Skip to content

fix(evm-adapters) remove checkTopic error from expectEmit in testFailExpectEmitWithCall1 and testFailExpectEmitWithCall2#925

Closed
ghost wants to merge 1 commit intomasterfrom
unknown repository
Closed

fix(evm-adapters) remove checkTopic error from expectEmit in testFailExpectEmitWithCall1 and testFailExpectEmitWithCall2#925
ghost wants to merge 1 commit intomasterfrom
unknown repository

Conversation

@ghost
Copy link

@ghost ghost commented Mar 13, 2022

Motivation

The function testFailExpectEmitWithCall1 is supposed to fail due to "data is different." However, the test is failing due to two reasons, which are

  1. "data is different"
  2. checkTopic3 is false

In other words, even if the data is the same as emitted by t4(), i.e., emit Transfer(address(this), address(1338), 100 gwei); the test will also fail. Hence, it is best to let this test to only fail due to "data is different."

Solution

emit Transfer(address(this), address(1337), 1 gwei); will fail only due to "data is different"

@gakonst
Copy link
Member

gakonst commented Mar 14, 2022

@brockelmore i am not sure that this PR makes the test tighter actually?

@onbjerg onbjerg added the L-ignore Log: ignore PR in changelog label Mar 15, 2022
@gakonst
Copy link
Member

gakonst commented Mar 17, 2022

this is checked in the fuzztests for expectemit in https://github.com/gakonst/foundry/pull/965, closing - thanks for the contribution either way @bblanc42!

@gakonst gakonst closed this Mar 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

L-ignore Log: ignore PR in changelog

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants