refactor(forge/test): cache initial executor, don't clone options (#7286

)

* refactor(forge/test): cache initial executor, don't clone options

* chore: clippy

* fix: don't share Db and state

* fix: don't even share the builder (???)

* fix: fuzz tests must also start with test

* chore: simplify filtering

* fix: filter

* fix: filter 2

* chore: comment, logs

crates/common/src/compile.rs

@@ -19,6 +19,7 @@ use std::{
     path::{Path, PathBuf},
     result,
     str::FromStr,
+    time::Instant,
 };
 
 /// Builder type to configure how to compile a project.
@@ -185,7 +186,7 @@ impl ProjectCompiler {
         let output = foundry_compilers::report::with_scoped(&reporter, || {
             tracing::debug!("compiling project");
 
-            let timer = std::time::Instant::now();
+            let timer = Instant::now();
             let r = f();
             let elapsed = timer.elapsed();
 

crates/evm/evm/src/executors/builder.rs

@@ -10,7 +10,7 @@ use revm::primitives::{Env, SpecId};
 ///
 /// [`Cheatcodes`]: super::inspector::Cheatcodes
 /// [`InspectorStack`]: super::inspector::InspectorStack
-#[derive(Debug)]
+#[derive(Clone, Debug)]
 #[must_use = "builders do nothing unless you call `build` on them"]
 pub struct ExecutorBuilder {
     /// The configuration used to build an [InspectorStack].

crates/forge/bin/cmd/coverage.rs

@@ -313,9 +313,7 @@ impl CoverageArgs {
         let known_contracts = runner.known_contracts.clone();
         let filter = self.filter;
         let (tx, rx) = channel::<(String, SuiteResult)>();
-        let handle = tokio::task::spawn(async move {
-            runner.test(&filter, tx, runner.test_options.clone()).await
-        });
+        let handle = tokio::task::spawn(async move { runner.test(&filter, tx).await });
 
         // Add hit data to the coverage report
         let data = rx

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

@@ -200,7 +200,7 @@ impl TestArgs {
             .sender(evm_opts.sender)
             .with_fork(evm_opts.get_fork(&config, env.clone()))
             .with_cheats_config(CheatsConfig::new(&config, evm_opts.clone(), None))
-            .with_test_options(test_options.clone())
+            .with_test_options(test_options)
             .enable_isolation(evm_opts.isolate)
             .build(project_root, output, env, evm_opts)?;
 
@@ -215,7 +215,7 @@ impl TestArgs {
             *test_pattern = Some(debug_test_pattern.clone());
         }
 
-        let outcome = self.run_tests(runner, config, verbosity, &filter, test_options).await?;
+        let outcome = self.run_tests(runner, config, verbosity, &filter).await?;
 
         if should_debug {
             // There is only one test.
@@ -250,15 +250,14 @@ impl TestArgs {
         config: Config,
         verbosity: u8,
         filter: &ProjectPathsAwareFilter,
-        test_options: TestOptions,
     ) -> eyre::Result<TestOutcome> {
         if self.list {
             return list(runner, filter, self.json);
         }
 
         trace!(target: "forge::test", "running all tests");
 
-        let num_filtered = runner.matching_test_function_count(filter);
+        let num_filtered = runner.matching_test_functions(filter).count();
         if num_filtered == 0 {
             println!();
             if filter.is_empty() {
@@ -273,7 +272,8 @@ impl TestArgs {
                 // Try to suggest a test when there's no match
                 if let Some(test_pattern) = &filter.args().test_pattern {
                     let test_name = test_pattern.as_str();
-                    let candidates = runner.get_tests(filter);
+                    // Filter contracts but not test functions.
+                    let candidates = runner.all_test_functions(filter).map(|f| &f.name);
                     if let Some(suggestion) = utils::did_you_mean(test_name, candidates).pop() {
                         println!("\nDid you mean `{suggestion}`?");
                     }
@@ -289,7 +289,7 @@ impl TestArgs {
         }
 
         if self.json {
-            let results = runner.test_collect(filter, test_options).await;
+            let results = runner.test_collect(filter).await;
             println!("{}", serde_json::to_string(&results)?);
             return Ok(TestOutcome::new(results, self.allow_failure));
         }
@@ -305,7 +305,7 @@ impl TestArgs {
         let timer = Instant::now();
         let handle = tokio::task::spawn({
             let filter = filter.clone();
-            async move { runner.test(&filter, tx, test_options).await }
+            async move { runner.test(&filter, tx).await }
         });
 
         let mut gas_report =

crates/forge/src/lib.rs

@@ -1,13 +1,11 @@
 #[macro_use]
 extern crate tracing;
 
-use alloy_primitives::B256;
 use foundry_compilers::ProjectCompileOutput;
 use foundry_config::{
     validate_profiles, Config, FuzzConfig, InlineConfig, InlineConfigError, InlineConfigParser,
     InvariantConfig, NatSpec,
 };
-
 use proptest::test_runner::{RngAlgorithm, TestRng, TestRunner};
 use std::path::Path;
 
@@ -146,20 +144,20 @@ impl TestOptions {
 
     pub fn fuzzer_with_cases(&self, cases: u32) -> TestRunner {
         // TODO: Add Options to modify the persistence
-        let cfg = proptest::test_runner::Config {
+        let config = proptest::test_runner::Config {
             failure_persistence: None,
             cases,
             max_global_rejects: self.fuzz.max_test_rejects,
             ..Default::default()
         };
 
-        if let Some(ref fuzz_seed) = self.fuzz.seed {
-            trace!(target: "forge::test", "building deterministic fuzzer with seed {}", fuzz_seed);
-            let rng = TestRng::from_seed(RngAlgorithm::ChaCha, &B256::from(*fuzz_seed).0);
-            TestRunner::new_with_rng(cfg, rng)
+        if let Some(seed) = &self.fuzz.seed {
+            trace!(target: "forge::test", %seed, "building deterministic fuzzer");
+            let rng = TestRng::from_seed(RngAlgorithm::ChaCha, &seed.to_be_bytes::<32>());
+            TestRunner::new_with_rng(config, rng)
         } else {
             trace!(target: "forge::test", "building stochastic fuzzer");
-            TestRunner::new(cfg)
+            TestRunner::new(config)
         }
     }
 }

crates/forge/src/multi_runner.rs

@@ -26,6 +26,7 @@ use std::{
     fmt::Debug,
     path::Path,
     sync::{mpsc, Arc},
+    time::Instant,
 };
 
 pub type DeployableContracts = BTreeMap<ArtifactId, (JsonAbi, Bytes, Vec<Bytes>)>;
@@ -65,57 +66,47 @@ pub struct MultiContractRunner {
 }
 
 impl MultiContractRunner {
-    /// Returns the number of matching tests
-    pub fn matching_test_function_count(&self, filter: &dyn TestFilter) -> usize {
-        self.matching_test_functions(filter).count()
+    /// Returns an iterator over all contracts that match the filter.
+    pub fn matching_contracts<'a>(
+        &'a self,
+        filter: &'a dyn TestFilter,
+    ) -> impl Iterator<Item = (&ArtifactId, &(JsonAbi, Bytes, Vec<Bytes>))> {
+        self.contracts.iter().filter(|&(id, (abi, _, _))| matches_contract(id, abi, filter))
     }
 
-    /// Returns all test functions matching the filter
+    /// Returns an iterator over all test functions that match the filter.
     pub fn matching_test_functions<'a>(
         &'a self,
         filter: &'a dyn TestFilter,
     ) -> impl Iterator<Item = &Function> {
-        self.contracts
-            .iter()
-            .filter(|(id, _)| filter.matches_path(&id.source) && filter.matches_contract(&id.name))
-            .flat_map(|(_, (abi, _, _))| {
-                abi.functions().filter(|func| filter.matches_test(&func.signature()))
-            })
+        self.matching_contracts(filter)
+            .flat_map(|(_, (abi, _, _))| abi.functions())
+            .filter(|func| is_matching_test(func, filter))
     }
 
-    /// Get an iterator over all test contract functions that matches the filter path and contract
-    /// name
-    fn filtered_tests<'a>(&'a self, filter: &'a dyn TestFilter) -> impl Iterator<Item = &Function> {
+    /// Returns an iterator over all test functions in contracts that match the filter.
+    pub fn all_test_functions<'a>(
+        &'a self,
+        filter: &'a dyn TestFilter,
+    ) -> impl Iterator<Item = &Function> {
         self.contracts
             .iter()
             .filter(|(id, _)| filter.matches_path(&id.source) && filter.matches_contract(&id.name))
             .flat_map(|(_, (abi, _, _))| abi.functions())
-    }
-
-    /// Get all test names matching the filter
-    pub fn get_tests(&self, filter: &dyn TestFilter) -> Vec<String> {
-        self.filtered_tests(filter)
-            .map(|func| func.name.clone())
-            .filter(|name| name.is_test())
-            .collect()
+            .filter(|func| func.is_test() || func.is_invariant_test())
     }
 
     /// Returns all matching tests grouped by contract grouped by file (file -> (contract -> tests))
     pub fn list(&self, filter: &dyn TestFilter) -> BTreeMap<String, BTreeMap<String, Vec<String>>> {
-        self.contracts
-            .iter()
-            .filter(|(id, _)| filter.matches_path(&id.source) && filter.matches_contract(&id.name))
-            .filter(|(_, (abi, _, _))| abi.functions().any(|func| filter.matches_test(&func.name)))
+        self.matching_contracts(filter)
             .map(|(id, (abi, _, _))| {
                 let source = id.source.as_path().display().to_string();
                 let name = id.name.clone();
                 let tests = abi
                     .functions()
-                    .filter(|func| func.name.is_test())
-                    .filter(|func| filter.matches_test(&func.signature()))
+                    .filter(|func| is_matching_test(func, filter))
                     .map(|func| func.name.clone())
                     .collect::<Vec<_>>();
-
                 (source, name, tests)
             })
             .fold(BTreeMap::new(), |mut acc, (source, name, tests)| {
@@ -129,12 +120,8 @@ impl MultiContractRunner {
     /// The same as [`test`](Self::test), but returns the results instead of streaming them.
     ///
     /// Note that this method returns only when all tests have been executed.
-    pub async fn test_collect(
-        &mut self,
-        filter: &dyn TestFilter,
-        test_options: TestOptions,
-    ) -> BTreeMap<String, SuiteResult> {
-        self.test_iter(filter, test_options).await.collect()
+    pub async fn test_collect(&mut self, filter: &dyn TestFilter) -> BTreeMap<String, SuiteResult> {
+        self.test_iter(filter).await.collect()
     }
 
     /// Executes _all_ tests that match the given `filter`.
@@ -145,10 +132,9 @@ impl MultiContractRunner {
     pub async fn test_iter(
         &mut self,
         filter: &dyn TestFilter,
-        test_options: TestOptions,
     ) -> impl Iterator<Item = (String, SuiteResult)> {
         let (tx, rx) = mpsc::channel();
-        self.test(filter, tx, test_options).await;
+        self.test(filter, tx).await;
         rx.into_iter()
     }
 
@@ -158,50 +144,40 @@ impl MultiContractRunner {
     /// before executing all contracts and their tests in _parallel_.
     ///
     /// Each Executor gets its own instance of the `Backend`.
-    pub async fn test(
-        &mut self,
-        filter: &dyn TestFilter,
-        stream_result: mpsc::Sender<(String, SuiteResult)>,
-        test_options: TestOptions,
-    ) {
+    pub async fn test(&mut self, filter: &dyn TestFilter, tx: mpsc::Sender<(String, SuiteResult)>) {
         trace!("running all tests");
 
-        // the db backend that serves all the data, each contract gets its own instance
+        // The DB backend that serves all the data.
         let db = Backend::spawn(self.fork.take()).await;
-
-        self.contracts
-            .par_iter()
-            .filter(|(id, _)| filter.matches_path(&id.source) && filter.matches_contract(&id.name))
-            .filter(|(_, (abi, _, _))| abi.functions().any(|func| filter.matches_test(&func.name)))
-            .for_each_with(stream_result, |stream_result, (id, (abi, deploy_code, libs))| {
-                let executor = ExecutorBuilder::new()
-                    .inspectors(|stack| {
-                        stack
-                            .cheatcodes(self.cheats_config.clone())
-                            .trace(self.evm_opts.verbosity >= 3 || self.debug)
-                            .debug(self.debug)
-                            .coverage(self.coverage)
-                            .enable_isolation(self.isolation)
-                    })
-                    .spec(self.evm_spec)
-                    .gas_limit(self.evm_opts.gas_limit())
-                    .build(self.env.clone(), db.clone());
-                let identifier = id.identifier();
-                trace!(contract=%identifier, "start executing all tests in contract");
-
-                let result = self.run_tests(
-                    &identifier,
-                    abi,
-                    executor,
-                    deploy_code.clone(),
-                    libs,
-                    filter,
-                    test_options.clone(),
-                );
-                trace!(contract=?identifier, "executed all tests in contract");
-
-                let _ = stream_result.send((identifier, result));
+        let executor = ExecutorBuilder::new()
+            .inspectors(|stack| {
+                stack
+                    .cheatcodes(self.cheats_config.clone())
+                    .trace(self.evm_opts.verbosity >= 3 || self.debug)
+                    .debug(self.debug)
+                    .coverage(self.coverage)
+                    .enable_isolation(self.isolation)
             })
+            .spec(self.evm_spec)
+            .gas_limit(self.evm_opts.gas_limit())
+            .build(self.env.clone(), db);
+
+        let find_timer = Instant::now();
+        let contracts = self.matching_contracts(filter).collect::<Vec<_>>();
+        let find_time = find_timer.elapsed();
+        debug!(
+            "Found {} test contracts out of {} in {:?}",
+            contracts.len(),
+            self.contracts.len(),
+            find_time,
+        );
+
+        contracts.par_iter().for_each_with(tx, |tx, &(id, (abi, deploy_code, libs))| {
+            let identifier = id.identifier();
+            let executor = executor.clone();
+            let result = self.run_tests(&identifier, abi, executor, deploy_code, libs, filter);
+            let _ = tx.send((identifier, result));
+        })
     }
 
     #[allow(clippy::too_many_arguments)]
@@ -210,20 +186,17 @@ impl MultiContractRunner {
         name: &str,
         contract: &JsonAbi,
         executor: Executor,
-        deploy_code: Bytes,
+        deploy_code: &Bytes,
         libs: &[Bytes],
         filter: &dyn TestFilter,
-        test_options: TestOptions,
     ) -> SuiteResult {
-        let span = info_span!("run_tests");
-        if !span.is_disabled() {
-            if enabled!(tracing::Level::TRACE) {
-                span.record("contract", name);
-            } else {
-                span.record("contract", get_contract_name(name));
-            }
+        let mut span_name = name;
+        if !enabled!(tracing::Level::TRACE) {
+            span_name = get_contract_name(span_name);
         }
-        let _guard = span.enter();
+        let _guard = info_span!("run_tests", name = span_name).entered();
+
+        debug!("start executing all tests in contract");
 
         let runner = ContractRunner::new(
             name,
@@ -236,7 +209,11 @@ impl MultiContractRunner {
             libs,
             self.debug,
         );
-        runner.run_tests(filter, test_options, Some(&self.known_contracts))
+        let r = runner.run_tests(filter, &self.test_options, Some(&self.known_contracts));
+
+        debug!(duration=?r.duration, "executed all tests in contract");
+
+        r
     }
 }
 
@@ -395,3 +372,13 @@ impl MultiContractRunnerBuilder {
         })
     }
 }
+
+fn matches_contract(id: &ArtifactId, abi: &JsonAbi, filter: &dyn TestFilter) -> bool {
+    (filter.matches_path(&id.source) && filter.matches_contract(&id.name)) &&
+        abi.functions().any(|func| is_matching_test(func, filter))
+}
+
+/// Returns `true` if the function is a test function that matches the given filter.
+pub(crate) fn is_matching_test(func: &Function, filter: &dyn TestFilter) -> bool {
+    (func.is_test() || func.is_invariant_test()) && filter.matches_test(&func.signature())
+}