foundry-rs / foundry

Foundry is a blazing fast, portable and modular toolkit for Ethereum application development written in Rust.
https://getfoundry.sh
Apache License 2.0
8.27k stars 1.74k forks source link

feat(`cheatcodes`): specify mis-matched fields on `expectEmit` failure #592

Open EzraWeller opened 2 years ago

EzraWeller commented 2 years ago

Component

Forge

Describe the feature you would like

Using the expectEmit function from the vm cheats foundry provides for testing, failures don't provide much information about what was mismatched.

Example:

% forge test -vvvv
compiling...
success.
Running 1 test for RepsTest.json:RepsTest
[FAIL. Reason: Log != expected log] testNewRep():(uint256) (gas: 5917)
Traces:
  [3840356] RepsTest::setUp()
    ├─ → new WETH@0xce71…c246
    │   └─ ← 3486 bytes of code
    ├─ → new Reps@0x185a…1aea
    │   └─ ← 7706 bytes of code
    ├─ [1305] Reps::name()
    │   └─ ← "Test"
    ├─ [1305] Reps::symbol()
    │   └─ ← "TST"
    ├─ [306] Reps::weth()
    │   └─ ← 0xce71065d4017f316ec606fe4422e11eb2c47c246
    ├─ → new CentralizedArbitrator@0xefc5…b132
    │   └─ ← 6101 bytes of code
    └─ ← ()
  [5917] RepsTest::testNewRep()
    ├─ [0] VM::expectEmit(true, true, true, true)
    │   └─ ← ()
    ├─ [2363] Reps::repCount()
    │   └─ ← 0
    └─ ← "Log != expected log"

Failed tests:
[FAIL. Reason: Log != expected log] testNewRep():(uint256) (gas: 5917)

It would be great to get more details about which fields were mismatched in this situation.

Additional context

No response

mds1 commented 1 year ago

Closing this in favor of the more generic https://github.com/foundry-rs/foundry/issues/928

mds1 commented 1 year ago

@Evalir reopened this as I think it’s a good solution to keeping the current behavior, while also solving https://github.com/foundry-rs/foundry/issues/5117#issuecomment-1599680238, without having to implement broader improvements/refactors as mentioned in #928

topocount commented 1 month ago

@zerosnacks @klkvr I'm looking into implementing this (as a follow-up to or as a part of #8686) and am curious what the intended behavior would be for situations where multiple instances of the same event are emitted in a given tx, but none match. Should the params of the events that were emitted but don't quite match be highlighted red?

I think highlighting params of events that match the expected topic 0 but fail another topic or data equivalence check is the way to go, just want to see if there are better alternatives.

edit to add: we should constrain this to an expected address if applicable as well, of course

topocount commented 1 month ago

Additionally, based on my understanding of the smart contract interfaces, we wouldn't be able to individually highlight data (unindexed) params, right? We'd have to parse and compare the abi-encoded bytestrings to determine which params don't match as well.

zerosnacks commented 1 month ago

Hi @topocount, great!

Should the params of the events that were emitted but don't quite match be highlighted red?

That would make sense to me, though limited to the mismatch to avoid warning / error fatigue

Additionally, based on my understanding of the smart contract interfaces, we wouldn't be able to individually highlight data (unindexed) params, right? We'd have to parse and compare the abi-encoded bytestrings to determine which params don't match as well.

I believe that to be correct, worst case you could diff the bytestring and just display that