More pretty logs when running fork tests

[PaulRBerg]

When running all tests or just all fork tests, it should be possible to avoid dumping this ugly calldata, which makes it quite difficult to see what particular function failed.

[andreivladbrg]

Hmm, since we're fuzzing the Merkle data (a long array of data), I'm not sure if there's a solution for this.

[PaulRBerg]

We can write a Bash script that uses grep or smth to filter out the calldata

[smol-ninja]

Yeah, its very annoying. But warp has a cool feature that works for me in identifying the failing function without having to scroll up the way:

If you click on the marked region, the warp takes you to the beginning of the command execution and then you can scroll down to see which function failed.

PS: Now I think you all know about this hacky feature :))

[PaulRBerg]

Aah, that's a cool feature I didn't know about, thanks.

But I still think the Bash script would be helpful

[andreivladbrg]

The main reason for these ugly logs in fork/fuzz tests is the use of expectEmit in our tests.

Shub and I were discussing this on Slack, and we agreed that the best fix is to remove them. Doing so would clean up the logs and make debugging much easier and faster, since we’d get explicit assertion failures instead.

Personally, I always end up commenting out the expectEmit lines when debugging fork/fuzz tests — it’s impossible to debug the calldata.

We’ll still keep event tests in concrete/unit tests, which should be sufficient

Curious what you think, @PaulRBerg

[smol-ninja]

Though I agree with Andrei but I also think testing events in fork/fuzz is equally important especially in production. We may not always have getters for all the values used in the events.

Re. poor debugging experience, I agree its bad but it only takes a bit of an effort to debug it by commenting the events locally.

Just a thought that came to while reading this message in Slack.

[PaulRBerg]

We should keep testing the events in the fork tests because events serve as a critical 'communication layer' between the contracts and the outside world, e.g. via the subgraphs. When we test the events on a mainnet fork, it's a form of 'stress testing'.

The solution isn't really that hard to build — a Bash script that looks for the output and filters out the content using some regex. AI should be able to do it.

Also: consider reaching out to Foundry to complain about this and suggest the ability to save the long error logs into a file or smth.

[andreivladbrg]

The solution isn't really that hard to build — a Bash script that looks for the output and filters out the content using some regex. AI should be able to do it.

My point wasn't just about "pretty" logs, but the debugging process itself. We won't be able to ever debug that the real reason of failling (I now believe it would have been better to open a separate discussion/issue instead, The reason I wrote it here is because it would implicitly fix this issue)

For the same test, compare it reverting with an incorrect event log or assertion:

[FAIL: log != expected log] test_WhenEndTimeNotInPast() 
// vs specific assertion
[FAIL: total forgone amount: 7920000000000000000000 != 7919999999999999999999] test_WhenEndTimeNotInPast()

failing here, instead of here

We should keep testing the events in the fork tests because events serve as a critical 'communication layer' between the contracts and the outside world, e.g. via the subgraphs. When we test the events on a mainnet fork, it's a form of 'stress testing'

Good point about the "communication layer" — though what’s important IMO is to test that the events are emitted correctly before deploying the contracts. This is why I suggested keeping them in the concrete tests. Once the contract is deployed, simply testing that the events are emitted is basically just testing that the EVM works as expected (similar to the argument that was made in the past about "testing the payable functionality").

Also: consider reaching out to Foundry to complain about this and suggest the ability to save the long error logs into a file or smth.

Tbh, I personally don’t care that much about how long or short the Foundry error logs are — what matters to me is whether I can find the exact reason somewhere

[PaulRBerg]

OK in this case please open another issue/discussion because you're making a different request

simply testing that the events are emitted is basically just testing that the EVM works as expected (similar to the argument that was made in the past about "testing the payable functionality").

This is incorrect but let's talk about in that other discussion