fix: parallel mutant generation

There are two fixes here:
- fix for parallel mutant generation (using indexed file can cause file
  name collisions, so we are using hash of the mutant in the mutant file
  name instead)
- disabling process's ability to generate coverage report as by doing
  that we configure TRACING_ENABLED flag deep within MoveVM which is a
  statically configured flag that cannot be reset anymore. From now on,
  we will ask users to generate `aptos move test --coverage`

Cargo.toml

@@ -20,6 +20,7 @@ repository = "https://github.com/eigerco/move-spec-testing"
 rust-version = "1.78.0"
 
 [workspace.dependencies]
+ahash = "0.8"
 anyhow = "1.0"
 aptos = { git = "https://github.com/aptos-labs/aptos-core.git", branch = "main" }
 aptos-framework = { git = "https://github.com/aptos-labs/aptos-core.git", branch = "main" }

move-mutation-test/src/mutation_test.rs

@@ -160,19 +160,28 @@ fn run_tests<W: WriteColor + Send>(
         aptos_test_feature_flags_genesis(),
         gas_limit,
         cost_table,
-        cfg.apply_coverage,
+        // We cannot use `true` here since that would set a static variable TRACING_ENABLED deep
+        // within MoveVM to true, and that could cause a huge slowdown, even test failures in the
+        // later "mutation-test" phase.
+        // Until we can somehow reconfigure:
+        // https://github.com/aptos-labs/aptos-core/blob/2bb2d43037a93d883729869d65c7c6c75b028fa1/third_party/move/move-vm/runtime/src/tracing.rs#L40
+        // we are forced to avoid computing coverage before running the tests.
+        // How it works: compute_coverage sets `MOVE_VM_TRACE` env variable that configures this
+        // once_cell value above and then we can't change it back anymore.
+        false,
         &mut error_writer,
     )
     .map_err(|err| Error::msg(format!("failed to run unit tests: {err:#}")))?;
 
-    // Disk space optimization:
     if cfg.apply_coverage {
+        // Disk space optimization:
         let trace_path = package_path.join(".trace");
-        info!("removing {}", trace_path.display());
         // Our tool doesn't use the .trace file at all, only the .coverage_map.mvcov file, and
         // since the tool copy package directory to temp directories for when running tests,
         // so let's keep copied directory as small as possible.
-        let _ = fs::remove_file(trace_path);
+        if fs::remove_file(&trace_path).is_ok() {
+            info!("removing {}", trace_path.display());
+        }
     }
 
     match result {

move-mutation-test/tests/integration_tests.rs

@@ -33,8 +33,10 @@ fn test_run_mutation_test(path: &Path, expected_report: String) -> datatest_stab
         dump_state: false,
         filter: None,
         ignore_compile_warnings: false,
-        apply_coverage: true,
-        gas_limit: 1_000,
+        // TODO(rqnsom): maybe we could do set it to true, but it would require `aptos` command in
+        // the `build.rs` - using `process::Command` slowed down the execution a lot
+        apply_coverage: false,
+        gas_limit: 2000,
     };
 
     let report_file = PathBuf::from("report.txt");

move-mutator/Cargo.toml

@@ -15,6 +15,7 @@ name = "move-mutator"
 path = "src/main.rs"
 
 [dependencies]
+ahash = { workspace = true }
 anyhow = { workspace = true }
 clap = { workspace = true }
 codespan = { workspace = true }

move-mutator/src/coverage.rs

@@ -1,5 +1,5 @@
 use crate::compiler::compile_package;
-use anyhow::Error;
+use anyhow::{bail, Error};
 use codespan::Span;
 use move_command_line_common::files::FileHash;
 use move_compiler::compiled_unit::{CompiledUnit, NamedCompiledModule};
@@ -15,6 +15,8 @@ use std::{
     path::{Path, PathBuf},
 };
 
