From 9c962b94309fe5e2b241d6c28e95c4dd8abe70b8 Mon Sep 17 00:00:00 2001 From: zerosnacks Date: Thu, 30 Jan 2025 11:48:49 +0100 Subject: [PATCH 1/8] add gas_snapshot_check configuration option w/ FORGE_SNAPSHOT_CHECK, checking for bool value - not just existence --- crates/config/src/lib.rs | 3 +++ crates/forge/bin/cmd/test/mod.rs | 6 +++++- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/crates/config/src/lib.rs b/crates/config/src/lib.rs index 891fd163b0194..de4cefc83c5d3 100644 --- a/crates/config/src/lib.rs +++ b/crates/config/src/lib.rs @@ -200,6 +200,8 @@ pub struct Config { pub cache_path: PathBuf, /// where the gas snapshots are stored pub snapshots: PathBuf, + /// whether to check for differences against previously stored gas snapshots + pub gas_snapshot_check: bool, /// where the broadcast logs are stored pub broadcast: PathBuf, /// additional solc allow paths for `--allow-paths` @@ -2316,6 +2318,7 @@ impl Default for Config { cache_path: "cache".into(), broadcast: "broadcast".into(), snapshots: "snapshots".into(), + gas_snapshot_check: false, allow_paths: vec![], include_paths: vec![], force: false, diff --git a/crates/forge/bin/cmd/test/mod.rs b/crates/forge/bin/cmd/test/mod.rs index 019e44e8c3255..4ec628cb9b71f 100644 --- a/crates/forge/bin/cmd/test/mod.rs +++ b/crates/forge/bin/cmd/test/mod.rs @@ -118,6 +118,10 @@ pub struct TestArgs { #[arg(long, env = "FORGE_GAS_REPORT")] gas_report: bool, + /// Check gas snapshots against previous runs. + #[arg(long, env = "FORGE_SNAPSHOT_CHECK")] + gas_snapshot_check: bool, + /// Exit with code 0 even if a test fails. #[arg(long, env = "FORGE_ALLOW_FAILURE")] allow_failure: bool, @@ -664,7 +668,7 @@ impl TestArgs { if !gas_snapshots.is_empty() { // Check for differences in gas snapshots if `FORGE_SNAPSHOT_CHECK` is set. // Exiting early with code 1 if differences are found. - if std::env::var("FORGE_SNAPSHOT_CHECK").is_ok() { + if self.gas_snapshot_check || config.gas_snapshot_check { let differences_found = gas_snapshots.clone().into_iter().fold( false, |mut found, (group, snapshots)| { From bd173dd707355593d5cddda12203579ff907552e Mon Sep 17 00:00:00 2001 From: zerosnacks Date: Thu, 30 Jan 2025 12:19:41 +0100 Subject: [PATCH 2/8] add additional test to display behaviour --- crates/forge/tests/cli/config.rs | 114 +++++++++++++++++++++++++++++++ 1 file changed, 114 insertions(+) diff --git a/crates/forge/tests/cli/config.rs b/crates/forge/tests/cli/config.rs index cd33ce79303e6..b81538b019b84 100644 --- a/crates/forge/tests/cli/config.rs +++ b/crates/forge/tests/cli/config.rs @@ -40,6 +40,7 @@ forgetest!(can_extract_config_values, |prj, cmd| { cache: true, cache_path: "test-cache".into(), snapshots: "snapshots".into(), + gas_snapshot_check: false, broadcast: "broadcast".into(), force: true, evm_version: EvmVersion::Byzantium, @@ -978,6 +979,7 @@ libraries = [] cache = true cache_path = "cache" snapshots = "snapshots" +gas_snapshot_check = false broadcast = "broadcast" allow_paths = [] include_paths = [] @@ -1365,3 +1367,115 @@ optimizer_runs = 0 "#]]); }); + +forgetest_init!(test_gas_snapshot_check_config, |prj, cmd| { + // Default settings: gas_snapshot_check disabled. + cmd.forge_fuse().args(["config"]).assert_success().stdout_eq(str![[r#" +... +gas_snapshot_check = false +... + +"#]]); + + prj.insert_ds_test(); + + prj.add_source( + "Flare.sol", + r#" +contract Flare { + bytes32[] public data; + + function run(uint256 n_) public { + for (uint256 i = 0; i < n_; i++) { + data.push(keccak256(abi.encodePacked(i))); + } + } +} + "#, + ) + .unwrap(); + + let test_contract = |n: u32| { + format!( + r#" +import "./test.sol"; +import "./Flare.sol"; + +interface Vm {{ + function startSnapshotGas(string memory name) external; + function stopSnapshotGas() external returns (uint256); +}} + +contract GasSnapshotCheckTest is DSTest {{ + Vm constant vm = Vm(HEVM_ADDRESS); + + Flare public flare; + + function setUp() public {{ + flare = new Flare(); + }} + + function testSnapshotGasSectionExternal() public {{ + vm.startSnapshotGas("testAssertGasExternal"); + flare.run({}); + vm.stopSnapshotGas(); + }} +}} + "#, + n + ) + }; + + prj.add_source("GasSnapshotCheckTest.sol", &test_contract(1)).unwrap(); + + cmd.forge_fuse().args(["test"]).assert_success().stdout_eq(str![[r#" +... +Ran 1 test for src/GasSnapshotCheckTest.sol:GasSnapshotCheckTest +[PASS] testSnapshotGasSectionExternal() ([GAS]) +Suite result: ok. 1 passed; 0 failed; 0 skipped; [ELAPSED] +... +"#]]); + + // Enable gas_snapshot_check + let config = Config { gas_snapshot_check: true, ..Default::default() }; + prj.write_config(config); + cmd.forge_fuse().args(["config"]).assert_success().stdout_eq(str![[r#" +... +gas_snapshot_check = true +... + +"#]]); + + prj.add_source("GasSnapshotCheckTest.sol", &test_contract(2)).unwrap(); + + cmd.forge_fuse().args(["test"]).assert_failure().stderr_eq(str![[r#" +... +[GasSnapshotCheckTest] Failed to match snapshots: +- [testAssertGasExternal] [..] → [..] + +Error: Snapshots differ from previous run +... +"#]]); + + // Disable gas_snapshot_check + let config = Config { gas_snapshot_check: false, ..Default::default() }; + prj.write_config(config); + cmd.forge_fuse().args(["test"]).assert_success().stdout_eq(str![[r#" +... +Ran 1 test for src/GasSnapshotCheckTest.sol:GasSnapshotCheckTest +[PASS] testSnapshotGasSectionExternal() ([GAS]) +Suite result: ok. 1 passed; 0 failed; 0 skipped; [ELAPSED] +... +"#]]); + + // Re-enable gas_snapshot_check, the new value has been stored. + let config = Config { gas_snapshot_check: true, ..Default::default() }; + prj.write_config(config); + cmd.forge_fuse().args(["test"]).assert_success().stdout_eq(str![[r#" +... +Ran 1 test for src/GasSnapshotCheckTest.sol:GasSnapshotCheckTest +[PASS] testSnapshotGasSectionExternal() ([GAS]) +Suite result: ok. 1 passed; 0 failed; 0 skipped; [ELAPSED] +... +"#]]); +}); From ceb2b197bc81c5484b6a0537f276789671b83a97 Mon Sep 17 00:00:00 2001 From: zerosnacks Date: Thu, 30 Jan 2025 12:59:23 +0100 Subject: [PATCH 3/8] improve docs --- crates/forge/bin/cmd/test/mod.rs | 22 +++++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) diff --git a/crates/forge/bin/cmd/test/mod.rs b/crates/forge/bin/cmd/test/mod.rs index 4ec628cb9b71f..f5c49f8783383 100644 --- a/crates/forge/bin/cmd/test/mod.rs +++ b/crates/forge/bin/cmd/test/mod.rs @@ -120,7 +120,7 @@ pub struct TestArgs { /// Check gas snapshots against previous runs. #[arg(long, env = "FORGE_SNAPSHOT_CHECK")] - gas_snapshot_check: bool, + gas_snapshot_check: Option, /// Exit with code 0 even if a test fails. #[arg(long, env = "FORGE_ALLOW_FAILURE")] @@ -666,9 +666,25 @@ impl TestArgs { // Write gas snapshots to disk if any were collected. if !gas_snapshots.is_empty() { - // Check for differences in gas snapshots if `FORGE_SNAPSHOT_CHECK` is set. + // By default `gas_snapshot_check` is set to `false` in the config. + // + // The user can either: + // - Set `FORGE_SNAPSHOT_CHECK=true` in the environment. + // - Pass `--gas-snapshot-check=true` as a CLI argument. + // - Set `gas_snapshot_check = true` in the config. + // + // If the user passes `--gas-snapshot-check=` then it will override the config + // and the environment variable, disabling the check if `false` is passed. + // // Exiting early with code 1 if differences are found. - if self.gas_snapshot_check || config.gas_snapshot_check { + let should_check_for_differences = + if let Some(gas_snapshot_check) = self.gas_snapshot_check { + gas_snapshot_check + } else { + config.gas_snapshot_check + }; + + if should_check_for_differences { let differences_found = gas_snapshots.clone().into_iter().fold( false, |mut found, (group, snapshots)| { From a2a845f02ef2b13cf08051ff164048ad4bce8fa5 Mon Sep 17 00:00:00 2001 From: zerosnacks Date: Thu, 30 Jan 2025 13:19:35 +0100 Subject: [PATCH 4/8] fix clippy --- crates/forge/tests/cli/config.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/crates/forge/tests/cli/config.rs b/crates/forge/tests/cli/config.rs index b81538b019b84..ef09212223844 100644 --- a/crates/forge/tests/cli/config.rs +++ b/crates/forge/tests/cli/config.rs @@ -1417,12 +1417,11 @@ contract GasSnapshotCheckTest is DSTest {{ function testSnapshotGasSectionExternal() public {{ vm.startSnapshotGas("testAssertGasExternal"); - flare.run({}); + flare.run({n}); vm.stopSnapshotGas(); }} }} - "#, - n + "# ) }; From f5eaf2550391c6eb12f831c13954e859b5361f7d Mon Sep 17 00:00:00 2001 From: zerosnacks Date: Thu, 30 Jan 2025 13:48:08 +0100 Subject: [PATCH 5/8] improve test suite, tests all combinations exhaustively --- crates/forge/tests/cli/config.rs | 75 ++++++++++++++++++++++++++++++-- 1 file changed, 71 insertions(+), 4 deletions(-) diff --git a/crates/forge/tests/cli/config.rs b/crates/forge/tests/cli/config.rs index ef09212223844..8c51d0fe4819b 100644 --- a/crates/forge/tests/cli/config.rs +++ b/crates/forge/tests/cli/config.rs @@ -1425,8 +1425,8 @@ contract GasSnapshotCheckTest is DSTest {{ ) }; + // Assert that gas_snapshot_check is disabled by default prj.add_source("GasSnapshotCheckTest.sol", &test_contract(1)).unwrap(); - cmd.forge_fuse().args(["test"]).assert_success().stdout_eq(str![[r#" ... Ran 1 test for src/GasSnapshotCheckTest.sol:GasSnapshotCheckTest @@ -1445,8 +1445,8 @@ gas_snapshot_check = true "#]]); + // Replace the test contract with a new one that will fail the gas snapshot check prj.add_source("GasSnapshotCheckTest.sol", &test_contract(2)).unwrap(); - cmd.forge_fuse().args(["test"]).assert_failure().stderr_eq(str![[r#" ... [GasSnapshotCheckTest] Failed to match snapshots: @@ -1456,7 +1456,7 @@ Error: Snapshots differ from previous run ... "#]]); - // Disable gas_snapshot_check + // Disable gas_snapshot_check, assert that running the test will pass let config = Config { gas_snapshot_check: false, ..Default::default() }; prj.write_config(config); cmd.forge_fuse().args(["test"]).assert_success().stdout_eq(str![[r#" @@ -1467,7 +1467,8 @@ Suite result: ok. 1 passed; 0 failed; 0 skipped; [ELAPSED] ... "#]]); - // Re-enable gas_snapshot_check, the new value has been stored. + // Re-enable gas_snapshot_check + // Assert that the new value has been stored from the previous run and re-run the test let config = Config { gas_snapshot_check: true, ..Default::default() }; prj.write_config(config); cmd.forge_fuse().args(["test"]).assert_success().stdout_eq(str![[r#" @@ -1476,5 +1477,71 @@ Ran 1 test for src/GasSnapshotCheckTest.sol:GasSnapshotCheckTest [PASS] testSnapshotGasSectionExternal() ([GAS]) Suite result: ok. 1 passed; 0 failed; 0 skipped; [ELAPSED] ... +"#]]); + + // Replace the test contract with a new one that will fail the gas_snapshot_check + prj.add_source("GasSnapshotCheckTest.sol", &test_contract(3)).unwrap(); + cmd.forge_fuse().args(["test"]).assert_failure().stderr_eq(str![[r#" +... +[GasSnapshotCheckTest] Failed to match snapshots: +- [testAssertGasExternal] [..] → [..] + +Error: Snapshots differ from previous run +... +"#]]); + + // Test that `--gas-snapshot-check=false` flag can be used to disable the gas_snapshot_check + cmd.forge_fuse().args(["test", "--gas-snapshot-check=false"]).assert_success().stdout_eq(str![ + [r#" +... +Ran 1 test for src/GasSnapshotCheckTest.sol:GasSnapshotCheckTest +[PASS] testSnapshotGasSectionExternal() ([GAS]) +Suite result: ok. 1 passed; 0 failed; 0 skipped; [ELAPSED] +... +"#] + ]); + + // Disable gas_snapshot_check in the config file + // Enable using `FORGE_SNAPSHOT_CHECK` environment variable + // Assert that this will override the config file value + let config = Config { gas_snapshot_check: false, ..Default::default() }; + prj.write_config(config); + prj.add_source("GasSnapshotCheckTest.sol", &test_contract(4)).unwrap(); + cmd.forge_fuse(); + cmd.env("FORGE_SNAPSHOT_CHECK", "true"); + cmd.args(["test"]).assert_failure().stderr_eq(str![[r#" +... +[GasSnapshotCheckTest] Failed to match snapshots: +- [testAssertGasExternal] [..] → [..] + +Error: Snapshots differ from previous run +... +"#]]); + + // Assert that `--gas-snapshot-check=true` flag can be used to enable the gas_snapshot_check + // even when `FORGE_SNAPSHOT_CHECK` is set to false in the environment variable + cmd.forge_fuse(); + cmd.env("FORGE_SNAPSHOT_CHECK", "false"); + cmd.args(["test", "--gas-snapshot-check=true"]).assert_failure().stderr_eq(str![[r#" +... +[GasSnapshotCheckTest] Failed to match snapshots: +- [testAssertGasExternal] [..] → [..] + +Error: Snapshots differ from previous run +... +"#]]); + + // + + // Finally assert that `--gas-snapshot-check=false` flag can be used to disable the + // gas_snapshot_check even when `FORGE_SNAPSHOT_CHECK` is set to true + cmd.forge_fuse(); + cmd.env("FORGE_SNAPSHOT_CHECK", "true"); + cmd.args(["test", "--gas-snapshot-check=false"]).assert_success().stdout_eq(str![[r#" +... +Ran 1 test for src/GasSnapshotCheckTest.sol:GasSnapshotCheckTest +[PASS] testSnapshotGasSectionExternal() ([GAS]) +Suite result: ok. 1 passed; 0 failed; 0 skipped; [ELAPSED] +... "#]]); }); From d04c1165d682fa3d164aabb649ac295728b96b68 Mon Sep 17 00:00:00 2001 From: zerosnacks Date: Thu, 30 Jan 2025 13:49:34 +0100 Subject: [PATCH 6/8] fix failing test --- crates/forge/tests/cli/config.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/crates/forge/tests/cli/config.rs b/crates/forge/tests/cli/config.rs index 8c51d0fe4819b..587bc3c54f7b2 100644 --- a/crates/forge/tests/cli/config.rs +++ b/crates/forge/tests/cli/config.rs @@ -1134,6 +1134,7 @@ exclude = [] "cache": true, "cache_path": "cache", "snapshots": "snapshots", + "gas_snapshot_check": false, "broadcast": "broadcast", "allow_paths": [], "include_paths": [], From d56e16d7d340972c8ee48db43803ef3a336c2b11 Mon Sep 17 00:00:00 2001 From: zerosnacks Date: Thu, 30 Jan 2025 14:56:49 +0100 Subject: [PATCH 7/8] fix nit --- crates/forge/bin/cmd/test/mod.rs | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/crates/forge/bin/cmd/test/mod.rs b/crates/forge/bin/cmd/test/mod.rs index f5c49f8783383..87ba45f1936db 100644 --- a/crates/forge/bin/cmd/test/mod.rs +++ b/crates/forge/bin/cmd/test/mod.rs @@ -677,14 +677,7 @@ impl TestArgs { // and the environment variable, disabling the check if `false` is passed. // // Exiting early with code 1 if differences are found. - let should_check_for_differences = - if let Some(gas_snapshot_check) = self.gas_snapshot_check { - gas_snapshot_check - } else { - config.gas_snapshot_check - }; - - if should_check_for_differences { + if self.gas_snapshot_check.unwrap_or(config.gas_snapshot_check) { let differences_found = gas_snapshots.clone().into_iter().fold( false, |mut found, (group, snapshots)| { From 50f0b1a35735a58a10fb504b662e1491b4157147 Mon Sep 17 00:00:00 2001 From: zerosnacks Date: Thu, 30 Jan 2025 15:29:53 +0100 Subject: [PATCH 8/8] small nits --- crates/forge/tests/cli/config.rs | 24 +++++++++++------------- 1 file changed, 11 insertions(+), 13 deletions(-) diff --git a/crates/forge/tests/cli/config.rs b/crates/forge/tests/cli/config.rs index 587bc3c54f7b2..d8e8810e9a682 100644 --- a/crates/forge/tests/cli/config.rs +++ b/crates/forge/tests/cli/config.rs @@ -1426,7 +1426,7 @@ contract GasSnapshotCheckTest is DSTest {{ ) }; - // Assert that gas_snapshot_check is disabled by default + // Assert that gas_snapshot_check is disabled by default. prj.add_source("GasSnapshotCheckTest.sol", &test_contract(1)).unwrap(); cmd.forge_fuse().args(["test"]).assert_success().stdout_eq(str![[r#" ... @@ -1436,7 +1436,7 @@ Suite result: ok. 1 passed; 0 failed; 0 skipped; [ELAPSED] ... "#]]); - // Enable gas_snapshot_check + // Enable gas_snapshot_check. let config = Config { gas_snapshot_check: true, ..Default::default() }; prj.write_config(config); cmd.forge_fuse().args(["config"]).assert_success().stdout_eq(str![[r#" @@ -1446,7 +1446,7 @@ gas_snapshot_check = true "#]]); - // Replace the test contract with a new one that will fail the gas snapshot check + // Replace the test contract with a new one that will fail the gas snapshot check. prj.add_source("GasSnapshotCheckTest.sol", &test_contract(2)).unwrap(); cmd.forge_fuse().args(["test"]).assert_failure().stderr_eq(str![[r#" ... @@ -1457,7 +1457,7 @@ Error: Snapshots differ from previous run ... "#]]); - // Disable gas_snapshot_check, assert that running the test will pass + // Disable gas_snapshot_check, assert that running the test will pass. let config = Config { gas_snapshot_check: false, ..Default::default() }; prj.write_config(config); cmd.forge_fuse().args(["test"]).assert_success().stdout_eq(str![[r#" @@ -1469,7 +1469,7 @@ Suite result: ok. 1 passed; 0 failed; 0 skipped; [ELAPSED] "#]]); // Re-enable gas_snapshot_check - // Assert that the new value has been stored from the previous run and re-run the test + // Assert that the new value has been stored from the previous run and re-run the test. let config = Config { gas_snapshot_check: true, ..Default::default() }; prj.write_config(config); cmd.forge_fuse().args(["test"]).assert_success().stdout_eq(str![[r#" @@ -1480,7 +1480,7 @@ Suite result: ok. 1 passed; 0 failed; 0 skipped; [ELAPSED] ... "#]]); - // Replace the test contract with a new one that will fail the gas_snapshot_check + // Replace the test contract with a new one that will fail the gas_snapshot_check. prj.add_source("GasSnapshotCheckTest.sol", &test_contract(3)).unwrap(); cmd.forge_fuse().args(["test"]).assert_failure().stderr_eq(str![[r#" ... @@ -1491,7 +1491,7 @@ Error: Snapshots differ from previous run ... "#]]); - // Test that `--gas-snapshot-check=false` flag can be used to disable the gas_snapshot_check + // Test that `--gas-snapshot-check=false` flag can be used to disable the gas_snapshot_check. cmd.forge_fuse().args(["test", "--gas-snapshot-check=false"]).assert_success().stdout_eq(str![ [r#" ... @@ -1502,9 +1502,9 @@ Suite result: ok. 1 passed; 0 failed; 0 skipped; [ELAPSED] "#] ]); - // Disable gas_snapshot_check in the config file - // Enable using `FORGE_SNAPSHOT_CHECK` environment variable - // Assert that this will override the config file value + // Disable gas_snapshot_check in the config file. + // Enable using `FORGE_SNAPSHOT_CHECK` environment variable. + // Assert that this will override the config file value. let config = Config { gas_snapshot_check: false, ..Default::default() }; prj.write_config(config); prj.add_source("GasSnapshotCheckTest.sol", &test_contract(4)).unwrap(); @@ -1520,7 +1520,7 @@ Error: Snapshots differ from previous run "#]]); // Assert that `--gas-snapshot-check=true` flag can be used to enable the gas_snapshot_check - // even when `FORGE_SNAPSHOT_CHECK` is set to false in the environment variable + // even when `FORGE_SNAPSHOT_CHECK` is set to false in the environment variable. cmd.forge_fuse(); cmd.env("FORGE_SNAPSHOT_CHECK", "false"); cmd.args(["test", "--gas-snapshot-check=true"]).assert_failure().stderr_eq(str![[r#" @@ -1532,8 +1532,6 @@ Error: Snapshots differ from previous run ... "#]]); - // - // Finally assert that `--gas-snapshot-check=false` flag can be used to disable the // gas_snapshot_check even when `FORGE_SNAPSHOT_CHECK` is set to true cmd.forge_fuse();