diff --git a/crates/common/src/traits.rs b/crates/common/src/traits.rs index 69e5570a15fef..3da6ff9847837 100644 --- a/crates/common/src/traits.rs +++ b/crates/common/src/traits.rs @@ -8,7 +8,7 @@ use std::{fmt, path::Path}; /// Test filter. pub trait TestFilter: Send + Sync { /// Returns whether the test should be included. - fn matches_test(&self, test_name: &str) -> bool; + fn matches_test(&self, test_signature: &str) -> bool; /// Returns whether the contract should be included. fn matches_contract(&self, contract_name: &str) -> bool; @@ -17,6 +17,30 @@ pub trait TestFilter: Send + Sync { fn matches_path(&self, path: &Path) -> bool; } +impl<'a> dyn TestFilter + 'a { + /// Returns `true` if the function is a test function that matches the given filter. + pub fn matches_test_function(&self, func: &Function) -> bool { + func.is_any_test() && self.matches_test(&func.signature()) + } +} + +/// A test filter that filters out nothing. +#[derive(Clone, Debug, Default)] +pub struct EmptyTestFilter(()); +impl TestFilter for EmptyTestFilter { + fn matches_test(&self, _test_signature: &str) -> bool { + true + } + + fn matches_contract(&self, _contract_name: &str) -> bool { + true + } + + fn matches_path(&self, _path: &Path) -> bool { + true + } +} + /// Extension trait for `Function`. pub trait TestFunctionExt { /// Returns the kind of test function. diff --git a/crates/forge/src/cmd/test/filter.rs b/crates/forge/src/cmd/test/filter.rs index 9857efc12c43b..f39535a97bb92 100644 --- a/crates/forge/src/cmd/test/filter.rs +++ b/crates/forge/src/cmd/test/filter.rs @@ -106,13 +106,13 @@ impl FileFilter for FilterArgs { } impl TestFilter for FilterArgs { - fn matches_test(&self, test_name: &str) -> bool { + fn matches_test(&self, test_signature: &str) -> bool { let mut ok = true; if let Some(re) = &self.test_pattern { - ok = ok && re.is_match(test_name); + ok = ok && re.is_match(test_signature); } if let Some(re) = &self.test_pattern_inverse { - ok = ok && !re.is_match(test_name); + ok = ok && !re.is_match(test_signature); } ok } @@ -207,8 +207,8 @@ impl FileFilter for ProjectPathsAwareFilter { } impl TestFilter for ProjectPathsAwareFilter { - fn matches_test(&self, test_name: &str) -> bool { - self.args_filter.matches_test(test_name) + fn matches_test(&self, test_signature: &str) -> bool { + self.args_filter.matches_test(test_signature) } fn matches_contract(&self, contract_name: &str) -> bool { diff --git a/crates/forge/src/cmd/test/mod.rs b/crates/forge/src/cmd/test/mod.rs index d04d2f172a234..a645d5391829a 100644 --- a/crates/forge/src/cmd/test/mod.rs +++ b/crates/forge/src/cmd/test/mod.rs @@ -1,9 +1,9 @@ use super::{install, test::filter::ProjectPathsAwareFilter, watch::WatchArgs}; use crate::{ - MultiContractRunner, MultiContractRunnerBuilder, TestFilter, + MultiContractRunner, MultiContractRunnerBuilder, decode::decode_console_logs, gas_report::GasReport, - multi_runner::matches_contract, + multi_runner::{is_test_contract, matches_artifact}, result::{SuiteResult, TestOutcome, TestStatus}, traces::{ CallTraceDecoderBuilder, InternalTraceMode, TraceKind, @@ -21,15 +21,12 @@ use foundry_cli::{ opts::{BuildOpts, GlobalArgs}, utils::{self, LoadConfig}, }; -use foundry_common::{TestFunctionExt, compile::ProjectCompiler, evm::EvmArgs, fs, shell}; +use foundry_common::{ + EmptyTestFilter, TestFunctionExt, compile::ProjectCompiler, evm::EvmArgs, fs, shell, +}; use foundry_compilers::{ - ProjectCompileOutput, - artifacts::output_selection::OutputSelection, - compilers::{ - Language, - multi::{MultiCompiler, MultiCompilerLanguage}, - }, - utils::source_files_iter, + ProjectCompileOutput, artifacts::output_selection::OutputSelection, + compilers::multi::MultiCompiler, }; use foundry_config::{ Config, figment, @@ -209,76 +206,44 @@ impl TestArgs { self.execute_tests().await } - /// Returns sources which include any tests to be executed. - /// If no filters are provided, sources are filtered by existence of test/invariant methods in - /// them, If filters are provided, sources are additionally filtered by them. + /// Returns a list of files that need to be compiled in order to run all the tests that match + /// the given filter. + /// + /// This means that it will return all sources that are not test contracts or that match the + /// filter. We want to compile all non-test sources always because tests might depend on them + /// dynamically through cheatcodes. + /// + /// Returns `None` if all sources should be compiled. #[instrument(target = "forge::test", skip_all)] pub fn get_sources_to_compile( &self, config: &Config, - filter: &ProjectPathsAwareFilter, - ) -> Result> { + test_filter: &ProjectPathsAwareFilter, + ) -> Result>> { + // An empty filter doesn't filter out anything. + if test_filter.is_empty() { + return Ok(None); + } + let mut project = config.create_project(true, true)?; project.update_output_selection(|selection| { *selection = OutputSelection::common_output_selection(["abi".to_string()]); }); - let output = project.compile()?; - if output.has_compiler_errors() { sh_println!("{output}")?; eyre::bail!("Compilation failed"); } - // ABIs of all sources - let abis = output - .into_artifacts() - .filter_map(|(id, artifact)| artifact.abi.map(|abi| (id, abi))) - .collect::>(); - - // Filter sources by their abis and contract names. - let mut test_sources = abis - .iter() - .filter(|(id, abi)| matches_contract(id, abi, filter)) - .map(|(id, _)| id.source.clone()) + let sources = output + .artifact_ids() + .filter_map(|(id, artifact)| artifact.abi.as_ref().map(|abi| (id, abi))) + .filter(|(id, abi)| { + !is_test_contract(abi.functions()) || matches_artifact(test_filter, id, abi) + }) + .map(|(id, _)| id.source) .collect::>(); - - if test_sources.is_empty() { - if filter.is_empty() { - sh_println!( - "No tests found in project! \ - Forge looks for functions that starts with `test`." - )?; - } else { - sh_println!("No tests match the provided pattern:")?; - sh_print!("{filter}")?; - - // 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 = abis - .into_iter() - .filter(|(id, _)| { - filter.matches_path(&id.source) && filter.matches_contract(&id.name) - }) - .flat_map(|(_, abi)| abi.functions.into_keys()) - .collect::>(); - if let Some(suggestion) = utils::did_you_mean(test_name, candidates).pop() { - sh_println!("\nDid you mean `{suggestion}`?")?; - } - } - } - - eyre::bail!("No tests to run"); - } - - // Always recompile all sources to ensure that `getCode` cheatcode can use any artifact. - test_sources.extend(source_files_iter( - &project.paths.sources, - MultiCompilerLanguage::FILE_EXTENSIONS, - )); - - Ok(test_sources) + Ok(Some(sources)) } /// Executes all the tests in the project. @@ -312,13 +277,10 @@ impl TestArgs { let filter = self.filter(&config)?; trace!(target: "forge::test", ?filter, "using filter"); - let sources_to_compile = self.get_sources_to_compile(&config, &filter)?; - let compiler = ProjectCompiler::new() .dynamic_test_linking(config.dynamic_test_linking) .quiet(shell::is_json() || self.junit) - .files(sources_to_compile); - + .files(self.get_sources_to_compile(&config, &filter)?.unwrap_or_default()); let output = compiler.compile(&project)?; // Create test options from general project settings and compiler output. @@ -457,6 +419,32 @@ impl TestArgs { let silent = self.gas_report && shell::is_json() || self.summary && shell::is_json(); let num_filtered = runner.matching_test_functions(filter).count(); + + if num_filtered == 0 { + let mut total_tests = num_filtered; + if !filter.is_empty() { + total_tests = runner.matching_test_functions(&EmptyTestFilter::default()).count(); + } + if total_tests == 0 { + sh_println!( + "No tests found in project! Forge looks for functions that start with `test`" + )?; + } else { + let mut msg = format!("no tests match the provided pattern:\n{filter}"); + // 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(); + // 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() { + write!(msg, "\nDid you mean `{suggestion}`?")?; + } + } + sh_warn!("{msg}")?; + } + return Ok(TestOutcome::empty(false)); + } + if num_filtered != 1 && (self.debug || self.flamegraph || self.flamechart) { let action = if self.flamegraph { "generate a flamegraph" @@ -915,7 +903,14 @@ fn list(runner: MultiContractRunner, filter: &ProjectPathsAwareFilter) -> Result /// Load persisted filter (with last test run failures) from file. fn last_run_failures(config: &Config) -> Option { match fs::read_to_string(&config.test_failures_file) { - Ok(filter) => Some(Regex::new(&filter).unwrap()), + Ok(filter) => Regex::new(&filter) + .inspect_err(|e| { + _ = sh_warn!( + "failed to parse test filter from {:?}: {e}", + config.test_failures_file + ) + }) + .ok(), Err(_) => None, } } diff --git a/crates/forge/src/multi_runner.rs b/crates/forge/src/multi_runner.rs index de212943de8b1..2ba9a3a447eb0 100644 --- a/crates/forge/src/multi_runner.rs +++ b/crates/forge/src/multi_runner.rs @@ -89,7 +89,7 @@ impl MultiContractRunner { &'a self, filter: &'b dyn TestFilter, ) -> impl Iterator + 'b { - self.contracts.iter().filter(|&(id, c)| matches_contract(id, &c.abi, filter)) + self.contracts.iter().filter(|&(id, c)| matches_artifact(filter, id, &c.abi)) } /// Returns an iterator over all test functions that match the filter. @@ -99,7 +99,7 @@ impl MultiContractRunner { ) -> impl Iterator + 'b { self.matching_contracts(filter) .flat_map(|(_, c)| c.abi.functions()) - .filter(|func| is_matching_test(func, filter)) + .filter(|func| filter.matches_test_function(func)) } /// Returns an iterator over all test functions in contracts that match the filter. @@ -123,7 +123,7 @@ impl MultiContractRunner { let tests = c .abi .functions() - .filter(|func| is_matching_test(func, filter)) + .filter(|func| filter.matches_test_function(func)) .map(|func| func.name.clone()) .collect::>(); (source, name, tests) @@ -566,12 +566,23 @@ impl MultiContractRunnerBuilder { } } -pub 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)) +pub fn matches_artifact(filter: &dyn TestFilter, id: &ArtifactId, abi: &JsonAbi) -> bool { + matches_contract(filter, &id.source, &id.name, abi.functions()) } -/// 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_any_test() && filter.matches_test(&func.signature()) +pub(crate) fn matches_contract( + filter: &dyn TestFilter, + path: &Path, + contract_name: &str, + functions: impl IntoIterator>, +) -> bool { + (filter.matches_path(path) && filter.matches_contract(contract_name)) + && functions.into_iter().any(|func| filter.matches_test_function(func.borrow())) +} + +/// Returns `true` if the given contract is a test contract. +pub(crate) fn is_test_contract( + functions: impl IntoIterator>, +) -> bool { + functions.into_iter().any(|func| func.borrow().is_any_test()) } diff --git a/crates/forge/src/runner.rs b/crates/forge/src/runner.rs index afe6071e5e950..f94e4c2d980c2 100644 --- a/crates/forge/src/runner.rs +++ b/crates/forge/src/runner.rs @@ -3,7 +3,7 @@ use crate::{ MultiContractRunner, TestFilter, fuzz::BaseCounterExample, - multi_runner::{TestContract, TestRunnerConfig, is_matching_test}, + multi_runner::{TestContract, TestRunnerConfig}, progress::{TestsProgress, start_fuzz_progress}, result::{SuiteResult, TestResult, TestSetup}, }; @@ -369,7 +369,7 @@ impl<'a> ContractRunner<'a> { .contract .abi .functions() - .filter(|func| is_matching_test(func, filter)) + .filter(|func| filter.matches_test_function(func)) .collect::>(); debug!( "Found {} test functions out of {} in {:?}", diff --git a/crates/forge/tests/cli/test_cmd.rs b/crates/forge/tests/cli/test_cmd.rs index 32362a904aed4..3d1af8bb5bf72 100644 --- a/crates/forge/tests/cli/test_cmd.rs +++ b/crates/forge/tests/cli/test_cmd.rs @@ -3,7 +3,7 @@ use alloy_primitives::U256; use anvil::{NodeConfig, spawn}; use foundry_test_utils::{ - rpc, str, + TestCommand, rpc, str, util::{OTHER_SOLC_VERSION, OutputExt, SOLC_VERSION}, }; use similar_asserts::assert_eq; @@ -36,54 +36,39 @@ forgetest!(can_set_filter_values, |prj, cmd| { assert_eq!(config.coverage_pattern_inverse, None); }); -// tests that warning is displayed when there are no tests in project +fn dummy_test_filter(cmd: &mut TestCommand) { + cmd.args(["test", "--match-test", "testA.*", "--no-match-test", "testB.*"]); + cmd.args(["--match-contract", "TestC.*", "--no-match-contract", "TestD.*"]); + cmd.args(["--match-path", "*TestE*", "--no-match-path", "*TestF*"]); +} + +// tests that a warning is displayed when there are no tests in project, regardless of filters forgetest!(warn_no_tests, |prj, cmd| { + // Must add at least one source to not fail earlier. prj.add_source( "dummy", r" contract Dummy {} ", ); - // set up command - cmd.args(["test"]); - // run command and assert - cmd.assert_failure().stdout_eq(str![[r#" -No tests found in project! Forge looks for functions that starts with `test`. + cmd.arg("test").assert_success().stdout_eq(str![[r#" +... +No tests found in project! Forge looks for functions that start with `test` "#]]); -}); -// tests that warning is displayed with pattern when no tests match -forgetest!(warn_no_tests_match, |prj, cmd| { - prj.add_source( - "dummy", - r" -contract Dummy {} -", - ); - - // set up command - cmd.args(["test", "--match-test", "testA.*", "--no-match-test", "testB.*"]); - cmd.args(["--match-contract", "TestC.*", "--no-match-contract", "TestD.*"]); - cmd.args(["--match-path", "*TestE*", "--no-match-path", "*TestF*"]); - - // run command and assert - cmd.assert_failure().stdout_eq(str![[r#" -No tests match the provided pattern: - match-test: `testA.*` - no-match-test: `testB.*` - match-contract: `TestC.*` - no-match-contract: `TestD.*` - match-path: `*TestE*` - no-match-path: `*TestF*` + cmd.forge_fuse(); + dummy_test_filter(&mut cmd); + cmd.assert_success().stdout_eq(str![[r#" +... +No tests found in project! Forge looks for functions that start with `test` "#]]); }); -// tests that suggestion is provided with pattern when no tests match +// tests that a warning is displayed if there are tests but none match a non-empty filter forgetest!(suggest_when_no_tests_match, |prj, cmd| { - // set up project prj.add_source( "TestE.t.sol", r" @@ -94,14 +79,9 @@ contract TestC { ", ); - // set up command - cmd.args(["test", "--match-test", "testA.*", "--no-match-test", "testB.*"]); - cmd.args(["--match-contract", "TestC.*", "--no-match-contract", "TestD.*"]); - cmd.args(["--match-path", "*TestE*", "--no-match-path", "*TestF*"]); - - // run command and assert - cmd.assert_failure().stdout_eq(str![[r#" -No tests match the provided pattern: + dummy_test_filter(&mut cmd); + cmd.assert_success().stderr_eq(str![[r#" +Warning: no tests match the provided pattern: match-test: `testA.*` no-match-test: `testB.*` match-contract: `TestC.*` @@ -509,10 +489,8 @@ forgetest_init!(exit_code_error_on_fail_fast, |prj, cmd| { prj.wipe_contracts(); prj.add_source("failing_test", FAILING_TEST); - // set up command cmd.args(["test", "--fail-fast"]); - // run command and assert error exit code cmd.assert_empty_stderr(); }); @@ -520,10 +498,8 @@ forgetest_init!(exit_code_error_on_fail_fast_with_json, |prj, cmd| { prj.wipe_contracts(); prj.add_source("failing_test", FAILING_TEST); - // set up command cmd.args(["test", "--fail-fast", "--json"]); - // run command and assert error exit code cmd.assert_empty_stderr(); }); diff --git a/crates/forge/tests/cli/test_optimizer.rs b/crates/forge/tests/cli/test_optimizer.rs index c6ac2e5f5ca8d..701edc3b34884 100644 --- a/crates/forge/tests/cli/test_optimizer.rs +++ b/crates/forge/tests/cli/test_optimizer.rs @@ -41,7 +41,7 @@ forgetest_init!(toggle_invalidate_cache_on_test, |prj, cmd| { // All files are built with optimized tests. cmd.args(["test"]).with_no_redact().assert_success().stdout_eq(str![[r#" ... -Compiling 21 files with [..] +Compiling 23 files with [..] ... "#]]); @@ -60,7 +60,7 @@ No files changed, compilation skipped // All files are rebuilt with preprocessed cache false. cmd.with_no_redact().assert_success().stdout_eq(str![[r#" ... -Compiling 21 files with [..] +Compiling 23 files with [..] ... "#]]); @@ -121,7 +121,7 @@ contract CounterTest is Test { } "#, ); - // All 20 files are compiled on first run. + // All files are compiled on first run. cmd.args(["test"]).with_no_redact().assert_success().stdout_eq(str![[r#" ... Compiling 21 files with [..] diff --git a/crates/test-utils/src/filter.rs b/crates/test-utils/src/filter.rs index 7bef934e2d981..b7813bde14376 100644 --- a/crates/test-utils/src/filter.rs +++ b/crates/test-utils/src/filter.rs @@ -73,13 +73,13 @@ impl Filter { } impl TestFilter for Filter { - fn matches_test(&self, test_name: &str) -> bool { + fn matches_test(&self, test_signature: &str) -> bool { if let Some(exclude) = &self.exclude_tests - && exclude.is_match(test_name) + && exclude.is_match(test_signature) { return false; } - self.test_regex.is_match(test_name) + self.test_regex.is_match(test_signature) } fn matches_contract(&self, contract_name: &str) -> bool {