+const COVERAGE_MAP_NAME: &str = ".coverage_map.mvcov";
+
 /// Contains all uncovered spans in the project.
 #[derive(Debug, Default)]
 pub(crate) struct Coverage {
@@ -32,7 +34,14 @@ impl Coverage {
     ) -> anyhow::Result<()> {
         info!("computing coverage");
 
-        let coverage_map = CoverageMap::from_binary_file(package_path.join(".coverage_map.mvcov"))
+        let coverage_file = package_path.join(COVERAGE_MAP_NAME);
+        if !coverage_file.exists() {
+            bail!(
+                "Coverage map not found, please run `aptos move test --coverage` for the package"
+            );
+        }
+
+        let coverage_map = CoverageMap::from_binary_file(coverage_file)
             .map_err(|e| Error::msg(format!("failed to retrieve the coverage map: {e}")))?;
 
         let mut coverage_config = build_config.clone();

move-mutator/src/lib.rs

@@ -185,7 +185,8 @@ pub fn run_move_mutator(
                 }
             }
 
-            let Ok(mutant_path) = output::setup_mutant_path(&output_dir, &path) else {
+            let mutant_id = mutated_info.unique_id();
+            let Ok(mutant_path) = output::setup_mutant_path(&output_dir, &path, mutant_id) else {
                 // If we cannot set up the mutant path, we skip the mutant.
                 trace!("Cannot set up mutant path for {path:?}");
                 return None;

move-mutator/src/operator.rs

@@ -3,14 +3,16 @@
 // SPDX-License-Identifier: Apache-2.0
 
 use crate::report::Mutation;
+use ahash::RandomState;
 use codespan::FileId;
 use std::{
     fmt,
     fmt::{Debug, Display},
+    hash::Hash,
 };
 
 /// Mutation result that contains the mutated source code and the modification that was applied.
-#[derive(Debug, Clone, PartialEq)]
+#[derive(Debug, Clone, PartialEq, Hash)]
 pub struct MutantInfo {
     /// The mutated source code.
     pub mutated_source: String,
@@ -26,6 +28,12 @@ impl MutantInfo {
             mutation,
         }
     }
+
+    /// Calculates the unique hash of the mutant.
+    pub fn unique_id(&self) -> u64 {
+        let fixed_randomness = RandomState::with_seeds(1, 2, 3, 4);
+        fixed_randomness.hash_one(self)
+    }
 }
 
 /// Trait for mutation operators.

move-mutator/src/output.rs

@@ -17,13 +17,13 @@ use std::{
 /// according to its relative path inside the package. If the file is not inside any package,
 /// it creates the directory structure in the output directory.
 /// Example:
-/// The file to be mutated is located in "/a/b/c/sources/X/Y/file.move" (`file_path`).
+/// The file to be mutated is located in "/a/b/c/sources/X/Y/file.move" (`original_file`).
 /// This function constructs the following output path for file.move:
 /// "`output_dir/X/Y/file_index.move`"
 /// It finds the package root for the file, which is "/a/b/c", then it append the relative path to the output directory.
 ///
 /// If the file is not inside any package, it creates the directory structure in the output directory like:
-/// The file to be mutated is located in "/a/b/c/file.move" (`file_path`).
+/// The file to be mutated is located in "/a/b/c/file.move" (`original_file`).
 /// This function constructs the following output path for file.move:
 /// "`output_dir/file_index.move`"
 ///
@@ -39,10 +39,14 @@ use std::{
 /// # Returns
 ///
 /// * `PathBuf` - The path to the mutant.
-pub(crate) fn setup_mutant_path(output_dir: &Path, file_path: &Path) -> anyhow::Result<PathBuf> {
-    trace!("Trying to set up mutant path for {file_path:?}");
+pub(crate) fn setup_mutant_path(
+    output_dir: &Path,
+    original_file: &Path,
+    unique_mutant_id: u64,
+) -> anyhow::Result<PathBuf> {
+    trace!("Trying to set up mutant path for {original_file:?}");
 
-    let file_path_canonicalized = file_path.canonicalize()?;
+    let file_path_canonicalized = original_file.canonicalize()?;
 
     // Try to find package root for the file. If the file is not inside any package, assume that it is a single file.
     let root = SourcePackageLayout::try_find_root(&file_path_canonicalized);
@@ -66,31 +70,21 @@ pub(crate) fn setup_mutant_path(output_dir: &Path, file_path: &Path) -> anyhow::
     if let Err(e) = fs::create_dir_all(&output_struct) {
         if e.kind() != std::io::ErrorKind::AlreadyExists {
             return Err(anyhow::anyhow!(
-                "Cannot create directory structure for {file_path:?} in {output_dir:?}"
+                "Cannot create directory structure for {original_file:?} in {output_dir:?}"
             ));
         }
     }
 
-    let filename = file_path
+    let filename = original_file
         .file_stem()
-        .ok_or(anyhow::anyhow!("Cannot get file stem of {file_path:?}"))?;
+        .ok_or(anyhow::anyhow!("Cannot get file stem of {original_file:?}"))?;
 
     // Deal with the file as OsString to avoid problems with non-UTF8 characters.
-    let filename = filename.to_os_string();
-    for i in 0u32..u32::MAX {
-        let mut mutant_path = filename.clone();
-        mutant_path.push(OsString::from(format!("_mut{i}.move")));
-
-        let mutant_path = output_struct.join(mutant_path);
-        if !mutant_path.exists() {
-            return Ok(mutant_path);
-        }
-    }
+    let mut mutant_path = filename.to_os_string();
+    mutant_path.push(OsString::from(format!("_mutant_{unique_mutant_id:x}.move")));
 
-    Err(anyhow::anyhow!(
-        "There is more than {} mutants in {output_dir:?}",
-        u32::MAX
-    ))
+    let mutant_path = output_struct.join(mutant_path);
+    Ok(mutant_path)
 }
 
 /// Sets up the output directory for the mutants.
@@ -140,63 +134,80 @@ mod tests {
     fn setup_mutant_path_handles_non_utf8_characters() {
         let output_dir = Path::new("mutants_output");
         let filename = Path::new("💖");
+        let mutant_id = 0;
+
         fs::File::create(filename).unwrap();
-        let result = setup_mutant_path(output_dir, filename);
+        let result = setup_mutant_path(output_dir, filename, mutant_id);
         fs::remove_file(filename).unwrap();
         fs::remove_dir_all(output_dir).unwrap();
         assert!(result.is_ok());
         assert_eq!(
             result.unwrap(),
-            PathBuf::from("mutants_output/💖_mut0.move")
+            PathBuf::from(format!("mutants_output/💖_mutant_{mutant_id}.move"))
         );
     }
 
     #[test]
     fn setup_mutant_path_handles_file_without_extension() {
         let output_dir = Path::new("mutants_output_no_extension");
         let filename = Path::new("file1");
+        let mutant_id = 0;
+
         fs::File::create(filename).unwrap();
-        let result = setup_mutant_path(output_dir, filename);
+        let result = setup_mutant_path(output_dir, filename, mutant_id);
         fs::remove_file(filename).unwrap();
         fs::remove_dir_all(output_dir).unwrap();
         assert!(result.is_ok());
         assert_eq!(
             result.unwrap(),
-            PathBuf::from("mutants_output_no_extension/file1_mut0.move")
+            PathBuf::from(format!(
+                "mutants_output_no_extension/file1_mutant_{mutant_id}.move"
+            ))
         );
     }
 
     #[test]
     fn setup_mutant_path_creates_correct_path() {
         let output_dir = Path::new("mutants_output_correct");
         let filename = Path::new("test_correct");
+        let mutant_id = 0;
+
         fs::File::create(filename).unwrap();
-        let result = setup_mutant_path(output_dir, filename);
+        let result = setup_mutant_path(output_dir, filename, mutant_id);
         fs::remove_file(filename).unwrap();
         fs::remove_dir_all(output_dir).unwrap();
         assert!(result.is_ok());
         assert_eq!(
             result.unwrap(),
-            PathBuf::from("mutants_output_correct/test_correct_mut0.move")
+            PathBuf::from(format!(
+                "mutants_output_correct/test_correct_mutant_{mutant_id}.move"
+            ))
         );
     }
 
     #[test]
     fn setup_mutant_path_handles_empty_output_dir() {
         let output_dir = Path::new("");
         let filename = Path::new("test_empty");
+        let mutant_id = 0;
+
         fs::File::create(filename).unwrap();
-        let result = setup_mutant_path(output_dir, filename);
+        let result = setup_mutant_path(output_dir, filename, mutant_id);
         fs::remove_file(filename).unwrap();
         assert!(result.is_ok());
-        assert_eq!(result.unwrap(), PathBuf::from("test_empty_mut0.move"));
+        assert_eq!(
+            result.unwrap(),
+            PathBuf::from(format!("test_empty_mutant_{mutant_id}.move"))
+        );
     }
 
     #[test]
     fn setup_mutant_path_handles_empty_filename() {
         let output_dir = Path::new("mutants_output_empty_filename");
         let filename = Path::new("");
-        let result = setup_mutant_path(output_dir, filename);
+        let mutant_id = 0;
+
+        let result = setup_mutant_path(output_dir, filename, mutant_id);
         assert!(result.is_err());
     }
 

move-mutator/src/report.rs

@@ -115,7 +115,7 @@ impl Default for Report {
 
 /// The `Range` struct represents a range with a start and end.
 /// It is used to represent the location of a mutation inside the source file.
-#[derive(Debug, Clone, Copy, Serialize, Deserialize, PartialEq)]
+#[derive(Debug, Clone, Copy, Serialize, Deserialize, PartialEq, Hash)]
 pub struct Range {
     /// The start of the range.
     start: usize,
@@ -139,7 +139,7 @@ impl Range {
 ///
 /// It contains the location of the modification, the name of the mutation operator, the old value and the new value.
 /// It is used to represent a single modification inside a `ReportEntry`.
-#[derive(Debug, Clone, Serialize, Deserialize, PartialEq)]
+#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Hash)]
 pub struct Mutation {
     /// The location of the modification.
     changed_place: Range,