forge: only hide fuzz tests from snapshots (still show in tests)

cli/src/cmd/forge/snapshot.rs

@@ -11,7 +11,7 @@ use crate::cmd::{
 use ansi_term::Colour;
 use clap::{Parser, ValueHint};
 use eyre::Context;
-use forge::TestKindGas;
+use forge::{TestKind, TestKindGas};
 use once_cell::sync::Lazy;
 use regex::Regex;
 use std::{
@@ -36,9 +36,11 @@ pub struct SnapshotArgs {
     /// All test arguments are supported
     #[clap(flatten)]
     test: test::TestArgs,
+
     /// Additional configs for test results
     #[clap(flatten)]
     config: SnapshotConfig,
+
     #[clap(
         help = "Compare against a snapshot and display changes from the snapshot. Takes an optional snapshot file, [default: .gas-snapshot]",
         conflicts_with = "snap",
@@ -47,6 +49,7 @@ pub struct SnapshotArgs {
         value_name = "SNAPSHOT_FILE",
     )]
     diff: Option<Option<PathBuf>>,
+
     #[clap(
         help = "Run snapshot in 'check' mode and compares against an existing snapshot file, [default: .gas-snapshot]. Exits with 0 if snapshots match. Exits with 1 and prints a diff otherwise",
         conflicts_with = "diff",
@@ -55,15 +58,21 @@ pub struct SnapshotArgs {
         value_name = "SNAPSHOT_FILE",
     )]
     check: Option<Option<PathBuf>>,
+
     #[clap(help = "How to format the output.", long)]
     format: Option<Format>,
+
     #[clap(
         help = "Output file for the snapshot.",
         default_value = ".gas-snapshot",
         long,
         value_name = "SNAPSHOT_FILE"
     )]
     snap: PathBuf,
