tests: run ganache with --blockTime 1 & adjust tests #36
Conversation
| .onConfirmation(onConfirmationSpy) | ||
| .onceConfirmedReceipt(3, onceConfirmedReceiptSpy) | ||
| .onceTxRevert(onceTxRevertSpy); | ||
| } catch (error) { |
There was a problem hiding this comment.
There should be a fail within the try block to ensure an exception was thrown. Same pattern should apply to the subsequent checks (instead of isUndefined).
There was a problem hiding this comment.
What you mean by fail within the block? .send should throw, that's what testing here. As for txHash isUndefined I'm not sure what you mean. Am I miss something? :)
There was a problem hiding this comment.
The pattern to properly check exceptions:
try {
doStuff();
assert.fail(); // we shouldn’t reach this line
} catch (e) {
// validate the error
}
There was a problem hiding this comment.
Btw, you could also use fail for callbacks that shouldn’t be called. Maybe shorter/clearer than spy...
| assert.equal(onceConfirmedReceiptSpy.callCount, 0); | ||
| }); | ||
|
|
||
| /** This doesn't work with ganache --blockTime 1 : can't get error in any ways with web3 beta36 */ |
There was a problem hiding this comment.
I didn't try this particular yet, good point.
test/Transaction.test.js
Outdated
| assert.deepEqual(error, errorCaught); | ||
| }); | ||
|
|
||
| assert.isUndefined(txHash); |
There was a problem hiding this comment.
What I meant instead of isUndefined:
try {
await tx.getTxHash();
assert.fail();
} catch (error) {
assert.deepEqual(error, errorCaught);
}
There was a problem hiding this comment.
Though I suppose other styles are possible (see https://www.chaijs.com/plugins/chai-as-promised/)
There was a problem hiding this comment.
I used chai-as-promised even in this test. I can't recall why not in this particular case, maybe it was less convenient.
--blockTime 1(which is closer how real chains work) and adjust tests to behaviourTransactionSendErrorif.sendfails..getTxHash,getTxReceiptandgetConfirmedReceiptare also rejected with that error