From 0d2a49078b932318793a865b97fc7aa2d63e2504 Mon Sep 17 00:00:00 2001 From: Mablr <59505383+mablr@users.noreply.github.com> Date: Fri, 29 May 2026 15:45:57 +0200 Subject: [PATCH 1/8] fix(mutation): key cached results by execution inputs --- crates/forge/src/mutation/mod.rs | 144 ++++++++++++++---- crates/forge/src/mutation/orchestrator.rs | 153 +++++++++++++++++++- crates/forge/tests/cli/test_cmd/mutation.rs | 88 ++++++++++- 3 files changed, 348 insertions(+), 37 deletions(-) diff --git a/crates/forge/src/mutation/mod.rs b/crates/forge/src/mutation/mod.rs index 5e40dce4be3fc..72c7d3a47e01d 100644 --- a/crates/forge/src/mutation/mod.rs +++ b/crates/forge/src/mutation/mod.rs @@ -24,6 +24,13 @@ use solar::{ parse::Parser, }; +#[derive(Clone, Copy)] +enum CacheKind<'a> { + Mutants, + Results { execution_key: &'a str }, + Survived { execution_key: &'a str }, +} + pub mod mutant; mod mutators; pub mod orchestrator; @@ -320,12 +327,13 @@ impl MutationHandler { // Note: we now get the build hash directly from the recent compile output (see test flow) - /// Returns the cache file path for the given build hash and extension. + /// Returns the cache file path for the given build hash and cache kind. /// The filename encodes a hash of the full contract path to prevent collisions /// between files with the same stem in different directories, and a hash of /// the active mutation config so changes to enabled operators invalidate - /// previously cached mutants/results. - fn cache_file_path(&self, hash: &str, ext: &str) -> PathBuf { + /// previously cached mutants. Result-like caches also include an execution + /// key so stale outcomes are not reused after test/config/EVM changes. + fn cache_file_path(&self, hash: &str, kind: CacheKind<'_>) -> PathBuf { use std::{ collections::hash_map::DefaultHasher, hash::{Hash, Hasher}, @@ -334,42 +342,45 @@ impl MutationHandler { self.contract_to_mutate.hash(&mut hasher); let path_hash = hasher.finish(); - // Hash the effective set of enabled mutation operators so cache entries - // are invalidated when the user changes `include_operators` / - // `exclude_operators` in their config. Also fold in the timeout so a - // changed budget invalidates previously cached results that may have - // been collected under a different (possibly larger) limit. + // Hash the effective set of enabled mutation operators so mutant cache + // entries are invalidated when the user changes `include_operators` / + // `exclude_operators` in their config. // - // We also fold in the active `--mutate-contract` regex pattern, because - // running with vs. without that filter produces a different set of - // mutants for the same file. Without this, a cached results vector - // collected under one filter could be silently reused under another - // (or absent) filter and report the wrong survival status. - let mut cfg_hasher = DefaultHasher::new(); + // Also fold in the active `--mutate-contract` regex pattern, because + // running with vs. without that filter produces a different mutant set + // for the same file. + let mut mutant_cfg_hasher = DefaultHasher::new(); + // Version salt for this mutant-set cache schema. Bump this if the + // inputs that define generated mutants change. + "mutant-set-v1".hash(&mut mutant_cfg_hasher); for op in self.config.mutation.enabled_operators() { - op.to_string().hash(&mut cfg_hasher); + op.to_string().hash(&mut mutant_cfg_hasher); } - self.config.mutation.timeout.hash(&mut cfg_hasher); match self.contract_filter.as_ref() { Some(re) => { - "filter:".hash(&mut cfg_hasher); - re.as_str().hash(&mut cfg_hasher); + "filter:".hash(&mut mutant_cfg_hasher); + re.as_str().hash(&mut mutant_cfg_hasher); } - None => "nofilter".hash(&mut cfg_hasher), + None => "nofilter".hash(&mut mutant_cfg_hasher), } - let cfg_hash = cfg_hasher.finish(); + let mutant_cfg_hash = mutant_cfg_hasher.finish(); + + let (ext, execution_suffix) = match kind { + CacheKind::Mutants => ("mutants", String::new()), + CacheKind::Results { execution_key } => ("results", format!("_{execution_key}")), + CacheKind::Survived { execution_key } => ("survived", format!("_{execution_key}")), + }; let stem = self.contract_to_mutate.file_stem().and_then(|s| s.to_str()).unwrap_or("unknown"); - self.config - .root - .join(&self.config.mutation_dir) - .join(format!("{hash}_{stem}_{path_hash:x}_{cfg_hash:x}.{ext}")) + self.config.root.join(&self.config.mutation_dir).join(format!( + "{hash}_{stem}_{path_hash:x}_{mutant_cfg_hash:x}{execution_suffix}.{ext}" + )) } /// Persists cached mutants using build hash for cache invalidation. pub fn persist_cached_mutants(&self, hash: &str, mutants: &[Mutant]) -> std::io::Result<()> { - let cache_file = self.cache_file_path(hash, "mutants"); + let cache_file = self.cache_file_path(hash, CacheKind::Mutants); if let Some(dir) = cache_file.parent() { std::fs::create_dir_all(dir)?; } @@ -381,9 +392,10 @@ impl MutationHandler { pub fn persist_cached_results( &self, hash: &str, + execution_key: &str, results: &[(Mutant, crate::mutation::mutant::MutationResult)], ) -> std::io::Result<()> { - let cache_file = self.cache_file_path(hash, "results"); + let cache_file = self.cache_file_path(hash, CacheKind::Results { execution_key }); if let Some(dir) = cache_file.parent() { std::fs::create_dir_all(dir)?; } @@ -446,7 +458,7 @@ impl MutationHandler { /// Retrieves cached mutants using build hash. pub fn retrieve_cached_mutants(&self, hash: &str) -> Option> { - let cache_file = self.cache_file_path(hash, "mutants"); + let cache_file = self.cache_file_path(hash, CacheKind::Mutants); let data = std::fs::read_to_string(cache_file).ok()?; serde_json::from_str(&data).ok() } @@ -455,8 +467,9 @@ impl MutationHandler { pub fn retrieve_cached_mutant_results( &self, hash: &str, + execution_key: &str, ) -> Option> { - let cache_file = self.cache_file_path(hash, "results"); + let cache_file = self.cache_file_path(hash, CacheKind::Results { execution_key }); let data = std::fs::read_to_string(cache_file).ok()?; serde_json::from_str(&data).ok() } @@ -472,8 +485,8 @@ impl MutationHandler { } /// Persist survived spans to cache for adaptive mutation testing. - pub fn persist_survived_spans(&self, hash: &str) -> std::io::Result<()> { - let cache_file = self.cache_file_path(hash, "survived"); + pub fn persist_survived_spans(&self, hash: &str, execution_key: &str) -> std::io::Result<()> { + let cache_file = self.cache_file_path(hash, CacheKind::Survived { execution_key }); if let Some(dir) = cache_file.parent() { std::fs::create_dir_all(dir)?; } @@ -483,8 +496,8 @@ impl MutationHandler { } /// Retrieve survived spans from cache. - pub fn retrieve_survived_spans(&mut self, hash: &str) -> bool { - let cache_file = self.cache_file_path(hash, "survived"); + pub fn retrieve_survived_spans(&mut self, hash: &str, execution_key: &str) -> bool { + let cache_file = self.cache_file_path(hash, CacheKind::Survived { execution_key }); if let Ok(data) = std::fs::read_to_string(cache_file) && let Ok(pairs) = serde_json::from_str::>(&data) @@ -496,3 +509,70 @@ impl MutationHandler { false } } + +#[cfg(test)] +mod tests { + use super::*; + use foundry_config::Config; + use tempfile::TempDir; + + fn test_handler(config: Config) -> MutationHandler { + let source = config.root.join("src").join("Counter.sol"); + MutationHandler::new(source, Arc::new(config)) + } + + fn test_config() -> (TempDir, Config) { + let temp = TempDir::new().unwrap(); + let config = Config { + root: temp.path().to_path_buf(), + mutation_dir: "cache/mutation".into(), + ..Default::default() + }; + (temp, config) + } + + #[test] + fn result_cache_path_includes_execution_key() { + let (_temp, config) = test_config(); + let handler = test_handler(config); + + let first = + handler.cache_file_path("build", CacheKind::Results { execution_key: "exec-a" }); + let second = + handler.cache_file_path("build", CacheKind::Results { execution_key: "exec-b" }); + let mutants = handler.cache_file_path("build", CacheKind::Mutants); + + assert_ne!(first, second); + assert_ne!(first, mutants); + assert_ne!(second, mutants); + } + + #[test] + fn survived_span_cache_path_includes_execution_key() { + let (_temp, config) = test_config(); + let handler = test_handler(config); + + let first = + handler.cache_file_path("build", CacheKind::Survived { execution_key: "exec-a" }); + let second = + handler.cache_file_path("build", CacheKind::Survived { execution_key: "exec-b" }); + + assert_ne!(first, second); + } + + #[test] + fn mutant_cache_path_ignores_execution_only_timeout() { + let (_temp, mut first_config) = test_config(); + let root = first_config.root.clone(); + let mut second_config = first_config.clone(); + + first_config.mutation.timeout = Some(1); + second_config.root = root; + second_config.mutation.timeout = Some(99); + + let first = test_handler(first_config).cache_file_path("build", CacheKind::Mutants); + let second = test_handler(second_config).cache_file_path("build", CacheKind::Mutants); + + assert_eq!(first, second); + } +} diff --git a/crates/forge/src/mutation/orchestrator.rs b/crates/forge/src/mutation/orchestrator.rs index de9bb403a5c35..91eb9081c485b 100644 --- a/crates/forge/src/mutation/orchestrator.rs +++ b/crates/forge/src/mutation/orchestrator.rs @@ -8,6 +8,7 @@ use std::{path::PathBuf, sync::Arc, time::Instant}; +use alloy_primitives::keccak256; use eyre::{Result, WrapErr}; use foundry_cli::utils::FoundryPathExt; use foundry_common::sh_println; @@ -24,6 +25,23 @@ use crate::mutation::{ runner::run_mutations_parallel_with_progress, }; +#[derive(Clone, Debug, Eq, Ord, PartialEq, PartialOrd, serde::Serialize)] +struct ArtifactCacheFingerprint { + source: String, + name: String, + version: String, + build_id: String, + profile: String, +} + +#[derive(serde::Serialize)] +struct ExecutionCacheFingerprint<'a> { + schema: &'static str, + config: &'a Config, + evm_opts: &'a EvmOpts, + artifacts: &'a [ArtifactCacheFingerprint], +} + /// Configuration for mutation testing run. pub struct MutationRunConfig { /// Paths to mutate (if empty, use all source files). @@ -80,6 +98,7 @@ pub async fn run_mutation_testing( // Determine which paths to mutate let mutate_paths = resolve_mutate_paths(&config, output, &mutation_config)?; + let execution_cache_key = mutation_execution_cache_key(&config, output, &evm_opts)?; if !mutation_config.show_progress && !json_output { sh_println!("Running mutation tests with {} parallel workers...", num_workers)?; @@ -109,7 +128,8 @@ pub async fn run_mutation_testing( .unwrap_or_default(); // Check for cached results - if let Some(prior) = handler.retrieve_cached_mutant_results(&build_id) { + if let Some(prior) = handler.retrieve_cached_mutant_results(&build_id, &execution_cache_key) + { if !mutation_config.show_progress && !json_output { sh_println!(" Using cached results for {} mutants", prior.len())?; } @@ -144,7 +164,7 @@ pub async fn run_mutation_testing( // Load survived spans for adaptive mutation testing. Only loaded after // we successfully obtained mutants for this build, so a stale survived // cache from a different mutant set is not applied. - handler.retrieve_survived_spans(&build_id); + handler.retrieve_survived_spans(&build_id, &execution_cache_key); // Sort mutations by span for optimal adaptive testing mutants.sort_by(|a, b| { @@ -211,9 +231,10 @@ pub async fn run_mutation_testing( if !mutants.is_empty() && !build_id.is_empty() { let _ = handler.persist_cached_mutants(&build_id, &mutants); if complete_run { - let _ = handler.persist_cached_results(&build_id, &results_vec); + let _ = + handler.persist_cached_results(&build_id, &execution_cache_key, &results_vec); } - let _ = handler.persist_survived_spans(&build_id); + let _ = handler.persist_survived_spans(&build_id, &execution_cache_key); } mutation_summary.merge(handler.get_report()); @@ -237,6 +258,51 @@ pub async fn run_mutation_testing( Ok(MutationRunResult { summary: mutation_summary, cancelled, duration_secs }) } +/// Build the cache discriminator for mutation *results*. +/// +/// Mutant generation only depends on the source build + selected mutators, but +/// result correctness depends on the compiled test universe and execution +/// settings. Hashing the full serialized config intentionally includes fuzz / +/// invariant settings, test filters, fs permissions, sender/balance/env values, +/// and future config fields unless explicitly skipped by `Config` itself. The +/// artifact fingerprint covers source and test build IDs, so changing a test +/// file invalidates result caches even when the mutated source did not change. +fn mutation_execution_cache_key( + config: &Config, + output: &ProjectCompileOutput, + evm_opts: &EvmOpts, +) -> Result { + let artifacts = output + .artifact_ids() + .map(|(id, _)| ArtifactCacheFingerprint { + source: id.source.display().to_string(), + name: id.name, + version: id.version.to_string(), + build_id: id.build_id, + profile: id.profile, + }) + .collect::>(); + mutation_execution_cache_key_from_parts(config, evm_opts, artifacts) +} + +fn mutation_execution_cache_key_from_parts( + config: &Config, + evm_opts: &EvmOpts, + mut artifacts: Vec, +) -> Result { + artifacts.sort(); + let fingerprint = ExecutionCacheFingerprint { + schema: "mutation-results-v1", + config, + evm_opts, + artifacts: &artifacts, + }; + let encoded = serde_json::to_vec(&fingerprint) + .wrap_err("failed to encode mutation execution cache key")?; + + Ok(keccak256(encoded).to_string()) +} + /// Resolve which paths to mutate based on configuration. fn resolve_mutate_paths( config: &Config, @@ -300,3 +366,82 @@ fn resolve_mutate_paths( Ok(paths) } + +#[cfg(test)] +mod tests { + use super::*; + + fn artifact(build_id: &str) -> ArtifactCacheFingerprint { + ArtifactCacheFingerprint { + source: "src/Counter.sol".to_string(), + name: "Counter".to_string(), + version: "0.8.30".to_string(), + build_id: build_id.to_string(), + profile: "default".to_string(), + } + } + + #[test] + fn execution_cache_key_changes_when_fuzz_config_changes() { + let first = Config::default(); + let mut second = first.clone(); + second.fuzz.runs += 1; + + let evm_opts = EvmOpts::default(); + let artifacts = vec![artifact("build-a")]; + + let first_key = + mutation_execution_cache_key_from_parts(&first, &evm_opts, artifacts.clone()).unwrap(); + let second_key = + mutation_execution_cache_key_from_parts(&second, &evm_opts, artifacts).unwrap(); + + assert_ne!(first_key, second_key); + } + + #[test] + fn execution_cache_key_changes_when_evm_options_change() { + let config = Config::default(); + let first = EvmOpts::default(); + let mut second = first.clone(); + second.memory_limit = first.memory_limit + 1; + + let artifacts = vec![artifact("build-a")]; + + let first_key = + mutation_execution_cache_key_from_parts(&config, &first, artifacts.clone()).unwrap(); + let second_key = + mutation_execution_cache_key_from_parts(&config, &second, artifacts).unwrap(); + + assert_ne!(first_key, second_key); + } + + #[test] + fn execution_cache_key_changes_when_compiled_artifacts_change() { + let config = Config::default(); + let evm_opts = EvmOpts::default(); + + let first_key = + mutation_execution_cache_key_from_parts(&config, &evm_opts, vec![artifact("build-a")]) + .unwrap(); + let second_key = + mutation_execution_cache_key_from_parts(&config, &evm_opts, vec![artifact("build-b")]) + .unwrap(); + + assert_ne!(first_key, second_key); + } + + #[test] + fn execution_cache_key_sorts_artifacts_before_hashing() { + let config = Config::default(); + let evm_opts = EvmOpts::default(); + + let first = vec![artifact("build-a"), artifact("build-b")]; + let second = vec![artifact("build-b"), artifact("build-a")]; + + let first_key = mutation_execution_cache_key_from_parts(&config, &evm_opts, first).unwrap(); + let second_key = + mutation_execution_cache_key_from_parts(&config, &evm_opts, second).unwrap(); + + assert_eq!(first_key, second_key); + } +} diff --git a/crates/forge/tests/cli/test_cmd/mutation.rs b/crates/forge/tests/cli/test_cmd/mutation.rs index bff1350a421a1..42e0c4d834045 100644 --- a/crates/forge/tests/cli/test_cmd/mutation.rs +++ b/crates/forge/tests/cli/test_cmd/mutation.rs @@ -1,6 +1,10 @@ // CLI integration tests for mutation testing -use foundry_test_utils::str; +use foundry_test_utils::{str, util::OutputExt}; + +fn mutation_summary(stdout: &str) -> serde_json::Value { + serde_json::from_str::(stdout.trim()).unwrap()["summary"].clone() +} forgetest_init!(can_run_mutation_testing, |prj, cmd| { prj.add_source( @@ -263,6 +267,88 @@ Survived mutants "#]]); }); +forgetest_init!(mutation_result_cache_invalidates_when_tests_change, |prj, _cmd| { + prj.add_source( + "Calculator.sol", + r#" +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.13; + +contract Calculator { + function add(uint256 a, uint256 b) public pure returns (uint256) { + return a + b; + } +} +"#, + ); + + prj.add_test( + "Calculator.t.sol", + r#" +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.13; + +import "../src/Calculator.sol"; + +contract CalculatorTest { + Calculator public calculator; + + function setUp() public { + calculator = new Calculator(); + } + + function test_Add() public view { + calculator.add(1, 2); + } +} +"#, + ); + + let mut weak_cmd = prj.forge_command(); + let weak_stdout = weak_cmd + .args(["test", "--mutate", "src/Calculator.sol", "--mutation-jobs", "1", "--json"]) + .assert_success() + .get_output() + .stdout_lossy(); + let weak_summary = mutation_summary(&weak_stdout); + + prj.add_test( + "Calculator.t.sol", + r#" +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.13; + +import "../src/Calculator.sol"; + +contract CalculatorTest { + Calculator public calculator; + + function setUp() public { + calculator = new Calculator(); + } + + function test_Add() public view { + assert(calculator.add(1, 2) == 3); + } +} +"#, + ); + + let mut strong_cmd = prj.forge_command(); + let strong_stdout = strong_cmd + .args(["test", "--mutate", "src/Calculator.sol", "--mutation-jobs", "1", "--json"]) + .assert_success() + .get_output() + .stdout_lossy(); + let strong_summary = mutation_summary(&strong_stdout); + + assert_eq!(weak_summary["total"], strong_summary["total"]); + assert!( + strong_summary["killed"].as_u64().unwrap() > weak_summary["killed"].as_u64().unwrap(), + "expected changed tests to invalidate cached mutation results: weak={weak_summary}, strong={strong_summary}", + ); +}); + // Test require/assert mutation for security-critical patterns forgetest_init!(mutation_testing_require_mutator, |prj, cmd| { // A contract with security-critical require checks (access control, input validation) From 95ed58b99f3e0b85a9d484a8e6b5f2bcf684b646 Mon Sep 17 00:00:00 2001 From: Mablr <59505383+mablr@users.noreply.github.com> Date: Fri, 29 May 2026 17:32:39 +0200 Subject: [PATCH 2/8] fix(mutation): include execution inputs in cache key --- crates/forge/src/mutation/orchestrator.rs | 70 +++++++++++++++++------ 1 file changed, 53 insertions(+), 17 deletions(-) diff --git a/crates/forge/src/mutation/orchestrator.rs b/crates/forge/src/mutation/orchestrator.rs index 91eb9081c485b..afd1fcaab3e6b 100644 --- a/crates/forge/src/mutation/orchestrator.rs +++ b/crates/forge/src/mutation/orchestrator.rs @@ -11,7 +11,7 @@ use std::{path::PathBuf, sync::Arc, time::Instant}; use alloy_primitives::keccak256; use eyre::{Result, WrapErr}; use foundry_cli::utils::FoundryPathExt; -use foundry_common::sh_println; +use foundry_common::{compile::ProjectCompiler, sh_println}; use foundry_compilers::{ Language, ProjectCompileOutput, compilers::multi::{MultiCompiler, MultiCompilerLanguage}, @@ -39,6 +39,7 @@ struct ExecutionCacheFingerprint<'a> { schema: &'static str, config: &'a Config, evm_opts: &'a EvmOpts, + num_workers: usize, artifacts: &'a [ArtifactCacheFingerprint], } @@ -98,7 +99,12 @@ pub async fn run_mutation_testing( // Determine which paths to mutate let mutate_paths = resolve_mutate_paths(&config, output, &mutation_config)?; - let execution_cache_key = mutation_execution_cache_key(&config, output, &evm_opts)?; + let execution_cache_output = ProjectCompiler::new() + .dynamic_test_linking(config.dynamic_test_linking) + .quiet(json_output) + .compile(&config.project()?)?; + let execution_cache_key = + mutation_execution_cache_key(&config, &execution_cache_output, &evm_opts, num_workers)?; if !mutation_config.show_progress && !json_output { sh_println!("Running mutation tests with {} parallel workers...", num_workers)?; @@ -265,12 +271,15 @@ pub async fn run_mutation_testing( /// settings. Hashing the full serialized config intentionally includes fuzz / /// invariant settings, test filters, fs permissions, sender/balance/env values, /// and future config fields unless explicitly skipped by `Config` itself. The -/// artifact fingerprint covers source and test build IDs, so changing a test -/// file invalidates result caches even when the mutated source did not change. +/// artifact fingerprint covers the unfiltered source and test build IDs, so +/// changing a test file invalidates result caches even when the outer test run +/// was filtered. Worker count is included because adaptive span skipping is +/// concurrency-sensitive. fn mutation_execution_cache_key( config: &Config, output: &ProjectCompileOutput, evm_opts: &EvmOpts, + num_workers: usize, ) -> Result { let artifacts = output .artifact_ids() @@ -282,12 +291,13 @@ fn mutation_execution_cache_key( profile: id.profile, }) .collect::>(); - mutation_execution_cache_key_from_parts(config, evm_opts, artifacts) + mutation_execution_cache_key_from_parts(config, evm_opts, num_workers, artifacts) } fn mutation_execution_cache_key_from_parts( config: &Config, evm_opts: &EvmOpts, + num_workers: usize, mut artifacts: Vec, ) -> Result { artifacts.sort(); @@ -295,6 +305,7 @@ fn mutation_execution_cache_key_from_parts( schema: "mutation-results-v1", config, evm_opts, + num_workers, artifacts: &artifacts, }; let encoded = serde_json::to_vec(&fingerprint) @@ -391,9 +402,10 @@ mod tests { let artifacts = vec![artifact("build-a")]; let first_key = - mutation_execution_cache_key_from_parts(&first, &evm_opts, artifacts.clone()).unwrap(); + mutation_execution_cache_key_from_parts(&first, &evm_opts, 1, artifacts.clone()) + .unwrap(); let second_key = - mutation_execution_cache_key_from_parts(&second, &evm_opts, artifacts).unwrap(); + mutation_execution_cache_key_from_parts(&second, &evm_opts, 1, artifacts).unwrap(); assert_ne!(first_key, second_key); } @@ -408,9 +420,9 @@ mod tests { let artifacts = vec![artifact("build-a")]; let first_key = - mutation_execution_cache_key_from_parts(&config, &first, artifacts.clone()).unwrap(); + mutation_execution_cache_key_from_parts(&config, &first, 1, artifacts.clone()).unwrap(); let second_key = - mutation_execution_cache_key_from_parts(&config, &second, artifacts).unwrap(); + mutation_execution_cache_key_from_parts(&config, &second, 1, artifacts).unwrap(); assert_ne!(first_key, second_key); } @@ -420,12 +432,20 @@ mod tests { let config = Config::default(); let evm_opts = EvmOpts::default(); - let first_key = - mutation_execution_cache_key_from_parts(&config, &evm_opts, vec![artifact("build-a")]) - .unwrap(); - let second_key = - mutation_execution_cache_key_from_parts(&config, &evm_opts, vec![artifact("build-b")]) - .unwrap(); + let first_key = mutation_execution_cache_key_from_parts( + &config, + &evm_opts, + 1, + vec![artifact("build-a")], + ) + .unwrap(); + let second_key = mutation_execution_cache_key_from_parts( + &config, + &evm_opts, + 1, + vec![artifact("build-b")], + ) + .unwrap(); assert_ne!(first_key, second_key); } @@ -438,10 +458,26 @@ mod tests { let first = vec![artifact("build-a"), artifact("build-b")]; let second = vec![artifact("build-b"), artifact("build-a")]; - let first_key = mutation_execution_cache_key_from_parts(&config, &evm_opts, first).unwrap(); + let first_key = + mutation_execution_cache_key_from_parts(&config, &evm_opts, 1, first).unwrap(); let second_key = - mutation_execution_cache_key_from_parts(&config, &evm_opts, second).unwrap(); + mutation_execution_cache_key_from_parts(&config, &evm_opts, 1, second).unwrap(); assert_eq!(first_key, second_key); } + + #[test] + fn execution_cache_key_changes_when_worker_count_changes() { + let config = Config::default(); + let evm_opts = EvmOpts::default(); + let artifacts = vec![artifact("build-a")]; + + let first_key = + mutation_execution_cache_key_from_parts(&config, &evm_opts, 1, artifacts.clone()) + .unwrap(); + let second_key = + mutation_execution_cache_key_from_parts(&config, &evm_opts, 4, artifacts).unwrap(); + + assert_ne!(first_key, second_key); + } } From e2e436e8a067d4ebcf7a966cdd82115fc333d0e0 Mon Sep 17 00:00:00 2001 From: zerosnacks <95942363+zerosnacks@users.noreply.github.com> Date: Wed, 20 May 2026 16:45:27 +0200 Subject: [PATCH 3/8] fix(mutation): honor test filters/isolation, tighten cache key, drop compound assignment mutation, reject incompatible flags - runner: thread FilterArgs and --isolate through MutationRunConfig into compile_and_test_inner; rebuild ProjectPathsAwareFilter against the per-mutant temp config so --match-test/--match-contract/--match-path work and mutants exercise the same test set/execution model as baseline - mod: add runtime_context_digest folded into the per-file cache key so cached results aren't reused when filters, isolation, fork URL/block, sender, or initial_balance change - orchestrator: compute runtime_context_digest once per run and bind it to every MutationHandler - binary_op_mutator: stop mutating compound assignments (a += b would silently be rewritten to a - b dropping the assignment); add regression tests - cmd/test: bail when --mutate is combined with --list/--debug/ --flamegraph/--flamechart/--junit instead of silently mixing modes - fs_permissions guard now narrowed to permissions whose path can reach symlinked dependency trees (lib/node_modules/dependencies) Amp-Thread-ID: https://ampcode.com/threads/T-019e4567-e7ca-717e-bcc0-bb67a3667c4d Co-authored-by: Amp --- crates/forge/src/cmd/test/mod.rs | 104 ++++++++++++++++-- .../mutation/mutators/binary_op_mutator.rs | 19 +++- .../mutators/tests/binary_op_mutator_test.rs | 6 + crates/forge/src/mutation/orchestrator.rs | 18 ++- crates/forge/src/mutation/runner.rs | 65 ++++++++--- 5 files changed, 179 insertions(+), 33 deletions(-) diff --git a/crates/forge/src/cmd/test/mod.rs b/crates/forge/src/cmd/test/mod.rs index ea21dc0610be0..e7b96482c67e3 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, @@ -64,7 +64,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}; @@ -353,6 +353,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; @@ -582,9 +615,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 \ @@ -593,13 +623,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}" ); } @@ -612,6 +687,13 @@ 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, ...) and isolation flag the baseline run used, + // so every mutant exercises the exact same test set under the + // same execution model. Without this, mutant runs silently + // diverge from baseline and produce false kills/survivors. + filter_args: self.filter.clone(), + isolate: evm_opts_for_mutation.isolate, }; let result = run_mutation_testing( diff --git a/crates/forge/src/mutation/mutators/binary_op_mutator.rs b/crates/forge/src/mutation/mutators/binary_op_mutator.rs index c62ea6c2cdaf5..cc0e7894bedf2 100644 --- a/crates/forge/src/mutation/mutators/binary_op_mutator.rs +++ b/crates/forge/src/mutation/mutators/binary_op_mutator.rs @@ -9,6 +9,12 @@ pub struct BinaryOpMutator; impl Mutator for BinaryOpMutator { fn generate_mutants(&self, context: &MutationContext<'_>) -> Result> { let expr = context.expr.ok_or_eyre("BinaryOpMutator: no expression")?; + // Compound assignments (`a += b`, etc.) are intentionally not mutated + // here: the mutation text we build is `"lhs new_op rhs"` and the + // replacement span covers the whole assignment, which would silently + // rewrite `a += b` to `a - b` and corrupt the test. See + // `is_applicable` — those expressions are filtered out up-front so + // this case is unreachable, but guard defensively. let (bin_op, _op_span, lhs, rhs) = get_bin_op_parts(expr)?; let op = bin_op.kind; @@ -82,18 +88,19 @@ impl Mutator for BinaryOpMutator { return false; } - match ctxt.expr.unwrap().kind { - ExprKind::Binary(_, _, _) => true, - ExprKind::Assign(_, bin_op, _) => bin_op.is_some(), - _ => false, - } + // We only mutate plain binary expressions. Compound assignments + // (`a += b`, `a *= b`, ...) are deliberately excluded: the textual + // replacement we build is `"lhs new_op rhs"` over the whole assignment + // span, which would rewrite `a += b` into `a - b` (dropping the + // assignment) rather than `a -= b`. Until we emit `lhs new_op= rhs` + // for the compound case, leave it alone. + matches!(ctxt.expr.unwrap().kind, ExprKind::Binary(_, _, _)) } } /// 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>)> { 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)), _ => eyre::bail!("BinaryOpMutator: unexpected expression kind"), } 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..3088767f3e160 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,10 @@ 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 assignments are intentionally skipped: the current textual + // replacement would rewrite `a += b` to `a - b` (dropping the assignment) + // instead of `a -= b`, so the mutator must report them as inapplicable. + compound_assign_add: "a += b" => None; + compound_assign_sub: "a -= b" => None; + compound_assign_mul: "a *= b" => None; ); diff --git a/crates/forge/src/mutation/orchestrator.rs b/crates/forge/src/mutation/orchestrator.rs index afd1fcaab3e6b..cdf3cc5babc97 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 { @@ -203,6 +213,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 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) From 2b72826e2e2fc0da58cd8a8d3e45919000cc8faa Mon Sep 17 00:00:00 2001 From: zerosnacks <95942363+zerosnacks@users.noreply.github.com> Date: Wed, 20 May 2026 17:13:19 +0200 Subject: [PATCH 4/8] fix(mutation): address fresh oracle review blockers and mediums Blockers - filter parity: pass materialized baseline filter (filter.args().clone()) into MutationRunConfig so positional 'forge test ' and --rerun reach mutant runs too; raw self.filter dropped them silently - bail when baseline matched zero tests; without it every compileable mutant was reported as 'Alive' - negative literal mutation: add OwnedLiteral::NegatedNumber(U256) formatted as '-{val}'; the old Number(-*val) wrapped via two's complement and produced wrong-source mutants - mutator test harness: wrap snippets in valid Solidity, panic on parse failure, and check emitted replacement text. Fixed 6 stale test expectations and 1 unmatched delegate-pattern case the old vacuous harness was hiding Mediums - survived-span resume: load retrieve_survived_spans BEFORE mutant generation/cached load and filter cached mutants through should_skip_span so cross-run adaptive skipping actually works - runtime context digest: hash the full serialized EvmOpts (networks, env, gas-limit toggles, fork, sender, balance, ...) instead of a hand-picked 4-field subset that missed several mutation-affecting knobs - deterministic output: BTreeMap for JSON survived_mutants with sorted entries; reporter sorts survived list by (path, line, col, span, mutation); persisted results_vec sorted by span - --mutate-contract: intersect with explicit --mutate instead of silently overriding them; explicit paths are now respected - workspace: copy 'script/' when present and distinct from src/test so projects that import script-side helpers don't see false Invalid mutants Amp-Thread-ID: https://ampcode.com/threads/T-019e4567-e7ca-717e-bcc0-bb67a3667c4d Co-authored-by: Amp --- crates/forge/src/cmd/test/mod.rs | 28 ++++-- crates/forge/src/mutation/mod.rs | 36 ++++++-- crates/forge/src/mutation/mutant.rs | 11 ++- .../mutation/mutators/assignment_mutator.rs | 10 ++- .../mutators/tests/assignment_mutator_test.rs | 13 +-- .../tests/delete_expression_mutator_test.rs | 5 +- .../tests/elim_delegate_mutator_test.rs | 9 +- .../src/mutation/mutators/tests/helper.rs | 85 ++++++++++++------- crates/forge/src/mutation/orchestrator.rs | 78 +++++++++++------ crates/forge/src/mutation/reporter.rs | 25 +++++- crates/forge/src/workspace.rs | 12 +++ 11 files changed, 230 insertions(+), 82 deletions(-) diff --git a/crates/forge/src/cmd/test/mod.rs b/crates/forge/src/cmd/test/mod.rs index e7b96482c67e3..842958c4483c8 100644 --- a/crates/forge/src/cmd/test/mod.rs +++ b/crates/forge/src/cmd/test/mod.rs @@ -583,6 +583,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. @@ -688,11 +701,16 @@ impl TestArgs { show_progress: self.show_progress, json_output, // Carry the same filter args (--match-test, --match-contract, - // --match-path, ...) and isolation flag the baseline run used, - // so every mutant exercises the exact same test set under the - // same execution model. Without this, mutant runs silently - // diverge from baseline and produce false kills/survivors. - filter_args: self.filter.clone(), + // --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, }; diff --git a/crates/forge/src/mutation/mod.rs b/crates/forge/src/mutation/mod.rs index 72c7d3a47e01d..a2f642dfa65c2 100644 --- a/crates/forge/src/mutation/mod.rs +++ b/crates/forge/src/mutation/mod.rs @@ -1,8 +1,4 @@ -use std::{ - collections::{HashMap, HashSet}, - path::PathBuf, - sync::Arc, -}; +use std::{collections::HashSet, path::PathBuf, sync::Arc}; use crate::mutation::{ mutant::{Mutant, MutationResult}, @@ -146,9 +142,17 @@ 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(); + use std::collections::BTreeMap; + + let mut survived_mutants: BTreeMap> = BTreeMap::new(); for mutant in &self.survived { let file_path = mutant.relative_path(); @@ -156,6 +160,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 +187,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: std::collections::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/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/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..7d604e891c97f 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,50 @@ 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; + Ok(mutant_visitor + .mutation_to_conduct + .into_iter() + .map(|m| m.mutation.to_string()) + .collect()) + }); - 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::>() - ); + // 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, + ) + }); - 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::>() + if let Some(expected) = test_case.expected_mutations { + assert_eq!( + mutations.len(), + expected.len(), + "Expected {} mutations, got {}: {:?}", + expected.len(), + mutations.len(), + mutations, + ); + + for mutation_str in &mutations { + assert!( + expected.contains(&mutation_str.as_str()), + "Unexpected mutation: {mutation_str}. Expected one of: {expected:?}", ); } - - Ok(()) - }); + } 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 cdf3cc5babc97..2c6a84c55ad69 100644 --- a/crates/forge/src/mutation/orchestrator.rs +++ b/crates/forge/src/mutation/orchestrator.rs @@ -162,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() @@ -177,11 +187,6 @@ pub async fn run_mutation_testing( continue; } - // Load survived spans for adaptive mutation testing. Only loaded after - // we successfully obtained mutants for this build, so a stale survived - // cache from a different mutant set is not applied. - handler.retrieve_survived_spans(&build_id, &execution_cache_key); - // Sort mutations by span for optimal adaptive testing mutants.sort_by(|a, b| { a.span.lo().0.cmp(&b.span.lo().0).then_with(|| b.span.hi().0.cmp(&a.span.hi().0)) @@ -246,6 +251,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 { @@ -327,35 +340,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()); @@ -385,6 +389,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/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); From 30ef5603b5ccbcf782e0bf51018611034fcd14b3 Mon Sep 17 00:00:00 2001 From: Mablr <59505383+mablr@users.noreply.github.com> Date: Fri, 29 May 2026 19:09:49 +0200 Subject: [PATCH 5/8] fix: `BTreeMap` import --- crates/forge/src/mutation/mod.rs | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/crates/forge/src/mutation/mod.rs b/crates/forge/src/mutation/mod.rs index a2f642dfa65c2..b9283d8ac8de4 100644 --- a/crates/forge/src/mutation/mod.rs +++ b/crates/forge/src/mutation/mod.rs @@ -1,4 +1,8 @@ -use std::{collections::HashSet, path::PathBuf, sync::Arc}; +use std::{ + collections::{BTreeMap, HashSet}, + path::PathBuf, + sync::Arc, +}; use crate::mutation::{ mutant::{Mutant, MutationResult}, @@ -150,8 +154,6 @@ impl MutationsSummary { /// completion order leaks into the JSON and breaks downstream diffing, /// snapshot tests, and reproducibility. pub fn to_json_output(&self, duration_secs: f64) -> MutationJsonOutput { - use std::collections::BTreeMap; - let mut survived_mutants: BTreeMap> = BTreeMap::new(); for mutant in &self.survived { @@ -194,7 +196,7 @@ impl MutationsSummary { #[derive(Debug, Clone, Serialize)] pub struct MutationJsonOutput { pub summary: MutationSummaryJson, - pub survived_mutants: std::collections::BTreeMap>, + pub survived_mutants: BTreeMap>, } /// Summary section of JSON output From 3bf1c7bf365b807d99a027c3d032c870094a2bbd Mon Sep 17 00:00:00 2001 From: Mablr <59505383+mablr@users.noreply.github.com> Date: Fri, 29 May 2026 19:11:39 +0200 Subject: [PATCH 6/8] fix mutation compound assignment handling --- .../mutation/mutators/binary_op_mutator.rs | 40 +++++++++---------- .../mutators/tests/binary_op_mutator_test.rs | 9 ++--- crates/forge/tests/cli/test_cmd/mutation.rs | 10 ++--- 3 files changed, 28 insertions(+), 31 deletions(-) diff --git a/crates/forge/src/mutation/mutators/binary_op_mutator.rs b/crates/forge/src/mutation/mutators/binary_op_mutator.rs index cc0e7894bedf2..1168953dbc085 100644 --- a/crates/forge/src/mutation/mutators/binary_op_mutator.rs +++ b/crates/forge/src/mutation/mutators/binary_op_mutator.rs @@ -9,13 +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")?; - // Compound assignments (`a += b`, etc.) are intentionally not mutated - // here: the mutation text we build is `"lhs new_op rhs"` and the - // replacement span covers the whole assignment, which would silently - // rewrite `a += b` to `a - b` and corrupt the test. See - // `is_applicable` — those expressions are filtered out up-front so - // this case is unreachable, but guard defensively. - 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![ @@ -53,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; @@ -68,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 }, @@ -88,20 +88,20 @@ impl Mutator for BinaryOpMutator { return false; } - // We only mutate plain binary expressions. Compound assignments - // (`a += b`, `a *= b`, ...) are deliberately excluded: the textual - // replacement we build is `"lhs new_op rhs"` over the whole assignment - // span, which would rewrite `a += b` into `a - b` (dropping the - // assignment) rather than `a -= b`. Until we emit `lhs new_op= rhs` - // for the compound case, leave it alone. - matches!(ctxt.expr.unwrap().kind, ExprKind::Binary(_, _, _)) + 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::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/binary_op_mutator_test.rs b/crates/forge/src/mutation/mutators/tests/binary_op_mutator_test.rs index 3088767f3e160..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,10 +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 assignments are intentionally skipped: the current textual - // replacement would rewrite `a += b` to `a - b` (dropping the assignment) - // instead of `a -= b`, so the mutator must report them as inapplicable. - compound_assign_add: "a += b" => None; - compound_assign_sub: "a -= b" => None; - compound_assign_mul: "a *= b" => 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/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] ... "#]]); }); From eb5aa0d54110cb07abad2746e9768bc63ada3315 Mon Sep 17 00:00:00 2001 From: Mablr <59505383+mablr@users.noreply.github.com> Date: Fri, 29 May 2026 19:34:02 +0200 Subject: [PATCH 7/8] test(mutation): compare full mutator output sets --- .../src/mutation/mutators/tests/helper.rs | 20 ++++++------------- 1 file changed, 6 insertions(+), 14 deletions(-) diff --git a/crates/forge/src/mutation/mutators/tests/helper.rs b/crates/forge/src/mutation/mutators/tests/helper.rs index 7d604e891c97f..88d0ca477192d 100644 --- a/crates/forge/src/mutation/mutators/tests/helper.rs +++ b/crates/forge/src/mutation/mutators/tests/helper.rs @@ -73,21 +73,13 @@ pub trait MutatorTester { }); if let Some(expected) = test_case.expected_mutations { - assert_eq!( - mutations.len(), - expected.len(), - "Expected {} mutations, got {}: {:?}", - expected.len(), - mutations.len(), - mutations, - ); + let mut actual = mutations; + actual.sort(); - for mutation_str in &mutations { - assert!( - expected.contains(&mutation_str.as_str()), - "Unexpected mutation: {mutation_str}. Expected one of: {expected:?}", - ); - } + 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(), From 5e8e7cb2ddfd8cc6c2e0ab86a0d2107be5619141 Mon Sep 17 00:00:00 2001 From: Mablr <59505383+mablr@users.noreply.github.com> Date: Fri, 29 May 2026 19:51:04 +0200 Subject: [PATCH 8/8] fix: clean-up --- crates/forge/src/mutation/mod.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/crates/forge/src/mutation/mod.rs b/crates/forge/src/mutation/mod.rs index 72c7d3a47e01d..b54c27f32f0fc 100644 --- a/crates/forge/src/mutation/mod.rs +++ b/crates/forge/src/mutation/mod.rs @@ -563,11 +563,9 @@ mod tests { #[test] fn mutant_cache_path_ignores_execution_only_timeout() { let (_temp, mut first_config) = test_config(); - let root = first_config.root.clone(); let mut second_config = first_config.clone(); first_config.mutation.timeout = Some(1); - second_config.root = root; second_config.mutation.timeout = Some(99); let first = test_handler(first_config).cache_file_path("build", CacheKind::Mutants);