search

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.26k stars 1.73k forks source link

chore: raise warning upon encountering a test function with the `testFail*` prefix #4437

Open rafales opened 1 year ago

rafales commented 1 year ago

Component

Forge

Describe the feature you would like

Hey. I just ran into a problem that wasted more time that I'm willing to admit.

Foundry has a feature where tests named testFailSomething are expected to fail. It's easy to forget about it.

So I found myself in a situation where test is failing for some unknown reason and all I get is this message:

[FAIL. Reason: Assertion failed.]

It would be good if this particular failure had a more user-friendly message, pointing to the reason behind the failure. It's easy to come up with a test name like testFailsWhenCalledContractFailed and not know about the testFailXXX feature.

Message like FAIL. Reason: test was expected to revert, but it did not. or something similar.

Additional context

No response

mds1 commented 1 year ago

@gakonst We should probably just remove testFail at this point, things like this have happened a few times and I don't think we need support for it anymore. Similar to https://github.com/foundry-rs/foundry/issues/3153#issuecomment-1445486191, we should look for tests named testFail* and print an issue/warning to inform users not to use this test name/format.

@rafales In the meantime I'd suggest following the naming conventions in https://book.getfoundry.sh/tutorials/best-practices, which are test_RevertIf_Condition

abhiram11 commented 1 year ago

We can prefer to use vm.expectRevert() in the function itself, rather than naming a function that starts with testFail. I feel it is possible that testFail may be deprecated so it's better to change this developer practice asap.

zerosnacks commented 1 month ago

Related: https://github.com/foundry-rs/book/issues/813