assertRevert.js passes when no revert occurred

[skmgoldin]

🎉 Description

https://github.com/OpenZeppelin/zeppelin-solidity/blob/b3a86029286569ac5656381be2398d5e3af7e92b/test/helpers/assertRevert.js

In assertRevert, the desired behavior is to fail the test if the provided promise does not throw an error containing the substring revert. As-written it will pass even if the promise does not throw any error. This is because assert.fail() throws an error, which is caught in the catch, and in the error message is the string Expected revert not received, which contains the substring revert.

assert.fail() does not magically fail a test, it just throws an error (but regardless of its expected/actual content) like any other assert. Because the error message contains the substring revert, revertFound will be true even if the promise never threw anything.

• 🐛 This is a bug report.
• 📈 This is a feature request.

🔢 Code To Reproduce Issue [ Good To Have ]

mkdir assertfail
cd assertfail
mkdir test
npm i mocha

Then in the test directory add this as a file:

var assert = require('assert');

it('should not consider this a revert', () => {
  try {
    assert.fail('This is not a real revert');
  } catch (error) {
    const revertFound = error.message.search('revert') >= 0;
    console.log(`Was a revert found? ${revertFound}`);
    assert.strictEqual(revertFound, false, `Did not expect a revert, but the error message contained it as a substring: ${error}`);
  }
});

Then run ./node_modules/mocha/bin/mocha

You will see this test fails. revertFound is true, but no EVM revert happened here.

👍 Other Information

The assert.fail() should go after (outside) the catch block, and there should be a return; at the end of (inside) the catch block.

@maurelian helped to discover this.

[simondlr]

I'm getting different behaviour when assertRevert.js is used in truffle 4.0.6 vs doing it like the above.

In truffle (4.0.6 & sol 0.4.19), error.message is just:

assert.fail()

with the above, I get the same behaviour:

This is not a real revert

Not sure why? Perhaps different assert libraries are being used?

[afiorenza]

I'm having the same issue. Any update on this?

Thanks!