diff --git a/NEWS.md b/NEWS.md index e4122ef2..3871b2e5 100644 --- a/NEWS.md +++ b/NEWS.md @@ -4,6 +4,8 @@ - New: Mutate `proc_macro` targets and functions. +- New: Write diffs to dedicated files under `mutants.out/diff/`. The filename is included in the mutant json output. + ## 24.9.0 - Fixed: Avoid generating empty string elements in `ENCODED_RUSTFLAGS` when `--cap-lints` is set. In some situations these could cause a compiler error complaining about the empty argument. diff --git a/book/src/mutants-out.md b/book/src/mutants-out.md index 32450ffe..20182c09 100644 --- a/book/src/mutants-out.md +++ b/book/src/mutants-out.md @@ -19,6 +19,9 @@ The output directory contains: * An `outcomes.json` file describing the results of all tests, and summary counts of each outcome. +* A `diff/` directory, containing a diff file for each mutation, relative to the unmutated baseline. + `mutants.json` includes for each mutant the name of the diff file. + * A `logs/` directory, with one log file for each mutation plus the baseline unmutated case. The log contains the diff of the mutation plus the output from cargo. `outcomes.json` includes for each mutant the name of the log file. @@ -31,4 +34,4 @@ The contents of the directory and the format of these files is subject to change These files are incrementally updated while cargo-mutants runs, so other programs can read them to follow progress. -There is generally no reason to include this directory in version control, so it is recommended that you add `/mutants.out*` to your `.gitignore` file. This will exclude both `mutants.out` and `mutants.out.old`. +There is generally no reason to include this directory in version control, so it is recommended that you add `/mutants.out*` to your `.gitignore` file or equivalent. This will exclude both `mutants.out` and `mutants.out.old`. diff --git a/src/build_dir.rs b/src/build_dir.rs index d2132bf5..8be3e757 100644 --- a/src/build_dir.rs +++ b/src/build_dir.rs @@ -3,6 +3,7 @@ //! A directory containing mutated source to run cargo builds and tests. use std::fmt::{self, Debug}; +use std::fs::write; use tempfile::TempDir; use tracing::info; @@ -77,6 +78,14 @@ impl BuildDir { pub fn path(&self) -> &Utf8Path { self.path.as_path() } + + pub fn overwrite_file(&self, relative_path: &Utf8Path, code: &str) -> Result<()> { + let full_path = self.path.join(relative_path); + // for safety, don't follow symlinks + ensure!(full_path.is_file(), "{full_path:?} is not a file"); + write(&full_path, code.as_bytes()) + .with_context(|| format!("failed to write code to {full_path:?}")) + } } #[cfg(test)] diff --git a/src/cargo.rs b/src/cargo.rs index b86ec01f..3ea434ab 100644 --- a/src/cargo.rs +++ b/src/cargo.rs @@ -9,6 +9,7 @@ use itertools::Itertools; use tracing::{debug, debug_span, warn}; use crate::outcome::PhaseResult; +use crate::output::ScenarioOutput; use crate::package::Package; use crate::process::{Process, ProcessStatus}; use crate::*; @@ -21,7 +22,7 @@ pub fn run_cargo( packages: Option<&[&Package]>, phase: Phase, timeout: Option, - log_file: &mut LogFile, + scenario_output: &mut ScenarioOutput, options: &Options, console: &Console, ) -> Result { @@ -45,7 +46,7 @@ pub fn run_cargo( build_dir.path(), timeout, jobserver, - log_file, + scenario_output, console, )?; check_interrupted()?; @@ -161,6 +162,9 @@ fn cargo_argv( /// Return adjusted CARGO_ENCODED_RUSTFLAGS, including any changes to cap-lints. /// +/// It seems we have to set this in the environment because Cargo doesn't expose +/// a way to pass it in as an option from all commands? +/// /// This does not currently read config files; it's too complicated. /// /// See diff --git a/src/console.rs b/src/console.rs index 0226f892..10fa10c7 100644 --- a/src/console.rs +++ b/src/console.rs @@ -6,12 +6,11 @@ use std::borrow::Cow; use std::fmt::Write; use std::fs::File; use std::io; -use std::path::{Path, PathBuf}; use std::sync::{Arc, Mutex}; use std::time::{Duration, Instant}; use anyhow::Context; -use camino::Utf8Path; +use camino::{Utf8Path, Utf8PathBuf}; use console::{style, StyledObject}; use humantime::format_duration; use nutmeg::Destination; @@ -73,9 +72,9 @@ impl Console { /// Update that a cargo task is starting. pub fn scenario_started( &self, - dir: &Path, + dir: &Utf8Path, scenario: &Scenario, - log_file: &Utf8Path, + log_file: File, ) -> Result<()> { let start = Instant::now(); let scenario_model = ScenarioModel::new(dir, scenario, start, log_file)?; @@ -88,7 +87,7 @@ impl Console { /// Update that cargo finished. pub fn scenario_finished( &self, - dir: &Path, + dir: &Utf8Path, scenario: &Scenario, outcome: &ScenarioOutcome, options: &Options, @@ -149,13 +148,13 @@ impl Console { self.message(&s); } - pub fn start_copy(&self, dir: &Path) { + pub fn start_copy(&self, dir: &Utf8Path) { self.view.update(|model| { model.copy_models.push(CopyModel::new(dir.to_owned())); }); } - pub fn finish_copy(&self, dir: &Path) { + pub fn finish_copy(&self, dir: &Utf8Path) { self.view.update(|model| { let idx = model .copy_models @@ -166,7 +165,7 @@ impl Console { }); } - pub fn copy_progress(&self, dest: &Path, total_bytes: u64) { + pub fn copy_progress(&self, dest: &Utf8Path, total_bytes: u64) { self.view.update(|model| { model .copy_models @@ -197,13 +196,13 @@ impl Console { } /// A new phase of this scenario started. - pub fn scenario_phase_started(&self, dir: &Path, phase: Phase) { + pub fn scenario_phase_started(&self, dir: &Utf8Path, phase: Phase) { self.view.update(|model| { model.find_scenario_mut(dir).phase_started(phase); }) } - pub fn scenario_phase_finished(&self, dir: &Path, phase: Phase) { + pub fn scenario_phase_finished(&self, dir: &Utf8Path, phase: Phase) { self.view.update(|model| { model.find_scenario_mut(dir).phase_finished(phase); }) @@ -451,14 +450,14 @@ impl nutmeg::Model for LabModel { } impl LabModel { - fn find_scenario_mut(&mut self, dir: &Path) -> &mut ScenarioModel { + fn find_scenario_mut(&mut self, dir: &Utf8Path) -> &mut ScenarioModel { self.scenario_models .iter_mut() .find(|sm| sm.dir == *dir) .expect("scenario directory not found") } - fn remove_scenario(&mut self, dir: &Path) { + fn remove_scenario(&mut self, dir: &Utf8Path) { self.scenario_models.retain(|sm| sm.dir != *dir); } } @@ -488,7 +487,7 @@ impl nutmeg::Model for WalkModel { /// It draws the command and some description of what scenario is being tested. struct ScenarioModel { /// The directory where this is being built: unique across all models. - dir: PathBuf, + dir: Utf8PathBuf, name: Cow<'static, str>, phase_start: Instant, phase: Option, @@ -499,10 +498,10 @@ struct ScenarioModel { impl ScenarioModel { fn new( - dir: &Path, + dir: &Utf8Path, scenario: &Scenario, start: Instant, - log_file: &Utf8Path, + log_file: File, ) -> Result { let log_tail = TailFile::new(log_file).context("Failed to open log file")?; Ok(ScenarioModel { @@ -556,13 +555,13 @@ impl nutmeg::Model for ScenarioModel { /// A Nutmeg model for progress in copying a tree. struct CopyModel { - dest: PathBuf, + dest: Utf8PathBuf, bytes_copied: u64, start: Instant, } impl CopyModel { - fn new(dest: PathBuf) -> CopyModel { + fn new(dest: Utf8PathBuf) -> CopyModel { CopyModel { dest, start: Instant::now(), diff --git a/src/copy_tree.rs b/src/copy_tree.rs index e1e7c29c..ea759fd1 100644 --- a/src/copy_tree.rs +++ b/src/copy_tree.rs @@ -46,7 +46,10 @@ pub fn copy_tree( .suffix(".tmp") .tempdir() .context("create temp dir")?; - let dest = temp_dir.path(); + let dest = temp_dir + .path() + .try_into() + .context("Convert path to UTF-8")?; console.start_copy(dest); for entry in WalkBuilder::new(from_path) .standard_filters(gitignore) diff --git a/src/lab.rs b/src/lab.rs index 8fad797c..e7e6c2ab 100644 --- a/src/lab.rs +++ b/src/lab.rs @@ -5,7 +5,6 @@ use std::cmp::{max, min}; use std::panic::resume_unwind; -use std::path::Path; use std::sync::Mutex; use std::thread; use std::time::Instant; @@ -209,53 +208,62 @@ fn test_scenario( options: &Options, console: &Console, ) -> Result { - let mut log_file = output_mutex + let mut scenario_output = output_mutex .lock() - .expect("lock output_dir to create log") - .create_log(scenario)?; - log_file.message(&scenario.to_string()); + .expect("lock output_dir to start scenario") + .start_scenario(scenario)?; + let dir = build_dir.path(); + console.scenario_started(dir, scenario, scenario_output.open_log_read()?)?; + let phases: &[Phase] = if options.check_only { &[Phase::Check] } else { &[Phase::Build, Phase::Test] }; - let applied = scenario - .mutant() - .map(|mutant| { - // TODO: This is slightly inefficient as it computes the mutated source twice, - // once for the diff and once to write it out. - log_file.message(&format!("mutation diff:\n{}", mutant.diff())); - mutant.apply(build_dir) - }) - .transpose()?; - let dir: &Path = build_dir.path().as_ref(); - console.scenario_started(dir, scenario, log_file.path())?; + if let Some(mutant) = scenario.mutant() { + let mutated_code = mutant.mutated_code(); + let diff = scenario.mutant().unwrap().diff(&mutated_code); + scenario_output.write_diff(&diff)?; + mutant.apply(build_dir, &mutated_code)?; + } - let mut outcome = ScenarioOutcome::new(&log_file, scenario.clone()); + let mut outcome = ScenarioOutcome::new(&scenario_output, scenario.clone()); for &phase in phases { console.scenario_phase_started(dir, phase); let timeout = match phase { Phase::Test => timeouts.test, Phase::Build | Phase::Check => timeouts.build, }; - let phase_result = run_cargo( + match run_cargo( build_dir, jobserver, Some(test_packages), phase, timeout, - &mut log_file, + &mut scenario_output, options, console, - )?; - let success = phase_result.is_success(); // so we can move it away - outcome.add_phase_result(phase_result); - console.scenario_phase_finished(dir, phase); - if !success { - break; + ) { + Ok(phase_result) => { + let success = phase_result.is_success(); // so we can move it away + outcome.add_phase_result(phase_result); + console.scenario_phase_finished(dir, phase); + if !success { + break; + } + } + Err(err) => { + // Some unexpected internal error that stops the program. + if let Some(mutant) = scenario.mutant() { + mutant.revert(build_dir)?; + return Err(err); + } + } } } - drop(applied); + if let Some(mutant) = scenario.mutant() { + mutant.revert(build_dir)?; + } output_mutex .lock() .expect("lock output dir to add outcome") diff --git a/src/list.rs b/src/list.rs index a3045040..bf1a6397 100644 --- a/src/list.rs +++ b/src/list.rs @@ -37,9 +37,10 @@ pub(crate) fn list_mutants( for mutant in mutants { let mut obj = serde_json::to_value(mutant)?; if options.emit_diffs { - obj.as_object_mut() - .unwrap() - .insert("diff".to_owned(), json!(mutant.diff())); + obj.as_object_mut().unwrap().insert( + "diff".to_owned(), + json!(mutant.diff(&mutant.mutated_code())), + ); } list.push(obj); } @@ -51,7 +52,7 @@ pub(crate) fn list_mutants( for mutant in mutants { writeln!(out, "{}", mutant.name(options.show_line_col, colors))?; if options.emit_diffs { - writeln!(out, "{}", mutant.diff())?; + writeln!(out, "{}", mutant.diff(&mutant.mutated_code()))?; } } } diff --git a/src/log_file.rs b/src/log_file.rs deleted file mode 100644 index 9fb0e45d..00000000 --- a/src/log_file.rs +++ /dev/null @@ -1,90 +0,0 @@ -// Copyright 2021-2023 Martin Pool - -//! Manage per-scenario log files, which contain the output from cargo -//! and test cases, mixed with commentary from cargo-mutants. - -use std::fs::{File, OpenOptions}; -use std::io::{self, Write}; - -use anyhow::Context; -use camino::{Utf8Path, Utf8PathBuf}; - -use crate::Result; - -/// Text inserted in log files to make important sections more visible. -pub const LOG_MARKER: &str = "***"; - -/// A log file for execution of a single scenario. -#[derive(Debug)] -pub struct LogFile { - path: Utf8PathBuf, - write_to: File, -} - -impl LogFile { - pub fn create_in(log_dir: &Utf8Path, scenario_name: &str) -> Result { - let basename = clean_filename(scenario_name); - for i in 0..1000 { - let t = if i == 0 { - format!("{basename}.log") - } else { - format!("{basename}_{i:03}.log") - }; - let path = log_dir.join(t); - match OpenOptions::new() - .read(true) - .append(true) - .create_new(true) - .open(&path) - { - Ok(write_to) => return Ok(LogFile { path, write_to }), - Err(e) if e.kind() == io::ErrorKind::AlreadyExists => continue, - Err(e) => return Err(anyhow::Error::from(e).context("create test log file")), - } - } - unreachable!( - "couldn't create any test log in {:?} for {:?}", - log_dir, scenario_name, - ); - } - - /// Open the log file to append more content. - pub fn open_append(&self) -> Result { - OpenOptions::new() - .append(true) - .open(&self.path) - .with_context(|| format!("open {} for append", self.path)) - } - - /// Write a message, with a marker. Ignore errors. - pub fn message(&mut self, message: &str) { - write!(self.write_to, "\n{LOG_MARKER} {message}\n").expect("write message to log"); - } - - pub fn path(&self) -> &Utf8Path { - &self.path - } -} - -fn clean_filename(s: &str) -> String { - s.replace('/', "__") - .chars() - .map(|c| match c { - '\\' | ' ' | ':' | '<' | '>' | '?' | '*' | '|' | '"' => '_', - c => c, - }) - .collect::() -} - -#[cfg(test)] -mod test { - use super::*; - - #[test] - fn clean_filename_removes_special_characters() { - assert_eq!( - clean_filename("1/2\\3:4<5>6?7*8|9\"0"), - "1__2_3_4_5_6_7_8_9_0" - ); - } -} diff --git a/src/main.rs b/src/main.rs index f0843c93..ce5123af 100644 --- a/src/main.rs +++ b/src/main.rs @@ -16,7 +16,6 @@ mod in_diff; mod interrupt; mod lab; mod list; -mod log_file; mod manifest; mod mutate; mod options; @@ -56,7 +55,6 @@ use crate::in_diff::diff_filter; use crate::interrupt::check_interrupted; use crate::lab::test_mutants; use crate::list::{list_files, list_mutants, FmtToIoWrite}; -use crate::log_file::LogFile; use crate::manifest::fix_manifest; use crate::mutate::{Genre, Mutant}; use crate::options::{Colors, Options, TestTool}; diff --git a/src/mutate.rs b/src/mutate.rs index 44256e6c..f5a69b50 100644 --- a/src/mutate.rs +++ b/src/mutate.rs @@ -1,20 +1,19 @@ -// Copyright 2021-2023 Martin Pool +// Copyright 2021-2024 Martin Pool //! Mutations to source files, and inference of interesting mutations to apply. use std::fmt; -use std::fs; use std::sync::Arc; -use anyhow::{ensure, Context, Result}; +use anyhow::Result; use console::{style, StyledObject}; use serde::ser::{SerializeStruct, Serializer}; use serde::Serialize; use similar::TextDiff; -use tracing::error; use tracing::trace; use crate::build_dir::BuildDir; +use crate::output::clean_filename; use crate::package::Package; use crate::source::SourceFile; use crate::span::Span; @@ -171,11 +170,14 @@ impl Mutant { } /// Return a unified diff for the mutant. - pub fn diff(&self) -> String { + /// + /// The mutated text must be passed in because we should have already computed + /// it, and don't want to pointlessly recompute it here. + pub fn diff(&self, mutated_code: &str) -> String { let old_label = self.source_file.tree_relative_slashes(); // There shouldn't be any newlines, but just in case... let new_label = self.describe_change().replace('\n', " "); - TextDiff::from_lines(self.source_file.code(), &self.mutated_code()) + TextDiff::from_lines(self.source_file.code(), mutated_code) .unified_diff() .context_radius(8) .header(&old_label, &new_label) @@ -183,34 +185,27 @@ impl Mutant { } /// Apply this mutant to the relevant file within a BuildDir. - pub fn apply<'a>(&'a self, build_dir: &'a BuildDir) -> Result { + pub fn apply(&self, build_dir: &BuildDir, mutated_code: &str) -> Result<()> { trace!(?self, "Apply mutant"); - self.write_in_dir(build_dir, &self.mutated_code())?; - Ok(AppliedMutant { - mutant: self, - build_dir, - }) - } - - fn unapply(&self, build_dir: &BuildDir) -> Result<()> { - trace!(?self, "Unapply mutant"); - self.write_in_dir(build_dir, self.source_file.code()) + build_dir.overwrite_file(&self.source_file.tree_relative_path, mutated_code) } - fn write_in_dir(&self, build_dir: &BuildDir, code: &str) -> Result<()> { - let path = build_dir.path().join(&self.source_file.tree_relative_path); - // for safety, don't follow symlinks - ensure!(path.is_file(), "{path:?} is not a file"); - fs::write(&path, code.as_bytes()) - .with_context(|| format!("failed to write mutated code to {path:?}")) + pub fn revert(&self, build_dir: &BuildDir) -> Result<()> { + trace!(?self, "Revert mutant"); + build_dir.overwrite_file( + &self.source_file.tree_relative_path, + self.source_file.code(), + ) } /// Return a string describing this mutant that's suitable for building a log file name, /// but can contain slashes. pub fn log_file_name_base(&self) -> String { + // TODO: Also include a unique number so that they can't collide, even + // with similar mutants on the same line? format!( "{filename}_line_{line}_col_{col}", - filename = self.source_file.tree_relative_slashes(), + filename = clean_filename(&self.source_file.tree_relative_slashes()), line = self.span.start.line, col = self.span.start.column, ) @@ -247,22 +242,6 @@ impl Serialize for Mutant { } } -/// Manages the lifetime of a mutant being applied to a build directory; when -/// dropped, the mutant is unapplied. -#[must_use] -pub struct AppliedMutant<'a> { - mutant: &'a Mutant, - build_dir: &'a BuildDir, -} - -impl Drop for AppliedMutant<'_> { - fn drop(&mut self) { - if let Err(e) = self.mutant.unapply(self.build_dir) { - error!("Failed to unapply mutant: {}", e); - } - } -} - #[cfg(test)] mod test { use indoc::indoc; diff --git a/src/outcome.rs b/src/outcome.rs index 7ef33229..72413032 100644 --- a/src/outcome.rs +++ b/src/outcome.rs @@ -3,11 +3,11 @@ //! The outcome of running a single mutation scenario, or a whole lab. use std::fmt; -use std::fs; use std::time::Duration; use std::time::Instant; use humantime::format_duration; +use output::ScenarioOutput; use serde::ser::SerializeStruct; use serde::Serialize; use serde::Serializer; @@ -143,7 +143,11 @@ impl LabOutcome { pub struct ScenarioOutcome { /// A file holding the text output from running this test. // TODO: Maybe this should be a log object? + output_dir: Utf8PathBuf, log_path: Utf8PathBuf, + /// The path relative to `mutants.out` for a file showing the diff between the unmutated + /// and mutated source. Only present for mutant scenarios. + diff_path: Option, /// What kind of scenario was being built? pub scenario: Scenario, /// For each phase, the duration and the cargo result. @@ -155,20 +159,23 @@ impl Serialize for ScenarioOutcome { where S: Serializer, { - // custom serialize to omit inessential info - let mut ss = serializer.serialize_struct("Outcome", 4)?; + // custom serialize to omit inessential info and to inline a summary. + let mut ss = serializer.serialize_struct("Outcome", 5)?; ss.serialize_field("scenario", &self.scenario)?; - ss.serialize_field("log_path", &self.log_path)?; ss.serialize_field("summary", &self.summary())?; + ss.serialize_field("log_path", &self.log_path)?; + ss.serialize_field("diff_path", &self.diff_path)?; ss.serialize_field("phase_results", &self.phase_results)?; ss.end() } } impl ScenarioOutcome { - pub fn new(log_file: &LogFile, scenario: Scenario) -> ScenarioOutcome { + pub fn new(scenario_output: &ScenarioOutput, scenario: Scenario) -> ScenarioOutcome { ScenarioOutcome { - log_path: log_file.path().to_owned(), + output_dir: scenario_output.output_dir.to_owned(), + log_path: scenario_output.log_path().to_owned(), + diff_path: scenario_output.diff_path.to_owned(), scenario, phase_results: Vec::new(), } @@ -179,7 +186,7 @@ impl ScenarioOutcome { } pub fn get_log_content(&self) -> Result { - fs::read_to_string(&self.log_path).context("read log file") + read_to_string(self.output_dir.join(&self.log_path)).context("read log file") } pub fn last_phase(&self) -> Phase { @@ -324,7 +331,9 @@ mod test { #[test] fn find_phase_result() { let outcome = ScenarioOutcome { + output_dir: "output".into(), log_path: "log".into(), + diff_path: Some("mutant.diff".into()), scenario: Scenario::Baseline, phase_results: vec![ PhaseResult { diff --git a/src/output.rs b/src/output.rs index a78020bb..e7d9deab 100644 --- a/src/output.rs +++ b/src/output.rs @@ -2,7 +2,9 @@ //! A `mutants.out` directory holding logs and other output. -use std::fs::{self, File, OpenOptions}; +use std::collections::hash_map::Entry; +use std::collections::HashMap; +use std::fs::{create_dir, remove_dir_all, rename, write, File, OpenOptions}; use std::io::{BufWriter, Write}; use std::path::Path; use std::thread::sleep; @@ -86,7 +88,7 @@ impl LockFile { #[derive(Debug)] pub struct OutputDir { path: Utf8PathBuf, - log_dir: Utf8PathBuf, + #[allow(unused)] // Lifetime controls the file lock lock_file: File, /// A file holding a list of missed mutants as text, one per line. @@ -98,6 +100,9 @@ pub struct OutputDir { unviable_list: File, /// The accumulated overall lab outcome. pub lab_outcome: LabOutcome, + /// Log filenames which have already been used, and the number of times that each + /// basename has been used. + used_log_names: HashMap, } impl OutputDir { @@ -113,27 +118,30 @@ impl OutputDir { /// the lock to be released. The returned `OutputDir` holds a lock for its lifetime. pub fn new(in_dir: &Utf8Path) -> Result { if !in_dir.exists() { - fs::create_dir(in_dir).context("create output parent directory {in_dir:?}")?; + create_dir(in_dir).context("create output parent directory {in_dir:?}")?; } let output_dir = in_dir.join(OUTDIR_NAME); if output_dir.exists() { LockFile::acquire_lock(output_dir.as_ref())?; // Now release the lock for a bit while we move the directory. This might be // slightly racy. + // TODO: Move the lock outside the directory, . let rotated = in_dir.join(ROTATED_NAME); if rotated.exists() { - fs::remove_dir_all(&rotated).with_context(|| format!("remove {:?}", &rotated))?; + remove_dir_all(&rotated).with_context(|| format!("remove {:?}", &rotated))?; } - fs::rename(&output_dir, &rotated) + rename(&output_dir, &rotated) .with_context(|| format!("move {:?} to {:?}", &output_dir, &rotated))?; } - fs::create_dir(&output_dir) + create_dir(&output_dir) .with_context(|| format!("create output directory {:?}", &output_dir))?; let lock_file = LockFile::acquire_lock(output_dir.as_std_path()) .context("create lock.json lock file")?; let log_dir = output_dir.join("log"); - fs::create_dir(&log_dir).with_context(|| format!("create log directory {:?}", &log_dir))?; + create_dir(&log_dir).with_context(|| format!("create log directory {:?}", &log_dir))?; + let diff_dir = output_dir.join("diff"); + create_dir(&diff_dir).context("create diff dir")?; // Create text list files. let mut list_file_options = OpenOptions::new(); @@ -153,25 +161,37 @@ impl OutputDir { Ok(OutputDir { path: output_dir, lab_outcome: LabOutcome::new(), - log_dir, lock_file, missed_list, caught_list, timeout_list, unviable_list, + used_log_names: HashMap::new(), }) } - /// Create a new log for a given scenario. - /// - /// Returns the [File] to which subprocess output should be sent, and a LogFile to read it - /// later. - pub fn create_log(&self, scenario: &Scenario) -> Result { - LogFile::create_in(&self.log_dir, &scenario.log_file_name_base()) + /// Allocate a sequence number and the output files for a scenario. + pub fn start_scenario(&mut self, scenario: &Scenario) -> Result { + let scenario_name = match scenario { + Scenario::Baseline => "baseline".into(), + Scenario::Mutant(mutant) => mutant.log_file_name_base(), + }; + let basename = match self.used_log_names.entry(scenario_name.clone()) { + Entry::Occupied(mut e) => { + let index = e.get_mut(); + *index += 1; + format!("{scenario_name}_{index:03}") + } + Entry::Vacant(e) => { + e.insert(0); + scenario_name + } + }; + ScenarioOutput::new(&self.path, scenario, &basename) } - #[allow(dead_code)] /// Return the path of the `mutants.out` directory. + #[allow(unused)] pub fn path(&self) -> &Utf8Path { &self.path } @@ -265,9 +285,87 @@ pub fn load_previously_caught(output_parent_dir: &Utf8Path) -> Result, +} + +impl ScenarioOutput { + fn new(output_dir: &Utf8Path, scenario: &Scenario, basename: &str) -> Result { + let log_path = Utf8PathBuf::from(format!("log/{basename}.log")); + let log_file = File::options() + .append(true) + .create_new(true) + .read(true) + .open(output_dir.join(&log_path))?; + let diff_path = if scenario.is_mutant() { + Some(Utf8PathBuf::from(format!("diff/{basename}.diff"))) + } else { + None + }; + let mut scenario_output = Self { + output_dir: output_dir.to_owned(), + log_path, + log_file, + diff_path, + }; + scenario_output.message(&scenario.to_string())?; + Ok(scenario_output) + } + + pub fn log_path(&self) -> &Utf8Path { + &self.log_path + } + + pub fn write_diff(&mut self, diff: &str) -> Result<()> { + self.message(&format!("mutation diff:\n{}", diff))?; + let diff_path = self.diff_path.as_ref().expect("should know the diff path"); + write(self.output_dir.join(diff_path), diff.as_bytes()) + .with_context(|| format!("write diff to {diff_path}")) + } + + /// Open a new handle reading from the start of the log file. + pub fn open_log_read(&self) -> Result { + let path = self.output_dir.join(&self.log_path); + OpenOptions::new() + .read(true) + .open(&path) + .with_context(|| format!("reopen {} for read", path)) + } + + /// Open a new handle that appends to the log file, so that it can be passed to a subprocess. + pub fn open_log_append(&self) -> Result { + let path = self.output_dir.join(&self.log_path); + OpenOptions::new() + .append(true) + .open(&path) + .with_context(|| format!("reopen {} for append", path)) + } + + /// Write a message, with a marker. + pub fn message(&mut self, message: &str) -> Result<()> { + write!(self.log_file, "\n*** {message}\n").context("write message to log") + } +} + +pub fn clean_filename(s: &str) -> String { + s.replace('/', "__") + .chars() + .map(|c| match c { + '\\' | ' ' | ':' | '<' | '>' | '?' | '*' | '|' | '"' => '_', + c => c, + }) + .collect::() +} + #[cfg(test)] mod test { - use fs::write; + use std::fs::write; + use indoc::indoc; use itertools::Itertools; use pretty_assertions::assert_eq; @@ -278,7 +376,7 @@ mod test { fn minimal_source_tree() -> TempDir { let tmp = tempdir().unwrap(); let path = tmp.path(); - fs::write( + write( path.join("Cargo.toml"), indoc! { br#" # enough for a test @@ -289,8 +387,8 @@ mod test { }, ) .unwrap(); - fs::create_dir(path.join("src")).unwrap(); - fs::write(path.join("src/lib.rs"), b"fn foo() {}").unwrap(); + create_dir(path.join("src")).unwrap(); + write(path.join("src/lib.rs"), b"fn foo() {}").unwrap(); tmp } @@ -310,6 +408,14 @@ mod test { .collect_vec() } + #[test] + fn clean_filename_removes_special_characters() { + assert_eq!( + clean_filename("1/2\\3:4<5>6?7*8|9\"0"), + "1__2_3_4_5_6_7_8_9_0" + ); + } + #[test] fn create_output_dir() { let tmp = minimal_source_tree(); @@ -323,6 +429,7 @@ mod test { "Cargo.toml", "mutants.out", "mutants.out/caught.txt", + "mutants.out/diff", "mutants.out/lock.json", "mutants.out/log", "mutants.out/missed.txt", @@ -333,7 +440,6 @@ mod test { ] ); assert_eq!(output_dir.path(), workspace.dir.join("mutants.out")); - assert_eq!(output_dir.log_dir, workspace.dir.join("mutants.out/log")); assert!(output_dir.path().join("lock.json").is_file()); } @@ -343,17 +449,15 @@ mod test { let temp_dir_path = Utf8Path::from_path(temp_dir.path()).unwrap(); // Create an initial output dir with one log. - let output_dir = OutputDir::new(temp_dir_path).unwrap(); - output_dir.create_log(&Scenario::Baseline).unwrap(); - assert!(temp_dir - .path() - .join("mutants.out/log/baseline.log") - .is_file()); + let mut output_dir = OutputDir::new(temp_dir_path).unwrap(); + let scenario_output = output_dir.start_scenario(&Scenario::Baseline).unwrap(); + assert!(temp_dir_path.join("mutants.out/log/baseline.log").is_file()); drop(output_dir); // release the lock. + drop(scenario_output); // The second time we create it in the same directory, the old one is moved away. - let output_dir = OutputDir::new(temp_dir_path).unwrap(); - output_dir.create_log(&Scenario::Baseline).unwrap(); + let mut output_dir = OutputDir::new(temp_dir_path).unwrap(); + output_dir.start_scenario(&Scenario::Baseline).unwrap(); assert!(temp_dir .path() .join("mutants.out.old/log/baseline.log") @@ -365,8 +469,8 @@ mod test { drop(output_dir); // The third time (and later), the .old directory is removed. - let output_dir = OutputDir::new(temp_dir_path).unwrap(); - output_dir.create_log(&Scenario::Baseline).unwrap(); + let mut output_dir = OutputDir::new(temp_dir_path).unwrap(); + output_dir.start_scenario(&Scenario::Baseline).unwrap(); assert!(temp_dir .path() .join("mutants.out/log/baseline.log") diff --git a/src/process.rs b/src/process.rs index 27b6ceee..e62d4d26 100644 --- a/src/process.rs +++ b/src/process.rs @@ -21,7 +21,7 @@ use tracing::{debug, debug_span, error, span, trace, warn, Level}; use crate::console::Console; use crate::interrupt::check_interrupted; -use crate::log_file::LogFile; +use crate::output::ScenarioOutput; use crate::Result; /// How frequently to check if a subprocess finished. @@ -42,10 +42,10 @@ impl Process { cwd: &Utf8Path, timeout: Option, jobserver: &Option, - log_file: &mut LogFile, + scenario_output: &mut ScenarioOutput, console: &Console, ) -> Result { - let mut child = Process::start(argv, env, cwd, timeout, jobserver, log_file)?; + let mut child = Process::start(argv, env, cwd, timeout, jobserver, scenario_output)?; let process_status = loop { if let Some(exit_status) = child.poll()? { break exit_status; @@ -54,7 +54,7 @@ impl Process { sleep(WAIT_POLL_INTERVAL); } }; - log_file.message(&format!("result: {process_status:?}")); + scenario_output.message(&format!("result: {process_status:?}"))?; Ok(process_status) } @@ -65,11 +65,11 @@ impl Process { cwd: &Utf8Path, timeout: Option, jobserver: &Option, - log_file: &mut LogFile, + scenario_output: &mut ScenarioOutput, ) -> Result { let start = Instant::now(); let quoted_argv = cheap_shell_quote(argv); - log_file.message("ed_argv); + scenario_output.message("ed_argv)?; debug!(%quoted_argv, "start process"); let os_env = env.iter().map(|(k, v)| (OsStr::new(k), OsStr::new(v))); let mut child = Command::new(&argv[0]); @@ -77,8 +77,8 @@ impl Process { .args(&argv[1..]) .envs(os_env) .stdin(Stdio::null()) - .stdout(log_file.open_append()?) - .stderr(log_file.open_append()?) + .stdout(scenario_output.open_log_append()?) + .stderr(scenario_output.open_log_append()?) .current_dir(cwd); jobserver.as_ref().map(|js| js.configure(&mut child)); #[cfg(unix)] diff --git a/src/scenario.rs b/src/scenario.rs index 413fdc8c..ea616d91 100644 --- a/src/scenario.rs +++ b/src/scenario.rs @@ -35,11 +35,4 @@ impl Scenario { Scenario::Mutant(mutant) => Some(mutant), } } - - pub fn log_file_name_base(&self) -> String { - match self { - Scenario::Baseline => "baseline".into(), - Scenario::Mutant(mutant) => mutant.log_file_name_base(), - } - } } diff --git a/src/tail_file.rs b/src/tail_file.rs index 8ba3efb8..d459da4d 100644 --- a/src/tail_file.rs +++ b/src/tail_file.rs @@ -1,10 +1,9 @@ -// Copyright 2021-2023 Martin Pool +// Copyright 2021-2024 Martin Pool //! Tail a log file: watch for new writes and return the last line. use std::fs::File; use std::io::Read; -use std::path::Path; use anyhow::Context; @@ -18,9 +17,8 @@ pub struct TailFile { } impl TailFile { - /// Watch the given path. - pub fn new>(path: P) -> Result { - let file = File::open(path.as_ref()).context("Open log file")?; + /// Watch for newly appended data in a file. + pub fn new(file: File) -> Result { Ok(TailFile { file, last_line_seen: String::new(), @@ -66,7 +64,8 @@ mod test { fn last_line_of_file() { let mut tempfile = tempfile::NamedTempFile::new().unwrap(); let path: Utf8PathBuf = tempfile.path().to_owned().try_into().unwrap(); - let mut tailer = TailFile::new(path).unwrap(); + let reopened = File::open(&path).unwrap(); + let mut tailer = TailFile::new(reopened).unwrap(); assert_eq!( tailer.last_line().unwrap(), diff --git a/tests/main.rs b/tests/main.rs index 0d881b46..782216bb 100644 --- a/tests/main.rs +++ b/tests/main.rs @@ -2,6 +2,7 @@ //! Tests for cargo-mutants CLI layer. +use std::collections::HashSet; use std::env; use std::fs::{self, read_dir, read_to_string}; use std::path::Path; @@ -85,6 +86,33 @@ fn tree_with_child_directories_is_well_tested() { ) .unwrap(), ); + // The outcomes all have `diff_path` keys and they all identify files. + let outcomes_json = + read_to_string(tmp_src_dir.path().join("mutants.out/outcomes.json")).unwrap(); + let json: serde_json::Value = serde_json::from_str(&outcomes_json).unwrap(); + let mut all_diffs = HashSet::new(); + for outcome_json in json["outcomes"].as_array().unwrap() { + dbg!(&outcome_json); + if outcome_json["scenario"].as_str() == Some("Baseline") { + assert!( + outcome_json + .get("diff_path") + .expect("has a diff_path") + .is_null(), + "diff_path should be null" + ); + } else { + let diff_path = outcome_json["diff_path"].as_str().unwrap(); + let full_diff_path = tmp_src_dir.path().join("mutants.out").join(diff_path); + assert!(full_diff_path.is_file(), "{diff_path:?} is not a file"); + assert!(all_diffs.insert(diff_path)); + let diff_content = read_to_string(&full_diff_path).expect("read diff file"); + assert!( + diff_content.starts_with("--- src/"), + "diff content in {full_diff_path:?} doesn't look right:\n{diff_content}" + ); + } + } } #[test]