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

feat(fuzz) - add test progress #7914

Merged
merged 14 commits into from
Jun 7, 2024
Merged
Show file tree
Hide file tree
Changes from 7 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
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

55 changes: 55 additions & 0 deletions crates/cli/src/utils/cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,61 @@ macro_rules! update_progress {
};
}

/// Creates progress object and progress bar.
#[macro_export]
macro_rules! init_tests_progress {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know this is how it exists already, but I'd like these macros to be helper functions and/or structs instead, since that's all they are

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I removed those macros and moved logic in structs within newly progress mod, pls check 7aaafc8

($tests_len:expr,$threads_no:expr) => {{
let progress = MultiProgress::new();
let pb = progress.add(indicatif::ProgressBar::new($tests_len as u64));
pb.set_style(
indicatif::ProgressStyle::with_template("{bar:40.cyan/blue} {pos:>7}/{len:7} {msg}")
.unwrap()
.progress_chars("##-"),
);
pb.set_message(format!("completed (with {} threads)", $threads_no as u64));
(progress, pb)
}};
}

/// Creates progress entry for test suite, having `[spinner] Test suite name` format.
#[macro_export]
macro_rules! init_test_suite_progress {
($overall_progress:expr, $suite_name:expr) => {{
let pb = $overall_progress.add(indicatif::ProgressBar::new_spinner());
pb.set_style(
indicatif::ProgressStyle::with_template("{spinner} {wide_msg:.bold.dim}")
.unwrap()
.tick_chars("⠁⠂⠄⡀⢀⠠⠐⠈ "),
);
pb.set_message(format!("{} ", $suite_name));
pb.enable_steady_tick(Duration::from_millis(100));
pb
}};
}

/// Creates progress entry for fuzz tests.
/// Set the prefix and total number of runs. Message is updated during execution with current phase.
/// Test progress is placed under test suite progress entry so all tests within suite are grouped.
#[macro_export]
macro_rules! init_fuzz_test_progress {
($progress:expr, $test_name:expr, $runs:expr) => {{
let test_progress = $progress.map(|(overall_progress, suite_progress)| {
let pb = overall_progress
.insert_after(suite_progress, indicatif::ProgressBar::new($runs as u64));
pb.set_style(
indicatif::ProgressStyle::with_template(
" ↪ {prefix:.bold.dim}: [{pos}/{len}]{msg} Runs",
)
.unwrap()
.tick_chars("⠁⠂⠄⡀⢀⠠⠐⠈ "),
);
pb.set_prefix(format!("{}", $test_name));
pb
});
test_progress
}};
}

