From 32b0aa86e4fcf3d55777457f70af3d4dd2692bfd Mon Sep 17 00:00:00 2001 From: DaniPopes <57450786+DaniPopes@users.noreply.github.com> Date: Wed, 14 Feb 2024 03:07:52 +0100 Subject: [PATCH] chore: reorder some stuff --- crates/forge/bin/cmd/coverage.rs | 2 +- crates/forge/bin/cmd/test/mod.rs | 47 +++++++------ crates/forge/src/multi_runner.rs | 116 ++++++++++++++----------------- 3 files changed, 78 insertions(+), 87 deletions(-) diff --git a/crates/forge/bin/cmd/coverage.rs b/crates/forge/bin/cmd/coverage.rs index 771bd8bb0906..ab78bdff2ff3 100644 --- a/crates/forge/bin/cmd/coverage.rs +++ b/crates/forge/bin/cmd/coverage.rs @@ -310,7 +310,7 @@ impl CoverageArgs { ..Default::default() }) .set_coverage(true) - .build(root.clone(), output, env, evm_opts)?; + .build(&root, output, env, evm_opts)?; // Run tests let known_contracts = runner.known_contracts.clone(); diff --git a/crates/forge/bin/cmd/test/mod.rs b/crates/forge/bin/cmd/test/mod.rs index 9834d067e6e6..0034c6549264 100644 --- a/crates/forge/bin/cmd/test/mod.rs +++ b/crates/forge/bin/cmd/test/mod.rs @@ -186,29 +186,28 @@ impl TestArgs { // Prepare the test builder let should_debug = self.debug.is_some(); - let runner_builder = MultiContractRunnerBuilder::default() + // Clone the output only if we actually need it later for the debugger. + let output_clone = should_debug.then(|| output.clone()); + + let runner = MultiContractRunnerBuilder::default() .set_debug(should_debug) .initial_balance(evm_opts.initial_balance) .evm_spec(config.evm_spec_id()) .sender(evm_opts.sender) .with_fork(evm_opts.get_fork(&config, env.clone())) .with_cheats_config(CheatsConfig::new(&config, evm_opts.clone())) - .with_test_options(test_options.clone()); - - // Clone the output only if we actually need it later for the debugger. - let output_clone = should_debug.then(|| output.clone()); - - let runner = runner_builder.build(project_root, output, env, evm_opts)?; + .with_test_options(test_options.clone()) + .build(project_root, output, env, evm_opts)?; - if should_debug { - filter.args_mut().test_pattern = self.debug.clone(); - let num_filtered = runner.matching_test_function_count(&filter); - if num_filtered != 1 { + if let Some(debug_test_pattern) = &self.debug { + let test_pattern = &mut filter.args_mut().test_pattern; + if test_pattern.is_some() { eyre::bail!( - "{num_filtered} tests matched your criteria, but exactly 1 test must match in order to run the debugger.\n\n\ - Use --match-contract and --match-path to further limit the search." + "Cannot specify both --debug and --match-test. \ + Use --match-contract and --match-path to further limit the search instead." ); } + *test_pattern = Some(debug_test_pattern.clone()); } let outcome = self.run_tests(runner, config, verbosity, &filter, test_options).await?; @@ -232,7 +231,9 @@ impl TestArgs { } // There is only one test. - let test = outcome.into_tests_cloned().next().unwrap(); + let Some(test) = outcome.into_tests_cloned().next() else { + return Err(eyre::eyre!("no tests were executed")); + }; // Run the debugger. let mut builder = Debugger::builder() @@ -262,17 +263,10 @@ impl TestArgs { return list(runner, filter, self.json); } - if let Some(debug_regex) = self.debug.as_ref() { - let mut filter = filter.clone(); - filter.args_mut().test_pattern = Some(debug_regex.clone()); - let results = runner.test_collect(&filter, test_options).await; - return Ok(TestOutcome::new(results, self.allow_failure)); - } - trace!(target: "forge::test", "running all tests"); - if runner.matching_test_function_count(filter) == 0 { - println!(); + let num_filtered = runner.matching_test_function_count(filter); + if num_filtered == 0 { if filter.is_empty() { println!( "No tests found in project! \ @@ -292,6 +286,13 @@ impl TestArgs { } } } + if self.debug.is_some() && num_filtered != 1 { + eyre::bail!( + "{num_filtered} tests matched your criteria, but exactly 1 test must match in order to run the debugger.\n\n\ + Use --match-contract and --match-path to further limit the search.\n\ + Filter used:\n{filter}" + ); + } if self.json { let results = runner.test_collect(filter, test_options).await; diff --git a/crates/forge/src/multi_runner.rs b/crates/forge/src/multi_runner.rs index 01c2aa1f6fd9..261468ef800f 100644 --- a/crates/forge/src/multi_runner.rs +++ b/crates/forge/src/multi_runner.rs @@ -9,9 +9,7 @@ use alloy_json_abi::{Function, JsonAbi}; use alloy_primitives::{Address, Bytes, U256}; use eyre::{OptionExt, Result}; use foundry_common::{ContractsByArtifact, TestFunctionExt}; -use foundry_compilers::{ - contracts::ArtifactContracts, Artifact, ArtifactId, ArtifactOutput, ProjectCompileOutput, -}; +use foundry_compilers::{contracts::ArtifactContracts, Artifact, ArtifactId, ProjectCompileOutput}; use foundry_evm::{ backend::Backend, executors::{Executor, ExecutorBuilder}, @@ -232,6 +230,7 @@ impl MultiContractRunner { /// Builder used for instantiating the multi-contract runner #[derive(Clone, Debug, Default)] +#[must_use = "builders do nothing unless you call `build` on them"] pub struct MultiContractRunnerBuilder { /// The address which will be used to deploy the initial contracts and send all /// transactions @@ -253,36 +252,75 @@ pub struct MultiContractRunnerBuilder { } impl MultiContractRunnerBuilder { + pub fn sender(mut self, sender: Address) -> Self { + self.sender = Some(sender); + self + } + + pub fn initial_balance(mut self, initial_balance: U256) -> Self { + self.initial_balance = initial_balance; + self + } + + pub fn evm_spec(mut self, spec: SpecId) -> Self { + self.evm_spec = Some(spec); + self + } + + pub fn with_fork(mut self, fork: Option) -> Self { + self.fork = fork; + self + } + + pub fn with_cheats_config(mut self, cheats_config: CheatsConfig) -> Self { + self.cheats_config = Some(cheats_config); + self + } + + pub fn with_test_options(mut self, test_options: TestOptions) -> Self { + self.test_options = Some(test_options); + self + } + + pub fn set_coverage(mut self, enable: bool) -> Self { + self.coverage = enable; + self + } + + pub fn set_debug(mut self, enable: bool) -> Self { + self.debug = enable; + self + } + /// Given an EVM, proceeds to return a runner which is able to execute all tests /// against that evm - pub fn build( + pub fn build( self, - root: impl AsRef, - output: ProjectCompileOutput, + root: &Path, + output: ProjectCompileOutput, env: revm::primitives::Env, evm_opts: EvmOpts, - ) -> Result - where - A: ArtifactOutput + Debug, - { - let output = output.with_stripped_file_prefixes(&root); + ) -> Result { // This is just the contracts compiled, but we need to merge this with the read cached // artifacts. let contracts = output + .with_stripped_file_prefixes(root) .into_artifacts() .map(|(i, c)| (i, c.into_contract_bytecode())) .collect::(); let source_paths = contracts .iter() - .map(|(i, _)| (i.identifier(), root.as_ref().join(&i.source).to_string_lossy().into())) + .map(|(i, _)| (i.identifier(), root.join(&i.source).to_string_lossy().into())) .collect::>(); - // create a mapping of name => (abi, deployment code, Vec) - let mut deployable_contracts = DeployableContracts::default(); - let linker = Linker::new(root.as_ref(), contracts); + let linker = Linker::new(root, contracts); + + // Create a mapping of name => (abi, deployment code, Vec) + let mut deployable_contracts = DeployableContracts::default(); let mut known_contracts = ContractsByArtifact::default(); + for (id, contract) in &linker.contracts.0 { let abi = contract.abi.as_ref().ok_or_eyre("we should have an abi by now")?; @@ -333,52 +371,4 @@ impl MultiContractRunnerBuilder { test_options: self.test_options.unwrap_or_default(), }) } - - #[must_use] - pub fn sender(mut self, sender: Address) -> Self { - self.sender = Some(sender); - self - } - - #[must_use] - pub fn initial_balance(mut self, initial_balance: U256) -> Self { - self.initial_balance = initial_balance; - self - } - - #[must_use] - pub fn evm_spec(mut self, spec: SpecId) -> Self { - self.evm_spec = Some(spec); - self - } - - #[must_use] - pub fn with_fork(mut self, fork: Option) -> Self { - self.fork = fork; - self - } - - #[must_use] - pub fn with_cheats_config(mut self, cheats_config: CheatsConfig) -> Self { - self.cheats_config = Some(cheats_config); - self - } - - #[must_use] - pub fn with_test_options(mut self, test_options: TestOptions) -> Self { - self.test_options = Some(test_options); - self - } - - #[must_use] - pub fn set_coverage(mut self, enable: bool) -> Self { - self.coverage = enable; - self - } - - #[must_use] - pub fn set_debug(mut self, enable: bool) -> Self { - self.debug = enable; - self - } }