From 1e4a7b66357f61560426d7156de68d67d1b9440a Mon Sep 17 00:00:00 2001 From: DaniPopes <57450786+DaniPopes@users.noreply.github.com> Date: Fri, 1 Mar 2024 09:40:39 +0100 Subject: [PATCH 1/9] refactor(forge/test): cache initial executor, don't clone options --- crates/common/src/compile.rs | 3 +- crates/forge/bin/cmd/coverage.rs | 4 +- crates/forge/bin/cmd/test/mod.rs | 9 +- crates/forge/src/multi_runner.rs | 118 ++++++++++++------------- crates/forge/src/runner.rs | 10 +-- crates/forge/tests/it/cheats.rs | 18 ++-- crates/forge/tests/it/config.rs | 52 ++--------- crates/forge/tests/it/core.rs | 38 ++++---- crates/forge/tests/it/fork.rs | 16 ++-- crates/forge/tests/it/fuzz.rs | 57 +++++------- crates/forge/tests/it/inline.rs | 13 +-- crates/forge/tests/it/invariant.rs | 120 +++++++------------------- crates/forge/tests/it/test_helpers.rs | 41 ++++++++- 13 files changed, 209 insertions(+), 290 deletions(-) diff --git a/crates/common/src/compile.rs b/crates/common/src/compile.rs index 1132e99bb509..935e098eeefe 100644 --- a/crates/common/src/compile.rs +++ b/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(); diff --git a/crates/forge/bin/cmd/coverage.rs b/crates/forge/bin/cmd/coverage.rs index 0c1232d7a8fa..718d1079058a 100644 --- a/crates/forge/bin/cmd/coverage.rs +++ b/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 diff --git a/crates/forge/bin/cmd/test/mod.rs b/crates/forge/bin/cmd/test/mod.rs index b81a5b362edc..8202cb650706 100644 --- a/crates/forge/bin/cmd/test/mod.rs +++ b/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,7 +250,6 @@ impl TestArgs { config: Config, verbosity: u8, filter: &ProjectPathsAwareFilter, - test_options: TestOptions, ) -> eyre::Result { if self.list { return list(runner, filter, self.json); @@ -289,7 +288,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 +304,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 = diff --git a/crates/forge/src/multi_runner.rs b/crates/forge/src/multi_runner.rs index 699df0da0566..02f3d8ba8b73 100644 --- a/crates/forge/src/multi_runner.rs +++ b/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)>; @@ -129,12 +130,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 { - self.test_iter(filter, test_options).await.collect() + pub async fn test_collect(&mut self, filter: &dyn TestFilter) -> BTreeMap { + self.test_iter(filter).await.collect() } /// Executes _all_ tests that match the given `filter`. @@ -145,10 +142,9 @@ impl MultiContractRunner { pub async fn test_iter( &mut self, filter: &dyn TestFilter, - test_options: TestOptions, ) -> impl Iterator { let (tx, rx) = mpsc::channel(); - self.test(filter, tx, test_options).await; + self.test(filter, tx).await; rx.into_iter() } @@ -158,50 +154,44 @@ 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 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 + .contracts + .iter() + .filter(|&(id, (abi, ..))| matches_contract(id, abi, filter)) + .collect::>(); + 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 result = + self.run_tests(&identifier, abi, executor.clone(), deploy_code, libs, filter); + let _ = tx.send((identifier, result)); + }) } #[allow(clippy::too_many_arguments)] @@ -210,20 +200,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 +223,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 +386,14 @@ 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.matches_test(&func.signature()))) +} + +/// Returns `true` if the function is a test function that matches the given filter. +pub(crate) fn is_matching_test(func: &Function, match_sig: impl FnOnce() -> bool) -> bool { + (func.is_test() || func.is_fuzz_test() || func.is_invariant_test()) && match_sig() +} diff --git a/crates/forge/src/runner.rs b/crates/forge/src/runner.rs index f94da089c536..b01fd8363987 100644 --- a/crates/forge/src/runner.rs +++ b/crates/forge/src/runner.rs @@ -1,6 +1,7 @@ //! The Forge test runner. use crate::{ + multi_runner::is_matching_test, result::{SuiteResult, TestKind, TestResult, TestSetup, TestStatus}, TestFilter, TestOptions, }; @@ -40,7 +41,7 @@ pub struct ContractRunner<'a> { /// Library contracts to be deployed before the test contract pub predeploy_libs: &'a [Bytes], /// The deployed contract's code - pub code: Bytes, + pub code: &'a Bytes, /// The test contract's ABI pub contract: &'a JsonAbi, /// Revert decoder. Contains all known errors. @@ -59,7 +60,7 @@ impl<'a> ContractRunner<'a> { name: &'a str, executor: Executor, contract: &'a JsonAbi, - code: Bytes, + code: &'a Bytes, initial_balance: U256, sender: Option
, revert_decoder: &'a RevertDecoder, @@ -185,7 +186,7 @@ impl<'a> ContractRunner<'a> { pub fn run_tests( mut self, filter: &dyn TestFilter, - test_options: TestOptions, + test_options: &TestOptions, known_contracts: Option<&ContractsByArtifact>, ) -> SuiteResult { info!("starting tests"); @@ -259,9 +260,8 @@ impl<'a> ContractRunner<'a> { let functions = self .contract .functions() - .filter(|func| func.is_test() || func.is_invariant_test()) .map(|func| (func.signature(), func)) - .filter(|(sig, _func)| filter.matches_test(sig)) + .filter(|(sig, func)| is_matching_test(func, || filter.matches_test(&sig))) .collect::>(); let find_time = find_timer.elapsed(); debug!( diff --git a/crates/forge/tests/it/cheats.rs b/crates/forge/tests/it/cheats.rs index 078361fab8e0..2959014bc0e1 100644 --- a/crates/forge/tests/it/cheats.rs +++ b/crates/forge/tests/it/cheats.rs @@ -10,15 +10,17 @@ use foundry_test_utils::Filter; /// Executes all cheat code tests but not fork cheat codes #[tokio::test(flavor = "multi_thread")] async fn test_cheats_local() { - let mut config = Config::with_root(PROJECT.root()); - config.fs_permissions = FsPermissions::new(vec![PathPermission::read_write("./")]); - let runner = runner_with_config(config); - let filter = + let mut filter = Filter::new(".*", ".*", &format!(".*cheats{RE_PATH_SEPARATOR}*")).exclude_paths("Fork"); - // on windows exclude ffi tests since no echo and file test that expect a certain file path - #[cfg(windows)] - let filter = filter.exclude_tests("(Ffi|File|Line|Root)"); + // Exclude FFI tests on Windows because no `echo`, and file tests that expect certain file paths + if cfg!(windows) { + filter = filter.exclude_tests("(Ffi|File|Line|Root)"); + } + + let mut config = Config::with_root(PROJECT.root()); + config.fs_permissions = FsPermissions::new(vec![PathPermission::read_write("./")]); + let runner = runner_with_config(config).await; - TestConfig::with_filter(runner.await, filter).run().await; + TestConfig::with_filter(runner, filter).run().await; } diff --git a/crates/forge/tests/it/config.rs b/crates/forge/tests/it/config.rs index 542f9a88e3d5..c56a626c49a8 100644 --- a/crates/forge/tests/it/config.rs +++ b/crates/forge/tests/it/config.rs @@ -1,13 +1,12 @@ //! Test config. -use crate::test_helpers::{COMPILED, EVM_OPTS, PROJECT}; +use crate::test_helpers::{COMPILED, EVM_OPTS, PROJECT, TEST_OPTS}; use forge::{ result::{SuiteResult, TestStatus}, - MultiContractRunner, MultiContractRunnerBuilder, TestOptions, TestOptionsBuilder, + MultiContractRunner, MultiContractRunnerBuilder, }; use foundry_config::{ - fs_permissions::PathPermission, Config, FsPermissions, FuzzConfig, FuzzDictionaryConfig, - InvariantConfig, RpcEndpoint, RpcEndpoints, + fs_permissions::PathPermission, Config, FsPermissions, RpcEndpoint, RpcEndpoints, }; use foundry_evm::{ decode::decode_console_logs, @@ -25,7 +24,6 @@ pub struct TestConfig { pub runner: MultiContractRunner, pub should_fail: bool, pub filter: Filter, - pub opts: TestOptions, } impl TestConfig { @@ -39,7 +37,7 @@ impl TestConfig { pub fn with_filter(runner: MultiContractRunner, filter: Filter) -> Self { init_tracing(); - Self { runner, should_fail: false, filter, opts: test_opts() } + Self { runner, should_fail: false, filter } } pub fn evm_spec(mut self, spec: SpecId) -> Self { @@ -58,7 +56,7 @@ impl TestConfig { /// Executes the test runner pub async fn test(&mut self) -> BTreeMap { - self.runner.test_collect(&self.filter, self.opts.clone()).await + self.runner.test_collect(&self.filter).await } pub async fn run(&mut self) { @@ -110,41 +108,6 @@ impl TestConfig { } } -/// Returns the [`TestOptions`] used by the tests. -pub fn test_opts() -> TestOptions { - TestOptionsBuilder::default() - .fuzz(FuzzConfig { - runs: 256, - max_test_rejects: 65536, - seed: None, - dictionary: FuzzDictionaryConfig { - include_storage: true, - include_push_bytes: true, - dictionary_weight: 40, - max_fuzz_dictionary_addresses: 10_000, - max_fuzz_dictionary_values: 10_000, - }, - }) - .invariant(InvariantConfig { - runs: 256, - depth: 15, - fail_on_revert: false, - call_override: false, - dictionary: FuzzDictionaryConfig { - dictionary_weight: 80, - include_storage: true, - include_push_bytes: true, - max_fuzz_dictionary_addresses: 10_000, - max_fuzz_dictionary_values: 10_000, - }, - shrink_sequence: true, - shrink_run_limit: 2usize.pow(18u32), - preserve_state: false, - }) - .build(&COMPILED, &PROJECT.paths.root) - .expect("Config loaded") -} - pub fn manifest_root() -> &'static Path { let mut root = Path::new(env!("CARGO_MANIFEST_DIR")); // need to check here where we're executing the test from, if in `forge` we need to also allow @@ -158,7 +121,9 @@ pub fn manifest_root() -> &'static Path { /// Builds a base runner pub fn base_runner() -> MultiContractRunnerBuilder { init_tracing(); - MultiContractRunnerBuilder::default().sender(EVM_OPTS.sender) + MultiContractRunnerBuilder::default() + .sender(EVM_OPTS.sender) + .with_test_options(TEST_OPTS.clone()) } /// Builds a non-tracing runner @@ -178,7 +143,6 @@ pub async fn runner_with_config(mut config: Config) -> MultiContractRunner { let env = opts.evm_env().await.expect("could not instantiate fork environment"); let output = COMPILED.clone(); base_runner() - .with_test_options(test_opts()) .with_cheats_config(CheatsConfig::new(&config, opts.clone(), None)) .sender(config.sender) .build(root, output, env, opts.clone()) diff --git a/crates/forge/tests/it/core.rs b/crates/forge/tests/it/core.rs index 660ab0677785..29ce68c9eedf 100644 --- a/crates/forge/tests/it/core.rs +++ b/crates/forge/tests/it/core.rs @@ -8,8 +8,9 @@ use std::{collections::BTreeMap, env}; #[tokio::test(flavor = "multi_thread")] async fn test_core() { + let filter = Filter::new(".*", ".*", ".*core"); let mut runner = runner().await; - let results = runner.test_collect(&Filter::new(".*", ".*", ".*core"), test_opts()).await; + let results = runner.test_collect(&filter).await; assert_multiple( &results, @@ -77,8 +78,9 @@ async fn test_core() { #[tokio::test(flavor = "multi_thread")] async fn test_linking() { + let filter = Filter::new(".*", ".*", ".*linking"); let mut runner = runner().await; - let results = runner.test_collect(&Filter::new(".*", ".*", ".*linking"), test_opts()).await; + let results = runner.test_collect(&filter).await; assert_multiple( &results, @@ -110,8 +112,9 @@ async fn test_linking() { #[tokio::test(flavor = "multi_thread")] async fn test_logs() { + let filter = Filter::new(".*", ".*", ".*logs"); let mut runner = runner().await; - let results = runner.test_collect(&Filter::new(".*", ".*", ".*logs"), test_opts()).await; + let results = runner.test_collect(&filter).await; assert_multiple( &results, @@ -670,38 +673,31 @@ async fn test_logs() { #[tokio::test(flavor = "multi_thread")] async fn test_env_vars() { - let mut runner = runner().await; - - // test `setEnv` first, and confirm that it can correctly set environment variables, - // so that we can use it in subsequent `env*` tests - let _ = runner.test_collect(&Filter::new("testSetEnv", ".*", ".*"), test_opts()).await; let env_var_key = "_foundryCheatcodeSetEnvTestKey"; let env_var_val = "_foundryCheatcodeSetEnvTestVal"; - let res = env::var(env_var_key); - assert!( - res.is_ok() && res.unwrap() == env_var_val, - "Test `testSetEnv` did not pass as expected. -Reason: `setEnv` failed to set an environment variable `{env_var_key}={env_var_val}`" - ); + env::remove_var(env_var_key); + + let filter = Filter::new("testSetEnv", ".*", ".*"); + let mut runner = runner().await; + let _ = runner.test_collect(&filter).await; + + assert_eq!(env::var(env_var_key).unwrap(), env_var_val); } #[tokio::test(flavor = "multi_thread")] async fn test_doesnt_run_abstract_contract() { + let filter = Filter::new(".*", ".*", ".*Abstract.t.sol".to_string().as_str()); let mut runner = runner().await; - let results = runner - .test_collect( - &Filter::new(".*", ".*", ".*Abstract.t.sol".to_string().as_str()), - test_opts(), - ) - .await; + let results = runner.test_collect(&filter).await; assert!(results.get("core/Abstract.t.sol:AbstractTestBase").is_none()); assert!(results.get("core/Abstract.t.sol:AbstractTest").is_some()); } #[tokio::test(flavor = "multi_thread")] async fn test_trace() { + let filter = Filter::new(".*", ".*", ".*trace"); let mut runner = tracing_runner().await; - let suite_result = runner.test_collect(&Filter::new(".*", ".*", ".*trace"), test_opts()).await; + let suite_result = runner.test_collect(&filter).await; // TODO: This trace test is very basic - it is probably a good candidate for snapshot // testing. diff --git a/crates/forge/tests/it/fork.rs b/crates/forge/tests/it/fork.rs index f34cc59bb3c7..76a06e57b670 100644 --- a/crates/forge/tests/it/fork.rs +++ b/crates/forge/tests/it/fork.rs @@ -11,17 +11,13 @@ use foundry_test_utils::Filter; /// Executes reverting fork test #[tokio::test(flavor = "multi_thread")] async fn test_cheats_fork_revert() { + let filter = Filter::new( + "testNonExistingContractRevert", + ".*", + &format!(".*cheats{RE_PATH_SEPARATOR}Fork"), + ); let mut runner = runner().await; - let suite_result = runner - .test_collect( - &Filter::new( - "testNonExistingContractRevert", - ".*", - &format!(".*cheats{RE_PATH_SEPARATOR}Fork"), - ), - test_opts(), - ) - .await; + let suite_result = runner.test_collect(&filter).await; assert_eq!(suite_result.len(), 1); for (_, SuiteResult { test_results, .. }) in suite_result { diff --git a/crates/forge/tests/it/fuzz.rs b/crates/forge/tests/it/fuzz.rs index 2eefa530d623..011497f4ce47 100644 --- a/crates/forge/tests/it/fuzz.rs +++ b/crates/forge/tests/it/fuzz.rs @@ -8,16 +8,11 @@ use std::collections::BTreeMap; #[tokio::test(flavor = "multi_thread")] async fn test_fuzz() { + let filter = Filter::new(".*", ".*", ".*fuzz/") + .exclude_tests(r"invariantCounter|testIncrement\(address\)|testNeedle\(uint256\)|testSuccessChecker\(uint256\)|testSuccessChecker2\(int256\)|testSuccessChecker3\(uint32\)") + .exclude_paths("invariant"); let mut runner = runner().await; - - let suite_result = runner - .test_collect( - &Filter::new(".*", ".*", ".*fuzz/") - .exclude_tests(r"invariantCounter|testIncrement\(address\)|testNeedle\(uint256\)|testSuccessChecker\(uint256\)|testSuccessChecker2\(int256\)|testSuccessChecker3\(uint32\)") - .exclude_paths("invariant"), - test_opts(), - ) - .await; + let suite_result = runner.test_collect(&filter).await; assert!(!suite_result.is_empty()); @@ -27,15 +22,17 @@ async fn test_fuzz() { "testPositive(uint256)" | "testPositive(int256)" | "testSuccessfulFuzz(uint128,uint128)" | - "testToStringFuzz(bytes32)" => assert!( - result.status == TestStatus::Success, + "testToStringFuzz(bytes32)" => assert_eq!( + result.status, + TestStatus::Success, "Test {} did not pass as expected.\nReason: {:?}\nLogs:\n{}", test_name, result.reason, result.decoded_logs.join("\n") ), - _ => assert!( - result.status == TestStatus::Failure, + _ => assert_eq!( + result.status, + TestStatus::Failure, "Test {} did not fail as expected.\nReason: {:?}\nLogs:\n{}", test_name, result.reason, @@ -48,16 +45,11 @@ async fn test_fuzz() { #[tokio::test(flavor = "multi_thread")] async fn test_successful_fuzz_cases() { + let filter = Filter::new(".*", ".*", ".*fuzz/FuzzPositive") + .exclude_tests(r"invariantCounter|testIncrement\(address\)|testNeedle\(uint256\)") + .exclude_paths("invariant"); let mut runner = runner().await; - - let suite_result = runner - .test_collect( - &Filter::new(".*", ".*", ".*fuzz/FuzzPositive") - .exclude_tests(r"invariantCounter|testIncrement\(address\)|testNeedle\(uint256\)") - .exclude_paths("invariant"), - test_opts(), - ) - .await; + let suite_result = runner.test_collect(&filter).await; assert!(!suite_result.is_empty()); @@ -66,8 +58,9 @@ async fn test_successful_fuzz_cases() { match test_name.as_str() { "testSuccessChecker(uint256)" | "testSuccessChecker2(int256)" | - "testSuccessChecker3(uint32)" => assert!( - result.status == TestStatus::Success, + "testSuccessChecker3(uint32)" => assert_eq!( + result.status, + TestStatus::Success, "Test {} did not pass as expected.\nReason: {:?}\nLogs:\n{}", test_name, result.reason, @@ -84,17 +77,13 @@ async fn test_successful_fuzz_cases() { #[tokio::test(flavor = "multi_thread")] #[ignore] async fn test_fuzz_collection() { + let filter = Filter::new(".*", ".*", ".*fuzz/FuzzCollection.t.sol"); let mut runner = runner().await; - - let mut opts = test_opts(); - opts.invariant.depth = 100; - opts.invariant.runs = 1000; - opts.fuzz.runs = 1000; - opts.fuzz.seed = Some(U256::from(6u32)); - runner.test_options = opts.clone(); - - let results = - runner.test_collect(&Filter::new(".*", ".*", ".*fuzz/FuzzCollection.t.sol"), opts).await; + runner.test_options.invariant.depth = 100; + runner.test_options.invariant.runs = 1000; + runner.test_options.fuzz.runs = 1000; + runner.test_options.fuzz.seed = Some(U256::from(6u32)); + let results = runner.test_collect(&filter).await; assert_multiple( &results, diff --git a/crates/forge/tests/it/inline.rs b/crates/forge/tests/it/inline.rs index 0001bea298da..3503606d7088 100644 --- a/crates/forge/tests/it/inline.rs +++ b/crates/forge/tests/it/inline.rs @@ -13,14 +13,9 @@ use foundry_test_utils::Filter; #[tokio::test(flavor = "multi_thread")] async fn inline_config_run_fuzz() { - let opts = default_test_options(); - let filter = Filter::new(".*", ".*", ".*inline/FuzzInlineConf.t.sol"); - let mut runner = runner().await; - runner.test_options = opts.clone(); - - let result = runner.test_collect(&filter, opts).await; + let result = runner.test_collect(&filter).await; let suite_result: &SuiteResult = result.get("inline/FuzzInlineConf.t.sol:FuzzInlineConf").unwrap(); let test_result: &TestResult = @@ -39,12 +34,10 @@ async fn inline_config_run_fuzz() { async fn inline_config_run_invariant() { const ROOT: &str = "inline/InvariantInlineConf.t.sol"; - let opts = default_test_options(); let filter = Filter::new(".*", ".*", ".*inline/InvariantInlineConf.t.sol"); let mut runner = runner().await; - runner.test_options = opts.clone(); - - let result = runner.test_collect(&filter, opts).await; + runner.test_options = default_test_options(); + let result = runner.test_collect(&filter).await; let suite_result_1 = result.get(&format!("{ROOT}:InvariantInlineConf")).expect("Result exists"); let suite_result_2 = diff --git a/crates/forge/tests/it/invariant.rs b/crates/forge/tests/it/invariant.rs index 5959b0926de7..b5132fefab9f 100644 --- a/crates/forge/tests/it/invariant.rs +++ b/crates/forge/tests/it/invariant.rs @@ -1,6 +1,6 @@ //! Invariant tests. -use crate::config::*; +use crate::{config::*, test_helpers::TEST_OPTS}; use alloy_primitives::U256; use forge::{fuzz::CounterExample, result::TestStatus, TestOptions}; use foundry_test_utils::Filter; @@ -8,14 +8,9 @@ use std::collections::BTreeMap; #[tokio::test(flavor = "multi_thread")] async fn test_invariant() { + let filter = Filter::new(".*", ".*", ".*fuzz/invariant/(target|targetAbi|common)"); let mut runner = runner().await; - - let results = runner - .test_collect( - &Filter::new(".*", ".*", ".*fuzz/invariant/(target|targetAbi|common)"), - test_opts(), - ) - .await; + let results = runner.test_collect(&filter).await; assert_multiple( &results, @@ -148,18 +143,10 @@ async fn test_invariant() { #[tokio::test(flavor = "multi_thread")] async fn test_invariant_override() { + let filter = Filter::new(".*", ".*", ".*fuzz/invariant/common/InvariantReentrancy.t.sol"); let mut runner = runner().await; - - let mut opts = test_opts(); - opts.invariant.call_override = true; - runner.test_options = opts.clone(); - - let results = runner - .test_collect( - &Filter::new(".*", ".*", ".*fuzz/invariant/common/InvariantReentrancy.t.sol"), - opts, - ) - .await; + runner.test_options.invariant.call_override = true; + let results = runner.test_collect(&filter).await; assert_multiple( &results, @@ -172,20 +159,12 @@ async fn test_invariant_override() { #[tokio::test(flavor = "multi_thread")] async fn test_invariant_fail_on_revert() { + let filter = Filter::new(".*", ".*", ".*fuzz/invariant/common/InvariantHandlerFailure.t.sol"); let mut runner = runner().await; - - let mut opts = test_opts(); - opts.invariant.fail_on_revert = true; - opts.invariant.runs = 1; - opts.invariant.depth = 10; - runner.test_options = opts.clone(); - - let results = runner - .test_collect( - &Filter::new(".*", ".*", ".*fuzz/invariant/common/InvariantHandlerFailure.t.sol"), - opts, - ) - .await; + runner.test_options.invariant.fail_on_revert = true; + runner.test_options.invariant.runs = 1; + runner.test_options.invariant.depth = 10; + let results = runner.test_collect(&filter).await; assert_multiple( &results, @@ -205,19 +184,11 @@ async fn test_invariant_fail_on_revert() { #[tokio::test(flavor = "multi_thread")] #[ignore] async fn test_invariant_storage() { + let filter = Filter::new(".*", ".*", ".*fuzz/invariant/storage/InvariantStorageTest.t.sol"); let mut runner = runner().await; - - let mut opts = test_opts(); - opts.invariant.depth = 100 + (50 * cfg!(windows) as u32); - opts.fuzz.seed = Some(U256::from(6u32)); - runner.test_options = opts.clone(); - - let results = runner - .test_collect( - &Filter::new(".*", ".*", ".*fuzz/invariant/storage/InvariantStorageTest.t.sol"), - opts, - ) - .await; + runner.test_options.invariant.depth = 100 + (50 * cfg!(windows) as u32); + runner.test_options.fuzz.seed = Some(U256::from(6u32)); + let results = runner.test_collect(&filter).await; assert_multiple( &results, @@ -236,18 +207,10 @@ async fn test_invariant_storage() { #[tokio::test(flavor = "multi_thread")] #[cfg_attr(windows, ignore = "for some reason there's different rng")] async fn test_invariant_shrink() { + let filter = Filter::new(".*", ".*", ".*fuzz/invariant/common/InvariantInnerContract.t.sol"); let mut runner = runner().await; - - let mut opts = test_opts(); - opts.fuzz.seed = Some(U256::from(119u32)); - runner.test_options = opts.clone(); - - let results = runner - .test_collect( - &Filter::new(".*", ".*", ".*fuzz/invariant/common/InvariantInnerContract.t.sol"), - opts, - ) - .await; + runner.test_options.fuzz.seed = Some(U256::from(119u32)); + let results = runner.test_collect(&filter).await; let results = results.values().last().expect("`InvariantInnerContract.t.sol` should be testable."); @@ -287,7 +250,7 @@ async fn test_invariant_shrink() { #[tokio::test(flavor = "multi_thread")] #[cfg_attr(windows, ignore = "for some reason there's different rng")] async fn test_invariant_assert_shrink() { - let mut opts = test_opts(); + let mut opts = TEST_OPTS.clone(); opts.fuzz.seed = Some(U256::from(119u32)); // ensure assert and require shrinks to same sequence of 3 or less @@ -296,18 +259,14 @@ async fn test_invariant_assert_shrink() { } async fn test_shrink(opts: TestOptions, contract_pattern: &str) { + let filter = Filter::new( + ".*", + contract_pattern, + ".*fuzz/invariant/common/InvariantShrinkWithAssert.t.sol", + ); let mut runner = runner().await; runner.test_options = opts.clone(); - let results = runner - .test_collect( - &Filter::new( - ".*", - contract_pattern, - ".*fuzz/invariant/common/InvariantShrinkWithAssert.t.sol", - ), - opts, - ) - .await; + let results = runner.test_collect(&filter).await; let results = results.values().last().expect("`InvariantShrinkWithAssert` should be testable."); let result = results @@ -333,18 +292,11 @@ async fn test_shrink(opts: TestOptions, contract_pattern: &str) { #[tokio::test(flavor = "multi_thread")] async fn test_invariant_preserve_state() { + let filter = Filter::new(".*", ".*", ".*fuzz/invariant/common/InvariantPreserveState.t.sol"); let mut runner = runner().await; - - // should not fail with default options - let mut opts = test_opts(); - opts.invariant.fail_on_revert = true; - runner.test_options = opts.clone(); - let results = runner - .test_collect( - &Filter::new(".*", ".*", ".*fuzz/invariant/common/InvariantPreserveState.t.sol"), - opts, - ) - .await; + // Should not fail with default options. + runner.test_options.invariant.fail_on_revert = true; + let results = runner.test_collect(&filter).await; assert_multiple( &results, BTreeMap::from([( @@ -354,17 +306,9 @@ async fn test_invariant_preserve_state() { ); // same test should revert when preserve state enabled - let mut opts = test_opts(); - opts.invariant.fail_on_revert = true; - opts.invariant.preserve_state = true; - runner.test_options = opts.clone(); - - let results = runner - .test_collect( - &Filter::new(".*", ".*", ".*fuzz/invariant/common/InvariantPreserveState.t.sol"), - opts, - ) - .await; + runner.test_options.invariant.fail_on_revert = true; + runner.test_options.invariant.preserve_state = true; + let results = runner.test_collect(&filter).await; assert_multiple( &results, BTreeMap::from([( diff --git a/crates/forge/tests/it/test_helpers.rs b/crates/forge/tests/it/test_helpers.rs index 31a330d2afb0..873e3071f723 100644 --- a/crates/forge/tests/it/test_helpers.rs +++ b/crates/forge/tests/it/test_helpers.rs @@ -1,11 +1,12 @@ //! Test helpers for Forge integration tests. use alloy_primitives::U256; +use forge::{TestOptions, TestOptionsBuilder}; use foundry_compilers::{ artifacts::{Libraries, Settings}, Project, ProjectCompileOutput, ProjectPathsConfig, SolcConfig, }; -use foundry_config::Config; +use foundry_config::{Config, FuzzConfig, FuzzDictionaryConfig, InvariantConfig}; use foundry_evm::{ constants::CALLER, executors::{Executor, FuzzedExecutor}, @@ -53,7 +54,7 @@ pub static COMPILED: Lazy = Lazy::new(|| { write.write_all(b"1").unwrap(); out = project.compile(); drop(write); - }; + } let out = out.unwrap(); if out.has_compiler_errors() { @@ -79,6 +80,40 @@ pub static EVM_OPTS: Lazy = Lazy::new(|| EvmOpts { ..Default::default() }); +pub static TEST_OPTS: Lazy = Lazy::new(|| { + TestOptionsBuilder::default() + .fuzz(FuzzConfig { + runs: 256, + max_test_rejects: 65536, + seed: None, + dictionary: FuzzDictionaryConfig { + include_storage: true, + include_push_bytes: true, + dictionary_weight: 40, + max_fuzz_dictionary_addresses: 10_000, + max_fuzz_dictionary_values: 10_000, + }, + }) + .invariant(InvariantConfig { + runs: 256, + depth: 15, + fail_on_revert: false, + call_override: false, + dictionary: FuzzDictionaryConfig { + dictionary_weight: 80, + include_storage: true, + include_push_bytes: true, + max_fuzz_dictionary_addresses: 10_000, + max_fuzz_dictionary_values: 10_000, + }, + shrink_sequence: true, + shrink_run_limit: 2usize.pow(18u32), + preserve_state: false, + }) + .build(&COMPILED, &PROJECT.paths.root) + .expect("Config loaded") +}); + pub fn fuzz_executor(executor: Executor) -> FuzzedExecutor { let cfg = proptest::test_runner::Config { failure_persistence: None, ..Default::default() }; @@ -86,6 +121,6 @@ pub fn fuzz_executor(executor: Executor) -> FuzzedExecutor { executor, proptest::test_runner::TestRunner::new(cfg), CALLER, - crate::config::test_opts().fuzz, + TEST_OPTS.fuzz, ) } From 9718d90987f48c5d2516562255d11a813dd15c7e Mon Sep 17 00:00:00 2001 From: DaniPopes <57450786+DaniPopes@users.noreply.github.com> Date: Fri, 1 Mar 2024 10:14:40 +0100 Subject: [PATCH 2/9] chore: clippy --- crates/forge/src/lib.rs | 14 ++++++-------- crates/forge/src/runner.rs | 2 +- 2 files changed, 7 insertions(+), 9 deletions(-) diff --git a/crates/forge/src/lib.rs b/crates/forge/src/lib.rs index 16343fc09655..00481591e079 100644 --- a/crates/forge/src/lib.rs +++ b/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) } } } diff --git a/crates/forge/src/runner.rs b/crates/forge/src/runner.rs index b01fd8363987..d7b5734e3cbf 100644 --- a/crates/forge/src/runner.rs +++ b/crates/forge/src/runner.rs @@ -261,7 +261,7 @@ impl<'a> ContractRunner<'a> { .contract .functions() .map(|func| (func.signature(), func)) - .filter(|(sig, func)| is_matching_test(func, || filter.matches_test(&sig))) + .filter(|(sig, func)| is_matching_test(func, || filter.matches_test(sig))) .collect::>(); let find_time = find_timer.elapsed(); debug!( From c666f7e31b2f78755e9f5dac1622f309e5b798e8 Mon Sep 17 00:00:00 2001 From: DaniPopes <57450786+DaniPopes@users.noreply.github.com> Date: Fri, 1 Mar 2024 10:21:15 +0100 Subject: [PATCH 3/9] fix: don't share Db and state --- crates/evm/evm/src/executors/builder.rs | 2 +- crates/forge/src/multi_runner.rs | 12 ++++++------ 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/crates/evm/evm/src/executors/builder.rs b/crates/evm/evm/src/executors/builder.rs index ab9bd7629b8e..d7cca61a7f5e 100644 --- a/crates/evm/evm/src/executors/builder.rs +++ b/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]. diff --git a/crates/forge/src/multi_runner.rs b/crates/forge/src/multi_runner.rs index 02f3d8ba8b73..a2dd7cef53ef 100644 --- a/crates/forge/src/multi_runner.rs +++ b/crates/forge/src/multi_runner.rs @@ -157,9 +157,9 @@ impl MultiContractRunner { 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, each test must get its own instance. let db = Backend::spawn(self.fork.take()).await; - let executor = ExecutorBuilder::new() + let executor_builder = ExecutorBuilder::new() .inspectors(|stack| { stack .cheatcodes(self.cheats_config.clone()) @@ -169,8 +169,8 @@ impl MultiContractRunner { .enable_isolation(self.isolation) }) .spec(self.evm_spec) - .gas_limit(self.evm_opts.gas_limit()) - .build(self.env.clone(), db); + .gas_limit(self.evm_opts.gas_limit()); + let make_executor = || executor_builder.clone().build(self.env.clone(), db.clone()); let find_timer = Instant::now(); let contracts = self @@ -188,8 +188,8 @@ impl MultiContractRunner { contracts.par_iter().for_each_with(tx, |tx, &(id, (abi, deploy_code, libs))| { let identifier = id.identifier(); - let result = - self.run_tests(&identifier, abi, executor.clone(), deploy_code, libs, filter); + let executor = make_executor(); + let result = self.run_tests(&identifier, abi, executor, deploy_code, libs, filter); let _ = tx.send((identifier, result)); }) } From 3f904c3ce1dec2e07c79c20d6f20d58643ab43ba Mon Sep 17 00:00:00 2001 From: DaniPopes <57450786+DaniPopes@users.noreply.github.com> Date: Fri, 1 Mar 2024 10:27:28 +0100 Subject: [PATCH 4/9] fix: don't even share the builder (???) --- crates/forge/src/multi_runner.rs | 26 ++++++++++++++------------ 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/crates/forge/src/multi_runner.rs b/crates/forge/src/multi_runner.rs index a2dd7cef53ef..24fe043d1fc0 100644 --- a/crates/forge/src/multi_runner.rs +++ b/crates/forge/src/multi_runner.rs @@ -159,18 +159,20 @@ impl MultiContractRunner { // The DB backend that serves all the data, each test must get its own instance. let db = Backend::spawn(self.fork.take()).await; - let executor_builder = 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()); - let make_executor = || executor_builder.clone().build(self.env.clone(), db.clone()); + let make_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 find_timer = Instant::now(); let contracts = self From 741629061f67c4e20106d2f8ac87d80964c47fb3 Mon Sep 17 00:00:00 2001 From: DaniPopes <57450786+DaniPopes@users.noreply.github.com> Date: Fri, 1 Mar 2024 10:53:20 +0100 Subject: [PATCH 5/9] fix: fuzz tests must also start with test --- crates/forge/src/multi_runner.rs | 30 ++++++++++++++---------------- crates/forge/src/runner.rs | 2 ++ 2 files changed, 16 insertions(+), 16 deletions(-) diff --git a/crates/forge/src/multi_runner.rs b/crates/forge/src/multi_runner.rs index 24fe043d1fc0..fc058849bba1 100644 --- a/crates/forge/src/multi_runner.rs +++ b/crates/forge/src/multi_runner.rs @@ -159,20 +159,18 @@ impl MultiContractRunner { // The DB backend that serves all the data, each test must get its own instance. let db = Backend::spawn(self.fork.take()).await; - let make_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 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 find_timer = Instant::now(); let contracts = self @@ -190,7 +188,7 @@ impl MultiContractRunner { contracts.par_iter().for_each_with(tx, |tx, &(id, (abi, deploy_code, libs))| { let identifier = id.identifier(); - let executor = make_executor(); + let executor = executor.clone(); let result = self.run_tests(&identifier, abi, executor, deploy_code, libs, filter); let _ = tx.send((identifier, result)); }) @@ -397,5 +395,5 @@ fn matches_contract(id: &ArtifactId, abi: &JsonAbi, filter: &dyn TestFilter) -> /// Returns `true` if the function is a test function that matches the given filter. pub(crate) fn is_matching_test(func: &Function, match_sig: impl FnOnce() -> bool) -> bool { - (func.is_test() || func.is_fuzz_test() || func.is_invariant_test()) && match_sig() + (func.is_test() || func.is_invariant_test()) && match_sig() } diff --git a/crates/forge/src/runner.rs b/crates/forge/src/runner.rs index d7b5734e3cbf..370b79a5e486 100644 --- a/crates/forge/src/runner.rs +++ b/crates/forge/src/runner.rs @@ -290,10 +290,12 @@ impl<'a> ContractRunner<'a> { identified_contracts.as_ref().unwrap(), ) } else if func.is_fuzz_test() { + debug_assert!(func.is_test()); let runner = test_options.fuzz_runner(self.name, &func.name); let fuzz_config = test_options.fuzz_config(self.name, &func.name); self.run_fuzz_test(func, should_fail, runner, setup, *fuzz_config) } else { + debug_assert!(func.is_test()); self.run_test(func, should_fail, setup) }; (sig, res) From 0c27a4d5594845c22bc6c456d967833cf5d001c9 Mon Sep 17 00:00:00 2001 From: DaniPopes <57450786+DaniPopes@users.noreply.github.com> Date: Fri, 1 Mar 2024 11:27:09 +0100 Subject: [PATCH 6/9] chore: simplify filtering --- crates/forge/bin/cmd/test/mod.rs | 11 ++++-- crates/forge/src/multi_runner.rs | 59 +++++++++----------------------- crates/forge/src/runner.rs | 10 +++--- 3 files changed, 31 insertions(+), 49 deletions(-) diff --git a/crates/forge/bin/cmd/test/mod.rs b/crates/forge/bin/cmd/test/mod.rs index 8202cb650706..c306fd7cae25 100644 --- a/crates/forge/bin/cmd/test/mod.rs +++ b/crates/forge/bin/cmd/test/mod.rs @@ -20,7 +20,7 @@ use foundry_cli::{ use foundry_common::{ compile::{ContractSources, ProjectCompiler}, evm::EvmArgs, - shell, + shell, TestFunctionExt, }; use foundry_config::{ figment, @@ -257,7 +257,7 @@ impl TestArgs { 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() { @@ -272,7 +272,12 @@ 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 + .matching_contracts(filter) + .flat_map(|(_, (abi, _, _))| abi.functions()) + .filter(|func| func.is_test() || func.is_invariant_test()) + .map(|f| &f.name); if let Some(suggestion) = utils::did_you_mean(test_name, candidates).pop() { println!("\nDid you mean `{suggestion}`?"); } diff --git a/crates/forge/src/multi_runner.rs b/crates/forge/src/multi_runner.rs index fc058849bba1..2c2ebc819551 100644 --- a/crates/forge/src/multi_runner.rs +++ b/crates/forge/src/multi_runner.rs @@ -66,9 +66,12 @@ 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))> { + self.contracts.iter().filter(|&(id, (abi, _, _))| matches_contract(id, abi, filter)) } /// Returns all test functions matching the filter @@ -76,47 +79,22 @@ impl MultiContractRunner { &'a self, filter: &'a dyn TestFilter, ) -> impl Iterator { - 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())) - }) - } - - /// 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 { - self.contracts - .iter() - .filter(|(id, _)| filter.matches_path(&id.source) && filter.matches_contract(&id.name)) + self.matching_contracts(filter) .flat_map(|(_, (abi, _, _))| abi.functions()) - } - - /// Get all test names matching the filter - pub fn get_tests(&self, filter: &dyn TestFilter) -> Vec { - self.filtered_tests(filter) - .map(|func| func.name.clone()) - .filter(|name| name.is_test()) - .collect() + .filter(|func| is_matching_test(func, filter)) } /// Returns all matching tests grouped by contract grouped by file (file -> (contract -> tests)) pub fn list(&self, filter: &dyn TestFilter) -> BTreeMap>> { - 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::>(); - (source, name, tests) }) .fold(BTreeMap::new(), |mut acc, (source, name, tests)| { @@ -170,14 +148,10 @@ impl MultiContractRunner { }) .spec(self.evm_spec) .gas_limit(self.evm_opts.gas_limit()) - .build(self.env.clone(), db.clone()); + .build(self.env.clone(), db); let find_timer = Instant::now(); - let contracts = self - .contracts - .iter() - .filter(|&(id, (abi, ..))| matches_contract(id, abi, filter)) - .collect::>(); + let contracts = self.matching_contracts(filter).collect::>(); let find_time = find_timer.elapsed(); debug!( "Found {} test contracts out of {} in {:?}", @@ -389,11 +363,12 @@ 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.matches_test(&func.signature()))) + 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, match_sig: impl FnOnce() -> bool) -> bool { - (func.is_test() || func.is_invariant_test()) && match_sig() +pub(crate) fn is_matching_test(func: &Function, filter: &dyn TestFilter) -> bool { + // Avoid calculating the signature if possible. + (func.is_test() || func.is_invariant_test()) && + (filter.matches_test(&func.name) || filter.matches_test(&func.signature())) } diff --git a/crates/forge/src/runner.rs b/crates/forge/src/runner.rs index 370b79a5e486..bf890f10e0bb 100644 --- a/crates/forge/src/runner.rs +++ b/crates/forge/src/runner.rs @@ -260,8 +260,7 @@ impl<'a> ContractRunner<'a> { let functions = self .contract .functions() - .map(|func| (func.signature(), func)) - .filter(|(sig, func)| is_matching_test(func, || filter.matches_test(sig))) + .filter(|func| is_matching_test(func, filter)) .collect::>(); let find_time = find_timer.elapsed(); debug!( @@ -274,8 +273,10 @@ impl<'a> ContractRunner<'a> { let identified_contracts = has_invariants.then(|| load_contracts(setup.traces.clone(), known_contracts)); let test_results = functions - .into_par_iter() - .map(|(sig, func)| { + .par_iter() + .map(|&func| { + let sig = func.signature(); + let setup = setup.clone(); let should_fail = func.is_test_fail(); let res = if func.is_invariant_test() { @@ -298,6 +299,7 @@ impl<'a> ContractRunner<'a> { debug_assert!(func.is_test()); self.run_test(func, should_fail, setup) }; + (sig, res) }) .collect::>(); From 48d4660aa168f2ab148a41f7728b2485d1828681 Mon Sep 17 00:00:00 2001 From: DaniPopes <57450786+DaniPopes@users.noreply.github.com> Date: Fri, 1 Mar 2024 12:01:56 +0100 Subject: [PATCH 7/9] fix: filter --- crates/forge/bin/cmd/test/mod.rs | 8 ++------ crates/forge/src/multi_runner.rs | 14 +++++++++++++- 2 files changed, 15 insertions(+), 7 deletions(-) diff --git a/crates/forge/bin/cmd/test/mod.rs b/crates/forge/bin/cmd/test/mod.rs index c306fd7cae25..0d49b4640671 100644 --- a/crates/forge/bin/cmd/test/mod.rs +++ b/crates/forge/bin/cmd/test/mod.rs @@ -20,7 +20,7 @@ use foundry_cli::{ use foundry_common::{ compile::{ContractSources, ProjectCompiler}, evm::EvmArgs, - shell, TestFunctionExt, + shell, }; use foundry_config::{ figment, @@ -273,11 +273,7 @@ impl TestArgs { if let Some(test_pattern) = &filter.args().test_pattern { let test_name = test_pattern.as_str(); // Filter contracts but not test functions. - let candidates = runner - .matching_contracts(filter) - .flat_map(|(_, (abi, _, _))| abi.functions()) - .filter(|func| func.is_test() || func.is_invariant_test()) - .map(|f| &f.name); + 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}`?"); } diff --git a/crates/forge/src/multi_runner.rs b/crates/forge/src/multi_runner.rs index 2c2ebc819551..f6b23f7dc5d7 100644 --- a/crates/forge/src/multi_runner.rs +++ b/crates/forge/src/multi_runner.rs @@ -74,7 +74,7 @@ impl MultiContractRunner { 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, @@ -84,6 +84,18 @@ impl MultiContractRunner { .filter(|func| is_matching_test(func, filter)) } + /// 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 { + self.contracts + .iter() + .filter(|(id, _)| filter.matches_path(&id.source) && filter.matches_contract(&id.name)) + .flat_map(|(_, (abi, _, _))| abi.functions()) + .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>> { self.matching_contracts(filter) From 747cb628945947be35bf21dd50de7aafdd8e6aaa Mon Sep 17 00:00:00 2001 From: DaniPopes <57450786+DaniPopes@users.noreply.github.com> Date: Fri, 1 Mar 2024 12:15:58 +0100 Subject: [PATCH 8/9] fix: filter 2 --- crates/forge/src/multi_runner.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/crates/forge/src/multi_runner.rs b/crates/forge/src/multi_runner.rs index f6b23f7dc5d7..74d22c406574 100644 --- a/crates/forge/src/multi_runner.rs +++ b/crates/forge/src/multi_runner.rs @@ -380,7 +380,5 @@ fn matches_contract(id: &ArtifactId, abi: &JsonAbi, filter: &dyn TestFilter) -> /// 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 { - // Avoid calculating the signature if possible. - (func.is_test() || func.is_invariant_test()) && - (filter.matches_test(&func.name) || filter.matches_test(&func.signature())) + (func.is_test() || func.is_invariant_test()) && filter.matches_test(&func.signature()) } From 5a56349aee77ad1b7e152375e6ef3f9f2325b346 Mon Sep 17 00:00:00 2001 From: DaniPopes <57450786+DaniPopes@users.noreply.github.com> Date: Fri, 1 Mar 2024 12:27:34 +0100 Subject: [PATCH 9/9] chore: comment, logs --- crates/forge/src/multi_runner.rs | 2 +- crates/forge/src/runner.rs | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/forge/src/multi_runner.rs b/crates/forge/src/multi_runner.rs index 74d22c406574..ea6c79c98374 100644 --- a/crates/forge/src/multi_runner.rs +++ b/crates/forge/src/multi_runner.rs @@ -147,7 +147,7 @@ impl MultiContractRunner { 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 test must get its own instance. + // The DB backend that serves all the data. let db = Backend::spawn(self.fork.take()).await; let executor = ExecutorBuilder::new() .inspectors(|stack| { diff --git a/crates/forge/src/runner.rs b/crates/forge/src/runner.rs index bf890f10e0bb..ca3e8362940b 100644 --- a/crates/forge/src/runner.rs +++ b/crates/forge/src/runner.rs @@ -421,7 +421,7 @@ impl<'a> ContractRunner<'a> { // Record test execution time let duration = start.elapsed(); - debug!(?duration, gas, reverted, should_fail, success); + trace!(?duration, gas, reverted, should_fail, success); TestResult { status: match success { @@ -678,7 +678,7 @@ impl<'a> ContractRunner<'a> { // Record test execution time let duration = start.elapsed(); - debug!(?duration, success = %result.success); + trace!(?duration, success = %result.success); TestResult { status: match result.success {