foundry icon indicating copy to clipboard operation
foundry copied to clipboard

feat(forge): add contract ignore list for gas reports

Open danielrachi1 opened this issue 3 years ago • 4 comments

Motivation

See #2464

Solution

Create a new Config field called gas_reports_ignore and a new field in GasReport called ignore. (Default value for gas_reports_ignore is an empty Vec<String>.)

Pass gas_reports_ignore as an argument to function GasReport::new so GasReports are built with an ignore list.

The initialization of the variable report_contract decides if the contract has to be reported, using this logic: image

Were:

  • a: self.report_for.contains(&contract_name)
  • b: self.ignore.contains(&contract_name)
  • c: self.report_for.is_empty()
  • d: self.report_for.contains(&"*".to_string())

In simpler terms: Report the contract every time it's listed on report_for. Report all contracts if report_for is either empty or contains "*". If you're reporting all contracts but one of them is listed on ignore, don't report that one.

If the user listed the contract in 'gas_reports' (the foundry.toml field) a report for the contract is generated even if it's listed in the ignore list. This is addressed this way because getting a report you don't expect is preferable than not getting one you expect. A warning is printed to stderr indicating the "double listing".

danielrachi1 avatar Jul 31 '22 04:07 danielrachi1

Let me know if something is blocking you :)

onbjerg avatar Aug 04 '22 16:08 onbjerg

As I mentioned here, I need help testing it because I don't know how to write tests for it. So I'd really appreciate if you let me know how can I learn to test new features :)

danielrachi1 avatar Aug 04 '22 16:08 danielrachi1

@derch28 I think the best way currently would be to add a new forgetest! in cli/tests/it/cmd.rs and checking that the output of the command contains or does not contain what you expect - be careful not to match on the entirey of the output (use String::contains instead) since some of the output may change between each run.

A good starting point is reading some of the tests in there, figuring out what is closest to what you want to do (e.g. setting foundry.toml, adding a simple contract, running forge test --gas-report)

onbjerg avatar Aug 04 '22 16:08 onbjerg

@onbjerg Followed what you said and managed to write some tests :)

While testing, I realized that there was a simpler way to solve the issue and that allowed me to delete a function (analyze_trace) and a variable (report_for_all) that were no longer needed.

So, given that all the previous commits don't represent any logical change (because the PR-ready code was all made in the last one) I'm squashing them and force-pushing the result.

Should I edit the "Solution" Section of my first comment and explain how the new solution works there?

danielrachi1 avatar Aug 07 '22 08:08 danielrachi1