Cleanup after review

Author: grandizzy

crates/evm/evm/src/executors/mod.rs

@@ -1115,6 +1115,7 @@ impl FuzzTestTimer {
 }
 
 /// Helper struct to enable fail fast behavior: when one test fails, all other tests stop early.
+#[derive(Clone)]
 pub struct FailFast {
     /// Shared atomic flag set to `true` when a failure occurs.
     /// None if fail-fast is disabled.

crates/forge/src/cmd/test/mod.rs

@@ -355,6 +355,7 @@ impl TestArgs {
             .sender(evm_opts.sender)
             .with_fork(evm_opts.get_fork(&config, env.clone()))
             .enable_isolation(evm_opts.isolate)
+            .fail_fast(self.fail_fast)
             .odyssey(evm_opts.odyssey)
             .build::<MultiCompiler>(project_root, &output, env, evm_opts)?;
 
@@ -509,10 +510,9 @@ impl TestArgs {
         let (tx, rx) = channel::<(String, SuiteResult)>();
         let timer = Instant::now();
         let show_progress = config.show_progress;
-        let fail_fast = self.fail_fast;
         let handle = tokio::task::spawn_blocking({
             let filter = filter.clone();
-            move || runner.test(&filter, tx, show_progress, fail_fast)
+            move || runner.test(&filter, tx, show_progress)
         });
 
         // Set up trace identifiers.

crates/forge/src/multi_runner.rs

@@ -153,7 +153,7 @@ impl MultiContractRunner {
         filter: &dyn TestFilter,
     ) -> Result<impl Iterator<Item = (String, SuiteResult)>> {
         let (tx, rx) = mpsc::channel();
-        self.test(filter, tx, false, false)?;
+        self.test(filter, tx, false)?;
         Ok(rx.into_iter())
     }
 
@@ -168,7 +168,6 @@ impl MultiContractRunner {
         filter: &dyn TestFilter,
         tx: mpsc::Sender<(String, SuiteResult)>,
         show_progress: bool,
-        fail_fast: bool,
     ) -> Result<()> {
         let tokio_handle = tokio::runtime::Handle::current();
         trace!("running all tests");
@@ -186,8 +185,6 @@ impl MultiContractRunner {
             find_time,
         );
 
-        let fail_fast = FailFast::new(fail_fast);
-
         if show_progress {
             let tests_progress = TestsProgress::new(contracts.len(), rayon::current_num_threads());
             // Collect test suite results to stream at the end of test run.
@@ -204,7 +201,6 @@ impl MultiContractRunner {
                         filter,
                         &tokio_handle,
                         Some(&tests_progress),
-                        &fail_fast,
                     );
 
                     tests_progress
@@ -224,16 +220,14 @@ impl MultiContractRunner {
         } else {
             contracts.par_iter().for_each(|&(id, contract)| {
                 let _guard = tokio_handle.enter();
-                let result =
-                    self.run_test_suite(id, contract, &db, filter, &tokio_handle, None, &fail_fast);
+                let result = self.run_test_suite(id, contract, &db, filter, &tokio_handle, None);
                 let _ = tx.send((id.identifier(), result));
             })
         }
 
         Ok(())
     }
 
-    #[allow(clippy::too_many_arguments)]
     fn run_test_suite(
         &self,
         artifact_id: &ArtifactId,
@@ -242,7 +236,6 @@ impl MultiContractRunner {
         filter: &dyn TestFilter,
         tokio_handle: &tokio::runtime::Handle,
         progress: Option<&TestsProgress>,
-        fail_fast: &FailFast,
     ) -> SuiteResult {
         let identifier = artifact_id.identifier();
         let mut span_name = identifier.as_str();
@@ -266,7 +259,7 @@ impl MultiContractRunner {
             span,
             self,
         );
-        let r = runner.run_tests(filter, fail_fast);
+        let r = runner.run_tests(filter);
 
         debug!(duration=?r.duration, "executed all tests in contract");
 
@@ -303,6 +296,8 @@ pub struct TestRunnerConfig {
     pub isolation: bool,
     /// Whether to enable Odyssey features.
     pub odyssey: bool,
+    /// Whether to exit early on test failure.
+    pub fail_fast: FailFast,
 }
 
 impl TestRunnerConfig {
@@ -411,6 +406,8 @@ pub struct MultiContractRunnerBuilder {
     pub isolation: bool,
     /// Whether to enable Odyssey features.
     pub odyssey: bool,
+    /// Whether to exit early on test failure.
+    pub fail_fast: bool,
 }
 
 impl MultiContractRunnerBuilder {
@@ -426,6 +423,7 @@ impl MultiContractRunnerBuilder {
             isolation: Default::default(),
             decode_internal: Default::default(),
             odyssey: Default::default(),
+            fail_fast: false,
         }
     }
 
@@ -464,6 +462,11 @@ impl MultiContractRunnerBuilder {
         self
     }
 
+    pub fn fail_fast(mut self, fail_fast: bool) -> Self {
+        self.fail_fast = fail_fast;
+        self
+    }
+
     pub fn enable_isolation(mut self, enable: bool) -> Self {
         self.isolation = enable;
         self
@@ -545,15 +548,14 @@ impl MultiContractRunnerBuilder {
                 env,
                 spec_id: self.evm_spec.unwrap_or_else(|| self.config.evm_spec_id()),
                 sender: self.sender.unwrap_or(self.config.sender),
-
                 line_coverage: self.line_coverage,
                 debug: self.debug,
                 decode_internal: self.decode_internal,
                 inline_config: Arc::new(InlineConfig::new_parsed(output, &self.config)?),
                 isolation: self.isolation,
                 odyssey: self.odyssey,
-
                 config: self.config,
+                fail_fast: FailFast::new(self.fail_fast),
             },
         })
     }

crates/forge/src/runner.rs

@@ -18,7 +18,7 @@ use foundry_evm::{
     constants::CALLER,
     decode::RevertDecoder,
     executors::{
-        CallResult, EvmError, Executor, FailFast, ITest, RawCallResult,
+        CallResult, EvmError, Executor, ITest, RawCallResult,
         fuzz::FuzzedExecutor,
         invariant::{
             InvariantExecutor, InvariantFuzzError, check_sequence, replay_error, replay_run,
@@ -279,7 +279,7 @@ impl<'a> ContractRunner<'a> {
     }
 
     /// Runs all tests for a contract whose names match the provided regular expression
-    pub fn run_tests(mut self, filter: &dyn TestFilter, fail_fast: &FailFast) -> SuiteResult {
+    pub fn run_tests(mut self, filter: &dyn TestFilter) -> SuiteResult {
         let start = Instant::now();
         let mut warnings = Vec::new();
 
@@ -401,6 +401,8 @@ impl<'a> ContractRunner<'a> {
             return SuiteResult::new(start.elapsed(), [(instances, fail)].into(), warnings);
         }
 
+        let fail_fast = &self.tcfg.fail_fast;
+
         let test_results = functions
             .par_iter()
             .map(|&func| {
@@ -434,7 +436,6 @@ impl<'a> ContractRunner<'a> {
                     kind,
                     call_after_invariant,
                     identified_contracts.as_ref(),
-                    fail_fast,
                 );
                 res.duration = start.elapsed();
 
@@ -512,7 +513,6 @@ impl<'a> FunctionRunner<'a> {
         kind: TestFunctionKind,
         call_after_invariant: bool,
         identified_contracts: Option<&ContractsByAddress>,
-        fail_fast: &FailFast,
     ) -> TestResult {
         if let Err(e) = self.apply_function_inline_config(func) {
             self.result.single_fail(Some(e.to_string()));
@@ -521,16 +521,15 @@ impl<'a> FunctionRunner<'a> {
 
         match kind {
             TestFunctionKind::UnitTest { .. } => self.run_unit_test(func),
-            TestFunctionKind::FuzzTest { .. } => self.run_fuzz_test(func, fail_fast),
-            TestFunctionKind::TableTest => self.run_table_test(func, fail_fast),
+            TestFunctionKind::FuzzTest { .. } => self.run_fuzz_test(func),
+            TestFunctionKind::TableTest => self.run_table_test(func),
             TestFunctionKind::InvariantTest => {
                 let test_bytecode = &self.cr.contract.bytecode;
                 self.run_invariant_test(
                     func,
                     call_after_invariant,
                     identified_contracts.unwrap(),
                     test_bytecode,
-                    fail_fast,
                 )
             }
             _ => unreachable!(),
@@ -586,7 +585,7 @@ impl<'a> FunctionRunner<'a> {
     /// - `uint256[] public fixtureAmount = [2, 5]`
     /// - `bool[] public fixtureSwap = [true, false]` The `table_test` is then called with the pair
     ///   of args `(2, true)` and `(5, false)`.
-    fn run_table_test(mut self, func: &Function, fail_fast: &FailFast) -> TestResult {
+    fn run_table_test(mut self, func: &Function) -> TestResult {
         // Prepare unit test execution.
         if self.prepare_test(func).is_err() {
             return self.result;
@@ -642,7 +641,7 @@ impl<'a> FunctionRunner<'a> {
         );
 
         for i in 0..fixtures_len {
-            if fail_fast.should_stop() {
+            if self.tcfg.fail_fast.should_stop() {
                 return self.result;
             }
 
@@ -703,7 +702,6 @@ impl<'a> FunctionRunner<'a> {
         call_after_invariant: bool,
         identified_contracts: &ContractsByAddress,
         test_bytecode: &Bytes,
-        fail_fast: &FailFast,
     ) -> TestResult {
         // First, run the test normally to see if it needs to be skipped.
         if let Err(EvmError::Skip(reason)) = self.executor.call(
@@ -818,7 +816,7 @@ impl<'a> FunctionRunner<'a> {
             &self.setup.fuzz_fixtures,
             &self.setup.deployed_libs,
             progress.as_ref(),
-            fail_fast,
+            &self.tcfg.fail_fast,
         ) {
             Ok(x) => x,
             Err(e) => {
@@ -929,7 +927,7 @@ impl<'a> FunctionRunner<'a> {
     /// (therefore the fuzz test will use the modified state).
     /// State modifications of before test txes and fuzz test are discarded after test ends,
     /// similar to `eth_call`.
-    fn run_fuzz_test(mut self, func: &Function, fail_fast: &FailFast) -> TestResult {
+    fn run_fuzz_test(mut self, func: &Function) -> TestResult {
         // Prepare fuzz test execution.
         if self.prepare_test(func).is_err() {
             return self.result;
@@ -969,7 +967,7 @@ impl<'a> FunctionRunner<'a> {
             self.address,
             &self.cr.mcr.revert_decoder,
             progress.as_ref(),
-            fail_fast,
+            &self.tcfg.fail_fast,
         ) {
             Ok(x) => x,
             Err(e) => {