Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add --file flag to forge test --gas-report to write to file #8781

Closed
3esmit opened this issue Aug 31, 2024 · 3 comments
Closed

feat: add --file flag to forge test --gas-report to write to file #8781

3esmit opened this issue Aug 31, 2024 · 3 comments
Labels
A-gas-snapshots Area: gas snapshotting/reporting C-forge Command: forge T-blocked Type: blocked T-feature Type: feature

Comments

@3esmit
Copy link

3esmit commented Aug 31, 2024

Component

Forge

Describe the feature you would like

I would like that forge test --gas-report to be saved in a file, similar to what forge snapshot does.

This could be either a different flag in forge test --gas-report like forge-test --gas-report-file or probably the better would be forge snapshot --gas-report, so the gas report also generates a file called .gas-report with the table output of forge-test --gas-report

Additional context

forge test --gas-report seems more complete than forge snapshot, because it actually tests the gas costs for different scenarios, for example when storage is loaded, or empty, etc, or different call stacks, and create a very useful report to see how the changes impact in gas usage.

Currently I am using a custom commands on package.json to achieve this functionality:

  "scripts": {
"gas-report": "forge test --gas-report 2>&1 | (tee /dev/tty | awk '/Suite result:/ {found=1; buffer=\"\"; next} found && !/Ran/ {buffer=buffer $0 ORS} /Ran/ {found=0} END {printf \"%s\", buffer}' > .gas-report)",
"adorno": "pnpm prettier:write && forge fmt && forge snapshot && pnpm gas-report"
} 

Recently, forge changed a word in the test report, from "Test result" to "Suite result" that broke that command entirely in many repositories I am working. See vacp2p/foundry-template#31

This feature would avoid having to tweak with awk, and give peace of mind on both sides.

@3esmit 3esmit added T-feature Type: feature T-needs-triage Type: this issue needs to be labelled labels Aug 31, 2024
@zerosnacks
Copy link
Member

zerosnacks commented Sep 2, 2024

Hi @3esmit I think this issue can be resolved if forge test --gas-report --json yields a predictable and correct JSON serialized representation of the table layout. This would allow you to just pipe to file using builtins.

See: #8789

@zerosnacks zerosnacks changed the title Feature request: Gas-report output to file feat: add --file flag to forge test --gas-report to write to file Sep 2, 2024
@zerosnacks zerosnacks added C-forge Command: forge A-gas-snapshots Area: gas snapshotting/reporting T-blocked Type: blocked and removed T-needs-triage Type: this issue needs to be labelled labels Sep 2, 2024
@3esmit
Copy link
Author

3esmit commented Sep 24, 2024

@zerosnacks yes, if that command outputed the data in the table it would fix the issue.

I need something to keep track of all gas changes.. Ideally, gas-snapshot and gas-report become one single file, and both in json, so we can have some tool (maybe a plugin in coverage stuff) that would follow the gas cost of operations.

@3esmit
Copy link
Author

3esmit commented Oct 22, 2024

Closed by #9063

@3esmit 3esmit closed this as completed Oct 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-gas-snapshots Area: gas snapshotting/reporting C-forge Command: forge T-blocked Type: blocked T-feature Type: feature
Projects
None yet
Development

No branches or pull requests

2 participants