/// True if the network calculates gas costs differently.
pub fn has_different_gas_calc(chain_id: u64) -> bool {
if let Some(chain) = Chain::from(chain_id).named() {
Expand Down
1 change: 1 addition & 0 deletions crates/evm/evm/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -43,3 +43,4 @@ parking_lot = "0.12"
proptest = "1"
thiserror = "1"
tracing = "0.1"
indicatif = "0.17"
7 changes: 7 additions & 0 deletions crates/evm/evm/src/executors/fuzz/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ use foundry_evm_fuzz::{
BaseCounterExample, CounterExample, FuzzCase, FuzzError, FuzzFixtures, FuzzTestResult,
};
use foundry_evm_traces::CallTraceArena;
use indicatif::ProgressBar;
use proptest::test_runner::{TestCaseError, TestError, TestRunner};
use std::{borrow::Cow, cell::RefCell};

Expand Down Expand Up @@ -59,6 +60,7 @@ impl FuzzedExecutor {
address: Address,
should_fail: bool,
rd: &RevertDecoder,
progress: Option<&ProgressBar>,
) -> FuzzTestResult {
// Stores the first Fuzzcase
let first_case: RefCell<Option<FuzzCase>> = RefCell::default();
Expand Down Expand Up @@ -91,6 +93,11 @@ impl FuzzedExecutor {
let run_result = self.runner.clone().run(&strat, |calldata| {
let fuzz_res = self.single_fuzz(address, should_fail, calldata)?;

// If running with progress then increment current run.
if let Some(progress) = progress {
progress.inc(1);
};

match fuzz_res {
FuzzOutcome::Case(case) => {
let mut first_case = first_case.borrow_mut();
Expand Down
7 changes: 7 additions & 0 deletions crates/evm/evm/src/executors/invariant/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ use foundry_evm_fuzz::{
FuzzCase, FuzzFixtures, FuzzedCases,
};
use foundry_evm_traces::CallTraceArena;
use indicatif::ProgressBar;
use parking_lot::RwLock;
use proptest::{
strategy::{BoxedStrategy, Strategy},
Expand Down Expand Up @@ -139,6 +140,7 @@ impl<'a> InvariantExecutor<'a> {
&mut self,
invariant_contract: InvariantContract<'_>,
fuzz_fixtures: &FuzzFixtures,
progress: Option<&ProgressBar>,
) -> Result<InvariantFuzzTestResult> {
// Throw an error to abort test run if the invariant function accepts input params
if !invariant_contract.invariant_function.inputs.is_empty() {
Expand Down Expand Up @@ -315,6 +317,11 @@ impl<'a> InvariantExecutor<'a> {
// Revert state to not persist values between runs.
fuzz_state.revert();

// If running with progress then increment completed runs.
if let Some(progress) = progress {
progress.inc(1);
}

Ok(())
});

Expand Down
4 changes: 3 additions & 1 deletion crates/evm/evm/src/executors/invariant/replay.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ use foundry_evm_fuzz::{
BaseCounterExample,
};
use foundry_evm_traces::{load_contracts, TraceKind, Traces};
use indicatif::ProgressBar;
use parking_lot::RwLock;
use proptest::test_runner::TestError;
use revm::primitives::U256;
Expand Down Expand Up @@ -101,13 +102,14 @@ pub fn replay_error(
logs: &mut Vec<Log>,
traces: &mut Traces,
coverage: &mut Option<HitMaps>,
progress: Option<&ProgressBar>,
) -> Result<Vec<BaseCounterExample>> {
match failed_case.test_error {
// Don't use at the moment.
TestError::Abort(_) => Ok(vec![]),
TestError::Fail(_, ref calls) => {
// Shrink sequence of failed calls.
let calls = shrink_sequence(failed_case, calls, &executor)?;
let calls = shrink_sequence(failed_case, calls, &executor, progress)?;

set_up_inner_replay(&mut executor, &failed_case.inner_sequence);

Expand Down
15 changes: 14 additions & 1 deletion crates/evm/evm/src/executors/invariant/shrink.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,9 @@ use crate::executors::{invariant::error::FailedInvariantCaseData, Executor};
use alloy_primitives::{Address, Bytes, U256};
use foundry_evm_core::constants::CALLER;
use foundry_evm_fuzz::invariant::BasicTxDetails;
use indicatif::ProgressBar;
use proptest::bits::{BitSetLike, VarBitSet};
use std::borrow::Cow;
use std::{borrow::Cow, cmp::min};

#[derive(Clone, Copy, Debug)]
struct Shrink {
Expand Down Expand Up @@ -83,9 +84,17 @@ pub(crate) fn shrink_sequence(
failed_case: &FailedInvariantCaseData,
calls: &[BasicTxDetails],
executor: &Executor,
progress: Option<&ProgressBar>,
) -> eyre::Result<Vec<BasicTxDetails>> {
trace!(target: "forge::test", "Shrinking sequence of {} calls.", calls.len());

// Reset run count and display shrinking message.
if let Some(progress) = progress {
progress.set_length(min(calls.len(), failed_case.shrink_run_limit) as u64);
progress.reset();
progress.set_message(" Shrink");
}

// Special case test: the invariant is *unsatisfiable* - it took 0 calls to
// break the invariant -- consider emitting a warning.
let error_call_result =
Expand All @@ -112,6 +121,10 @@ pub(crate) fn shrink_sequence(
Ok((true, _)) if !shrinker.complicate() => break,
_ => {}
}

if let Some(progress) = progress {
progress.inc(1);
}
}

Ok(shrinker.current().map(|idx| &calls[idx]).cloned().collect())
Expand Down
7 changes: 6 additions & 1 deletion crates/forge/bin/cmd/test/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,10 @@ pub struct TestArgs {
/// Print detailed test summary table.
#[arg(long, help_heading = "Display options", requires = "summary")]
pub detailed: bool,

/// Show test execution progress.
#[arg(long)]
pub show_progress: bool,
}

impl TestArgs {
Expand Down Expand Up @@ -366,9 +370,10 @@ impl TestArgs {
// Run tests.
let (tx, rx) = channel::<(String, SuiteResult)>();
let timer = Instant::now();
let show_progress = self.show_progress;
let handle = tokio::task::spawn_blocking({
let filter = filter.clone();
move || runner.test(&filter, tx)
move || runner.test(&filter, tx, show_progress)
});

let mut gas_report = self
Expand Down
69 changes: 60 additions & 9 deletions crates/forge/src/multi_runner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ use crate::{result::SuiteResult, ContractRunner, TestFilter, TestOptions};
use alloy_json_abi::{Function, JsonAbi};
use alloy_primitives::{Address, Bytes, U256};
use eyre::Result;
use foundry_cli::{init_test_suite_progress, init_tests_progress};
use foundry_common::{get_contract_name, ContractsByArtifact, TestFunctionExt};
use foundry_compilers::{artifacts::Libraries, Artifact, ArtifactId, ProjectCompileOutput};
use foundry_config::Config;
Expand All @@ -12,6 +13,7 @@ use foundry_evm::{
inspectors::CheatsConfig, opts::EvmOpts, revm,
};
use foundry_linking::{LinkOutput, Linker};
use indicatif::{MultiProgress, ProgressBar};
use rayon::prelude::*;
use revm::primitives::SpecId;
use std::{
Expand All @@ -20,7 +22,7 @@ use std::{
fmt::Debug,
path::Path,
sync::{mpsc, Arc},
time::Instant,
time::{Duration, Instant},
};

#[derive(Debug, Clone)]
Expand Down Expand Up @@ -136,7 +138,7 @@ impl MultiContractRunner {
filter: &dyn TestFilter,
) -> impl Iterator<Item = (String, SuiteResult)> {
let (tx, rx) = mpsc::channel();
self.test(filter, tx);
self.test(filter, tx, false);
rx.into_iter()
}

Expand All @@ -146,7 +148,12 @@ impl MultiContractRunner {
/// before executing all contracts and their tests in _parallel_.
///
/// Each Executor gets its own instance of the `Backend`.
pub fn test(&mut self, filter: &dyn TestFilter, tx: mpsc::Sender<(String, SuiteResult)>) {
pub fn test(
&mut self,
filter: &dyn TestFilter,
tx: mpsc::Sender<(String, SuiteResult)>,
show_progress: bool,
) {
let handle = tokio::runtime::Handle::current();
trace!("running all tests");

Expand All @@ -163,11 +170,54 @@ impl MultiContractRunner {
find_time,
);

contracts.par_iter().for_each_with(tx, |tx, &(id, contract)| {
let _guard = handle.enter();
let result = self.run_tests(id, contract, db.clone(), filter, &handle);
let _ = tx.send((id.identifier(), result));
})
if show_progress {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok with this for now, but I think we want to have one single par_iter here and also handle all output in between progress bars

// Create overall tests progress and progress bar.
// Tests progress is passed to test runners for adding individual test suite progress.
// Progress bar is updated each time a test suite ends.
let (tests_progress, progress_bar) =
init_tests_progress!(contracts.len(), rayon::current_num_threads());
// Collect test suite results to stream at the end of test run.
let results: Vec<(String, SuiteResult)> = contracts
.par_iter()
.map(|&(id, contract)| {
let _guard = handle.enter();
// Add test suite progress to tests result.
let suite_progress = init_test_suite_progress!(tests_progress, id.name);

// Run tests, pass tests progress and current test suite progress in order
// to add specific run details.
let result = self.run_tests(
id,
contract,
db.clone(),
filter,
&handle,
Some((&tests_progress, &suite_progress)),
);

// Print result summary and remove test progress from overall progress.
tests_progress.suspend(|| {
println!("{}\n ↪ {}", id.name.clone(), result.summary());
});
suite_progress.finish_and_clear();
// Increment test progress bar to reflect completed test suite.
progress_bar.inc(1);

(id.identifier(), result)
})
.collect();
// Tests completed, remove progress and stream results.
tests_progress.clear().unwrap();
results.iter().for_each(|result| {
let _ = tx.send(result.to_owned());
});
} else {
contracts.par_iter().for_each_with(tx, |tx, &(id, contract)| {
let _guard = handle.enter();
let result = self.run_tests(id, contract, db.clone(), filter, &handle, None);
let _ = tx.send((id.identifier(), result));
})
}
}

fn run_tests(
Expand All @@ -177,6 +227,7 @@ impl MultiContractRunner {
db: Backend,
filter: &dyn TestFilter,
handle: &tokio::runtime::Handle,
progress: Option<(&MultiProgress, &ProgressBar)>,
) -> SuiteResult {
let identifier = artifact_id.identifier();
let mut span_name = identifier.as_str();
Expand Down Expand Up @@ -223,7 +274,7 @@ impl MultiContractRunner {
&self.revert_decoder,
self.debug,
);
let r = runner.run_tests(filter, &self.test_options, known_contracts, handle);
let r = runner.run_tests(filter, &self.test_options, known_contracts, handle, progress);

debug!(duration=?r.duration, "executed all tests in contract");

Expand Down
Loading
Loading