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.27k stars 1.74k forks source link

bug: `forge test --gas-report --json` yields unexpected data and misses data #8789

Closed zerosnacks closed 3 weeks ago

zerosnacks commented 2 months ago

Component

Forge

Have you ensured that all of these are up to date?

What version of Foundry are you on?

forge 0.2.0 (d75318c 2024-09-02T00:21:21.377809037Z)

What command(s) is the bug in?

forge test --gas-report --json

Operating System

Linux

Describe the bug

forge test --gas-report yields:

forge test --gas-report
[⠊] Compiling...
No files changed, compilation skipped

Ran 2 tests for test/Counter.t.sol:CounterTest
[PASS] testFuzz_SetNumber(uint256) (runs: 256, μ: 52450, ~: 52540)
[PASS] test_Increment() (gas: 52367)
Suite result: ok. 2 passed; 0 failed; 0 skipped; finished in 14.51ms (13.86ms CPU time)
| src/Counter.sol:Counter contract |                 |       |        |       |         |
|----------------------------------|-----------------|-------|--------|-------|---------|
| Deployment Cost                  | Deployment Size |       |        |       |         |
| 106703                           | 277             |       |        |       |         |
| Function Name                    | min             | avg   | median | max   | # calls |
| increment                        | 43404           | 43404 | 43404  | 43404 | 1       |
| number                           | 283             | 283   | 283    | 283   | 257     |
| setNumber                        | 23582           | 43298 | 43536  | 43866 | 258     |

Ran 1 test suite in 22.31ms (14.51ms CPU time): 2 tests passed, 0 failed, 0 skipped (2 total tests)

Whereas forge test --gas-report --json | jq yields:

{
  "test/Counter.t.sol:CounterTest": {
    "duration": "9ms 752us 128ns",
    "test_results": {
      "testFuzz_SetNumber(uint256)": {
        "status": "Success",
        "reason": null,
        "counterexample": null,
        "logs": [],
        "kind": {
          "Fuzz": {
            "first_case": {
              "calldata": "0x5c7f60d7000000000000000000000000000000038fe552585acc01d0757bbae604ea5dae",
              "gas": 74080,
              "stipend": 21396
            },
            "runs": 256,
            "mean_gas": 52232,
            "median_gas": 52576
          }
        },
        "labeled_addresses": {},
        "duration": {
          "secs": 0,
          "nanos": 9216613
        },
        "breakpoints": {}
      },
      "test_Increment()": {
        "status": "Success",
        "reason": null,
        "counterexample": null,
        "logs": [],
        "kind": {
          "Unit": {
            "gas": 52367
          }
        },
        "labeled_addresses": {},
        "duration": {
          "secs": 0,
          "nanos": 128063
        },
        "breakpoints": {}
      }
    },
    "warnings": []
  }
}

Which are the results of running the test whereas I would assume --json would yield a JSON representation of the table layout:

src/Counter.sol:Counter contract
Deployment Cost Deployment Size
106703 277
Function Name min avg median max # calls
increment 43404 43404 43404 43404 1
number 283 283 283 283 257
setNumber 23582 43294 43542 43866 258

This would address the feature raised in https://github.com/foundry-rs/foundry/issues/8781

A possible format could be:

[
    {
       "src/Counter.sol:Counter contract":  {
         "Deployment Cost": "106703",
         "Deployment Size": "277",
         "increment": {
           "min": "43404"
           "avg": "43404"
           "max": "43404"
           "# calls": "1"
         },
         "number": {
           "min": "283"
           "avg": "283"
           "max": "283"
           "# calls": "0"
         },
         "setNumber": {
            "min": "23582"
            "avg": "43294"
            "max": "43542"
            "# calls": "258"
         }
      }
    }
]
zerosnacks commented 2 months ago

I think this is part of a larger issue we should think about holistically - all commands should output the same information with --json as rendered to stdout and be aware of the context of other flags. Passing --json to forge test with --gas-report has a different expectation of output than without --gas-report.

3esmit commented 1 month ago

Would be very interesting such feature, because we can link follow up the metrics of gas usage in the contract as per commit basis, specially when running tasks on CI, and git hooks.

beeb commented 1 month ago

Suggestion for the output format: https://bencher.dev/docs/reference/bencher-metric-format/

Additional info: https://bencher.dev/docs/how-to/track-custom-benchmarks/

Not sure if there are other standards out there, but using this format would allow for an easy adapter to Bencher.

3esmit commented 1 month ago

@beeb Looks good.

Would be good if gas snapshot and gas report become a single file, as their information are complementary. I mean, gas-snapshot is the summary of gas costs for the tests, while gas-report is the individual cost of each operation on the smart contracts used by the tests.

I think that this feature would help us detect bad efficiencies in the code.

Skyge commented 3 weeks ago

Yes, it would be good if the result of gas report can be written to a single file. I love this feature.

zerosnacks commented 3 weeks ago

The proposed JSON output in https://github.com/foundry-rs/foundry/pull/9063 looks as follows:

{
   "gas":106715,
   "size":277,
   "functions":{
      "increment":{
         "increment()":{
            "calls":1,
            "min":43404,
            "mean":43404,
            "median":43404,
            "max":43404
         }
      },
      "number":{
         "number()":{
            "calls":257,
            "min":283,
            "mean":283,
            "median":283,
            "max":283
         }
      },
      "setNumber":{
         "setNumber(uint256)":{
            "calls":258,
            "min":23582,
            "mean":43384,
            "median":43542,
            "max":43866
         }
      }
   }
}

Equivalent to:

| src/Counter.sol:Counter contract |                 |       |        |       |         |
|----------------------------------|-----------------|-------|--------|-------|---------|
| Deployment Cost                  | Deployment Size |       |        |       |         |
| 106715                           | 277             |       |        |       |         |
| Function Name                    | min             | avg   | median | max   | # calls |
| increment                        | 43404           | 43404 | 43404  | 43404 | 1       |
| number                           | 283             | 283   | 283    | 283   | 257     |
| setNumber                        | 23582           | 43224 | 43554  | 43866 | 258     |

Each test suite is rendered on a new line and can easily be parsed with jq for next steps or piped to a file