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

Unexpected Behaviour with vm.expectRevert() #6331

Closed
Equious opened this issue Nov 16, 2023 · 5 comments
Closed

Unexpected Behaviour with vm.expectRevert() #6331

Equious opened this issue Nov 16, 2023 · 5 comments
Labels
T-bug Type: bug

Comments

@Equious
Copy link

Equious commented Nov 16, 2023

To summarize - vm.expectRevert(bytes("Custom Error Message")) will pass a test regardless of if the custom error message actually matches.

Contract function for reference:

function fund() public payable {
        require(msg.value >= MINIMUM_USD, "You need to spend more ETH!");
        s_addressToAmountFunded[msg.sender] += msg.value;
        s_funders.push(msg.sender);
    }

Test being run:

function testFailsWithoutEnoughETH() public {
        vm.expectRevert(bytes("Different Words!")); //expect a revert to happen
        fundMe.fund();
    }

Result in Terminal:

equious@DESKTOP-HTA2B2C:~/github-discussion-help/Foundry__FundMe$ forge test --mt testFailsWithoutEnoughETH -vvvv
[⠒] Compiling...
[⠃] Compiling 3 files with 0.8.21
[⠊] Solc 0.8.21 finished in 662.23ms
Compiler run successful!

Running 1 test for test/FundMeTest.t.sol:FundMeTest
[PASS] testFailsWithoutEnoughETH() (gas: 8672)
Traces:
  [8672] FundMeTest::testFailsWithoutEnoughETH()
    ├─ [0] VM::expectRevert(Different Words!)
    │   └─  ()
    ├─ [263] FundMe::fund()
    │   └─ ← revert: You need to spend more ETH!
    └─ ← Error != expected error: You need to spend more ETH! != Different Words!

Test result: ok. 1 passed; 0 failed; 0 skipped; finished in 1.38ms
 
Ran 1 test suites: 1 tests passed, 0 failed, 0 skipped (1 total tests)

The test seems to recognize the error is not what's expected, but passes anyway.

A further, perhaps related behaviour I can't explain is that removing the arguments passed to vm.expectRevert() - which should result in a pass for any revert - actually fails, despite catching the revert.

Function

function testFailsWithoutEnoughETH() public {
        vm.expectRevert(); //expect a revert to happen
        fundMe.fund();
    }

Result in Terminal:

  [8302] FundMeTest::testFailsWithoutEnoughETH()
    ├─ [0] VM::expectRevert(custom error f4844814:)
    │   └─  ()
    ├─ [263] FundMe::fund()
    │   └─ ← revert: You need to spend more ETH!
    └─  ()

Test result: FAILED. 0 passed; 1 failed; 0 skipped; finished in 1.28ms
 
Ran 1 test suites: 0 tests passed, 1 failed, 0 skipped (1 total tests)

Failing tests:
Encountered 1 failing test in test/FundMeTest.t.sol:FundMeTest
[FAIL. Reason: assertion failed] testFailsWithoutEnoughETH() (gas: 8302)

Encountered a total of 1 failing tests, 0 tests succeeded
@mds1 mds1 transferred this issue from foundry-rs/forge-std Nov 16, 2023
@gakonst gakonst added this to Foundry Nov 16, 2023
@github-project-automation github-project-automation bot moved this to Todo in Foundry Nov 16, 2023
@mds1 mds1 added the T-bug Type: bug label Nov 16, 2023
@DaniPopes
Copy link
Member

What Foundry version are you on?

@Equious
Copy link
Author

Equious commented Nov 16, 2023

v0.2.0 @DaniPopes

@gakonst
Copy link
Member

gakonst commented Nov 16, 2023

@Equious Can you share your forge -v? We haven't updated the version in the CLI but the date should update

@Equious
Copy link
Author

Equious commented Nov 16, 2023

image

Here you are! @gakonst

@DaniPopes
Copy link
Member

DaniPopes commented Nov 17, 2023

Ah I see what's happening here, @Equious your test name starts with "testFail" which expects the test to fail. Either rename the test to test_fails... (or anything other than starting with exactly "testFail"), or remove the expectRevert

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T-bug Type: bug
Projects
Archived in project
Development

No branches or pull requests

4 participants