diff --git a/crates/forge/src/cmd/test/mod.rs b/crates/forge/src/cmd/test/mod.rs index 14ebb613e270a..2c4ef0cfe50c1 100644 --- a/crates/forge/src/cmd/test/mod.rs +++ b/crates/forge/src/cmd/test/mod.rs @@ -1,4 +1,4 @@ -use super::{install, test::filter::ProjectPathsAwareFilter, watch::WatchArgs}; +use super::{install, watch::WatchArgs}; use crate::{ MultiContractRunner, MultiContractRunnerBuilder, decode::decode_console_logs, @@ -66,7 +66,7 @@ use yansi::Paint; mod filter; mod summary; use crate::{result::TestKind, traces::render_trace_arena_inner}; -pub use filter::FilterArgs; +pub use filter::{FilterArgs, ProjectPathsAwareFilter}; use quick_junit::{NonSuccessKind, Report, TestCase, TestCaseStatus, TestSuite}; use summary::{TestSummaryReport, format_invariant_metrics_table}; @@ -447,6 +447,39 @@ impl TestArgs { bail!("`fuzz.run` must be greater than 0"); } + // Mutation testing has bespoke orchestration (per-mutant temp + // workspaces, baseline + N mutants, aggregated mutation report). It is + // not compatible with the single-run debug / flame / list / junit + // modes — running them together would either mix incompatible output + // formats, or run the secondary mode against the baseline tests and + // then silently continue into mutation testing. Reject up front with a + // clear error rather than do the wrong thing. + if self.mutate.is_some() { + let mut conflicts = Vec::new(); + if self.list { + conflicts.push("--list"); + } + if self.debug { + conflicts.push("--debug"); + } + if self.flamegraph { + conflicts.push("--flamegraph"); + } + if self.flamechart { + conflicts.push("--flamechart"); + } + if self.junit { + conflicts.push("--junit"); + } + if !conflicts.is_empty() { + bail!( + "`--mutate` cannot be combined with: {}. Re-run without those flags to use \ + mutation testing.", + conflicts.join(", ") + ); + } + } + // Explicitly enable isolation for gas reports for more correct gas accounting. if self.gas_report { evm_opts.isolate = true; @@ -644,6 +677,19 @@ impl TestArgs { eyre::bail!("Cannot run mutation testing with failed tests"); } + // A green baseline that ran *zero* tests is not a useful baseline: + // every compileable mutant would be reported as `Alive` (no test + // failed, so nothing killed it), which produces a wildly + // misleading mutation report. Hard-error so users get an actual + // signal that their filter / path / setup matched nothing. + if outcome.tests().next().is_none() { + eyre::bail!( + "Mutation testing requires at least one matching baseline test; the current \ + filter/path selection matched zero tests. Loosen `--match-test` / \ + `--match-contract` / `--match-path` or check the project layout." + ); + } + // Explicit paths on --mutate cannot be combined with the --mutate-path // glob filter: clap can't express this directly because --mutate takes // an optional list of paths. @@ -676,9 +722,6 @@ impl TestArgs { // Detect both up front so users aren't surprised by races or // corruption of their real dependency tree. use foundry_config::fs_permissions::FsAccessPermission; - let has_broad_write = config_for_mutation.fs_permissions.permissions.iter().any(|p| { - matches!(p.access, FsAccessPermission::Write | FsAccessPermission::ReadWrite) - }); if config_for_mutation.ffi { eyre::bail!( "Mutation testing is unsafe with `ffi = true`: per-mutant workspaces share \ @@ -687,13 +730,58 @@ impl TestArgs { Disable ffi in your foundry.toml to run mutation tests." ); } - if has_broad_write { + + // Only refuse write-capable `fs_permissions` whose path can actually + // reach one of the symlinked dependency trees. Scoped writes (e.g. + // `./out`, `./snapshots`) are safe because they target paths that + // never resolve into the shared `lib`/`node_modules`/`dependencies` + // trees. + let root = &config_for_mutation.root; + let mut shared_dep_dirs: Vec = config_for_mutation.libs.clone(); + shared_dep_dirs.push(root.join("node_modules")); + shared_dep_dirs.push(root.join("dependencies")); + let shared_dep_dirs: Vec = + shared_dep_dirs.into_iter().map(|p| dunce::canonicalize(&p).unwrap_or(p)).collect(); + + let touches_shared_dep = |perm_path: &Path| -> bool { + let resolved = if perm_path.is_absolute() { + perm_path.to_path_buf() + } else { + root.join(perm_path) + }; + let canon = dunce::canonicalize(&resolved).unwrap_or(resolved); + shared_dep_dirs.iter().any(|dep| { + // Either the permission path is inside a shared dep dir + // (write directly into deps), or it is an ancestor of one + // (broad permission like `./` covers them transitively). + canon.starts_with(dep) || dep.starts_with(&canon) + }) + }; + + let unsafe_write_paths: Vec<&Path> = config_for_mutation + .fs_permissions + .permissions + .iter() + .filter(|p| { + matches!(p.access, FsAccessPermission::Write | FsAccessPermission::ReadWrite) + }) + .filter(|p| touches_shared_dep(&p.path)) + .map(|p| p.path.as_path()) + .collect(); + + if !unsafe_write_paths.is_empty() { + let paths = unsafe_write_paths + .iter() + .map(|p| format!(" - {}", p.display())) + .collect::>() + .join("\n"); eyre::bail!( - "Mutation testing is unsafe with write-capable `fs_permissions`: per-mutant \ - workspaces share symlinked dependency directories, and `vm.writeFile` \ - calls can race against or corrupt the real `lib`/`node_modules`/\ - `dependencies` trees. Restrict `fs_permissions` to read-only (or scope it \ - away from dependency paths) to run mutation tests." + "Mutation testing is unsafe with write-capable `fs_permissions` that can \ + reach the symlinked dependency trees (`lib`/`node_modules`/`dependencies`); \ + per-mutant workspaces share those trees, so `vm.writeFile` calls would race \ + against or corrupt your real dependencies. Restrict the following \ + `fs_permissions` entries to read-only or scope them away from dependency \ + paths:\n{paths}" ); } @@ -706,6 +794,18 @@ impl TestArgs { num_workers: self.mutation_jobs.unwrap_or(0), show_progress: self.show_progress, json_output, + // Carry the same filter args (--match-test, --match-contract, + // --match-path, positional path shorthand, --rerun, ...) and + // isolation flag the baseline actually used, so every mutant + // exercises the exact same test set under the same execution + // model. We pull from the materialized `filter`, not the raw + // CLI flags on `self`, because the baseline applies extras: + // the positional `forge test ` shorthand is folded into + // `path_pattern`, and `--rerun` injects last-run failures + // into `test_pattern`. Using `self.filter.clone()` would lose + // those and let mutant runs silently diverge from baseline. + filter_args: filter.args().clone(), + isolate: evm_opts_for_mutation.isolate, }; let result = run_mutation_testing( diff --git a/crates/forge/src/mutation/mod.rs b/crates/forge/src/mutation/mod.rs index b54c27f32f0fc..bd0d04782e07d 100644 --- a/crates/forge/src/mutation/mod.rs +++ b/crates/forge/src/mutation/mod.rs @@ -1,5 +1,5 @@ use std::{ - collections::{HashMap, HashSet}, + collections::{BTreeMap, HashSet}, path::PathBuf, sync::Arc, }; @@ -146,9 +146,15 @@ impl MutationsSummary { if valid_mutants == 0 { 0.0 } else { self.dead.len() as f64 / valid_mutants as f64 * 100.0 } } - /// Convert to JSON output format + /// Convert to JSON output format. + /// + /// Output is sorted deterministically: files in lexicographic order + /// (`BTreeMap` keys), and survived mutants within each file sorted by + /// `(span.lo, span.hi, mutation_text)`. Without this, parallel worker + /// completion order leaks into the JSON and breaks downstream diffing, + /// snapshot tests, and reproducibility. pub fn to_json_output(&self, duration_secs: f64) -> MutationJsonOutput { - let mut survived_mutants: HashMap> = HashMap::new(); + let mut survived_mutants: BTreeMap> = BTreeMap::new(); for mutant in &self.survived { let file_path = mutant.relative_path(); @@ -156,6 +162,17 @@ impl MutationsSummary { entry.push(SurvivedMutantJson::from_mutant(mutant)); } + for entries in survived_mutants.values_mut() { + entries.sort_by(|a, b| { + (a.line, a.column, &a.original, &a.mutant).cmp(&( + b.line, + b.column, + &b.original, + &b.mutant, + )) + }); + } + MutationJsonOutput { summary: MutationSummaryJson { total: self.total_mutants(), @@ -172,11 +189,14 @@ impl MutationsSummary { } } -/// JSON output for mutation testing results +/// JSON output for mutation testing results. +/// +/// Uses [`BTreeMap`] for `survived_mutants` so file ordering in the emitted +/// JSON is deterministic. #[derive(Debug, Clone, Serialize)] pub struct MutationJsonOutput { pub summary: MutationSummaryJson, - pub survived_mutants: HashMap>, + pub survived_mutants: BTreeMap>, } /// Summary section of JSON output diff --git a/crates/forge/src/mutation/mutant.rs b/crates/forge/src/mutation/mutant.rs index b6423b06ed0ea..4a86c6df108fc 100644 --- a/crates/forge/src/mutation/mutant.rs +++ b/crates/forge/src/mutation/mutant.rs @@ -108,12 +108,20 @@ pub enum OwnedStrKind { #[derive(Debug, Clone, Serialize, Deserialize)] pub enum OwnedLiteral { - Str { kind: OwnedStrKind, text: String }, + Str { + kind: OwnedStrKind, + text: String, + }, Number(alloy_primitives::U256), Rational(String), Address(String), Bool(bool), Err(String), + /// Signed-negation of a numeric literal (e.g. `-123`). We cannot represent + /// negative values inside `Number(U256)` (the cast wraps via two's + /// complement and renders as a huge unsigned literal), so we carry the + /// negation textually and render it as `-{val}`. + NegatedNumber(alloy_primitives::U256), } impl From<&LitKind<'_>> for OwnedLiteral { @@ -142,6 +150,7 @@ impl Display for OwnedLiteral { match self { Self::Bool(val) => write!(f, "{val}"), Self::Number(val) => write!(f, "{val}"), + Self::NegatedNumber(val) => write!(f, "-{val}"), Self::Rational(s) => write!(f, "{s}"), Self::Address(s) => write!(f, "{s}"), Self::Str { kind, text } => match kind { diff --git a/crates/forge/src/mutation/mutators/assignment_mutator.rs b/crates/forge/src/mutation/mutators/assignment_mutator.rs index f9b586d40f893..6e81f14e1547d 100644 --- a/crates/forge/src/mutation/mutators/assignment_mutator.rs +++ b/crates/forge/src/mutation/mutators/assignment_mutator.rs @@ -47,10 +47,14 @@ impl Mutator for AssignmentMutator { line_number, column_number, }, + // Negation of a numeric literal must be carried textually: + // applying `-*val` on a `U256` wraps via two's complement + // and would render as a huge unsigned literal (e.g. `1` + // becomes `2^256 - 1`), producing wrong-source mutants. Mutant { span: replacement_span, mutation: MutationType::Assignment(AssignVarTypes::Literal( - OwnedLiteral::Number(-*val), + OwnedLiteral::NegatedNumber(*val), )), path: context.path.clone(), original, @@ -63,6 +67,10 @@ impl Mutator for AssignmentMutator { OwnedLiteral::Rational(_) => Ok(vec![]), OwnedLiteral::Address(_) => Ok(vec![]), OwnedLiteral::Err(_) => Ok(vec![]), + // `NegatedNumber` is only ever constructed *as* a mutant; it + // does not appear as an original literal in the source AST, + // so there is nothing to mutate here. + OwnedLiteral::NegatedNumber(_) => Ok(vec![]), }, AssignVarTypes::Identifier(ref ident) => Ok(vec![ Mutant { diff --git a/crates/forge/src/mutation/mutators/binary_op_mutator.rs b/crates/forge/src/mutation/mutators/binary_op_mutator.rs index c62ea6c2cdaf5..1168953dbc085 100644 --- a/crates/forge/src/mutation/mutators/binary_op_mutator.rs +++ b/crates/forge/src/mutation/mutators/binary_op_mutator.rs @@ -9,7 +9,7 @@ pub struct BinaryOpMutator; impl Mutator for BinaryOpMutator { fn generate_mutants(&self, context: &MutationContext<'_>) -> Result> { let expr = context.expr.ok_or_eyre("BinaryOpMutator: no expression")?; - let (bin_op, _op_span, lhs, rhs) = get_bin_op_parts(expr)?; + let (bin_op, _op_span, lhs, rhs, compound_assignment) = get_bin_op_parts(expr)?; let op = bin_op.kind; let operations_bools = vec![ @@ -47,8 +47,11 @@ impl Mutator for BinaryOpMutator { let rhs_text = extract_span_text(source, rhs.span); let op_str = op.to_str(); - // Build original expression: "lhs op rhs" - let original_expr = format!("{lhs_text} {op_str} {rhs_text}"); + let original_expr = if compound_assignment { + format!("{lhs_text} {op_str}= {rhs_text}") + } else { + format!("{lhs_text} {op_str} {rhs_text}") + }; // Use the full expression span for the mutation (not just the operator span) let expr_span = context.span; @@ -62,8 +65,11 @@ impl Mutator for BinaryOpMutator { .into_iter() .filter(|&kind| kind != op) .map(|kind| { - // Build mutated expression: "lhs new_op rhs" - let mutated_expr = format!("{} {} {}", lhs_text, kind.to_str(), rhs_text); + let mutated_expr = if compound_assignment { + format!("{} {}= {}", lhs_text, kind.to_str(), rhs_text) + } else { + format!("{} {} {}", lhs_text, kind.to_str(), rhs_text) + }; Mutant { span: expr_span, mutation: MutationType::BinaryOpExpr { new_op: kind, mutated_expr }, @@ -82,19 +88,20 @@ impl Mutator for BinaryOpMutator { return false; } - match ctxt.expr.unwrap().kind { - ExprKind::Binary(_, _, _) => true, - ExprKind::Assign(_, bin_op, _) => bin_op.is_some(), - _ => false, - } + matches!( + ctxt.expr.unwrap().kind, + ExprKind::Binary(_, _, _) | ExprKind::Assign(_, Some(_), _) + ) } } /// Extract the binary operator, its span, and LHS/RHS expressions -fn get_bin_op_parts<'a>(expr: &'a Expr<'a>) -> Result<(BinOp, Span, &'a Expr<'a>, &'a Expr<'a>)> { +fn get_bin_op_parts<'a>( + expr: &'a Expr<'a>, +) -> Result<(BinOp, Span, &'a Expr<'a>, &'a Expr<'a>, bool)> { match &expr.kind { - ExprKind::Assign(lhs, Some(bin_op), rhs) => Ok((*bin_op, bin_op.span, lhs, rhs)), - ExprKind::Binary(lhs, op, rhs) => Ok((*op, op.span, lhs, rhs)), + ExprKind::Assign(lhs, Some(op), rhs) => Ok((*op, op.span, lhs, rhs, true)), + ExprKind::Binary(lhs, op, rhs) => Ok((*op, op.span, lhs, rhs, false)), _ => eyre::bail!("BinaryOpMutator: unexpected expression kind"), } } diff --git a/crates/forge/src/mutation/mutators/tests/assignment_mutator_test.rs b/crates/forge/src/mutation/mutators/tests/assignment_mutator_test.rs index a2d06384c59a9..9c3909cdbb345 100644 --- a/crates/forge/src/mutation/mutators/tests/assignment_mutator_test.rs +++ b/crates/forge/src/mutation/mutators/tests/assignment_mutator_test.rs @@ -2,11 +2,14 @@ use crate::mutation::mutators::{ assignment_mutator::AssignmentMutator, tests::helper::mutator_tests, }; +// Each emitted mutation only carries the *replacement text* for the RHS +// span — not the full statement. So `x = 123` mutates to `0` (zero) and +// `-123` (signed-negation), not `x = 0` / `x = -123`. mutator_tests!(AssignmentMutator; - assign_lit: "x = y" => Some(vec!["x = 0", "x = -y"]); - assign_number: "x = 123" => Some(vec!["x = 0", "x = -123"]); - assign_true: "x = true" => Some(vec!["x = false"]); - assign_false: "x = false" => Some(vec!["x = true"]); - assign_declare: "uint256 x = 123" => Some(vec!["uint256 x = 0", "uint256 x = -123"]); + assign_lit: "x = y" => Some(vec!["0", "-y"]); + assign_number: "x = 123" => Some(vec!["0", "-123"]); + assign_true: "x = true" => Some(vec!["false"]); + assign_false: "x = false" => Some(vec!["true"]); + assign_declare: "uint256 x = 123" => Some(vec!["0", "-123"]); non_assign: "a = b + c" => None; ); diff --git a/crates/forge/src/mutation/mutators/tests/binary_op_mutator_test.rs b/crates/forge/src/mutation/mutators/tests/binary_op_mutator_test.rs index afb4837d8aaab..ae169ba340c24 100644 --- a/crates/forge/src/mutation/mutators/tests/binary_op_mutator_test.rs +++ b/crates/forge/src/mutation/mutators/tests/binary_op_mutator_test.rs @@ -14,4 +14,7 @@ mutator_tests!(BinaryOpMutator; bit_or: "x | y" => Some(vec!["x + y", "x - y", "x * y", "x / y", "x % y", "x ** y", "x << y", "x >> y", "x >>> y", "x & y", "x ^ y"]); bit_xor: "x ^ y" => Some(vec!["x + y", "x - y", "x * y", "x / y", "x % y", "x ** y", "x << y", "x >> y", "x >>> y", "x & y", "x | y"]); non_binary: "a = true" => None; + compound_assign_add: "a += b" => Some(vec!["a >>= b", "a <<= b", "a >>>= b", "a &= b", "a |= b", "a ^= b", "a -= b", "a **= b", "a *= b", "a /= b", "a %= b"]); + compound_assign_sub: "a -= b" => Some(vec!["a >>= b", "a <<= b", "a >>>= b", "a &= b", "a |= b", "a ^= b", "a += b", "a **= b", "a *= b", "a /= b", "a %= b"]); + compound_assign_mul: "a *= b" => Some(vec!["a >>= b", "a <<= b", "a >>>= b", "a &= b", "a |= b", "a ^= b", "a += b", "a -= b", "a **= b", "a /= b", "a %= b"]); ); diff --git a/crates/forge/src/mutation/mutators/tests/delete_expression_mutator_test.rs b/crates/forge/src/mutation/mutators/tests/delete_expression_mutator_test.rs index 2519b423d1e7a..3d68858757319 100644 --- a/crates/forge/src/mutation/mutators/tests/delete_expression_mutator_test.rs +++ b/crates/forge/src/mutation/mutators/tests/delete_expression_mutator_test.rs @@ -2,7 +2,10 @@ use crate::mutation::mutators::{ delete_expression_mutator::DeleteExpressionMutator, tests::helper::mutator_tests, }; +// `delete x` is replaced by `assert(true)` (a no-op statement) — the test +// expects the mutation's *replacement text*, not the original expression +// stripped of the `delete` keyword. mutator_tests!(DeleteExpressionMutator; - delete_expr: "delete x" => Some(vec!["x"]); + delete_expr: "delete x" => Some(vec!["assert(true)"]); non_delete: "a = b + c" => None; ); diff --git a/crates/forge/src/mutation/mutators/tests/elim_delegate_mutator_test.rs b/crates/forge/src/mutation/mutators/tests/elim_delegate_mutator_test.rs index 6a08641fa3bcf..913aef0ef7bf8 100644 --- a/crates/forge/src/mutation/mutators/tests/elim_delegate_mutator_test.rs +++ b/crates/forge/src/mutation/mutators/tests/elim_delegate_mutator_test.rs @@ -2,7 +2,12 @@ use crate::mutation::mutators::{ elim_delegate_mutator::ElimDelegateMutator, tests::helper::mutator_tests, }; +// The mutator narrows the replacement span to just the `delegatecall` +// identifier and rewrites it to `call`, so the emitted mutation text is +// just `"call"`. It only matches plain `.delegatecall(args)` Call +// expressions; the variant with `{value: ...}` parses as a `CallOptions` +// wrapper and is intentionally left to a follow-up. mutator_tests!(ElimDelegateMutator; - delegate_expr: "address(this).delegatecall{value: 1 ether}(0)" => Some(vec!["address(this).call{value: 1 ether}(0)"]); - non_delegate: "address(this).call{value: 1 ether}(0)" => None; + delegate_expr: "target.delegatecall(data)" => Some(vec!["call"]); + non_delegate: "target.call(data)" => None; ); diff --git a/crates/forge/src/mutation/mutators/tests/helper.rs b/crates/forge/src/mutation/mutators/tests/helper.rs index 614502294f4f2..88d0ca477192d 100644 --- a/crates/forge/src/mutation/mutators/tests/helper.rs +++ b/crates/forge/src/mutation/mutators/tests/helper.rs @@ -18,16 +18,32 @@ pub struct MutatorTestCase<'a> { pub trait MutatorTester { fn test_mutator(mutator: M, test_case: MutatorTestCase<'_>) { + // Wrap the snippet in a minimal but valid Solidity source unit so the + // parser/visitor actually walks into expression contexts. Bare + // fragments like `"x + y"` or `"a += b"` are not parseable on their + // own; without wrapping, the parser would emit errors that the test + // harness used to swallow, silently making mutator tests vacuous. + let wrapped = format!( + "// SPDX-License-Identifier: MIT\n\ + pragma solidity ^0.8.0;\n\ + contract __TestC {{\n\ + function __test() public {{\n\ + {input};\n\ + }}\n\ + }}\n", + input = test_case.input, + ); + let sess = Session::builder().with_silent_emitter(None).build(); - let _ = sess.enter(|| -> solar::interface::Result<()> { + let outcome = sess.enter(|| -> solar::interface::Result> { let arena = Arena::new(); let mut parser = Parser::from_lazy_source_code( &sess, &arena, FileName::Real(PathBuf::from("test.sol")), - || Ok(test_case.input.to_string()), + || Ok(wrapped.clone()), )?; let ast = parser.parse_file().map_err(|e| e.emit())?; @@ -36,41 +52,42 @@ pub trait MutatorTester { PathBuf::from("test.sol"), vec![Box::new(mutator)], ) - .with_source(test_case.input); + .with_source(&wrapped); let _ = mutant_visitor.visit_source_unit(&ast); - let mutations = mutant_visitor.mutation_to_conduct; - - if let Some(expected) = test_case.expected_mutations { - assert_eq!( - mutations.len(), - expected.len(), - "Expected {} mutations, got {}: {:?}", - expected.len(), - mutations.len(), - mutations.iter().map(|m| m.mutation.to_string()).collect::>() - ); - - for mutation in &mutations { - let mutation_str = mutation.mutation.to_string(); - assert!( - expected.contains(&mutation_str.as_str()), - "Unexpected mutation: {mutation_str}. Expected one of: {expected:?}", - ); - } - } else { - assert_eq!( - mutations.len(), - 0, - "Expected no mutations, got {}: {:?}", - mutations.len(), - mutations.iter().map(|m| m.mutation.to_string()).collect::>() - ); - } + Ok(mutant_visitor + .mutation_to_conduct + .into_iter() + .map(|m| m.mutation.to_string()) + .collect()) + }); - Ok(()) + // Surface parse/visit errors instead of silently passing the test. + let mutations = outcome.unwrap_or_else(|_| { + panic!( + "mutator test input failed to parse/visit; wrapped source was:\n{wrapped}\n\ + raw input: {input:?}", + input = test_case.input, + ) }); + + if let Some(expected) = test_case.expected_mutations { + let mut actual = mutations; + actual.sort(); + + let mut expected = expected.into_iter().map(str::to_string).collect::>(); + expected.sort(); + + assert_eq!(actual, expected, "Unexpected mutation set for input {:?}", test_case.input); + } else { + assert!( + mutations.is_empty(), + "Expected no mutations, got {}: {:?}", + mutations.len(), + mutations, + ); + } } } diff --git a/crates/forge/src/mutation/orchestrator.rs b/crates/forge/src/mutation/orchestrator.rs index afd1fcaab3e6b..b11c1d7a33419 100644 --- a/crates/forge/src/mutation/orchestrator.rs +++ b/crates/forge/src/mutation/orchestrator.rs @@ -20,9 +20,12 @@ use foundry_compilers::{ use foundry_config::{Config, filter::GlobMatcher}; use foundry_evm::opts::EvmOpts; -use crate::mutation::{ - MutationHandler, MutationProgress, MutationReporter, MutationsSummary, mutant::MutationResult, - runner::run_mutations_parallel_with_progress, +use crate::{ + cmd::test::FilterArgs, + mutation::{ + MutationHandler, MutationProgress, MutationReporter, MutationsSummary, + mutant::MutationResult, runner::run_mutations_parallel_with_progress, + }, }; #[derive(Clone, Debug, Eq, Ord, PartialEq, PartialOrd, serde::Serialize)] @@ -57,6 +60,13 @@ pub struct MutationRunConfig { pub show_progress: bool, /// Whether to output JSON (suppress all other output). pub json_output: bool, + /// Test filter (`--match-test`, `--match-contract`, `--match-path`, ...) + /// applied identically to baseline and every mutant run so they exercise + /// the same test set. + pub filter_args: FilterArgs, + /// EVM isolation flag — mirrors the canonical `forge test` runner so + /// baseline and mutant runs use the same execution model. + pub isolate: bool, } impl MutationRunConfig { @@ -152,9 +162,19 @@ pub async fn run_mutation_testing( continue; } + // Load persisted survived spans *before* generating/loading mutants so + // they can actually steer the adaptive skip — both at AST generation + // time (via the span filter inside `generate_ast`) and when re-using + // a cached mutant list from a prior partial run. + handler.retrieve_survived_spans(&build_id, &execution_cache_key); + // Generate or load cached mutants let mut mutants = if let Some(ms) = handler.retrieve_cached_mutants(&build_id) { - ms + // When loading from cache, filter out mutants whose span already + // had a survivor in a previous run. Without this, a resumed run + // would re-test mutations the adaptive heuristic already knows + // are uninformative. + ms.into_iter().filter(|m| !handler.should_skip_span(m.span)).collect() } else { handler.generate_ast(json_output).await?; handler.mutations.clone() @@ -203,6 +223,8 @@ pub async fn run_mutation_testing( num_workers, progress.clone(), json_output, + mutation_config.filter_args.clone(), + mutation_config.isolate, )?; // Collect results for caching @@ -234,6 +256,14 @@ pub async fn run_mutation_testing( // - non-cancelled-but-short result vectors indicate a bug, not a hit // The mutants list itself is fine to persist (it's deterministic from // the AST + operator set) and so are survived spans (best-effort hint). + // + // Sort the persisted result vector by mutant span so the on-disk + // cache is independent of rayon worker completion order; otherwise + // the cache file changes content-hash run-to-run even when the + // outcomes are identical, defeating diffing and reproducibility. + results_vec.sort_by(|(a, _), (b, _)| { + a.span.lo().0.cmp(&b.span.lo().0).then_with(|| a.span.hi().0.cmp(&b.span.hi().0)) + }); if !mutants.is_empty() && !build_id.is_empty() { let _ = handler.persist_cached_mutants(&build_id, &mutants); if complete_run { @@ -315,35 +345,26 @@ fn mutation_execution_cache_key_from_parts( } /// Resolve which paths to mutate based on configuration. +/// +/// Resolution order: +/// 1. Pick the *base* set of candidate files: +/// - `--mutate-path ` → all source files matching the glob, OR +/// - explicit `--mutate PATH...` → those validated files, OR +/// - default → every Solidity file under `config.src`. +/// 2. If `--mutate-contract ` is set, intersect the base set with files that contain at +/// least one contract whose name matches the regex. The per-file contract filter still +/// re-applies inside the handler. fn resolve_mutate_paths( config: &Config, output: &ProjectCompileOutput, mutation_config: &MutationRunConfig, ) -> Result> { - let paths = if let Some(pattern) = &mutation_config.mutate_path_pattern { - // If --mutate-path is provided, use it to filter paths + // 1. Base path set. + let base: Vec = if let Some(pattern) = &mutation_config.mutate_path_pattern { source_files_iter(&config.src, MultiCompilerLanguage::FILE_EXTENSIONS) .filter(|entry| entry.is_sol() && !entry.is_sol_test() && pattern.is_match(entry)) .collect() - } else if let Some(contract_pattern) = &mutation_config.mutate_contract_pattern { - // If --mutate-contract is provided, use it to filter contracts - source_files_iter(&config.src, MultiCompilerLanguage::FILE_EXTENSIONS) - .filter(|entry| { - entry.is_sol() - && !entry.is_sol_test() - && output - .artifact_ids() - .filter(|(id, _)| id.source == *entry) - .any(|(id, _)| contract_pattern.is_match(&id.name)) - }) - .collect() - } else if mutation_config.mutate_paths.is_empty() { - // If --mutate is passed without arguments, use all Solidity files - source_files_iter(&config.src, MultiCompilerLanguage::FILE_EXTENSIONS) - .filter(|entry| entry.is_sol() && !entry.is_sol_test()) - .collect() - } else { - // If --mutate is passed with arguments, validate and use those paths + } else if !mutation_config.mutate_paths.is_empty() { let root_canon = config.root.canonicalize().wrap_err("failed to canonicalize project root")?; let mut validated = Vec::with_capacity(mutation_config.mutate_paths.len()); @@ -373,6 +394,26 @@ fn resolve_mutate_paths( validated.push(canon); } validated + } else { + source_files_iter(&config.src, MultiCompilerLanguage::FILE_EXTENSIONS) + .filter(|entry| entry.is_sol() && !entry.is_sol_test()) + .collect() + }; + + // 2. Intersect with `--mutate-contract` if set, so explicit `--mutate ` combined with + // `--mutate-contract ` does the principled thing (the listed files, restricted to + // those containing a matching contract) instead of silently expanding to every source file. + let paths = if let Some(contract_pattern) = &mutation_config.mutate_contract_pattern { + base.into_iter() + .filter(|entry| { + output + .artifact_ids() + .filter(|(id, _)| id.source == *entry) + .any(|(id, _)| contract_pattern.is_match(&id.name)) + }) + .collect() + } else { + base }; Ok(paths) diff --git a/crates/forge/src/mutation/reporter.rs b/crates/forge/src/mutation/reporter.rs index c075046f0622a..3dd4228c7c489 100644 --- a/crates/forge/src/mutation/reporter.rs +++ b/crates/forge/src/mutation/reporter.rs @@ -97,7 +97,30 @@ impl MutationReporter { let _ = sh_println!("{}", Paint::red("Survived mutants").bold()); let _ = sh_println!("{}", "─".repeat(60)); - for (i, mutant) in summary.get_survived().iter().enumerate() { + // Sort by (file, line, column, span, mutation text) so the + // reported order is deterministic across runs / worker counts. + // Workers complete in arbitrary order, so without this every run + // can permute the report. + let mut survived: Vec<&Mutant> = summary.get_survived().iter().collect(); + survived.sort_by(|a, b| { + ( + a.relative_path(), + a.line_number, + a.column_number, + a.span.lo().0, + a.span.hi().0, + a.mutation.to_string(), + ) + .cmp(&( + b.relative_path(), + b.line_number, + b.column_number, + b.span.lo().0, + b.span.hi().0, + b.mutation.to_string(), + )) + }); + for (i, mutant) in survived.iter().enumerate() { self.print_survived_mutant(i + 1, mutant); } } diff --git a/crates/forge/src/mutation/runner.rs b/crates/forge/src/mutation/runner.rs index 639e03bc3ea3c..bfec7192d0ba0 100644 --- a/crates/forge/src/mutation/runner.rs +++ b/crates/forge/src/mutation/runner.rs @@ -18,7 +18,7 @@ use std::{ }; use eyre::Result; -use foundry_common::{EmptyTestFilter, compile::ProjectCompiler, sh_eprintln, sh_println}; +use foundry_common::{compile::ProjectCompiler, sh_eprintln, sh_println}; use foundry_compilers::compilers::multi::MultiCompiler; use foundry_config::Config; #[cfg(feature = "optimism")] @@ -34,6 +34,7 @@ use tempfile::TempDir; use crate::{ MultiContractRunnerBuilder, + cmd::test::FilterArgs, mutation::{ SurvivedSpans, mutant::{Mutant, MutationResult}, @@ -147,6 +148,8 @@ pub fn run_mutations_parallel_with_progress( num_workers: usize, progress: Option, silent: bool, + filter_args: FilterArgs, + isolate: bool, ) -> Result> { let total = mutants.len(); if total == 0 { @@ -220,6 +223,8 @@ pub fn run_mutations_parallel_with_progress( let completed_results: Arc>> = Arc::new(Mutex::new(Vec::with_capacity(total))); + let filter_args = Arc::new(filter_args); + pool.install(|| { mutants.into_par_iter().for_each(|mutant| { // Skip if cancelled @@ -237,6 +242,8 @@ pub fn run_mutations_parallel_with_progress( &config, &evm_opts, &shared_state, + &filter_args, + isolate, ) })); @@ -305,6 +312,7 @@ pub fn run_mutations_parallel_with_progress( } /// Test a single mutant in an isolated temporary workspace. +#[allow(clippy::too_many_arguments)] fn test_single_mutant_isolated( mutant: Mutant, source_relative: &PathBuf, @@ -312,6 +320,8 @@ fn test_single_mutant_isolated( config: &Arc, evm_opts: &EvmOpts, shared_state: &Arc, + filter_args: &Arc, + isolate: bool, ) -> MutantTestResult { // Check if we should skip this mutant based on adaptive span tracking if shared_state.should_skip_span(mutant.span) { @@ -424,11 +434,17 @@ fn test_single_mutant_isolated( let timeout = config.mutation.timeout.map(|s| Duration::from_secs(s as u64)); let result = match timeout { - Some(budget) => { - run_compile_and_test_with_timeout(temp_config, evm_opts, budget, temp_dir, shared_state) - } + Some(budget) => run_compile_and_test_with_timeout( + temp_config, + evm_opts, + budget, + temp_dir, + shared_state, + filter_args.clone(), + isolate, + ), None => { - let res = match compile_and_test(&temp_config, evm_opts) { + let res = match compile_and_test(&temp_config, evm_opts, filter_args, isolate) { Ok(true) => MutationResult::Dead, Ok(false) => MutationResult::Alive, Err(_) => MutationResult::Invalid, @@ -460,12 +476,15 @@ fn test_single_mutant_isolated( /// directory is only dropped when the worker thread actually exits. On /// timeout the `JoinHandle` is parked in `shared_state.pending_workers` /// and joined at the end of the parallel run. +#[allow(clippy::too_many_arguments)] fn run_compile_and_test_with_timeout( config: Arc, evm_opts: &EvmOpts, budget: Duration, temp_dir: TempDir, shared_state: &Arc, + filter_args: Arc, + isolate: bool, ) -> MutationResult { let (tx, rx) = mpsc::channel::>(); let opts = evm_opts.clone(); @@ -473,13 +492,16 @@ fn run_compile_and_test_with_timeout( // thread exits. Do NOT capture by reference — the worker may outlive this // function on timeout. let cfg = Arc::clone(&config); + let filter_for_worker = Arc::clone(&filter_args); let spawn_result = std::thread::Builder::new() .stack_size(16 * 1024 * 1024) .name("mutation-worker".to_string()) .spawn(move || { - let res = panic::catch_unwind(AssertUnwindSafe(|| compile_and_test(&cfg, &opts))) - .unwrap_or_else(|_| Err(eyre::eyre!("worker panicked"))); + let res = panic::catch_unwind(AssertUnwindSafe(|| { + compile_and_test(&cfg, &opts, &filter_for_worker, isolate) + })) + .unwrap_or_else(|_| Err(eyre::eyre!("worker panicked"))); let _ = tx.send(res); // Keep `temp_dir` alive until *after* the worker is done with the // workspace. Dropping here (vs at function entry on timeout) @@ -560,21 +582,28 @@ fn apply_mutation(mutant: &Mutant, original_source: &str, dest_path: &Path) -> R /// Compile the project and run tests, returning true if any test failed (mutant killed). /// /// Dispatches to the correct network type based on `evm_opts.networks`. -fn compile_and_test(config: &Arc, evm_opts: &EvmOpts) -> Result { +fn compile_and_test( + config: &Arc, + evm_opts: &EvmOpts, + filter_args: &FilterArgs, + isolate: bool, +) -> Result { if evm_opts.networks.is_tempo() { - compile_and_test_inner::(config, evm_opts) + compile_and_test_inner::(config, evm_opts, filter_args, isolate) } else { #[cfg(feature = "optimism")] if evm_opts.networks.is_optimism() { - return compile_and_test_inner::(config, evm_opts); + return compile_and_test_inner::(config, evm_opts, filter_args, isolate); } - compile_and_test_inner::(config, evm_opts) + compile_and_test_inner::(config, evm_opts, filter_args, isolate) } } fn compile_and_test_inner( config: &Arc, evm_opts: &EvmOpts, + filter_args: &FilterArgs, + isolate: bool, ) -> Result { // Compile let compiler = @@ -582,6 +611,12 @@ fn compile_and_test_inner( let compile_output = compiler.compile(&config.project()?)?; + // Rebuild the per-mutant test filter so `--match-test`, `--match-contract`, + // `--match-path`, ... are honored against the temp workspace's paths + // (not the original project root). Without this the mutant runs would + // ignore user filters and execute a different test set than the baseline. + let filter = filter_args.clone().merge_with_config(config); + // Run tests - need a multi-threaded Tokio runtime since test() uses rayon internally // with par_iter, and rayon workers need tokio handle access let rt = tokio::runtime::Builder::new_multi_thread() @@ -595,16 +630,20 @@ fn compile_and_test_inner( let (evm_env, tx_env, fork_block) = evm_opts.env::, BlockEnvFor, TxEnvFor>().await?; - // Build test runner with fail-fast enabled + // Build test runner mirroring the canonical `forge test` runner: same + // isolation flag, same fail-fast semantics for mutation, and same + // filter so kept/skipped tests stay consistent across baseline and + // mutant runs. let mut runner = MultiContractRunnerBuilder::new(config.clone()) .set_debug(false) .initial_balance(evm_opts.initial_balance) .sender(evm_opts.sender) .with_fork(evm_opts.get_fork(config, evm_env.cfg_env.chain_id, fork_block)) + .enable_isolation(isolate) .fail_fast(true) .build::(&compile_output, evm_env, tx_env, evm_opts.clone())?; - runner.test_collect(&EmptyTestFilter::default()) + runner.test_collect(&filter) })?; // Check if any test failed (mutant killed) diff --git a/crates/forge/src/workspace.rs b/crates/forge/src/workspace.rs index b6730aa9f8040..3d81c81d63d5d 100644 --- a/crates/forge/src/workspace.rs +++ b/crates/forge/src/workspace.rs @@ -83,6 +83,18 @@ pub fn copy_project(config: &Config, temp_dir: &Path) -> Result<()> { copy_dir_recursive(&config.test, &temp_dir.join(&test_rel))?; } + // Copy `script/` too when present and distinct from src/test. Many real + // projects keep helper contracts, deployment scripts, or fixtures under + // `script/` and reference them from tests via relative imports. Without + // this, baselines that compile fine produce a sea of `Invalid` mutants + // for purely-environmental reasons. + if config.script.exists() && config.script != config.src && config.script != config.test { + let script_rel = relative_to_root(&config.root, &config.script); + ensure_safe_relative_path(&script_rel, "script", &config.script)?; + ensure_within_root(&config.root, &config.script, "script", &config.script)?; + copy_dir_recursive(&config.script, &temp_dir.join(&script_rel))?; + } + for lib_path in &config.libs { if lib_path.exists() { let lib_rel = relative_to_root(&config.root, lib_path); diff --git a/crates/forge/tests/cli/test_cmd/mutation.rs b/crates/forge/tests/cli/test_cmd/mutation.rs index 42e0c4d834045..c876ef107db70 100644 --- a/crates/forge/tests/cli/test_cmd/mutation.rs +++ b/crates/forge/tests/cli/test_cmd/mutation.rs @@ -516,16 +516,16 @@ MUTATION TESTING RESULTS ╭──────────┬───────────┬────────────╮ │ Status ┆ # Mutants ┆ % of Total │ ╞══════════╪═══════════╪════════════╡ -│ Survived ┆ 1 ┆ 1.6% │ +│ Survived ┆ 3 ┆ 4.8% │ ├╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌┤ -│ Killed ┆ 51 ┆ 82.3% │ +│ Killed ┆ 45 ┆ 72.6% │ ├╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌┤ -│ Invalid ┆ 9 ┆ 14.5% │ +│ Invalid ┆ 11 ┆ 17.7% │ ├╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌┤ -│ Skipped ┆ 1 ┆ 1.6% │ +│ Skipped ┆ 3 ┆ 4.8% │ ╰──────────┴───────────┴────────────╯ ... -Mutation Score: 98.1% (51/52 mutants killed); [ELAPSED] +Mutation Score: 93.8% (45/48 mutants killed); [ELAPSED] ... "#]]); });