forge: keep fuzz snapshots behind a flag #1086
Conversation
transmissions11
changed the title
transmissions11
changed the title
|
we might want a corresponding config option for |
|
also if anyone has ideas for a better/shorter flag name than |
| #[clap(long, env = "FORGE_GAS_REPORT")] | ||
| gas_report: bool, | ||
|
|
||
| /// Include the mean and median gas use of fuzz tests in the output. | ||
| #[clap(long, env = "FORGE_INCLUDE_FUZZ_TEST_GAS")] | ||
| pub include_fuzz_test_gas: bool, |
There was a problem hiding this comment.
will move this to config values as well
There was a problem hiding this comment.
how'd I go about doing that while still respecting the CLI arg?
There was a problem hiding this comment.
env always take precedent over config vals, but with bools it's a bit tricky, because if set to true in the config then you can't set it to false via cli easily...
So not sure whether to move this to config at all, perhaps the env var is sufficient?
There was a problem hiding this comment.
i think folks like @mds1 would def prefer a config option
There was a problem hiding this comment.
sg, this will require some minor refactoring because this is actually the first config val directly included in the TestArgs struct, so I'll rewrite the macro that does all the config magic,
moving this to a followup
| #[clap(long, env = "FORGE_GAS_REPORT")] | ||
| gas_report: bool, | ||
|
|
||
| /// Include the mean and median gas use of fuzz tests in the output. | ||
| #[clap(long, env = "FORGE_INCLUDE_FUZZ_TEST_GAS")] | ||
| pub include_fuzz_test_gas: bool, |
There was a problem hiding this comment.
this would make median and mean opt-in instead of opt-out.
should we rather flip it?
There was a problem hiding this comment.
I think anyone actually optimizing realizes these values are uselessly inconsistent and should be out-in as a result but as long as I can opt-out via config I'm willing to concede hehe
There was a problem hiding this comment.
sg, can you please run cargo +nightly fmt to fix the fmt errors? then this is good to merge
* ✨ Keep fuzz snapshots behind a flag * ♻️ cargo +nightly fmt
* revert #1086 * ✨ Hide fuzz tests unless flag
* revert #1086 * ✨ Hide fuzz tests unless flag * ⚡️ forge snapshot: don't run fuzz tests by default * ♻️ Combine args into a tuple * ♻️ Simplify write_to_snapshot_file * ♻️ Unused import * ♻️ fmt
Closes #1081