Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

📈assertRevert with message checking. #917

Closed
MoMannn opened this issue Apr 24, 2018 · 14 comments
Closed

📈assertRevert with message checking. #917

MoMannn opened this issue Apr 24, 2018 · 14 comments
Assignees
Labels
feature New contracts, functions, or helpers. good first issue Low hanging fruit for new contributors to get involved! on hold Put on hold for some reason that must be specified in a comment. tests Test suite and helpers.

Comments

@MoMannn
Copy link

MoMannn commented Apr 24, 2018

Provide assertRevert with the ability to check the revert message (added in solidity 0.4.22).

@nventuro
Copy link
Contributor

nventuro commented Jul 14, 2018

Not 100% sure on what you have in mind with this: do you want to provide an expected error message and have the assert fail on a mismatch?

@MoMannn
Copy link
Author

MoMannn commented Jul 14, 2018

Bingo, that is exactly what I meant.

@nventuro
Copy link
Contributor

With #326 on the horizon, it makes sense to add the capability to the helpers, but I don't know if we want to commit to error messages in OZ itself yet. @frangio WDYT?

@frangio
Copy link
Contributor

frangio commented Jul 17, 2018

There's #888 for discussing revert reasons (error messages) in OpenZeppelin itself.

It makes sense to move forward with this helper though even before we introduce reasons in OpenZeppelin.

@frangio frangio added tests Test suite and helpers. feature New contracts, functions, or helpers. and removed kind:improvement labels Jul 20, 2018
@nventuro nventuro added the good first issue Low hanging fruit for new contributors to get involved! label Jul 25, 2018
@justuswilhelm
Copy link
Contributor

Hi @MoMannn , I was looking into this issue and wanted to extend assertRevert as per your suggestion to check for revert() error messages. Now I realize, that I actually don't know how to access the error message in a Truffle test. Do you have any pointers on how to approach this?

@MoMannn
Copy link
Author

MoMannn commented Jul 27, 2018

@justuswilhelm Sorry I have never went into the details of truffle so I can't really help you here it is just a feature I would like to have.

@nventuro nventuro removed this from the v1.12.0 milestone Jul 27, 2018
@nventuro
Copy link
Contributor

Sadly, it looks like this isn't yet possible in web3: web3/web3.js#1707.

@nventuro nventuro added the on hold Put on hold for some reason that must be specified in a comment. label Jul 27, 2018
@frangio
Copy link
Contributor

frangio commented Aug 3, 2018

web3.js is not the culprit. There is actually no way to retrieve the revert reason from a node. EIP 758 is a proposal to solve that, and there's Geth (ethereum/go-ethereum#16424) and Ganache (trufflesuite/ganache#116) issues to implement it.

I guess we'll have to wait and see how that plays out.

@nventuro
Copy link
Contributor

nventuro commented Aug 21, 2018

@justuswilhelm looks like truffle 5.0 will allow for this to be possible: see the relevant beta release notes.

@remon-nashid
Copy link
Contributor

It is worth noting that checking revert reason is currently possible with Ganache CLI, as demoed in this repo https://github.com/remon-nashid/assertRevert.

@frangio
Copy link
Contributor

frangio commented Nov 22, 2018

@remon-nashid Nice! Are you interested in incorporating your code into the OpenZeppelin helper?

@remon-nashid
Copy link
Contributor

@frangio I have since found an equivalent to assertRevert and that is shoudFail.withMessage. I opened an issue earlier and submitted a PR to export that function. See #1502.

@remon-nashid
Copy link
Contributor

IMO this issue can be closed as 'assertRevert' is superseded by shouldFail.withMessage.

@MoMannn
Copy link
Author

MoMannn commented Nov 23, 2018

@remon-nashid agreed. Closing.

@MoMannn MoMannn closed this as completed Nov 23, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New contracts, functions, or helpers. good first issue Low hanging fruit for new contributors to get involved! on hold Put on hold for some reason that must be specified in a comment. tests Test suite and helpers.
Projects
None yet
Development

No branches or pull requests

6 participants