+
+    /// Include the mean and median gas use of fuzz tests in the snapshot.
+    #[clap(long, env = "FORGE_INCLUDE_FUZZ_TESTS")]
+    pub include_fuzz_tests: bool,
 }
 
 impl SnapshotArgs {
@@ -88,25 +97,24 @@ impl Cmd for SnapshotArgs {
     type Output = ();
 
     fn run(self) -> eyre::Result<()> {
-        let include_fuzz_test_gas = self.test.include_fuzz_test_gas;
         let outcome = self.test.run()?;
-        outcome.ensure_ok(include_fuzz_test_gas)?;
+        outcome.ensure_ok()?;
         let tests = self.config.apply(outcome);
 
         if let Some(path) = self.diff {
             let snap = path.as_ref().unwrap_or(&self.snap);
             let snaps = read_snapshot(snap)?;
-            diff(tests, include_fuzz_test_gas, snaps)?;
+            diff(tests, snaps)?;
         } else if let Some(path) = self.check {
             let snap = path.as_ref().unwrap_or(&self.snap);
             let snaps = read_snapshot(snap)?;
-            if check(tests, include_fuzz_test_gas, snaps) {
+            if check(tests, snaps) {
                 std::process::exit(0)
             } else {
                 std::process::exit(1)
             }
         } else {
-            write_to_snapshot_file(&tests, self.snap, include_fuzz_test_gas, self.format)?;
+            write_to_snapshot_file(&tests, self.snap, self.include_fuzz_tests, self.format)?;
         }
         Ok(())
     }
@@ -208,7 +216,6 @@ impl FromStr for SnapshotEntry {
                                     contract_name: file.as_str().to_string(),
                                     signature: sig.as_str().to_string(),
                                     gas_used: TestKindGas::Fuzz {
-                                        include_fuzz_test_gas: true,
                                         runs: runs.as_str().parse().unwrap(),
                                         median: med.as_str().parse().unwrap(),
                                         mean: avg.as_str().parse().unwrap(),
@@ -242,17 +249,22 @@ fn read_snapshot(path: impl AsRef<Path>) -> eyre::Result<Vec<SnapshotEntry>> {
 fn write_to_snapshot_file(
     tests: &[Test],
     path: impl AsRef<Path>,
-    include_fuzz_test_gas: bool,
+    include_fuzz_tests: bool,
     _format: Option<Format>,
 ) -> eyre::Result<()> {
     let mut out = String::new();
     for test in tests {
+        // Ignore fuzz tests if we're not including fuzz tests in the snapshot.
+        if matches!(test.result.kind, TestKind::Fuzz(..)) && !include_fuzz_tests {
+            continue
+        }
+
         writeln!(
             out,
             "{}:{} {}",
             test.contract_name(),
             test.signature,
-            test.result.kind.gas_used(include_fuzz_test_gas)
+            test.result.kind.gas_used()
         )?;
     }
     Ok(fs::write(path, out)?)
@@ -284,7 +296,7 @@ impl SnapshotDiff {
 /// Compares the set of tests with an existing snapshot
 ///
 /// Returns true all tests match
-fn check(tests: Vec<Test>, include_fuzz_test_gas: bool, snaps: Vec<SnapshotEntry>) -> bool {
+fn check(tests: Vec<Test>, snaps: Vec<SnapshotEntry>) -> bool {
     let snaps = snaps
         .into_iter()
         .map(|s| ((s.contract_name, s.signature), s.gas_used))
@@ -294,7 +306,7 @@ fn check(tests: Vec<Test>, include_fuzz_test_gas: bool, snaps: Vec<SnapshotEntry
         if let Some(target_gas) =
             snaps.get(&(test.contract_name().to_string(), test.signature.clone())).cloned()
         {
-            let source_gas = test.result.kind.gas_used(include_fuzz_test_gas);
+            let source_gas = test.result.kind.gas_used();
             if source_gas.gas() != target_gas.gas() {
                 eprintln!(
                     "Diff in \"{}::{}\": consumed \"{}\" gas, expected \"{}\" gas ",
@@ -318,11 +330,7 @@ fn check(tests: Vec<Test>, include_fuzz_test_gas: bool, snaps: Vec<SnapshotEntry
 }
 
 /// Compare the set of tests with an existing snapshot
-fn diff(
-    tests: Vec<Test>,
-    include_fuzz_test_gas: bool,
-    snaps: Vec<SnapshotEntry>,
-) -> eyre::Result<()> {
+fn diff(tests: Vec<Test>, snaps: Vec<SnapshotEntry>) -> eyre::Result<()> {
     let snaps = snaps
         .into_iter()
         .map(|s| ((s.contract_name, s.signature), s.gas_used))
@@ -340,7 +348,7 @@ fn diff(
             })?;
 
         diffs.push(SnapshotDiff {
-            source_gas_used: test.result.kind.gas_used(include_fuzz_test_gas),
+            source_gas_used: test.result.kind.gas_used(),
             signature: test.signature,
             target_gas_used,
         });
@@ -420,12 +428,7 @@ mod tests {
             SnapshotEntry {
                 contract_name: "Test".to_string(),
                 signature: "deposit()".to_string(),
-                gas_used: TestKindGas::Fuzz {
-                    runs: 256,
-                    median: 200,
-                    mean: 100,
-                    include_fuzz_test_gas: true
-                }
+                gas_used: TestKindGas::Fuzz { runs: 256, median: 200, mean: 100 }
             }
         );
     }

cli/src/cmd/forge/test.rs

@@ -130,10 +130,6 @@ pub struct TestArgs {
     #[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,
-
     /// Force the process to exit with code 0, even if the tests fail.
     #[clap(long, env = "FORGE_ALLOW_FAILURE")]
     allow_failure: bool,
@@ -239,7 +235,7 @@ impl Cmd for TestArgs {
                     };
                     debugger.run()?;
 
-                    Ok(TestOutcome::new(results, self.allow_failure, self.include_fuzz_test_gas))
+                    Ok(TestOutcome::new(results, self.allow_failure))
                 }
                 n =>
                     Err(
@@ -255,7 +251,7 @@ impl Cmd for TestArgs {
                 filter,
                 self.json,
                 self.allow_failure,
-                (self.include_fuzz_test_gas, self.gas_report, config.gas_reports),
+                (self.gas_report, config.gas_reports),
             )
         }
     }
@@ -275,7 +271,7 @@ pub struct Test {
 
 impl Test {
     pub fn gas_used(&self) -> u64 {
-        self.result.kind.gas_used(true).gas()
+        self.result.kind.gas_used().gas()
     }
 
     /// Returns the contract name of the artifact id
@@ -293,19 +289,13 @@ impl Test {
 pub struct TestOutcome {
     /// Whether failures are allowed
     pub allow_failure: bool,
-    /// Whether to include fuzz test gas usage in the output
-    pub include_fuzz_test_gas: bool,
     /// Results for each suite of tests `contract -> SuiteResult`
     pub results: BTreeMap<String, SuiteResult>,
 }
 
 impl TestOutcome {
-    fn new(
-        results: BTreeMap<String, SuiteResult>,
-        allow_failure: bool,
-        include_fuzz_test_gas: bool,
-    ) -> Self {
-        Self { results, include_fuzz_test_gas, allow_failure }
+    fn new(results: BTreeMap<String, SuiteResult>, allow_failure: bool) -> Self {
+        Self { results, allow_failure }
     }
 
     /// Iterator over all succeeding tests and their names
@@ -334,14 +324,14 @@ impl TestOutcome {
     }
 
     /// Checks if there are any failures and failures are disallowed
-    pub fn ensure_ok(&self, include_fuzz_test_gas: bool) -> eyre::Result<()> {
+    pub fn ensure_ok(&self) -> eyre::Result<()> {
         if !self.allow_failure {
             let failures = self.failures().count();
             if failures > 0 {
                 println!();
                 println!("Failed tests:");
                 for (name, result) in self.failures() {
-                    short_test_result(name, include_fuzz_test_gas, result);
+                    short_test_result(name, result);
                 }
                 println!();
 
@@ -377,7 +367,7 @@ impl TestOutcome {
     }
 }
 
-fn short_test_result(name: &str, include_fuzz_test_gas: bool, result: &forge::TestResult) {
+fn short_test_result(name: &str, result: &forge::TestResult) {
     let status = if result.success {
         Colour::Green.paint("[PASS]")
     } else {
@@ -397,7 +387,7 @@ fn short_test_result(name: &str, include_fuzz_test_gas: bool, result: &forge::Te
         Colour::Red.paint(txt)
     };
 
-    println!("{} {} {}", status, name, result.kind.gas_used(include_fuzz_test_gas));
+    println!("{} {} {}", status, name, result.kind.gas_used());
 }
 
 /// Runs all the tests
@@ -407,12 +397,12 @@ fn test(
     filter: Filter,
     json: bool,
     allow_failure: bool,
-    (include_fuzz_test_gas, gas_reporting, gas_reports): (bool, bool, Vec<String>),
+    (gas_reporting, gas_reports): (bool, Vec<String>),
 ) -> eyre::Result<TestOutcome> {
     if json {
         let results = runner.test(&filter, None)?;
         println!("{}", serde_json::to_string(&results)?);
-        Ok(TestOutcome::new(results, allow_failure, include_fuzz_test_gas))
+        Ok(TestOutcome::new(results, allow_failure))
     } else {
         let local_identifier = LocalTraceIdentifier::new(&runner.known_contracts);
         let (tx, rx) = channel::<(String, SuiteResult)>();
@@ -429,7 +419,7 @@ fn test(
                 println!("Running {} {} for {}", tests.len(), term, contract_name);
             }
             for (name, result) in &mut tests {
-                short_test_result(name, include_fuzz_test_gas, result);
+                short_test_result(name, result);
 
                 // We only display logs at level 2 and above
                 if verbosity >= 2 {
@@ -492,7 +482,6 @@ fn test(
             let block_outcome = TestOutcome::new(
                 [(contract_name.clone(), suite_result.clone())].into(),
                 allow_failure,
-                include_fuzz_test_gas,
             );
             println!("{}", block_outcome.summary());
             results.insert(contract_name, suite_result);
@@ -505,6 +494,6 @@ fn test(
         // reattach the thread
         let _ = handle.join();
 
-        Ok(TestOutcome::new(results, allow_failure, include_fuzz_test_gas))
+        Ok(TestOutcome::new(results, allow_failure))
     }
 }

cli/src/forge.rs

@@ -20,9 +20,8 @@ fn main() -> eyre::Result<()> {
             if cmd.build_args().is_watch() {
                 utils::block_on(watch::watch_test(cmd))?;
             } else {
-                let include_fuzz_test_gas = cmd.include_fuzz_test_gas;
                 let outcome = cmd.run()?;
-                outcome.ensure_ok(include_fuzz_test_gas)?;
+                outcome.ensure_ok()?;
             }
         }
         Subcommands::Bind(cmd) => {

forge/src/runner.rs

@@ -83,7 +83,7 @@ impl TestResult {
 #[derive(Debug, Clone, Eq, PartialEq)]
 pub enum TestKindGas {
     Standard(u64),
-    Fuzz { runs: usize, include_fuzz_test_gas: bool, mean: u64, median: u64 },
+    Fuzz { runs: usize, mean: u64, median: u64 },
 }
 
 impl fmt::Display for TestKindGas {
@@ -92,12 +92,8 @@ impl fmt::Display for TestKindGas {
             TestKindGas::Standard(gas) => {
                 write!(f, "(gas: {})", gas)
             }
-            TestKindGas::Fuzz { runs, include_fuzz_test_gas, mean, median } => {
-                if *include_fuzz_test_gas {
-                    write!(f, "(runs: {}, μ: {}, ~: {})", runs, mean, median)
-                } else {
-                    write!(f, "(runs: {})", runs)
-                }
+            TestKindGas::Fuzz { runs, mean, median } => {
+                write!(f, "(runs: {}, μ: {}, ~: {})", runs, mean, median)
             }
         }
     }
@@ -127,12 +123,11 @@ pub enum TestKind {
 
 impl TestKind {
     /// The gas consumed by this test
-    pub fn gas_used(&self, include_fuzz_test_gas: bool) -> TestKindGas {
+    pub fn gas_used(&self) -> TestKindGas {
         match self {
             TestKind::Standard(gas) => TestKindGas::Standard(*gas),
             TestKind::Fuzz(fuzzed) => TestKindGas::Fuzz {
                 runs: fuzzed.cases().len(),
-                include_fuzz_test_gas,
                 median: fuzzed.median_gas(false),
                 mean: fuzzed.mean_gas(false),
             },