How is expectRevert used properly?

Archive SDK
1

I'm litte confused how to use expectRevert from openzeppelin-test-helpers properly.

Given is my Contract and the following revert:

    require(openingTime >= block.timestamp, "opening time is in the past");

Then I have a test with the following code:

it('reverts if the opening time is in the past', async function () {
  await expectRevert(TokenSale.new(
    (await time.latest()).sub(time.duration.days(1)), closingTime, token.address
  ), 'opening time is in the past');
});

which is doing the revert due to its arguments in the function call.

But I still get the following error:

Error: Returned error: VM Exception while processing transaction: revert opening time is in the past -- Reason given: opening time is in the past.

Lets pay attention on the reason string:

revert opening time is in the past

Reason given: opening time is in the past.

The final dot was added somewhere inside expectRevert. It is not part of the reason string wether in the Contract nor in the test.

How can I bring this to work?

2 Likes
2

Ok, I have figured something out.

If I would change the reason-string in the require() statement to the text sufficed by a Dot, everything works as expected.

require(openingTime >= block.timestamp, “opening time is in the past.”);

But in my test I may not include the Dot:

await expectRevert(TokenSale.new(
    (await time.latest()).sub(time.duration.days(1)), closingTime, token.address
  ), 'opening time is in the past');
2 Likes
3

Given is the following contract code:

    require(address(token) != address(0), "Zero adress for token is invalid");
    require(openingTime >= block.timestamp, "opening time is in the past.");
    require(closingTime > openingTime, "closing time is before the opening time");

Please consider the suffixed Dot at the second require statement, which is not existing in the first and third require-statement.

This is my test code:

it('reverts if the opening time is in the past', async function () {
  await expectRevert(TokenSale.new(
    (await time.latest()).sub(time.duration.days(1)), closingTime, token.address
  ), 'opening time is in the past');
});

it('reverts if the closing time is before the opening time', async function () {
  await expectRevert(TokenSale.new(
    openingTime, openingTime.sub(time.duration.seconds(1)), token.address
  ), 'closing time is before the opening time');
});

it('reverts if the closing time equals the opening time', async function () {
  await expectRevert(TokenSale.new(
    openingTime, openingTime, token.address
  ), 'closing time is before the opening time');
});

it('reverts if the token is a zero address', async function () {
  await expectRevert(TokenSale.new(
    openingTime, openingTime, ZERO_ADDRESS
  ), 'Zero adress for token is invalid');
});

Nowhere is the dot inside here, but all tests are running through. Just the second require-statement needs this Dot. Weird.

If I would remove this Dot in the second require-statement, it would break again as describe one comment above:

Error: Returned error: VM Exception while processing transaction: revert opening time is in the past -- Reason given: opening time is in the past.

I'm not sure if this happens because I'm working currently in an office having 33 Degree Celcius, or is there something odd inside ^^

2 Likes
4

Hi @itinance

The error when it is displayed in the terminal includes a full stop at the end. I assume this is what is causing the confusion.

If you update your repo to include the tests I am happy to have a look.

I would suggest prefixing the reverts with the contract name as per the style OpenZeppelin is using for their revert reasons. OpenZeppelin 2.3.0, that way revert reasons are unique, so easier to find in multi-contract usage.

[Off topic] It’s so cold in Melbourne, I am jealous of your warm weather outside, though 33C in the office must be tough.

5

Hi @itinance just checking that your expectRevert tests are ok?

6

Hey @abcoathup, thanks for asking! I kept 0.3.2 as I felt more comfortable with it. The dot issue, that I have described above, was a hard issue for my client he don’t want to lose any time on it. We have a tiny deadline. But I will give it a try on the next project :slight_smile:

1 Like
7

I can confirm I am having unpredictable behavior from the expectRevert test myself. I don’t have the above issue, but something that feels similar. I’ll talk to @nventuro in the morning and create a github issue if I still have the problem in the morning. :slight_smile:

1 Like
8

So update: Actually I can only confirm that I had a typo that was causing it to not work. I can’t reproduce the above error once I tried it again.

It seems to work exactly as described in the documentation for me.

https://github.com/OpenZeppelin/openzeppelin-test-helpers

2 Likes