From bb3de5bad5f43e295903140ea379d135270a2391 Mon Sep 17 00:00:00 2001 From: Matthias Seitz Date: Tue, 26 Jul 2022 16:04:58 +0200 Subject: [PATCH] fix/perf(forge): improve glob path matching --- cli/src/cmd/forge/test/filter.rs | 266 +++++++++++++++++++++ cli/src/cmd/forge/{test.rs => test/mod.rs} | 196 +-------------- forge/src/multi_runner.rs | 1 + 3 files changed, 274 insertions(+), 189 deletions(-) create mode 100644 cli/src/cmd/forge/test/filter.rs rename cli/src/cmd/forge/{test.rs => test/mod.rs} (75%) diff --git a/cli/src/cmd/forge/test/filter.rs b/cli/src/cmd/forge/test/filter.rs new file mode 100644 index 000000000000..443d0c6a8c90 --- /dev/null +++ b/cli/src/cmd/forge/test/filter.rs @@ -0,0 +1,266 @@ +use crate::utils::FoundryPathExt; +use clap::Parser; +use ethers::solc::FileFilter; +use forge::TestFilter; +use foundry_config::Config; +use std::{fmt, path::Path, str::FromStr}; + +/// The filter to use during testing +/// +/// See also `FileFilter` +#[derive(Clone, Parser)] +pub struct Filter { + /// Only run test functions matching the specified regex pattern. + /// + /// Deprecated: See --match-test + #[clap(long = "match", short = 'm')] + pub pattern: Option, + + /// Only run test functions matching the specified regex pattern. + #[clap( + long = "match-test", + visible_alias = "mt", + conflicts_with = "pattern", + value_name = "REGEX" + )] + pub test_pattern: Option, + + /// Only run test functions that do not match the specified regex pattern. + #[clap( + long = "no-match-test", + visible_alias = "nmt", + conflicts_with = "pattern", + value_name = "REGEX" + )] + pub test_pattern_inverse: Option, + + /// Only run tests in contracts matching the specified regex pattern. + #[clap( + long = "match-contract", + visible_alias = "mc", + conflicts_with = "pattern", + value_name = "REGEX" + )] + pub contract_pattern: Option, + + /// Only run tests in contracts that do not match the specified regex pattern. + #[clap( + long = "no-match-contract", + visible_alias = "nmc", + conflicts_with = "pattern", + value_name = "REGEX" + )] + pub contract_pattern_inverse: Option, + + /// Only run tests in source files matching the specified glob pattern. + #[clap( + long = "match-path", + visible_alias = "mp", + conflicts_with = "pattern", + value_name = "GLOB" + )] + pub path_pattern: Option, + + /// Only run tests in source files that do not match the specified glob pattern. + #[clap( + name = "no-match-path", + long = "no-match-path", + visible_alias = "nmp", + conflicts_with = "pattern", + value_name = "GLOB" + )] + pub path_pattern_inverse: Option, +} + +impl Filter { + /// Merges the set filter globs with the config's values + pub fn with_merged_config(&self, config: &Config) -> Self { + let mut filter = self.clone(); + if filter.test_pattern.is_none() { + filter.test_pattern = config.test_pattern.clone().map(|p| p.into()); + } + if filter.test_pattern_inverse.is_none() { + filter.test_pattern_inverse = config.test_pattern_inverse.clone().map(|p| p.into()); + } + if filter.contract_pattern.is_none() { + filter.contract_pattern = config.contract_pattern.clone().map(|p| p.into()); + } + if filter.contract_pattern_inverse.is_none() { + filter.contract_pattern_inverse = + config.contract_pattern_inverse.clone().map(|p| p.into()); + } + if filter.path_pattern.is_none() { + filter.path_pattern = config.path_pattern.clone().map(Into::into); + } + if filter.path_pattern_inverse.is_none() { + filter.path_pattern_inverse = config.path_pattern_inverse.clone().map(Into::into); + } + filter + } +} + +impl fmt::Debug for Filter { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.debug_struct("Filter") + .field("match", &self.pattern.as_ref().map(|r| r.as_str())) + .field("match-test", &self.test_pattern.as_ref().map(|r| r.as_str())) + .field("no-match-test", &self.test_pattern_inverse.as_ref().map(|r| r.as_str())) + .field("match-contract", &self.contract_pattern.as_ref().map(|r| r.as_str())) + .field("no-match-contract", &self.contract_pattern_inverse.as_ref().map(|r| r.as_str())) + .field("match-path", &self.path_pattern.as_ref().map(|g| g.as_str())) + .field("no-match-path", &self.path_pattern_inverse.as_ref().map(|g| g.as_str())) + .finish_non_exhaustive() + } +} + +impl FileFilter for Filter { + /// Returns true if the file regex pattern match the `file` + /// + /// If no file regex is set this returns true if the file ends with `.t.sol`, see + /// [FoundryPathExr::is_sol_test()] + fn is_match(&self, file: &Path) -> bool { + if let Some(file) = file.as_os_str().to_str() { + if let Some(ref glob) = self.path_pattern { + return glob.is_match(file) + } + if let Some(ref glob) = self.path_pattern_inverse { + return !glob.is_match(file) + } + } + file.is_sol_test() + } +} + +impl TestFilter for Filter { + fn matches_test(&self, test_name: impl AsRef) -> bool { + let mut ok = true; + let test_name = test_name.as_ref(); + // Handle the deprecated option match + if let Some(re) = &self.pattern { + ok &= re.is_match(test_name); + } + if let Some(re) = &self.test_pattern { + ok &= re.is_match(test_name); + } + if let Some(re) = &self.test_pattern_inverse { + ok &= !re.is_match(test_name); + } + ok + } + + fn matches_contract(&self, contract_name: impl AsRef) -> bool { + let mut ok = true; + let contract_name = contract_name.as_ref(); + if let Some(re) = &self.contract_pattern { + ok &= re.is_match(contract_name); + } + if let Some(re) = &self.contract_pattern_inverse { + ok &= !re.is_match(contract_name); + } + ok + } + + fn matches_path(&self, path: impl AsRef) -> bool { + let mut ok = true; + let path = path.as_ref(); + if let Some(ref glob) = self.path_pattern { + ok &= glob.is_match(path); + } + if let Some(ref glob) = self.path_pattern_inverse { + ok &= !glob.is_match(path); + } + ok + } +} + +impl fmt::Display for Filter { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + let mut patterns = Vec::new(); + if let Some(ref p) = self.pattern { + patterns.push(format!("\tmatch: `{}`", p.as_str())); + } + if let Some(ref p) = self.test_pattern { + patterns.push(format!("\tmatch-test: `{}`", p.as_str())); + } + if let Some(ref p) = self.test_pattern_inverse { + patterns.push(format!("\tno-match-test: `{}`", p.as_str())); + } + if let Some(ref p) = self.contract_pattern { + patterns.push(format!("\tmatch-contract: `{}`", p.as_str())); + } + if let Some(ref p) = self.contract_pattern_inverse { + patterns.push(format!("\tno-match-contract: `{}`", p.as_str())); + } + if let Some(ref p) = self.path_pattern { + patterns.push(format!("\tmatch-path: `{}`", p.as_str())); + } + if let Some(ref p) = self.path_pattern_inverse { + patterns.push(format!("\tno-match-path: `{}`", p.as_str())); + } + write!(f, "{}", patterns.join("\n")) + } +} + +/// A `globset::Glob` that creates its `globset::GlobMatcher` when its created, so it doesn't need +/// to be compiled when the filter functions `TestFilter` functions are called. +#[derive(Debug, Clone)] +pub struct GlobMatcher { + /// The parsed glob + pub glob: globset::Glob, + /// The compiled `glob` + pub matcher: globset::GlobMatcher, +} + +// === impl GlobMatcher === + +impl GlobMatcher { + /// Tests whether the given path matches this pattern or not. + /// + /// The glob `./test/*` won't match absolute paths like `test/Contract.sol`, which is common + /// format here, so we also handle this case here + pub fn is_match(&self, path: &str) -> bool { + let mut matches = self.matcher.is_match(path); + if !matches && !path.starts_with("./") && self.as_str().starts_with("./") { + matches = self.matcher.is_match(format!("./{}", path)); + } + matches + } + + /// Returns the `Glob` string used to compile this matcher. + pub fn as_str(&self) -> &str { + self.glob.glob() + } +} + +impl fmt::Display for GlobMatcher { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + self.glob.fmt(f) + } +} + +impl FromStr for GlobMatcher { + type Err = globset::Error; + + fn from_str(s: &str) -> Result { + s.parse::().map(Into::into) + } +} + +impl From for GlobMatcher { + fn from(glob: globset::Glob) -> Self { + let matcher = glob.compile_matcher(); + Self { glob, matcher } + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn can_match_glob_paths() { + let matcher: GlobMatcher = "./test/*".parse().unwrap(); + assert!(matcher.is_match("test/Contract.sol")); + assert!(matcher.is_match("./test/Contract.sol")); + } +} diff --git a/cli/src/cmd/forge/test.rs b/cli/src/cmd/forge/test/mod.rs similarity index 75% rename from cli/src/cmd/forge/test.rs rename to cli/src/cmd/forge/test/mod.rs index af65b0082495..7f285baebad4 100644 --- a/cli/src/cmd/forge/test.rs +++ b/cli/src/cmd/forge/test/mod.rs @@ -7,10 +7,9 @@ use crate::{ compile, compile::ProjectCompiler, suggestions, utils, - utils::FoundryPathExt, }; use clap::{AppSettings, Parser}; -use ethers::solc::{utils::RuntimeOrHandle, FileFilter}; +use ethers::solc::utils::RuntimeOrHandle; use forge::{ decode::decode_console_logs, executor::{inspector::CheatsConfig, opts::EvmOpts}, @@ -20,200 +19,17 @@ use forge::{ identifier::{EtherscanIdentifier, LocalTraceIdentifier}, CallTraceDecoderBuilder, TraceKind, }, - MultiContractRunner, MultiContractRunnerBuilder, TestFilter, + MultiContractRunner, MultiContractRunnerBuilder, }; use foundry_common::evm::EvmArgs; use foundry_config::{figment::Figment, Config}; use regex::Regex; -use std::{ - collections::BTreeMap, - fmt, - path::{Path, PathBuf}, - sync::mpsc::channel, - thread, - time::Duration, -}; +use std::{collections::BTreeMap, path::PathBuf, sync::mpsc::channel, thread, time::Duration}; use tracing::trace; use watchexec::config::{InitConfig, RuntimeConfig}; use yansi::Paint; - -#[derive(Debug, Clone, Parser)] -pub struct Filter { - /// Only run test functions matching the specified regex pattern. - /// - /// Deprecated: See --match-test - #[clap(long = "match", short = 'm')] - pub pattern: Option, - - /// Only run test functions matching the specified regex pattern. - #[clap( - long = "match-test", - visible_alias = "mt", - conflicts_with = "pattern", - value_name = "REGEX" - )] - pub test_pattern: Option, - - /// Only run test functions that do not match the specified regex pattern. - #[clap( - long = "no-match-test", - visible_alias = "nmt", - conflicts_with = "pattern", - value_name = "REGEX" - )] - pub test_pattern_inverse: Option, - - /// Only run tests in contracts matching the specified regex pattern. - #[clap( - long = "match-contract", - visible_alias = "mc", - conflicts_with = "pattern", - value_name = "REGEX" - )] - pub contract_pattern: Option, - - /// Only run tests in contracts that do not match the specified regex pattern. - #[clap( - long = "no-match-contract", - visible_alias = "nmc", - conflicts_with = "pattern", - value_name = "REGEX" - )] - pub contract_pattern_inverse: Option, - - /// Only run tests in source files matching the specified glob pattern. - #[clap( - long = "match-path", - visible_alias = "mp", - conflicts_with = "pattern", - value_name = "GLOB" - )] - pub path_pattern: Option, - - /// Only run tests in source files that do not match the specified glob pattern. - #[clap( - name = "no-match-path", - long = "no-match-path", - visible_alias = "nmp", - conflicts_with = "pattern", - value_name = "GLOB" - )] - pub path_pattern_inverse: Option, -} - -impl Filter { - pub fn with_merged_config(&self, config: &Config) -> Self { - let mut filter = self.clone(); - if filter.test_pattern.is_none() { - filter.test_pattern = config.test_pattern.clone().map(|p| p.into()); - } - if filter.test_pattern_inverse.is_none() { - filter.test_pattern_inverse = config.test_pattern_inverse.clone().map(|p| p.into()); - } - if filter.contract_pattern.is_none() { - filter.contract_pattern = config.contract_pattern.clone().map(|p| p.into()); - } - if filter.contract_pattern_inverse.is_none() { - filter.contract_pattern_inverse = - config.contract_pattern_inverse.clone().map(|p| p.into()); - } - if filter.path_pattern.is_none() { - filter.path_pattern = config.path_pattern.clone(); - } - if filter.path_pattern_inverse.is_none() { - filter.path_pattern_inverse = config.path_pattern_inverse.clone(); - } - filter - } -} - -impl FileFilter for Filter { - /// Returns true if the file regex pattern match the `file` - /// - /// If no file regex is set this returns true if the file ends with `.t.sol`, see - /// [FoundryPathExr::is_sol_test()] - fn is_match(&self, file: &Path) -> bool { - if let Some(file) = file.as_os_str().to_str() { - if let Some(ref glob) = self.path_pattern { - return glob.compile_matcher().is_match(file) - } - if let Some(ref glob) = self.path_pattern_inverse { - return !glob.compile_matcher().is_match(file) - } - } - file.is_sol_test() - } -} - -impl TestFilter for Filter { - fn matches_test(&self, test_name: impl AsRef) -> bool { - let mut ok = true; - let test_name = test_name.as_ref(); - // Handle the deprecated option match - if let Some(re) = &self.pattern { - ok &= re.is_match(test_name); - } - if let Some(re) = &self.test_pattern { - ok &= re.is_match(test_name); - } - if let Some(re) = &self.test_pattern_inverse { - ok &= !re.is_match(test_name); - } - ok - } - - fn matches_contract(&self, contract_name: impl AsRef) -> bool { - let mut ok = true; - let contract_name = contract_name.as_ref(); - if let Some(re) = &self.contract_pattern { - ok &= re.is_match(contract_name); - } - if let Some(re) = &self.contract_pattern_inverse { - ok &= !re.is_match(contract_name); - } - ok - } - - fn matches_path(&self, path: impl AsRef) -> bool { - let mut ok = true; - let path = path.as_ref(); - if let Some(ref glob) = self.path_pattern { - ok &= glob.compile_matcher().is_match(path); - } - if let Some(ref glob) = self.path_pattern_inverse { - ok &= !glob.compile_matcher().is_match(path); - } - ok - } -} - -impl fmt::Display for Filter { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - let mut patterns = Vec::new(); - if let Some(ref p) = self.pattern { - patterns.push(format!("\tmatch: `{}`", p.as_str())); - } - if let Some(ref p) = self.test_pattern { - patterns.push(format!("\tmatch-test: `{}`", p.as_str())); - } - if let Some(ref p) = self.test_pattern_inverse { - patterns.push(format!("\tno-match-test: `{}`", p.as_str())); - } - if let Some(ref p) = self.contract_pattern { - patterns.push(format!("\tmatch-contract: `{}`", p.as_str())); - } - if let Some(ref p) = self.contract_pattern_inverse { - patterns.push(format!("\tno-match-contract: `{}`", p.as_str())); - } - if let Some(ref p) = self.path_pattern { - patterns.push(format!("\tmatch-path: `{}`", p.glob())); - } - if let Some(ref p) = self.path_pattern_inverse { - patterns.push(format!("\tno-match-path: `{}`", p.glob())); - } - write!(f, "{}", patterns.join("\n")) - } -} +mod filter; +pub use filter::Filter; // Loads project's figment and merges the build cli arguments into it foundry_config::impl_figment_convert!(TestArgs, opts, evm_opts); @@ -473,6 +289,8 @@ pub fn custom_run(args: TestArgs, include_fuzz_tests: bool) -> eyre::Result usize { self.contracts .iter()