Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(forge/test): cache initial executor, don't clone options #7286

Merged
merged 9 commits into from
Mar 1, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion crates/common/src/compile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ use std::{
path::{Path, PathBuf},
result,
str::FromStr,
time::Instant,
};

/// Builder type to configure how to compile a project.
Expand Down Expand Up @@ -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();

Expand Down
2 changes: 1 addition & 1 deletion crates/evm/evm/src/executors/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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].
Expand Down
4 changes: 1 addition & 3 deletions crates/forge/bin/cmd/coverage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
14 changes: 7 additions & 7 deletions crates/forge/bin/cmd/test/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)?;

Expand All @@ -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.
Expand Down Expand Up @@ -250,15 +250,14 @@ impl TestArgs {
config: Config,
verbosity: u8,
filter: &ProjectPathsAwareFilter,
test_options: TestOptions,
) -> eyre::Result<TestOutcome> {
if self.list {
return list(runner, filter, self.json);
}

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() {
Expand All @@ -273,7 +272,8 @@ 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.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}`?");
}
Expand All @@ -289,7 +289,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));
}
Expand All @@ -305,7 +305,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 =
Expand Down
14 changes: 6 additions & 8 deletions crates/forge/src/lib.rs
Original file line number Diff line number Diff line change
@@ -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;

Expand Down Expand Up @@ -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)
}
}
}
Expand Down
161 changes: 74 additions & 87 deletions crates/forge/src/multi_runner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ use std::{
fmt::Debug,
path::Path,
sync::{mpsc, Arc},
time::Instant,
};

pub type DeployableContracts = BTreeMap<ArtifactId, (JsonAbi, Bytes, Vec<Bytes>)>;
Expand Down Expand Up @@ -65,57 +66,47 @@ 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<Item = (&ArtifactId, &(JsonAbi, Bytes, Vec<Bytes>))> {
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,
) -> impl Iterator<Item = &Function> {
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()))
})
self.matching_contracts(filter)
.flat_map(|(_, (abi, _, _))| abi.functions())
.filter(|func| is_matching_test(func, filter))
}

/// 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<Item = &Function> {
/// 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<Item = &Function> {
self.contracts
.iter()
.filter(|(id, _)| filter.matches_path(&id.source) && filter.matches_contract(&id.name))
.flat_map(|(_, (abi, _, _))| abi.functions())
}

/// Get all test names matching the filter
pub fn get_tests(&self, filter: &dyn TestFilter) -> Vec<String> {
self.filtered_tests(filter)
.map(|func| func.name.clone())
.filter(|name| name.is_test())
.collect()
.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<String, BTreeMap<String, Vec<String>>> {
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::<Vec<_>>();

(source, name, tests)
})
.fold(BTreeMap::new(), |mut acc, (source, name, tests)| {
Expand All @@ -129,12 +120,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<String, SuiteResult> {
self.test_iter(filter, test_options).await.collect()
pub async fn test_collect(&mut self, filter: &dyn TestFilter) -> BTreeMap<String, SuiteResult> {
self.test_iter(filter).await.collect()
}

/// Executes _all_ tests that match the given `filter`.
Expand All @@ -145,10 +132,9 @@ impl MultiContractRunner {
pub async fn test_iter(
&mut self,
filter: &dyn TestFilter,
test_options: TestOptions,
) -> impl Iterator<Item = (String, SuiteResult)> {
let (tx, rx) = mpsc::channel();
self.test(filter, tx, test_options).await;
self.test(filter, tx).await;
rx.into_iter()
}

Expand All @@ -158,50 +144,40 @@ 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
// The DB backend that serves all the data.
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.matching_contracts(filter).collect::<Vec<_>>();
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 executor = executor.clone();
let result = self.run_tests(&identifier, abi, executor, deploy_code, libs, filter);
let _ = tx.send((identifier, result));
})
}

#[allow(clippy::too_many_arguments)]
Expand All @@ -210,20 +186,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,
Expand All @@ -236,7 +209,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
}
}

Expand Down Expand Up @@ -395,3 +372,13 @@ 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))
}

/// 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_test() || func.is_invariant_test()) && filter.matches_test(&func.signature())
}
Loading
Loading