From 85669c27554ecafb18f1c65770b5eaaef5c004c9 Mon Sep 17 00:00:00 2001 From: DaniPopes <57450786+DaniPopes@users.noreply.github.com> Date: Thu, 15 Feb 2024 00:29:03 +0200 Subject: [PATCH] perf/refactor: partial test runner refactor (#7109) * perf/refactor: partial test runner refactor * fix: update test output * fix: identify addresses if *any* trace * fix: clear addresses before decoding traces * fix: keep default labels * fix: mean overflow * chore: reorder some stuff * perf: speed up bytecode_diff_score in debug mode --- Cargo.toml | 13 +- clippy.toml | 2 +- crates/common/src/calc.rs | 56 ++- crates/common/src/contracts.rs | 28 +- crates/evm/core/src/decode.rs | 3 + crates/evm/fuzz/src/lib.rs | 22 +- crates/evm/traces/src/decoder/mod.rs | 20 +- crates/evm/traces/src/identifier/etherscan.rs | 2 +- crates/forge/bin/cmd/coverage.rs | 2 +- crates/forge/bin/cmd/snapshot.rs | 15 +- crates/forge/bin/cmd/test/mod.rs | 406 +++++------------- crates/forge/bin/cmd/test/summary.rs | 76 ++-- crates/forge/src/gas_report.rs | 99 ++--- crates/forge/src/multi_runner.rs | 122 +++--- crates/forge/src/result.rs | 248 ++++++++++- crates/forge/tests/cli/cmd.rs | 28 +- .../tests/fixtures/can_check_snapshot.stdout | 6 +- .../can_run_test_in_custom_test_folder.stdout | 6 +- .../tests/fixtures/can_test_repeatedly.stdout | 6 +- .../can_use_libs_in_multi_fork.stdout | 6 +- .../include_custom_types_in_traces.stdout | 6 +- crates/forge/tests/fixtures/repro_6531.stdout | 6 +- ...xactly_once_with_changed_versions.1.stdout | 6 +- ...xactly_once_with_changed_versions.2.stdout | 6 +- 24 files changed, 617 insertions(+), 573 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 97e815342e71..d4196400e1dc 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -28,7 +28,7 @@ resolver = "2" [workspace.package] version = "0.2.0" edition = "2021" -rust-version = "1.74" # Remember to update clippy.toml as well +rust-version = "1.75" # Remember to update clippy.toml as well authors = ["Foundry Contributors"] license = "MIT OR Apache-2.0" homepage = "https://github.com/foundry-rs/foundry" @@ -49,7 +49,10 @@ solang-parser.opt-level = 3 serde_json.opt-level = 3 # EVM +alloy-dyn-abi.opt-level = 3 +alloy-json-abi.opt-level = 3 alloy-primitives.opt-level = 3 +alloy-sol-type-parser.opt-level = 3 alloy-sol-types.opt-level = 3 hashbrown.opt-level = 3 keccak.opt-level = 3 @@ -62,12 +65,16 @@ sha2.opt-level = 3 sha3.opt-level = 3 tiny-keccak.opt-level = 3 -# keystores -scrypt.opt-level = 3 +# fuzzing +proptest.opt-level = 3 +foundry-evm-fuzz.opt-level = 3 # forking axum.opt-level = 3 +# keystores +scrypt.opt-level = 3 + # Local "release" mode, more optimized than dev but much faster to compile than release. [profile.local] inherits = "dev" diff --git a/clippy.toml b/clippy.toml index 866add684a96..cc4ad18b1872 100644 --- a/clippy.toml +++ b/clippy.toml @@ -1 +1 @@ -msrv = "1.74" +msrv = "1.75" diff --git a/crates/common/src/calc.rs b/crates/common/src/calc.rs index e404c4520166..bde75635ce7f 100644 --- a/crates/common/src/calc.rs +++ b/crates/common/src/calc.rs @@ -1,29 +1,28 @@ //! Commonly used calculations. use alloy_primitives::{Sign, U256}; -use std::ops::Div; -/// Returns the mean of the slice +/// Returns the mean of the slice. #[inline] -pub fn mean(values: &[U256]) -> U256 { +pub fn mean(values: &[u64]) -> u64 { if values.is_empty() { - return U256::ZERO + return 0; } - values.iter().copied().fold(U256::ZERO, |sum, val| sum + val).div(U256::from(values.len())) + (values.iter().map(|x| *x as u128).sum::() / values.len() as u128) as u64 } -/// Returns the median of a _sorted_ slice +/// Returns the median of a _sorted_ slice. #[inline] -pub fn median_sorted(values: &[U256]) -> U256 { +pub fn median_sorted(values: &[u64]) -> u64 { if values.is_empty() { - return U256::ZERO + return 0; } let len = values.len(); let mid = len / 2; if len % 2 == 0 { - (values[mid - 1] + values[mid]) / U256::from(2u64) + (values[mid - 1] + values[mid]) / 2 } else { values[mid] } @@ -70,49 +69,42 @@ mod tests { #[test] fn calc_mean_empty() { - let values: [U256; 0] = []; - let m = mean(&values); - assert_eq!(m, U256::ZERO); + let m = mean(&[]); + assert_eq!(m, 0); } #[test] fn calc_mean() { - let values = [ - U256::ZERO, - U256::from(1), - U256::from(2u64), - U256::from(3u64), - U256::from(4u64), - U256::from(5u64), - U256::from(6u64), - ]; - let m = mean(&values); - assert_eq!(m, U256::from(3u64)); + let m = mean(&[0, 1, 2, 3, 4, 5, 6]); + assert_eq!(m, 3); + } + + #[test] + fn calc_mean_overflow() { + let m = mean(&[0, 1, 2, u32::MAX as u64, 3, u16::MAX as u64, u64::MAX, 6]); + assert_eq!(m, 2305843009750573057); } #[test] fn calc_median_empty() { - let values: Vec = vec![]; - let m = median_sorted(&values); - assert_eq!(m, U256::from(0)); + let m = median_sorted(&[]); + assert_eq!(m, 0); } #[test] fn calc_median() { - let mut values = - vec![29, 30, 31, 40, 59, 61, 71].into_iter().map(U256::from).collect::>(); + let mut values = vec![29, 30, 31, 40, 59, 61, 71]; values.sort(); let m = median_sorted(&values); - assert_eq!(m, U256::from(40)); + assert_eq!(m, 40); } #[test] fn calc_median_even() { - let mut values = - vec![80, 90, 30, 40, 50, 60, 10, 20].into_iter().map(U256::from).collect::>(); + let mut values = vec![80, 90, 30, 40, 50, 60, 10, 20]; values.sort(); let m = median_sorted(&values); - assert_eq!(m, U256::from(45)); + assert_eq!(m, 45); } #[test] diff --git a/crates/common/src/contracts.rs b/crates/common/src/contracts.rs index 818dee04596a..cef420518c0f 100644 --- a/crates/common/src/contracts.rs +++ b/crates/common/src/contracts.rs @@ -103,6 +103,7 @@ pub type ContractsByAddress = BTreeMap; /// /// Returns a value between `0.0` (identical) and `1.0` (completely different). pub fn bytecode_diff_score<'a>(mut a: &'a [u8], mut b: &'a [u8]) -> f64 { + // Make sure `a` is the longer one. if a.len() < b.len() { std::mem::swap(&mut a, &mut b); } @@ -119,11 +120,36 @@ pub fn bytecode_diff_score<'a>(mut a: &'a [u8], mut b: &'a [u8]) -> f64 { } // Count different bytes. - n_different_bytes += std::iter::zip(a, b).filter(|(a, b)| a != b).count(); + // SAFETY: `a` is longer than `b`. + n_different_bytes += unsafe { count_different_bytes(a, b) }; n_different_bytes as f64 / a.len() as f64 } +/// Returns the amount of different bytes between two slices. +/// +/// # Safety +/// +/// `a` must be at least as long as `b`. +unsafe fn count_different_bytes(a: &[u8], b: &[u8]) -> usize { + // This could've been written as `std::iter::zip(a, b).filter(|(x, y)| x != y).count()`, + // however this function is very hot, and has been written to be as primitive as + // possible for lower optimization levels. + + let a_ptr = a.as_ptr(); + let b_ptr = b.as_ptr(); + let len = b.len(); + + let mut sum = 0; + let mut i = 0; + while i < len { + // SAFETY: `a` is at least as long as `b`, and `i` is in bound of `b`. + sum += unsafe { *a_ptr.add(i) != *b_ptr.add(i) } as usize; + i += 1; + } + sum +} + /// Flattens the contracts into (`id` -> (`JsonAbi`, `Vec`)) pairs pub fn flatten_contracts( contracts: &BTreeMap, diff --git a/crates/evm/core/src/decode.rs b/crates/evm/core/src/decode.rs index d4b889779859..b9b1d9f52163 100644 --- a/crates/evm/core/src/decode.rs +++ b/crates/evm/core/src/decode.rs @@ -41,6 +41,9 @@ pub fn decode_revert( }) } +/// Tries to decode an error message from the given revert bytes. +/// +/// See [`decode_revert`] for more information. pub fn maybe_decode_revert( err: &[u8], maybe_abi: Option<&JsonAbi>, diff --git a/crates/evm/fuzz/src/lib.rs b/crates/evm/fuzz/src/lib.rs index 55c1f1c3e5d5..ed3b50178f57 100644 --- a/crates/evm/fuzz/src/lib.rs +++ b/crates/evm/fuzz/src/lib.rs @@ -8,7 +8,7 @@ extern crate tracing; use alloy_dyn_abi::{DynSolValue, JsonAbiExt}; -use alloy_primitives::{Address, Bytes, Log, U256}; +use alloy_primitives::{Address, Bytes, Log}; use foundry_common::{calc, contracts::ContractsByAddress}; use foundry_evm_coverage::HitMaps; use foundry_evm_traces::CallTraceArena; @@ -157,18 +157,16 @@ pub struct FuzzTestResult { impl FuzzTestResult { /// Returns the median gas of all test cases pub fn median_gas(&self, with_stipend: bool) -> u64 { - let mut values = - self.gas_values(with_stipend).into_iter().map(U256::from).collect::>(); + let mut values = self.gas_values(with_stipend); values.sort_unstable(); - calc::median_sorted(&values).to::() + calc::median_sorted(&values) } /// Returns the average gas use of all test cases pub fn mean_gas(&self, with_stipend: bool) -> u64 { - let mut values = - self.gas_values(with_stipend).into_iter().map(U256::from).collect::>(); + let mut values = self.gas_values(with_stipend); values.sort_unstable(); - calc::mean(&values).to::() + calc::mean(&values) } fn gas_values(&self, with_stipend: bool) -> Vec { @@ -223,19 +221,17 @@ impl FuzzedCases { /// Returns the median gas of all test cases #[inline] pub fn median_gas(&self, with_stipend: bool) -> u64 { - let mut values = - self.gas_values(with_stipend).into_iter().map(U256::from).collect::>(); + let mut values = self.gas_values(with_stipend); values.sort_unstable(); - calc::median_sorted(&values).to::() + calc::median_sorted(&values) } /// Returns the average gas use of all test cases #[inline] pub fn mean_gas(&self, with_stipend: bool) -> u64 { - let mut values = - self.gas_values(with_stipend).into_iter().map(U256::from).collect::>(); + let mut values = self.gas_values(with_stipend); values.sort_unstable(); - calc::mean(&values).to::() + calc::mean(&values) } #[inline] diff --git a/crates/evm/traces/src/decoder/mod.rs b/crates/evm/traces/src/decoder/mod.rs index 0fac862b29eb..9057fb92a7a3 100644 --- a/crates/evm/traces/src/decoder/mod.rs +++ b/crates/evm/traces/src/decoder/mod.rs @@ -99,12 +99,14 @@ pub struct CallTraceDecoder { pub labels: HashMap, /// Contract addresses that have a receive function. pub receive_contracts: Vec
, + /// All known functions. pub functions: HashMap>, /// All known events. pub events: BTreeMap<(B256, usize), Vec>, /// All known errors. pub errors: JsonAbi, + /// A signature identifier for events and functions. pub signature_identifier: Option, /// Verbosity level @@ -143,7 +145,6 @@ impl CallTraceDecoder { Self { contracts: Default::default(), - labels: [ (CHEATCODE_ADDRESS, "VM".to_string()), (HARDHAT_CONSOLE_ADDRESS, "console".to_string()), @@ -152,6 +153,7 @@ impl CallTraceDecoder { (TEST_CONTRACT_ADDRESS, "DefaultTestContract".to_string()), ] .into(), + receive_contracts: Default::default(), functions: hh_funcs() .chain( @@ -162,20 +164,30 @@ impl CallTraceDecoder { ) .map(|(selector, func)| (selector, vec![func])) .collect(), - events: Console::abi::events() .into_values() .flatten() .map(|event| ((event.selector(), indexed_inputs(&event)), vec![event])) .collect(), - errors: Default::default(), + signature_identifier: None, - receive_contracts: Default::default(), verbosity: 0, } } + /// Clears all known addresses. + pub fn clear_addresses(&mut self) { + self.contracts.clear(); + + let default_labels = &Self::new().labels; + if self.labels.len() > default_labels.len() { + self.labels = default_labels.clone(); + } + + self.receive_contracts.clear(); + } + /// Identify unknown addresses in the specified call trace using the specified identifier. /// /// Unknown contracts are contracts that either lack a label or an ABI. diff --git a/crates/evm/traces/src/identifier/etherscan.rs b/crates/evm/traces/src/identifier/etherscan.rs index 1ddbf2457d6f..e995b36f9b66 100644 --- a/crates/evm/traces/src/identifier/etherscan.rs +++ b/crates/evm/traces/src/identifier/etherscan.rs @@ -99,7 +99,7 @@ impl TraceIdentifier for EtherscanIdentifier { where A: Iterator)>, { - trace!(target: "etherscanidentifier", "identify {:?} addresses", addresses.size_hint().1); + trace!("identify {:?} addresses", addresses.size_hint().1); let Some(client) = self.client.clone() else { // no client was configured 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/snapshot.rs b/crates/forge/bin/cmd/snapshot.rs index bd657abe8438..f60ab01bb01d 100644 --- a/crates/forge/bin/cmd/snapshot.rs +++ b/crates/forge/bin/cmd/snapshot.rs @@ -1,11 +1,8 @@ -use super::{ - test, - test::{Test, TestOutcome}, -}; +use super::test; use alloy_primitives::U256; use clap::{builder::RangedU64ValueParser, Parser, ValueHint}; use eyre::{Context, Result}; -use forge::result::TestKindReport; +use forge::result::{SuiteTestResult, TestKindReport, TestOutcome}; use foundry_cli::utils::STATIC_FUZZ_SEED; use once_cell::sync::Lazy; use regex::Regex; @@ -175,7 +172,7 @@ impl SnapshotConfig { true } - fn apply(&self, outcome: TestOutcome) -> Vec { + fn apply(&self, outcome: TestOutcome) -> Vec { let mut tests = outcome .into_tests() .filter(|test| self.is_in_gas_range(test.gas_used())) @@ -274,7 +271,7 @@ fn read_snapshot(path: impl AsRef) -> Result> { /// Writes a series of tests to a snapshot file after sorting them fn write_to_snapshot_file( - tests: &[Test], + tests: &[SuiteTestResult], path: impl AsRef, _format: Option, ) -> Result<()> { @@ -318,7 +315,7 @@ impl SnapshotDiff { /// Compares the set of tests with an existing snapshot /// /// Returns true all tests match -fn check(tests: Vec, snaps: Vec, tolerance: Option) -> bool { +fn check(tests: Vec, snaps: Vec, tolerance: Option) -> bool { let snaps = snaps .into_iter() .map(|s| ((s.contract_name, s.signature), s.gas_used)) @@ -352,7 +349,7 @@ fn check(tests: Vec, snaps: Vec, tolerance: Option) -> } /// Compare the set of tests with an existing snapshot -fn diff(tests: Vec, snaps: Vec) -> Result<()> { +fn diff(tests: Vec, snaps: Vec) -> Result<()> { let snaps = snaps .into_iter() .map(|s| ((s.contract_name, s.signature), s.gas_used)) diff --git a/crates/forge/bin/cmd/test/mod.rs b/crates/forge/bin/cmd/test/mod.rs index 10d188c4b6a2..102f93e31ef1 100644 --- a/crates/forge/bin/cmd/test/mod.rs +++ b/crates/forge/bin/cmd/test/mod.rs @@ -6,7 +6,7 @@ use forge::{ decode::decode_console_logs, gas_report::GasReport, inspectors::CheatsConfig, - result::{SuiteResult, TestResult, TestStatus}, + result::{SuiteResult, TestOutcome, TestStatus}, traces::{ identifier::{EtherscanIdentifier, LocalTraceIdentifier, SignaturesIdentifier}, CallTraceDecoderBuilder, TraceKind, @@ -21,7 +21,7 @@ use foundry_common::{ compact_to_contract, compile::{ContractSources, ProjectCompiler}, evm::EvmArgs, - get_contract_name, get_file_name, shell, + shell, }; use foundry_config::{ figment, @@ -33,7 +33,7 @@ use foundry_config::{ }; use foundry_debugger::Debugger; use regex::Regex; -use std::{collections::BTreeMap, fs, sync::mpsc::channel, time::Duration}; +use std::{collections::BTreeMap, fs, sync::mpsc::channel}; use watchexec::config::{InitConfig, RuntimeConfig}; use yansi::Paint; @@ -186,81 +186,35 @@ 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()); - - let runner = runner_builder.clone().build( - project_root, - output.clone(), - env.clone(), - evm_opts.clone(), - )?; + .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 known_contracts = runner.known_contracts.clone(); - let mut local_identifier = LocalTraceIdentifier::new(&known_contracts); - let remote_chain_id = runner.evm_opts.get_remote_chain_id(); - - let outcome = self - .run_tests(runner, config.clone(), verbosity, &filter, test_options.clone()) - .await?; + let outcome = self.run_tests(runner, config, verbosity, &filter, test_options).await?; if should_debug { - let tests = outcome.clone().into_tests(); - // todo(onbjerg): why do we bother decoding everything and having multiple decoders if - // we are only going to use the first one? (see `DebuggerArgs` below) - let mut decoders = Vec::new(); - for test in tests { - let mut result = test.result; - // Identify addresses in each trace - let mut builder = CallTraceDecoderBuilder::new() - .with_labels(result.labeled_addresses.clone()) - .with_local_identifier_abis(&local_identifier) - .with_verbosity(verbosity); - - // Signatures are of no value for gas reports - if !self.gas_report { - let sig_identifier = - SignaturesIdentifier::new(Config::foundry_cache_dir(), config.offline)?; - builder = builder.with_signature_identifier(sig_identifier.clone()); - } - - let mut decoder = builder.build(); - - if !result.traces.is_empty() { - // Set up identifiers - // Do not re-query etherscan for contracts that you've already queried today. - let mut etherscan_identifier = - EtherscanIdentifier::new(&config, remote_chain_id)?; - - // Decode the traces - for (_, trace) in &mut result.traces { - decoder.identify(trace, &mut local_identifier); - decoder.identify(trace, &mut etherscan_identifier); - } - } - - decoders.push(decoder); - } - - let mut sources: ContractSources = Default::default(); - for (id, artifact) in output.into_artifacts() { + let mut sources = ContractSources::default(); + for (id, artifact) in output_clone.unwrap().into_artifacts() { // Sources are only required for the debugger, but it *might* mean that there's // something wrong with the build and/or artifacts. if let Some(source) = artifact.source_file() { @@ -276,16 +230,20 @@ impl TestArgs { } } - let test = outcome.clone().into_tests().next().unwrap(); - let result = test.result; - // Run the debugger - let mut debugger = Debugger::builder() - // TODO: `Option::as_slice` in 1.75 - .debug_arenas(result.debug.as_ref().map(core::slice::from_ref).unwrap_or_default()) - .decoders(&decoders) + // There is only one test. + 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() + .debug_arenas(test.result.debug.as_slice()) .sources(sources) - .breakpoints(result.breakpoints) - .build(); + .breakpoints(test.result.breakpoints); + if let Some(decoder) = &outcome.decoder { + builder = builder.decoder(decoder); + } + let mut debugger = builder.build(); debugger.try_run()?; } @@ -305,16 +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 { + let num_filtered = runner.matching_test_function_count(filter); + if num_filtered == 0 { println!(); if filter.is_empty() { println!( @@ -335,6 +287,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; @@ -342,46 +301,58 @@ impl TestArgs { return Ok(TestOutcome::new(results, self.allow_failure)); } - // Set up identifiers + // Set up trace identifiers. let known_contracts = runner.known_contracts.clone(); let mut local_identifier = LocalTraceIdentifier::new(&known_contracts); let remote_chain_id = runner.evm_opts.get_remote_chain_id(); - // Do not re-query etherscan for contracts that you've already queried today. let mut etherscan_identifier = EtherscanIdentifier::new(&config, remote_chain_id)?; - // Set up test reporter channel + // Run tests. let (tx, rx) = channel::<(String, SuiteResult)>(); - - // Run tests let handle = tokio::task::spawn({ let filter = filter.clone(); async move { runner.test(&filter, tx, test_options).await } }); - let mut results = BTreeMap::new(); - let mut gas_report = GasReport::new(config.gas_reports, config.gas_reports_ignore); - let sig_identifier = - SignaturesIdentifier::new(Config::foundry_cache_dir(), config.offline)?; + let mut gas_report = + self.gas_report.then(|| GasReport::new(config.gas_reports, config.gas_reports_ignore)); + + // Build the trace decoder. + let mut builder = CallTraceDecoderBuilder::new() + .with_local_identifier_abis(&local_identifier) + .with_verbosity(verbosity); + // Signatures are of no value for gas reports. + if !self.gas_report { + builder = builder.with_signature_identifier(SignaturesIdentifier::new( + Config::foundry_cache_dir(), + config.offline, + )?); + } + let mut decoder = builder.build(); + + // We identify addresses if we're going to print *any* trace or gas report. + let identify_addresses = verbosity >= 3 || self.gas_report || self.debug.is_some(); - let mut total_passed = 0; - let mut total_failed = 0; - let mut total_skipped = 0; - let mut suite_results: Vec = Vec::new(); + let mut outcome = TestOutcome::empty(self.allow_failure); - 'outer: for (contract_name, suite_result) in rx { - results.insert(contract_name.clone(), suite_result.clone()); + let mut any_test_failed = false; + for (contract_name, suite_result) in rx { + let tests = &suite_result.test_results; - let mut tests = suite_result.test_results.clone(); + // Print suite header. println!(); for warning in suite_result.warnings.iter() { eprintln!("{} {warning}", Paint::yellow("Warning:").bold()); } if !tests.is_empty() { - let term = if tests.len() > 1 { "tests" } else { "test" }; - println!("Running {} {term} for {contract_name}", tests.len()); + let len = tests.len(); + let tests = if len > 1 { "tests" } else { "test" }; + println!("Ran {len} {tests} for {contract_name}"); } - for (name, result) in &mut tests { - short_test_result(name, result); + + // Process individual test results, printing logs and traces when necessary. + for (name, result) in tests { + shell::println(result.short_result(name))?; // We only display logs at level 2 and above if verbosity >= 2 { @@ -396,28 +367,27 @@ impl TestArgs { } } + // We shouldn't break out of the outer loop directly here so that we finish + // processing the remaining tests and print the suite summary. + any_test_failed |= result.status == TestStatus::Failure; + if result.traces.is_empty() { continue; } - // Identify addresses in each trace - let mut builder = CallTraceDecoderBuilder::new() - .with_labels(result.labeled_addresses.iter().map(|(a, s)| (*a, s.clone()))) - .with_local_identifier_abis(&local_identifier) - .with_verbosity(verbosity); - - // Signatures are of no value for gas reports - if !self.gas_report { - builder = builder.with_signature_identifier(sig_identifier.clone()); - } + // Clear the addresses and labels from previous runs. + decoder.clear_addresses(); + decoder + .labels + .extend(result.labeled_addresses.iter().map(|(k, v)| (*k, v.clone()))); - let mut decoder = builder.build(); - - // Decode the traces + // Identify addresses and decode traces. let mut decoded_traces = Vec::with_capacity(result.traces.len()); - for (kind, arena) in &mut result.traces { - decoder.identify(arena, &mut local_identifier); - decoder.identify(arena, &mut etherscan_identifier); + for (kind, arena) in &result.traces { + if identify_addresses { + decoder.identify(arena, &mut local_identifier); + decoder.identify(arena, &mut etherscan_identifier); + } // verbosity: // - 0..3: nothing @@ -441,52 +411,43 @@ impl TestArgs { if !decoded_traces.is_empty() { shell::println("Traces:")?; - decoded_traces.into_iter().try_for_each(shell::println)?; + for trace in &decoded_traces { + shell::println(trace)?; + } } - if self.gas_report { + if let Some(gas_report) = &mut gas_report { gas_report.analyze(&result.traces, &decoder).await; } - - // If the test failed, we want to stop processing the rest of the tests - if self.fail_fast && result.status == TestStatus::Failure { - break 'outer - } } - let block_outcome = TestOutcome::new( - [(contract_name.clone(), suite_result)].into(), - self.allow_failure, - ); - total_passed += block_outcome.successes().count(); - total_failed += block_outcome.failures().count(); - total_skipped += block_outcome.skips().count(); + // Print suite summary. + shell::println(suite_result.summary())?; - shell::println(block_outcome.summary())?; + // Add the suite result to the outcome. + outcome.results.insert(contract_name, suite_result); - if self.summary { - suite_results.push(block_outcome.clone()); + // Stop processing the remaining suites if any test failed and `fail_fast` is set. + if self.fail_fast && any_test_failed { + break; } } - if self.gas_report { + trace!(target: "forge::test", len=outcome.results.len(), %any_test_failed, "done with results"); + + outcome.decoder = Some(decoder); + + if let Some(gas_report) = gas_report { shell::println(gas_report.finalize())?; } - let num_test_suites = results.len(); - - if num_test_suites > 0 { - shell::println(format_aggregated_summary( - num_test_suites, - total_passed, - total_failed, - total_skipped, - ))?; + if !outcome.results.is_empty() { + shell::println(outcome.summary())?; if self.summary { let mut summary_table = TestSummaryReporter::new(self.detailed); shell::println("\n\nTest Summary:")?; - summary_table.print_summary(suite_results); + summary_table.print_summary(&outcome); } } @@ -498,9 +459,7 @@ impl TestArgs { } } - trace!(target: "forge::test", "received {} results", results.len()); - - Ok(TestOutcome::new(results, self.allow_failure)) + Ok(outcome) } /// Returns the flattened [`FilterArgs`] arguments merged with [`Config`]. @@ -550,157 +509,6 @@ impl Provider for TestArgs { } } -/// The result of a single test -#[derive(Clone, Debug)] -pub struct Test { - /// The identifier of the artifact/contract in the form of `:` - pub artifact_id: String, - /// The signature of the solidity test - pub signature: String, - /// Result of the executed solidity test - pub result: TestResult, -} - -impl Test { - pub fn gas_used(&self) -> u64 { - self.result.kind.report().gas() - } - - /// Returns the contract name of the artifact id - pub fn contract_name(&self) -> &str { - get_contract_name(&self.artifact_id) - } - - /// Returns the file name of the artifact id - pub fn file_name(&self) -> &str { - get_file_name(&self.artifact_id) - } -} - -/// Represents the bundled results of all tests -#[derive(Clone, Debug)] -pub struct TestOutcome { - /// Whether failures are allowed - pub allow_failure: bool, - /// Results for each suite of tests `contract -> SuiteResult` - pub results: BTreeMap, -} - -impl TestOutcome { - fn new(results: BTreeMap, allow_failure: bool) -> Self { - Self { results, allow_failure } - } - - /// Returns an iterator over all succeeding tests and their names. - pub fn successes(&self) -> impl Iterator { - self.tests().filter(|(_, t)| t.status == TestStatus::Success) - } - - /// Returns an iterator over all failing tests and their names. - pub fn failures(&self) -> impl Iterator { - self.tests().filter(|(_, t)| t.status == TestStatus::Failure) - } - - /// Returns an iterator over all skipped tests and their names. - pub fn skips(&self) -> impl Iterator { - self.tests().filter(|(_, t)| t.status == TestStatus::Skipped) - } - - /// Returns an iterator over all tests and their names. - pub fn tests(&self) -> impl Iterator { - self.results.values().flat_map(|suite| suite.tests()) - } - - /// Returns an iterator over all `Test`s. - pub fn into_tests(self) -> impl Iterator { - self.results - .into_iter() - .flat_map(|(file, SuiteResult { test_results, .. })| { - test_results.into_iter().map(move |t| (file.clone(), t)) - }) - .map(|(artifact_id, (signature, result))| Test { artifact_id, signature, result }) - } - - /// Checks if there are any failures and failures are disallowed - pub fn ensure_ok(&self) -> Result<()> { - let failures = self.failures().count(); - if self.allow_failure || failures == 0 { - return Ok(()); - } - - if !shell::verbosity().is_normal() { - // skip printing and exit early - std::process::exit(1); - } - - shell::println("")?; - shell::println("Failing tests:")?; - for (suite_name, suite) in self.results.iter() { - let failures = suite.failures().count(); - if failures == 0 { - continue; - } - - let term = if failures > 1 { "tests" } else { "test" }; - shell::println(format!("Encountered {failures} failing {term} in {suite_name}"))?; - for (name, result) in suite.failures() { - short_test_result(name, result); - } - shell::println("")?; - } - let successes = self.successes().count(); - shell::println(format!( - "Encountered a total of {} failing tests, {} tests succeeded", - Paint::red(failures.to_string()), - Paint::green(successes.to_string()) - ))?; - - std::process::exit(1); - } - - pub fn duration(&self) -> Duration { - self.results - .values() - .fold(Duration::ZERO, |acc, SuiteResult { duration, .. }| acc + *duration) - } - - pub fn summary(&self) -> String { - let failed = self.failures().count(); - let result = if failed == 0 { Paint::green("ok") } else { Paint::red("FAILED") }; - format!( - "Test result: {}. {} passed; {} failed; {} skipped; finished in {:.2?}", - result, - Paint::green(self.successes().count()), - Paint::red(failed), - Paint::yellow(self.skips().count()), - self.duration() - ) - } -} - -fn short_test_result(name: &str, result: &TestResult) { - shell::println(format!("{result} {name} {}", result.kind.report())).unwrap(); -} - -/// Formats the aggregated summary of all test suites into a string (for printing). -fn format_aggregated_summary( - num_test_suites: usize, - total_passed: usize, - total_failed: usize, - total_skipped: usize, -) -> String { - let total_tests = total_passed + total_failed + total_skipped; - format!( - " \nRan {} test suites: {} tests passed, {} failed, {} skipped ({} total tests)", - num_test_suites, - Paint::green(total_passed), - Paint::red(total_failed), - Paint::yellow(total_skipped), - total_tests - ) -} - /// Lists all matching tests fn list( runner: MultiContractRunner, diff --git a/crates/forge/bin/cmd/test/summary.rs b/crates/forge/bin/cmd/test/summary.rs index 5f8bd9650bd8..561ea6b6824c 100644 --- a/crates/forge/bin/cmd/test/summary.rs +++ b/crates/forge/bin/cmd/test/summary.rs @@ -48,64 +48,46 @@ impl TestSummaryReporter { Self { table, is_detailed } } - pub(crate) fn print_summary(&mut self, mut test_results: Vec) { - // Sort by suite name first - - // Using `sort_by_cached_key` so that the key extraction logic runs only once - test_results.sort_by_cached_key(|test_outcome| { - test_outcome - .results - .keys() - .next() - .and_then(|suite| suite.split(':').nth(1)) - .unwrap() - .to_string() - }); - + pub(crate) fn print_summary(&mut self, outcome: &TestOutcome) { // Traverse the test_results vector and build the table - for suite in &test_results { - for contract in suite.results.keys() { - let mut row = Row::new(); - let suite_name = contract.split(':').nth(1).unwrap(); - let suite_path = contract.split(':').nth(0).unwrap(); - - let passed = suite.successes().count(); - let mut passed_cell = Cell::new(passed).set_alignment(CellAlignment::Center); + for (contract, suite) in &outcome.results { + let mut row = Row::new(); + let (suite_path, suite_name) = contract.split_once(':').unwrap(); - let failed = suite.failures().count(); - let mut failed_cell = Cell::new(failed).set_alignment(CellAlignment::Center); + let passed = suite.successes().count(); + let mut passed_cell = Cell::new(passed).set_alignment(CellAlignment::Center); - let skipped = suite.skips().count(); - let mut skipped_cell = Cell::new(skipped).set_alignment(CellAlignment::Center); + let failed = suite.failures().count(); + let mut failed_cell = Cell::new(failed).set_alignment(CellAlignment::Center); - let duration = suite.duration(); + let skipped = suite.skips().count(); + let mut skipped_cell = Cell::new(skipped).set_alignment(CellAlignment::Center); - row.add_cell(Cell::new(suite_name)); + row.add_cell(Cell::new(suite_name)); - if passed > 0 { - passed_cell = passed_cell.fg(Color::Green); - } - row.add_cell(passed_cell); - - if failed > 0 { - failed_cell = failed_cell.fg(Color::Red); - } - row.add_cell(failed_cell); + if passed > 0 { + passed_cell = passed_cell.fg(Color::Green); + } + row.add_cell(passed_cell); - if skipped > 0 { - skipped_cell = skipped_cell.fg(Color::Yellow); - } - row.add_cell(skipped_cell); + if failed > 0 { + failed_cell = failed_cell.fg(Color::Red); + } + row.add_cell(failed_cell); - if self.is_detailed { - row.add_cell(Cell::new(suite_path)); - row.add_cell(Cell::new(format!("{:.2?}", duration).to_string())); - } + if skipped > 0 { + skipped_cell = skipped_cell.fg(Color::Yellow); + } + row.add_cell(skipped_cell); - self.table.add_row(row); + if self.is_detailed { + row.add_cell(Cell::new(suite_path)); + row.add_cell(Cell::new(format!("{:.2?}", suite.duration).to_string())); } + + self.table.add_row(row); } - // Print the summary table + println!("\n{}", self.table); } } diff --git a/crates/forge/src/gas_report.rs b/crates/forge/src/gas_report.rs index c537643ad618..9d416e36ff1c 100644 --- a/crates/forge/src/gas_report.rs +++ b/crates/forge/src/gas_report.rs @@ -5,7 +5,6 @@ use crate::{ hashbrown::HashSet, traces::{CallTraceArena, CallTraceDecoder, CallTraceNode, DecodedCallData, TraceKind}, }; -use alloy_primitives::U256; use comfy_table::{presets::ASCII_MARKDOWN, *}; use foundry_common::{calc, TestFunctionExt}; use serde::{Deserialize, Serialize}; @@ -37,10 +36,23 @@ impl GasReport { } /// Whether the given contract should be reported. + #[instrument(level = "trace", skip(self), ret)] fn should_report(&self, contract_name: &str) -> bool { if self.ignore.contains(contract_name) { - // could be specified in both ignore and report_for - return self.report_for.contains(contract_name) + let contains_anyway = self.report_for.contains(contract_name); + if contains_anyway { + // If the user listed the contract in 'gas_reports' (the foundry.toml field) a + // report for the contract is generated even if it's listed in the ignore + // list. This is addressed this way because getting a report you don't expect is + // preferable than not getting one you expect. A warning is printed to stderr + // indicating the "double listing". + eprintln!( + "{}: {} is listed in both 'gas_reports' and 'gas_reports_ignore'.", + yansi::Paint::yellow("warning").bold(), + contract_name + ); + } + return contains_anyway; } self.report_any || self.report_for.contains(contract_name) } @@ -58,48 +70,38 @@ impl GasReport { async fn analyze_node(&mut self, node: &CallTraceNode, decoder: &CallTraceDecoder) { let trace = &node.trace; - let decoded = decoder.decode_function(&node.trace).await; if trace.address == CHEATCODE_ADDRESS || trace.address == HARDHAT_CONSOLE_ADDRESS { - return + return; } - if let Some(name) = &decoded.contract { - let contract_name = name.rsplit(':').next().unwrap_or(name.as_str()); - // If the user listed the contract in 'gas_reports' (the foundry.toml field) a - // report for the contract is generated even if it's listed in the ignore - // list. This is addressed this way because getting a report you don't expect is - // preferable than not getting one you expect. A warning is printed to stderr - // indicating the "double listing". - if self.report_for.contains(contract_name) && self.ignore.contains(contract_name) { - eprintln!( - "{}: {} is listed in both 'gas_reports' and 'gas_reports_ignore'.", - yansi::Paint::yellow("warning").bold(), - contract_name - ); - } + let decoded = decoder.decode_function(&node.trace).await; + + let Some(name) = &decoded.contract else { return }; + let contract_name = name.rsplit(':').next().unwrap_or(name); + + if !self.should_report(contract_name) { + return; + } - if self.should_report(contract_name) { - let contract_info = self.contracts.entry(name.to_string()).or_default(); - - if trace.kind.is_any_create() { - contract_info.gas = U256::from(trace.gas_used); - contract_info.size = U256::from(trace.data.len()); - } else if let Some(DecodedCallData { signature, .. }) = decoded.func { - let name = signature.split('(').next().unwrap(); - // ignore any test/setup functions - let should_include = - !(name.is_test() || name.is_invariant_test() || name.is_setup()); - if should_include { - let gas_info = contract_info - .functions - .entry(name.to_string()) - .or_default() - .entry(signature.clone()) - .or_default(); - gas_info.calls.push(U256::from(trace.gas_used)); - } - } + let contract_info = self.contracts.entry(name.to_string()).or_default(); + if trace.kind.is_any_create() { + trace!(contract_name, "adding create gas info"); + contract_info.gas = trace.gas_used; + contract_info.size = trace.data.len(); + } else if let Some(DecodedCallData { signature, .. }) = decoded.func { + let name = signature.split('(').next().unwrap(); + // ignore any test/setup functions + let should_include = !(name.is_test() || name.is_invariant_test() || name.is_setup()); + if should_include { + trace!(contract_name, signature, "adding gas info"); + let gas_info = contract_info + .functions + .entry(name.to_string()) + .or_default() + .entry(signature.clone()) + .or_default(); + gas_info.calls.push(trace.gas_used); } } } @@ -114,7 +116,7 @@ impl GasReport { func.min = func.calls.first().copied().unwrap_or_default(); func.max = func.calls.last().copied().unwrap_or_default(); func.mean = calc::mean(&func.calls); - func.median = U256::from(calc::median_sorted(func.calls.as_slice())); + func.median = calc::median_sorted(func.calls.as_slice()); }); }); }); @@ -173,16 +175,17 @@ impl Display for GasReport { #[derive(Debug, Default, Serialize, Deserialize)] pub struct ContractInfo { - pub gas: U256, - pub size: U256, + pub gas: u64, + pub size: usize, + /// Function name -> Function signature -> GasInfo pub functions: BTreeMap>, } #[derive(Debug, Default, Serialize, Deserialize)] pub struct GasInfo { - pub calls: Vec, - pub min: U256, - pub mean: U256, - pub median: U256, - pub max: U256, + pub calls: Vec, + pub min: u64, + pub mean: u64, + pub median: u64, + pub max: u64, } diff --git a/crates/forge/src/multi_runner.rs b/crates/forge/src/multi_runner.rs index 0c876a640f29..a15934c7be3e 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")?; @@ -311,9 +349,9 @@ impl MultiContractRunnerBuilder { deployable_contracts.insert(id.clone(), (abi.clone(), bytecode, libs_to_deploy)); } - linked_contract.get_deployed_bytecode_bytes().map(|b| b.into_owned()).and_then( - |bytes| known_contracts.insert(id.clone(), (abi.clone(), bytes.to_vec())), - ); + if let Some(bytes) = linked_contract.get_deployed_bytecode_bytes() { + known_contracts.insert(id.clone(), (abi.clone(), bytes.to_vec())); + } } let errors = known_contracts.flatten_errors(); @@ -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 - } } diff --git a/crates/forge/src/result.rs b/crates/forge/src/result.rs index fe490dda45bd..2e22adc85b21 100644 --- a/crates/forge/src/result.rs +++ b/crates/forge/src/result.rs @@ -1,13 +1,13 @@ //! Test outcomes. use alloy_primitives::{Address, Log}; -use foundry_common::evm::Breakpoints; +use foundry_common::{evm::Breakpoints, get_contract_name, get_file_name, shell}; use foundry_evm::{ coverage::HitMaps, debug::DebugArena, executors::EvmError, fuzz::{CounterExample, FuzzCase}, - traces::{TraceKind, Traces}, + traces::{CallTraceDecoder, TraceKind, Traces}, }; use serde::{Deserialize, Serialize}; use std::{ @@ -17,14 +17,173 @@ use std::{ }; use yansi::Paint; -/// Results and duration for a set of tests included in the same test contract +/// The aggregated result of a test run. +#[derive(Clone, Debug)] +pub struct TestOutcome { + /// The results of all test suites by their identifier (`path:contract_name`). + /// + /// Essentially `identifier => signature => result`. + pub results: BTreeMap, + /// Whether to allow test failures without failing the entire test run. + pub allow_failure: bool, + /// The decoder used to decode traces and logs. + /// + /// This is `None` if traces and logs were not decoded. + /// + /// Note that `Address` fields only contain the last executed test case's data. + pub decoder: Option, +} + +impl TestOutcome { + /// Creates a new test outcome with the given results. + pub fn new(results: BTreeMap, allow_failure: bool) -> Self { + Self { results, allow_failure, decoder: None } + } + + /// Creates a new empty test outcome. + pub fn empty(allow_failure: bool) -> Self { + Self { results: BTreeMap::new(), allow_failure, decoder: None } + } + + /// Returns an iterator over all individual succeeding tests and their names. + pub fn successes(&self) -> impl Iterator { + self.tests().filter(|(_, t)| t.status == TestStatus::Success) + } + + /// Returns an iterator over all individual skipped tests and their names. + pub fn skips(&self) -> impl Iterator { + self.tests().filter(|(_, t)| t.status == TestStatus::Skipped) + } + + /// Returns an iterator over all individual failing tests and their names. + pub fn failures(&self) -> impl Iterator { + self.tests().filter(|(_, t)| t.status == TestStatus::Failure) + } + + /// Returns an iterator over all individual tests and their names. + pub fn tests(&self) -> impl Iterator { + self.results.values().flat_map(|suite| suite.tests()) + } + + /// Flattens the test outcome into a list of individual tests. + // TODO: Replace this with `tests` and make it return `TestRef<'_>` + pub fn into_tests_cloned(&self) -> impl Iterator + '_ { + self.results + .iter() + .flat_map(|(file, suite)| { + suite + .test_results + .iter() + .map(move |(sig, result)| (file.clone(), sig.clone(), result.clone())) + }) + .map(|(artifact_id, signature, result)| SuiteTestResult { + artifact_id, + signature, + result, + }) + } + + /// Flattens the test outcome into a list of individual tests. + pub fn into_tests(self) -> impl Iterator { + self.results + .into_iter() + .flat_map(|(file, suite)| { + suite.test_results.into_iter().map(move |t| (file.clone(), t)) + }) + .map(|(artifact_id, (signature, result))| SuiteTestResult { + artifact_id, + signature, + result, + }) + } + + /// Returns the number of tests that passed. + pub fn passed(&self) -> usize { + self.successes().count() + } + + /// Returns the number of tests that were skipped. + pub fn skipped(&self) -> usize { + self.skips().count() + } + + /// Returns the number of tests that failed. + pub fn failed(&self) -> usize { + self.failures().count() + } + + /// Calculates the total duration of all test suites. + pub fn duration(&self) -> Duration { + self.results.values().map(|suite| suite.duration).sum() + } + + /// Formats the aggregated summary of all test suites into a string (for printing). + pub fn summary(&self) -> String { + let num_test_suites = self.results.len(); + let suites = if num_test_suites == 1 { "suite" } else { "suites" }; + let total_passed = self.passed(); + let total_failed = self.failed(); + let total_skipped = self.skipped(); + let total_tests = total_passed + total_failed + total_skipped; + format!( + "\nRan {} test {}: {} tests passed, {} failed, {} skipped ({} total tests)", + num_test_suites, + suites, + Paint::green(total_passed), + Paint::red(total_failed), + Paint::yellow(total_skipped), + total_tests + ) + } + + /// Checks if there are any failures and failures are disallowed. + pub fn ensure_ok(&self) -> eyre::Result<()> { + let outcome = self; + let failures = outcome.failures().count(); + if outcome.allow_failure || failures == 0 { + return Ok(()); + } + + if !shell::verbosity().is_normal() { + // TODO: Avoid process::exit + std::process::exit(1); + } + + shell::println("")?; + shell::println("Failing tests:")?; + for (suite_name, suite) in outcome.results.iter() { + let failed = suite.failed(); + if failed == 0 { + continue; + } + + let term = if failed > 1 { "tests" } else { "test" }; + shell::println(format!("Encountered {failed} failing {term} in {suite_name}"))?; + for (name, result) in suite.failures() { + shell::println(result.short_result(name))?; + } + shell::println("")?; + } + let successes = outcome.passed(); + shell::println(format!( + "Encountered a total of {} failing tests, {} tests succeeded", + Paint::red(failures.to_string()), + Paint::green(successes.to_string()) + ))?; + + // TODO: Avoid process::exit + std::process::exit(1); + } +} + +/// A set of test results for a single test suite, which is all the tests in a single contract. #[derive(Clone, Debug, Serialize)] pub struct SuiteResult { - /// Total duration of the test run for this block of tests + /// Total duration of the test run for this block of tests. pub duration: Duration, - /// Individual test results. `test fn signature -> TestResult` + /// Individual test results: `test fn signature -> TestResult`. pub test_results: BTreeMap, - /// Warnings + /// Generated warnings. pub warnings: Vec, } @@ -37,16 +196,36 @@ impl SuiteResult { Self { duration, test_results, warnings } } - /// Iterator over all succeeding tests and their names + /// Returns an iterator over all individual succeeding tests and their names. pub fn successes(&self) -> impl Iterator { self.tests().filter(|(_, t)| t.status == TestStatus::Success) } - /// Iterator over all failing tests and their names + /// Returns an iterator over all individual skipped tests and their names. + pub fn skips(&self) -> impl Iterator { + self.tests().filter(|(_, t)| t.status == TestStatus::Skipped) + } + + /// Returns an iterator over all individual failing tests and their names. pub fn failures(&self) -> impl Iterator { self.tests().filter(|(_, t)| t.status == TestStatus::Failure) } + /// Returns the number of tests that passed. + pub fn passed(&self) -> usize { + self.successes().count() + } + + /// Returns the number of tests that were skipped. + pub fn skipped(&self) -> usize { + self.skips().count() + } + + /// Returns the number of tests that failed. + pub fn failed(&self) -> usize { + self.failures().count() + } + /// Iterator over all tests and their names pub fn tests(&self) -> impl Iterator { self.test_results.iter() @@ -61,8 +240,54 @@ impl SuiteResult { pub fn len(&self) -> usize { self.test_results.len() } + + /// Returns the summary of a single test suite. + pub fn summary(&self) -> String { + let failed = self.failed(); + let result = if failed == 0 { Paint::green("ok") } else { Paint::red("FAILED") }; + format!( + "Test result: {}. {} passed; {} failed; {} skipped; finished in {:.2?}", + result, + Paint::green(self.passed()), + Paint::red(failed), + Paint::yellow(self.skipped()), + self.duration, + ) + } +} + +/// The result of a single test in a test suite. +/// +/// This is flattened from a [`TestOutcome`]. +#[derive(Clone, Debug)] +pub struct SuiteTestResult { + /// The identifier of the artifact/contract in the form: + /// `:`. + pub artifact_id: String, + /// The function signature of the Solidity test. + pub signature: String, + /// The result of the executed test. + pub result: TestResult, } +impl SuiteTestResult { + /// Returns the gas used by the test. + pub fn gas_used(&self) -> u64 { + self.result.kind.report().gas() + } + + /// Returns the contract name of the artifact ID. + pub fn contract_name(&self) -> &str { + get_contract_name(&self.artifact_id) + } + + /// Returns the file name of the artifact ID. + pub fn file_name(&self) -> &str { + get_file_name(&self.artifact_id) + } +} + +/// The status of a test. #[derive(Clone, Copy, Debug, Default, PartialEq, Eq, Serialize, Deserialize)] pub enum TestStatus { Success, @@ -91,7 +316,7 @@ impl TestStatus { } } -/// The result of an executed solidity test +/// The result of an executed test. #[derive(Clone, Debug, Default, Serialize, Deserialize)] pub struct TestResult { /// The test status, indicating whether the test case succeeded, failed, or was marked as @@ -177,6 +402,11 @@ impl TestResult { pub fn is_fuzz(&self) -> bool { matches!(self.kind, TestKind::Fuzz { .. }) } + + /// Formats the test result into a string (for printing). + pub fn short_result(&self, name: &str) -> String { + format!("{self} {name} {}", self.kind.report()) + } } /// Data report by a test. diff --git a/crates/forge/tests/cli/cmd.rs b/crates/forge/tests/cli/cmd.rs index 222248f84e6c..a187169fae98 100644 --- a/crates/forge/tests/cli/cmd.rs +++ b/crates/forge/tests/cli/cmd.rs @@ -1291,36 +1291,34 @@ contract ContractThreeTest is DSTest { .unwrap(); // report for One - prj.write_config(Config { - gas_reports: (vec!["ContractOne".to_string()]), - gas_reports_ignore: (vec![]), - ..Default::default() - }); + prj.write_config(Config { gas_reports: vec!["ContractOne".to_string()], ..Default::default() }); cmd.forge_fuse(); let first_out = cmd.arg("test").arg("--gas-report").stdout_lossy(); - assert!(first_out.contains("foo") && !first_out.contains("bar") && !first_out.contains("baz")); + assert!( + first_out.contains("foo") && !first_out.contains("bar") && !first_out.contains("baz"), + "foo:\n{first_out}" + ); // report for Two - cmd.forge_fuse(); - prj.write_config(Config { - gas_reports: (vec!["ContractTwo".to_string()]), - ..Default::default() - }); + prj.write_config(Config { gas_reports: vec!["ContractTwo".to_string()], ..Default::default() }); cmd.forge_fuse(); let second_out = cmd.arg("test").arg("--gas-report").stdout_lossy(); assert!( - !second_out.contains("foo") && second_out.contains("bar") && !second_out.contains("baz") + !second_out.contains("foo") && second_out.contains("bar") && !second_out.contains("baz"), + "bar:\n{second_out}" ); // report for Three - cmd.forge_fuse(); prj.write_config(Config { - gas_reports: (vec!["ContractThree".to_string()]), + gas_reports: vec!["ContractThree".to_string()], ..Default::default() }); cmd.forge_fuse(); let third_out = cmd.arg("test").arg("--gas-report").stdout_lossy(); - assert!(!third_out.contains("foo") && !third_out.contains("bar") && third_out.contains("baz")); + assert!( + !third_out.contains("foo") && !third_out.contains("bar") && third_out.contains("baz"), + "baz:\n{third_out}" + ); }); forgetest!(gas_ignore_some_contracts, |prj, cmd| { diff --git a/crates/forge/tests/fixtures/can_check_snapshot.stdout b/crates/forge/tests/fixtures/can_check_snapshot.stdout index dffc5df49634..1846bd5778b6 100644 --- a/crates/forge/tests/fixtures/can_check_snapshot.stdout +++ b/crates/forge/tests/fixtures/can_check_snapshot.stdout @@ -2,8 +2,8 @@ Compiling 2 files with 0.8.23 Solc 0.8.23 finished in 424.55ms Compiler run successful! -Running 1 test for src/ATest.t.sol:ATest +Ran 1 test for src/ATest.t.sol:ATest [PASS] testExample() (gas: 168) Test result: ok. 1 passed; 0 failed; 0 skipped; finished in 4.42ms - -Ran 1 test suites: 1 tests passed, 0 failed, 0 skipped (1 total tests) + +Ran 1 test suite: 1 tests passed, 0 failed, 0 skipped (1 total tests) diff --git a/crates/forge/tests/fixtures/can_run_test_in_custom_test_folder.stdout b/crates/forge/tests/fixtures/can_run_test_in_custom_test_folder.stdout index 5cf274ebc7de..1701d90029b9 100644 --- a/crates/forge/tests/fixtures/can_run_test_in_custom_test_folder.stdout +++ b/crates/forge/tests/fixtures/can_run_test_in_custom_test_folder.stdout @@ -2,8 +2,8 @@ Compiling 2 files with 0.8.23 Solc 0.8.23 finished in 185.25ms Compiler run successful! -Running 1 test for src/nested/forge-tests/MyTest.t.sol:MyTest +Ran 1 test for src/nested/forge-tests/MyTest.t.sol:MyTest [PASS] testTrue() (gas: 168) Test result: ok. 1 passed; 0 failed; 0 skipped; finished in 2.93ms - -Ran 1 test suites: 1 tests passed, 0 failed, 0 skipped (1 total tests) + +Ran 1 test suite: 1 tests passed, 0 failed, 0 skipped (1 total tests) diff --git a/crates/forge/tests/fixtures/can_test_repeatedly.stdout b/crates/forge/tests/fixtures/can_test_repeatedly.stdout index 6d645a5606b5..3d782aa35872 100644 --- a/crates/forge/tests/fixtures/can_test_repeatedly.stdout +++ b/crates/forge/tests/fixtures/can_test_repeatedly.stdout @@ -1,8 +1,8 @@ No files changed, compilation skipped -Running 2 tests for test/Counter.t.sol:CounterTest +Ran 2 tests for test/Counter.t.sol:CounterTest [PASS] testFuzz_SetNumber(uint256) (runs: 256, μ: 26521, ~: 28387) [PASS] test_Increment() (gas: 28379) Test result: ok. 2 passed; 0 failed; 0 skipped; finished in 9.42ms - -Ran 1 test suites: 2 tests passed, 0 failed, 0 skipped (2 total tests) + +Ran 1 test suite: 2 tests passed, 0 failed, 0 skipped (2 total tests) diff --git a/crates/forge/tests/fixtures/can_use_libs_in_multi_fork.stdout b/crates/forge/tests/fixtures/can_use_libs_in_multi_fork.stdout index 4c954e6c37c9..5a5b2f621811 100644 --- a/crates/forge/tests/fixtures/can_use_libs_in_multi_fork.stdout +++ b/crates/forge/tests/fixtures/can_use_libs_in_multi_fork.stdout @@ -2,8 +2,8 @@ Compiling 2 files with 0.8.23 Solc 0.8.23 finished in 1.95s Compiler run successful! -Running 1 test for test/Contract.t.sol:ContractTest +Ran 1 test for test/Contract.t.sol:ContractTest [PASS] test() (gas: 70360) Test result: ok. 1 passed; 0 failed; 0 skipped; finished in 3.21s - -Ran 1 test suites: 1 tests passed, 0 failed, 0 skipped (1 total tests) + +Ran 1 test suite: 1 tests passed, 0 failed, 0 skipped (1 total tests) diff --git a/crates/forge/tests/fixtures/include_custom_types_in_traces.stdout b/crates/forge/tests/fixtures/include_custom_types_in_traces.stdout index be5d7706debb..19771e0920f5 100644 --- a/crates/forge/tests/fixtures/include_custom_types_in_traces.stdout +++ b/crates/forge/tests/fixtures/include_custom_types_in_traces.stdout @@ -2,7 +2,7 @@ Compiling 1 files with 0.8.23 Solc 0.8.23 finished in 798.51ms Compiler run successful! -Running 2 tests for test/Contract.t.sol:CustomTypesTest +Ran 2 tests for test/Contract.t.sol:CustomTypesTest [FAIL. Reason: PoolNotInitialized()] testErr() (gas: 231) Traces: [231] CustomTypesTest::testErr() @@ -15,8 +15,8 @@ Traces: └─ ← () Test result: FAILED. 1 passed; 1 failed; 0 skipped; finished in 3.88ms - -Ran 1 test suites: 1 tests passed, 1 failed, 0 skipped (2 total tests) + +Ran 1 test suite: 1 tests passed, 1 failed, 0 skipped (2 total tests) Failing tests: Encountered 1 failing test in test/Contract.t.sol:CustomTypesTest diff --git a/crates/forge/tests/fixtures/repro_6531.stdout b/crates/forge/tests/fixtures/repro_6531.stdout index d2c2596cb169..1dfca09d72ff 100644 --- a/crates/forge/tests/fixtures/repro_6531.stdout +++ b/crates/forge/tests/fixtures/repro_6531.stdout @@ -2,7 +2,7 @@ Compiling 1 files with 0.8.23 Compiler run successful! -Running 1 test for test/Contract.t.sol:USDCCallingTest +Ran 1 test for test/Contract.t.sol:USDCCallingTest [PASS] test() (gas: 16799) Traces: [16799] USDCCallingTest::test() @@ -15,5 +15,5 @@ Traces: └─ ← () Test result: ok. 1 passed; 0 failed; 0 skipped; finished in 3.43s - -Ran 1 test suites: 1 tests passed, 0 failed, 0 skipped (1 total tests) + +Ran 1 test suite: 1 tests passed, 0 failed, 0 skipped (1 total tests) diff --git a/crates/forge/tests/fixtures/runs_tests_exactly_once_with_changed_versions.1.stdout b/crates/forge/tests/fixtures/runs_tests_exactly_once_with_changed_versions.1.stdout index aee6fb691ce3..c46d083296fa 100644 --- a/crates/forge/tests/fixtures/runs_tests_exactly_once_with_changed_versions.1.stdout +++ b/crates/forge/tests/fixtures/runs_tests_exactly_once_with_changed_versions.1.stdout @@ -2,8 +2,8 @@ Compiling 2 files with 0.8.23 Solc 0.8.23 finished in 185.25ms Compiler run successful! -Running 1 test for src/Contract.t.sol:ContractTest +Ran 1 test for src/Contract.t.sol:ContractTest [PASS] testExample() (gas: 190) Test result: ok. 1 passed; 0 failed; 0 skipped; finished in 1.89ms - -Ran 1 test suites: 1 tests passed, 0 failed, 0 skipped (1 total tests) + +Ran 1 test suite: 1 tests passed, 0 failed, 0 skipped (1 total tests) diff --git a/crates/forge/tests/fixtures/runs_tests_exactly_once_with_changed_versions.2.stdout b/crates/forge/tests/fixtures/runs_tests_exactly_once_with_changed_versions.2.stdout index 691af81679df..28dbffcc86c3 100644 --- a/crates/forge/tests/fixtures/runs_tests_exactly_once_with_changed_versions.2.stdout +++ b/crates/forge/tests/fixtures/runs_tests_exactly_once_with_changed_versions.2.stdout @@ -2,8 +2,8 @@ Compiling 2 files with 0.8.22 Solc 0.8.22 finished in 185.25ms Compiler run successful! -Running 1 test for src/Contract.t.sol:ContractTest +Ran 1 test for src/Contract.t.sol:ContractTest [PASS] testExample() (gas: 190) Test result: ok. 1 passed; 0 failed; 0 skipped; finished in 1.89ms - -Ran 1 test suites: 1 tests passed, 0 failed, 0 skipped (1 total tests) + +Ran 1 test suite: 1 tests passed, 0 failed, 0 skipped (1 total tests)