From ab35d021a1df9575e7e3a5bef34329e9f2e1e1bc Mon Sep 17 00:00:00 2001 From: Matthias Seitz Date: Wed, 3 Aug 2022 16:02:43 +0200 Subject: [PATCH 1/2] refactor: use TestFunctionExt for string --- cli/src/cmd/utils.rs | 4 +- cli/src/compile.rs | 3 +- common/src/lib.rs | 2 + common/src/traits.rs | 80 ++++++++++++++++++++++++++++++++++++ evm/src/coverage/analysis.rs | 3 +- forge/src/gas_report.rs | 3 +- forge/src/lib.rs | 4 +- forge/src/multi_runner.rs | 7 ++-- forge/src/runner.rs | 7 ++-- forge/src/traits.rs | 39 ------------------ 10 files changed, 100 insertions(+), 52 deletions(-) create mode 100644 common/src/traits.rs delete mode 100644 forge/src/traits.rs diff --git a/cli/src/cmd/utils.rs b/cli/src/cmd/utils.rs index 2946ae393d89..9afa3aa78cee 100644 --- a/cli/src/cmd/utils.rs +++ b/cli/src/cmd/utils.rs @@ -12,6 +12,7 @@ use ethers::{ Artifact, ProjectCompileOutput, }, }; +use foundry_common::TestFunctionExt; use foundry_config::Chain as ConfigChain; use foundry_utils::Retry; use std::{collections::BTreeMap, path::PathBuf}; @@ -148,8 +149,7 @@ impl From for Retry { } pub fn needs_setup(abi: &Abi) -> bool { - let setup_fns: Vec<_> = - abi.functions().filter(|func| func.name.to_lowercase() == "setup").collect(); + let setup_fns: Vec<_> = abi.functions().filter(|func| func.name.is_setup()).collect(); for setup_fn in setup_fns.iter() { if setup_fn.name != "setUp" { diff --git a/cli/src/compile.rs b/cli/src/compile.rs index ad19ac90ef02..d4172010335e 100644 --- a/cli/src/compile.rs +++ b/cli/src/compile.rs @@ -6,6 +6,7 @@ use ethers::{ prelude::Graph, solc::{report::NoReporter, Artifact, FileFilter, Project, ProjectCompileOutput}, }; +use foundry_common::TestFunctionExt; use std::{ collections::BTreeMap, fmt::Display, @@ -188,7 +189,7 @@ impl ProjectCompiler { let dev_functions = contract.abi.as_ref().unwrap().abi.functions().into_iter().filter( |func| { - func.name.starts_with("test") || + func.name.is_test() || func.name.eq("IS_TEST") || func.name.eq("IS_SCRIPT") }, diff --git a/common/src/lib.rs b/common/src/lib.rs index 8d19352b5fb6..b0d92bd40bfa 100644 --- a/common/src/lib.rs +++ b/common/src/lib.rs @@ -7,4 +7,6 @@ pub mod errors; pub mod evm; pub mod fmt; pub mod fs; +pub mod traits; pub use constants::*; +pub use traits::*; diff --git a/common/src/traits.rs b/common/src/traits.rs new file mode 100644 index 000000000000..cd1a43de6fde --- /dev/null +++ b/common/src/traits.rs @@ -0,0 +1,80 @@ +//! Commonly used traits + +use ethers_core::abi::Function; + +/// Extension trait for matching tests +pub trait TestFilter: Send + Sync { + /// Returns whether the test should be included + fn matches_test(&self, test_name: impl AsRef) -> bool; + /// Returns whether the contract should be included + fn matches_contract(&self, contract_name: impl AsRef) -> bool; + /// Returns a contract with the given path should be included + fn matches_path(&self, path: impl AsRef) -> bool; +} + +/// Extension trait for `Function` +pub trait TestFunctionExt { + /// Whether this function should be executed as fuzz test + fn is_fuzz_test(&self) -> bool; + /// Whether this function is a test + fn is_test(&self) -> bool; + /// Whether this function is a test that should fail + fn is_test_fail(&self) -> bool; + /// Whether this function is a `setUp` function + fn is_setup(&self) -> bool; +} + +impl TestFunctionExt for Function { + fn is_fuzz_test(&self) -> bool { + // test functions that have inputs are considered fuzz tests as those inputs will be fuzzed + !self.inputs.is_empty() + } + + fn is_test(&self) -> bool { + self.name.is_test() + } + + fn is_test_fail(&self) -> bool { + self.name.is_test_fail() + } + + fn is_setup(&self) -> bool { + self.name.is_setup() + } +} + +impl<'a> TestFunctionExt for &'a str { + fn is_fuzz_test(&self) -> bool { + self.contains("fuzz") + } + + fn is_test(&self) -> bool { + self.starts_with("test") + } + + fn is_test_fail(&self) -> bool { + self.starts_with("testFail") + } + + fn is_setup(&self) -> bool { + self.to_lowercase() == "setup" + } +} + +impl TestFunctionExt for String { + fn is_fuzz_test(&self) -> bool { + self.as_str().is_fuzz_test() + } + + fn is_test(&self) -> bool { + self.as_str().is_test() + } + + fn is_test_fail(&self) -> bool { + self.as_str().is_test_fail() + } + + fn is_setup(&self) -> bool { + self.as_str().is_setup() + } +} diff --git a/evm/src/coverage/analysis.rs b/evm/src/coverage/analysis.rs index 3d83c5d6c7f5..4b83d35558be 100644 --- a/evm/src/coverage/analysis.rs +++ b/evm/src/coverage/analysis.rs @@ -1,5 +1,6 @@ use super::{ContractId, CoverageItem, CoverageItemKind, SourceLocation}; use ethers::solc::artifacts::ast::{self, Ast, Node, NodeType}; +use foundry_common::TestFunctionExt; use semver::Version; use std::collections::HashMap; use tracing::warn; @@ -482,7 +483,7 @@ impl SourceAnalyzer { )?; let is_test = items.iter().any(|item| { if let CoverageItemKind::Function { name } = &item.kind { - name.starts_with("test") + name.is_test() } else { false } diff --git a/forge/src/gas_report.rs b/forge/src/gas_report.rs index 96c3e546af54..060881b8330e 100644 --- a/forge/src/gas_report.rs +++ b/forge/src/gas_report.rs @@ -4,6 +4,7 @@ use crate::{ }; use comfy_table::{modifiers::UTF8_ROUND_CORNERS, presets::UTF8_FULL, *}; use ethers::types::U256; +use foundry_common::TestFunctionExt; use serde::{Deserialize, Serialize}; use std::{collections::BTreeMap, fmt::Display}; @@ -70,7 +71,7 @@ impl GasReport { } // TODO: More robust test contract filtering RawOrDecodedCall::Decoded(func, sig, _) - if !func.starts_with("test") && func != "setUp" => + if !func.is_test() && !func.is_setup() => { let function_report = contract_report .functions diff --git a/forge/src/lib.rs b/forge/src/lib.rs index 335af24b6b3e..dfdae9465445 100644 --- a/forge/src/lib.rs +++ b/forge/src/lib.rs @@ -12,8 +12,8 @@ pub use runner::ContractRunner; mod multi_runner; pub use multi_runner::{MultiContractRunner, MultiContractRunnerBuilder}; -mod traits; -pub use traits::*; +/// reexport +pub use foundry_common::traits::TestFilter; pub mod result; diff --git a/forge/src/multi_runner.rs b/forge/src/multi_runner.rs index a41d7ae0cd3b..22e1515bf7c0 100644 --- a/forge/src/multi_runner.rs +++ b/forge/src/multi_runner.rs @@ -6,6 +6,7 @@ use ethers::{ types::{Address, Bytes, U256}, }; use eyre::Result; +use foundry_common::TestFunctionExt; use foundry_evm::{ executor::{ backend::Backend, fork::CreateFork, inspector::CheatsConfig, opts::EvmOpts, Executor, @@ -74,7 +75,7 @@ impl MultiContractRunner { filter.matches_contract(&id.name) }) .flat_map(|(_, (abi, _, _))| abi.functions().map(|func| func.name.clone())) - .filter(|sig| sig.starts_with("test")) + .filter(|sig| sig.is_test()) .collect() } @@ -95,7 +96,7 @@ impl MultiContractRunner { let name = id.name.clone(); let tests = abi .functions() - .filter(|func| func.name.starts_with("test")) + .filter(|func| func.name.is_test()) .filter(|func| filter.matches_test(func.signature())) .map(|func| func.name.clone()) .collect::>(); @@ -282,7 +283,7 @@ impl MultiContractRunnerBuilder { let abi = contract.abi.expect("We should have an abi by now"); // if it's a test, add it to deployable contracts if abi.constructor.as_ref().map(|c| c.inputs.is_empty()).unwrap_or(true) && - abi.functions().any(|func| func.name.starts_with("test")) + abi.functions().any(|func| func.name.is_test()) { deployable_contracts.insert( id.clone(), diff --git a/forge/src/runner.rs b/forge/src/runner.rs index 2150c507ce10..78da2fb0b04f 100644 --- a/forge/src/runner.rs +++ b/forge/src/runner.rs @@ -7,6 +7,7 @@ use ethers::{ types::{Address, Bytes, U256}, }; use eyre::Result; +use foundry_common::TestFunctionExt; use foundry_evm::{ executor::{CallResult, DeployResult, EvmError, Executor}, fuzz::FuzzedExecutor, @@ -182,7 +183,7 @@ impl<'a> ContractRunner<'a> { let mut warnings = Vec::new(); let setup_fns: Vec<_> = - self.contract.functions().filter(|func| func.name.to_lowercase() == "setup").collect(); + self.contract.functions().filter(|func| func.name.is_setup()).collect(); let needs_setup = setup_fns.len() == 1 && setup_fns[0].name == "setUp"; @@ -247,11 +248,11 @@ impl<'a> ContractRunner<'a> { .functions() .into_iter() .filter(|func| { - func.name.starts_with("test") && + func.name.is_test() && filter.matches_test(func.signature()) && (include_fuzz_tests || func.inputs.is_empty()) }) - .map(|func| (func, func.name.starts_with("testFail"))) + .map(|func| (func, func.name.is_test_fail())) .collect(); let test_results = tests diff --git a/forge/src/traits.rs b/forge/src/traits.rs deleted file mode 100644 index ecd5c4d922dc..000000000000 --- a/forge/src/traits.rs +++ /dev/null @@ -1,39 +0,0 @@ -use ethers::abi::Function; - -/// Extension trait for matching tests -pub trait TestFilter: Send + Sync { - fn matches_test(&self, test_name: impl AsRef) -> bool; - fn matches_contract(&self, contract_name: impl AsRef) -> bool; - fn matches_path(&self, path: impl AsRef) -> bool; -} - -/// Extension trait for `Function` -pub(crate) trait TestFunctionExt { - /// Whether this function should be executed as fuzz test - fn is_fuzz_test(&self) -> bool; - /// Whether this function is a test - fn is_test(&self) -> bool; - /// Whether this function is a test that should fail - fn is_test_fail(&self) -> bool; - /// Whether this function is a `setUp` function - fn is_setup(&self) -> bool; -} - -impl TestFunctionExt for Function { - fn is_fuzz_test(&self) -> bool { - // test functions that have inputs are considered fuzz tests as those inputs will be fuzzed - !self.inputs.is_empty() - } - - fn is_test(&self) -> bool { - self.name.starts_with("test") - } - - fn is_test_fail(&self) -> bool { - self.name.starts_with("testFail") - } - - fn is_setup(&self) -> bool { - self.name.to_lowercase() == "setup" - } -} From 9e82234fda9244861fd31a95b88b86fe5c24aec1 Mon Sep 17 00:00:00 2001 From: Matthias Seitz Date: Wed, 3 Aug 2022 17:06:56 +0200 Subject: [PATCH 2/2] add unimplemented --- common/src/traits.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/src/traits.rs b/common/src/traits.rs index cd1a43de6fde..97d7a88f6718 100644 --- a/common/src/traits.rs +++ b/common/src/traits.rs @@ -45,7 +45,7 @@ impl TestFunctionExt for Function { impl<'a> TestFunctionExt for &'a str { fn is_fuzz_test(&self) -> bool { - self.contains("fuzz") + unimplemented!("no naming convention for fuzz tests.") } fn is_test(&self) -> bool {