Replaced assertJump, assertRevert and expectThrow with shouldFail. #1363
Conversation
nventuro
added
tests
test/helpers/shouldFail.js
Outdated
| return; | ||
| } | ||
|
|
||
| should.fail(`Expected '${message} failure not received`); |
There was a problem hiding this comment.
There's a missing ' after ${message}.
test/helpers/shouldFail.js
Outdated
| should.fail(`Expected '${message} failure not received`); | ||
| } | ||
|
|
||
| function reverting (promise) { |
There was a problem hiding this comment.
Either add async here, or remove it in the next two functions for consistency. I'd add it here.
There was a problem hiding this comment.
I forgot to remove it from the other two ones. It makes no sense to mark it async unless we await inside it, and since all await in this case is return a promise, I'd rather just do it directly.
There was a problem hiding this comment.
I'd like you to reconsider making the functions async. It makes the return value of the function always be a promise regardless of the type of the return value. It's also like having a type annotation, and likewise it helps when reading the code IMO.
| return shouldFailWithMessage(promise, 'invalid opcode'); | ||
| } | ||
|
|
||
| async function outOfGas (promise) { |
There was a problem hiding this comment.
We're not really using this one, so why have it at all? Same for throwing.
There was a problem hiding this comment.
We'll want to have them once we publish the helpers and remove them from this repo (part of #1204), I'd just leave them be instead of removing them and later bringing them back.
|
It does look like it was fixed there, but I'd rather not close the issue until we have a test for this helper (which will come after this PR). |
There was a problem hiding this comment.
Thank you @nventuro. The tests read nicer now.
I'm not sure if this is the mocha syntax though. Wouldn't be something like this.bounty.cancelBounty.should.fail.reverting ?
I have no idea if that's possible on js, and I don't know where one would put the arguments for cancelBounty.
So this is good to me.
|
Yup, the syntax is not the best. I blame |
Closes #1362
Fixes #942
I'm not 100% sold on the
shouldFail.revertingsyntax, but I like that it feels close to chai. Suggestions are welcome.