diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index c35ca317..6b806283 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -87,7 +87,7 @@ jobs: strategy: matrix: os: [macOS-latest, ubuntu-latest, windows-latest] - version: [stable, nightly, "1.74"] + version: [stable, nightly, "1.78"] runs-on: ${{ matrix.os }} steps: - uses: actions/checkout@v4 @@ -117,47 +117,20 @@ jobs: # Clippy checks can vary between versions in a way that makes it a bit # fiddly to satisfy them all, so only insist that they pass on stable. run: cargo clippy --all-targets --all-features -- -D warnings + - run: cargo update + - name: Test after cargo update + run: cargo test --workspace + - name: Downgrade to minimal versions + if: matrix.version == 'nightly' + run: cargo +nightly -Zdirect-minimal-versions update + - name: Test on minimal versions + if: matrix.version == 'nightly' + run: cargo test - name: Install locked - run: cargo install --locked --path . + run: cargo install --path . --locked - name: Install unlocked run: cargo install --path . - minimal-versions: - needs: [quick-test] - strategy: - matrix: - os: [macOS-latest, ubuntu-latest, windows-latest] - runs-on: ${{ matrix.os }} - steps: - - uses: actions/checkout@v4 - - uses: dtolnay/rust-toolchain@nightly - - uses: Swatinem/rust-cache@v2 - - run: cargo +nightly -Zdirect-minimal-versions update - - run: cargo build --all-targets - - uses: taiki-e/install-action@v2 - name: Install nextest using install-action - with: - tool: nextest - - run: cargo test - - # Run `cargo update` and check the tests still pass - maximal-versions: - needs: [quick-test] - strategy: - matrix: - os: [macOS-latest, ubuntu-latest, windows-latest] - runs-on: ${{ matrix.os }} - steps: - - uses: actions/checkout@v4 - - uses: dtolnay/rust-toolchain@nightly - - uses: Swatinem/rust-cache@v2 - - run: cargo update - - uses: taiki-e/install-action@v2 - name: Install nextest using install-action - with: - tool: nextest - - run: cargo test - tests-from-tarball: needs: [quick-test] strategy: @@ -224,6 +197,7 @@ jobs: run: > cargo mutants --no-shuffle -vV --in-diff git.diff --test-tool=${{matrix.test_tool}} --timeout=500 --build-timeout=500 + --exclude=windows.rs --exclude=console.rs - name: Archive mutants.out uses: actions/upload-artifact@v4 if: always() @@ -261,7 +235,8 @@ jobs: run: > cargo mutants --no-shuffle -vV --shard ${{ matrix.shard }}/10 --test-tool ${{ matrix.test_tool }} --baseline=skip --timeout=500 - --build-timeout=500 --in-place + --build-timeout=500 --in-place --exclude=windows.rs + --exclude=console.rs - name: Archive mutants.out uses: actions/upload-artifact@v4 if: always() @@ -270,16 +245,7 @@ jobs: path: mutants.out overall-result: - needs: - [ - quick-test, - test, - minimal-versions, - maximal-versions, - tests-from-tarball, - pr-mutants, - cargo-mutants, - ] + needs: [quick-test, test, tests-from-tarball, pr-mutants, cargo-mutants] runs-on: ubuntu-latest if: always() steps: diff --git a/Cargo.lock b/Cargo.lock index b52e3d3c..d2b8de31 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -216,7 +216,7 @@ dependencies = [ [[package]] name = "cargo-mutants" -version = "24.11.0" +version = "24.11.2" dependencies = [ "anyhow", "assert_cmd", @@ -235,11 +235,11 @@ dependencies = [ "ignore", "indoc", "insta", - "itertools 0.12.0", + "itertools 0.13.0", "jobserver", "lazy_static", "mutants 0.0.3 (registry+https://github.com/rust-lang/crates.io-index)", - "nix 0.28.0", + "nix 0.29.0", "num_cpus", "nutmeg", "patch", @@ -254,7 +254,7 @@ dependencies = [ "serde_json", "similar", "strum", - "syn 2.0.46", + "syn 2.0.90", "tempfile", "test-log", "time", @@ -277,9 +277,9 @@ dependencies = [ [[package]] name = "cargo_metadata" -version = "0.18.0" +version = "0.19.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "fb9ac64500cc83ce4b9f8dafa78186aa008c8dea77a09b94cd307fd0cd5022a8" +checksum = "8769706aad5d996120af43197bf46ef6ad0fda35216b4505f926a365a232d924" dependencies = [ "camino", "cargo-platform", @@ -303,9 +303,9 @@ checksum = "baf1de4339761588bc0619e3cbc0120ee582ebb74b53b4efbf79117bd2da40fd" [[package]] name = "cfg_aliases" -version = "0.1.1" +version = "0.2.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "fd16c4719339c4530435d38e511904438d07cce7950afa3718a84ac36c10e89e" +checksum = "613afe47fcd5fac7ccf1db93babcb082c5994d996f20b8b159f2ad1658eb5724" [[package]] name = "chrono" @@ -363,7 +363,7 @@ dependencies = [ "heck 0.4.1", "proc-macro2", "quote", - "syn 2.0.46", + "syn 2.0.90", ] [[package]] @@ -390,7 +390,7 @@ dependencies = [ "nom", "proc-macro2", "quote", - "syn 2.0.46", + "syn 2.0.90", ] [[package]] @@ -749,9 +749,9 @@ dependencies = [ [[package]] name = "itertools" -version = "0.12.0" +version = "0.13.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "25db6b064527c5d482d0423354fcd07a89a2dfe07b67892e62411946db7f07b0" +checksum = "413ee7dfc52ee1a4949ceeb7dbc8a33f2d6c088194d9f922fb8318faf1f01186" dependencies = [ "either", ] @@ -881,9 +881,9 @@ dependencies = [ [[package]] name = "nix" -version = "0.28.0" +version = "0.29.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ab2156c4fce2f8df6c499cc1c763e4394b7482525bf2a9701c9d79d215f519e4" +checksum = "71e2746dc3a24dd78b3cfcb7be93368c6de9963d30f43a6a73998a9cf4b17b46" dependencies = [ "bitflags 2.5.0", "cfg-if", @@ -1094,9 +1094,9 @@ dependencies = [ [[package]] name = "proc-macro2" -version = "1.0.74" +version = "1.0.92" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2de98502f212cfcea8d0bb305bd0f49d7ebdd75b64ba0a68f937d888f4e0d6db" +checksum = "37d3544b3f2748c54e147655edb5025752e2303145b5aefb3c3ea2c78b973bb0" dependencies = [ "unicode-ident", ] @@ -1270,14 +1270,14 @@ checksum = "a3385e45322e8f9931410f01b3031ec534c3947d0e94c18049af4d9f9907d4e0" dependencies = [ "proc-macro2", "quote", - "syn 2.0.46", + "syn 2.0.90", ] [[package]] name = "serde_json" -version = "1.0.117" +version = "1.0.118" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "455182ea6142b14f93f4bc5320a2b31c1f266b66a4a5c858b013302a5d8cbfc3" +checksum = "d947f6b3163d8857ea16c4fa0dd4840d52f3041039a85decd46867eb1abef2e4" dependencies = [ "itoa", "ryu", @@ -1351,7 +1351,7 @@ dependencies = [ "proc-macro2", "quote", "rustversion", - "syn 2.0.46", + "syn 2.0.90", ] [[package]] @@ -1367,9 +1367,9 @@ dependencies = [ [[package]] name = "syn" -version = "2.0.46" +version = "2.0.90" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "89456b690ff72fddcecf231caedbe615c59480c93358a93dfae7fc29e3ebbf0e" +checksum = "919d3b74a5dd0ccd15aeb8f93e7006bd9e14c295087c9896a110f490752bcf31" dependencies = [ "proc-macro2", "quote", @@ -1434,27 +1434,27 @@ checksum = "5999e24eaa32083191ba4e425deb75cdf25efefabe5aaccb7446dd0d4122a3f5" dependencies = [ "proc-macro2", "quote", - "syn 2.0.46", + "syn 2.0.90", ] [[package]] name = "thiserror" -version = "1.0.61" +version = "2.0.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c546c80d6be4bc6a00c0f01730c08df82eaa7a7a61f11d656526506112cc1709" +checksum = "2f49a1853cf82743e3b7950f77e0f4d622ca36cf4317cba00c767838bac8d490" dependencies = [ "thiserror-impl", ] [[package]] name = "thiserror-impl" -version = "1.0.61" +version = "2.0.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "46c3384250002a6d5af4d114f2845d37b57521033f30d5c3f46c4d70e1197533" +checksum = "8381894bb3efe0c4acac3ded651301ceee58a15d47c2e34885ed1908ad667061" dependencies = [ "proc-macro2", "quote", - "syn 2.0.46", + "syn 2.0.90", ] [[package]] @@ -1562,7 +1562,7 @@ checksum = "34704c8d6ebcbc939824180af020566b01a7c01f80641264eba0999f6c2b6be7" dependencies = [ "proc-macro2", "quote", - "syn 2.0.46", + "syn 2.0.90", ] [[package]] @@ -1674,7 +1674,7 @@ dependencies = [ "once_cell", "proc-macro2", "quote", - "syn 2.0.46", + "syn 2.0.90", "wasm-bindgen-shared", ] @@ -1696,7 +1696,7 @@ checksum = "e94f17b526d0a461a191c78ea52bbce64071ed5c04c9ffe424dcb38f74171bb7" dependencies = [ "proc-macro2", "quote", - "syn 2.0.46", + "syn 2.0.90", "wasm-bindgen-backend", "wasm-bindgen-shared", ] diff --git a/Cargo.toml b/Cargo.toml index cc8a3222..db560582 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "cargo-mutants" -version = "24.11.0" +version = "24.11.2" edition = "2021" authors = ["Martin Pool"] license = "MIT" @@ -9,7 +9,7 @@ repository = "https://github.com/sourcefrog/cargo-mutants" homepage = "https://mutants.rs/" categories = ["development-tools::testing"] keywords = ["testing", "mutants", "cargo", "mutation-testing", "coverage"] -rust-version = "1.74" +rust-version = "1.78" [package.metadata.wix] upgrade-guid = "CA7BFE8D-F3A7-4D1D-AE43-B7749110FA90" @@ -22,7 +22,7 @@ eula = false [dependencies] anyhow = "1.0.86" camino = "1.1.6" -cargo_metadata = "0.18" +cargo_metadata = "0.19" clap = { version = "4.4.1", features = [ "deprecated", "derive", @@ -39,7 +39,7 @@ globset = "0.4.10" humantime = "2.1.0" ignore = "0.4.20" indoc = "2.0.0" -itertools = "0.12" +itertools = "0.13" jobserver = "0.1" mutants = "0.0.3" num_cpus = "1.16" @@ -47,7 +47,7 @@ patch = "0.7" path-slash = "0.2" quote = "1.0.35" regex = "1.10" -serde_json = "1.0.117" +serde_json = "1.0.118" similar = "2.1" strum = { version = "0.26", features = ["derive"] } tempfile = "3.8" @@ -76,7 +76,7 @@ version = "2.0.46" features = ["full", "extra-traits", "visit"] [target.'cfg(unix)'.dependencies] -nix = { version = "0.28", features = ["process", "signal"] } +nix = { version = "0.29", features = ["process", "signal"] } [dev-dependencies] assert_cmd = "2.0" diff --git a/NEWS.md b/NEWS.md index c58471b4..6a3449f1 100644 --- a/NEWS.md +++ b/NEWS.md @@ -2,6 +2,16 @@ ## Unreleased +- Better estimation of time remaining, based on the time taken to test mutants so far, excluding the time for the baseline. + +## 24.11.2 + +- Changed: `.gitignore` (and other git ignore files) are only consulted when copying the tree if it is contained within a directory with a `.git` directory. + +- Fixed: `.gitignore` files above the git root directory are no longer read. In particular this fixes the problem where `.gitignore *` in the home directory would prevent copying any source trees. + +## 24.11.1 + - Changed: The arguments of calls to functions or methods named `with_capacity` are not mutated by default. This can be turned off with `--skip-calls-defaults=false` on the command line or `skip_calls_defaults = false` in `.cargo/mutants.toml`. - New: `--skip-calls=NAME,NAME` on the command line or `skip_calls = [NAMES..]` in `.cargo/mutants.toml` allows configuring other functions whose calls should never be mutated. diff --git a/book/book.toml b/book/book.toml index 228e3ed9..a9be9f36 100644 --- a/book/book.toml +++ b/book/book.toml @@ -6,6 +6,8 @@ src = "src" title = "cargo-mutants" [output.html] +git-repository-url = "https://github.com/sourcefrog/cargo-mutants" +edit-url-template = "https://github.com/sourcefrog/cargo-mutants/edit/main/book/{path}" [output.linkcheck] follow-web-links = true diff --git a/book/src/build-dirs.md b/book/src/build-dirs.md index 730f42bf..09e562eb 100644 --- a/book/src/build-dirs.md +++ b/book/src/build-dirs.md @@ -17,4 +17,8 @@ Files or directories matching these patterns are not copied: From 23.11.2, by default, cargo-mutants will not copy files that are excluded by gitignore patterns, to make copying faster in large trees. -This behavior can be turned off with `--gitignore=false`. +gitignore filtering is only used within trees containing a `.git` directory. + +The filter, based on the [`ignore` crate](https://docs.rs/ignore/), also respects global git ignore configuration in the home directory, as well as `.gitignore` files within the tree. + +This behavior can be turned off with `--gitignore=false`, causing ignored files to be copied. diff --git a/book/src/mutants-out.md b/book/src/mutants-out.md index 20182c09..e7cfff62 100644 --- a/book/src/mutants-out.md +++ b/book/src/mutants-out.md @@ -1,6 +1,7 @@ # The `mutants.out` directory -A `mutants.out` directory is created in the original source directory. You can put the output directory elsewhere with the `--output` option. +A `mutants.out` directory is created in the original source directory. You can put the output directory elsewhere with the `--output` option +or using `CARGO_MUTANTS_OUTPUT` environment variable or via `output` directive in the config file. On each run, any existing `mutants.out` is renamed to `mutants.out.old`, and any existing `mutants.out.old` is deleted. diff --git a/src/cargo.rs b/src/cargo.rs index 5360017d..79653999 100644 --- a/src/cargo.rs +++ b/src/cargo.rs @@ -2,6 +2,9 @@ //! Run Cargo as a subprocess, including timeouts and propagating signals. +#![warn(clippy::pedantic)] +#![allow(clippy::module_name_repetitions)] + use std::env; use std::iter::once; use std::time::{Duration, Instant}; @@ -16,14 +19,14 @@ use crate::options::{Options, TestTool}; use crate::outcome::{Phase, PhaseResult}; use crate::output::ScenarioOutput; use crate::package::PackageSelection; -use crate::process::{Process, ProcessStatus}; +use crate::process::{Exit, Process}; use crate::Result; /// Run cargo build, check, or test. #[allow(clippy::too_many_arguments)] // I agree it's a lot but I'm not sure wrapping in a struct would be better. pub fn run_cargo( build_dir: &BuildDir, - jobserver: &Option, + jobserver: Option<&jobserver::Client>, packages: &PackageSelection, phase: Phase, timeout: Option, @@ -56,7 +59,7 @@ pub fn run_cargo( )?; check_interrupted()?; debug!(?process_status, elapsed = ?start.elapsed()); - if let ProcessStatus::Failure(code) = process_status { + if let Exit::Failure(code) = process_status { // 100 "one or more tests failed" from ; // I'm not addind a dependency to just get one integer. if argv[1] == "nextest" && code != 100 { @@ -155,12 +158,7 @@ fn cargo_argv(packages: &PackageSelection, phase: Phase, options: &Options) -> V cargo_args.push("--all-features".to_owned()); } // N.B. it can make sense to have --all-features and also explicit features from non-default packages.` - cargo_args.extend( - features - .features - .iter() - .map(|f| format!("--features={}", f)), - ); + cargo_args.extend(features.features.iter().map(|f| format!("--features={f}"))); cargo_args.extend(options.additional_cargo_args.iter().cloned()); if phase == Phase::Test { cargo_args.extend(options.additional_cargo_test_args.iter().cloned()); @@ -168,7 +166,7 @@ fn cargo_argv(packages: &PackageSelection, phase: Phase, options: &Options) -> V cargo_args } -/// Return adjusted CARGO_ENCODED_RUSTFLAGS, including any changes to cap-lints. +/// 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? @@ -240,7 +238,7 @@ mod test { // let relative_manifest_path = Utf8PathBuf::from("testdata/something/Cargo.toml"); options .additional_cargo_test_args - .extend(["--lib", "--no-fail-fast"].iter().map(|s| s.to_string())); + .extend(["--lib", "--no-fail-fast"].iter().map(ToString::to_string)); // TODO: It wolud be a bit better to use `--manifest-path` here, to get // the fix for // but it's temporarily regressed. @@ -292,7 +290,7 @@ mod test { let mut options = Options::default(); options .additional_cargo_test_args - .extend(["--lib", "--no-fail-fast"].iter().map(|s| s.to_string())); + .extend(["--lib", "--no-fail-fast"].iter().map(|&s| s.to_string())); options .additional_cargo_args .extend(["--release".to_owned()]); diff --git a/src/config.rs b/src/config.rs index 65df1599..796d6094 100644 --- a/src/config.rs +++ b/src/config.rs @@ -14,7 +14,7 @@ use std::path::Path; use std::str::FromStr; use anyhow::Context; -use camino::Utf8Path; +use camino::{Utf8Path, Utf8PathBuf}; use serde::Deserialize; use crate::options::TestTool; @@ -45,6 +45,8 @@ pub struct Config { pub additional_cargo_test_args: Vec, /// Minimum test timeout, in seconds, as a floor on the autoset value. pub minimum_test_timeout: Option, + /// Output directory. + pub output: Option, /// Cargo profile. pub profile: Option, /// Skip calls to functions or methods with these names. diff --git a/src/console.rs b/src/console.rs index c6d6c1c0..07062426 100644 --- a/src/console.rs +++ b/src/console.rs @@ -3,13 +3,11 @@ //! Print messages and progress bars on the terminal. use std::borrow::Cow; -use std::fmt::Write; use std::fs::File; use std::io; use std::sync::{Arc, Mutex}; use std::time::{Duration, Instant}; -use anyhow::Context; use camino::{Utf8Path, Utf8PathBuf}; use console::{style, StyledObject}; use humantime::format_duration; @@ -19,10 +17,10 @@ use tracing_subscriber::fmt::MakeWriter; use tracing_subscriber::prelude::*; use crate::options::Colors; -use crate::outcome::{LabOutcome, SummaryOutcome}; +use crate::outcome::{LabOutcome, ScenarioOutcome, SummaryOutcome}; use crate::scenario::Scenario; use crate::tail_file::TailFile; -use crate::{Mutant, Options, Phase, Result, ScenarioOutcome}; +use crate::{Mutant, Options, Phase}; /// An interface to the console for the rest of cargo-mutants. /// @@ -43,14 +41,6 @@ impl Console { } } - pub fn set_colors_enabled(&self, colors: Colors) { - if let Some(colors) = colors.forced_value() { - ::console::set_colors_enabled(colors); - ::console::set_colors_enabled_stderr(colors); - } - // Otherwise, let the console crate decide, based on isatty, etc. - } - pub fn walk_tree_start(&self) { self.view .update(|model| model.walk_tree = Some(WalkModel::default())); @@ -70,18 +60,12 @@ impl Console { } /// Update that a cargo task is starting. - pub fn scenario_started( - &self, - dir: &Utf8Path, - scenario: &Scenario, - log_file: File, - ) -> Result<()> { + pub fn scenario_started(&self, dir: &Utf8Path, scenario: &Scenario, log_file: File) { let start = Instant::now(); - let scenario_model = ScenarioModel::new(dir, scenario, start, log_file)?; + let scenario_model = ScenarioModel::new(dir, scenario, start, log_file); self.view.update(|model| { model.scenario_models.push(scenario_model); }); - Ok(()) } /// Update that cargo finished. @@ -93,7 +77,9 @@ impl Console { options: &Options, ) { self.view.update(|model| { - model.mutants_done += scenario.is_mutant() as usize; + if scenario.is_mutant() { + model.mutants_done += 1; + } match outcome.summary() { SummaryOutcome::CaughtMutant => model.mutants_caught += 1, SummaryOutcome::MissedMutant => model.mutants_missed += 1, @@ -113,14 +99,11 @@ impl Console { return; } - let mut s = String::with_capacity(100); - write!( - s, + let mut s = format!( "{:8} {}", style_outcome(outcome), style_scenario(scenario, true), - ) - .unwrap(); + ); if options.show_times { let prs: Vec = outcome .phase_results() @@ -133,7 +116,8 @@ impl Console { ) }) .collect(); - let _ = write!(s, " in {}", prs.join(" + ")); + s.push_str(" in "); + s.push_str(&prs.join(" + ")); } if outcome.should_show_logs() || options.show_all_logs { s.push('\n'); @@ -172,7 +156,7 @@ impl Console { .iter_mut() .find(|m| m.dest == dest) .expect("copy in progress") - .bytes_copied(total_bytes) + .bytes_copied(total_bytes); }); } @@ -186,7 +170,7 @@ impl Console { self.view.update(|model| { model.n_mutants = n_mutants; model.lab_start_time = Some(Instant::now()); - }) + }); } /// Update that work is starting on testing a given number of mutants. @@ -199,13 +183,13 @@ impl Console { 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: &Utf8Path, phase: Phase) { self.view.update(|model| { model.find_scenario_mut(dir).phase_finished(phase); - }) + }); } pub fn lab_finished(&self, lab_outcome: &LabOutcome, start_time: Instant, options: &Options) { @@ -219,7 +203,7 @@ impl Console { } pub fn clear(&self) { - self.view.clear() + self.view.clear(); } pub fn message(&self, message: &str) { @@ -227,11 +211,11 @@ impl Console { // stderr... // self.view.clear(); - print!("{}", message); + print!("{message}"); } pub fn tick(&self) { - self.view.update(|_| ()) + self.view.update(|_| ()); } /// Return a tracing `MakeWriter` that will send messages via nutmeg to the console. @@ -254,8 +238,8 @@ impl Console { /// Configure tracing to send messages to the console and debug log. /// - /// The debug log is opened later and provided by [Console::set_debug_log]. - pub fn setup_global_trace(&self, console_trace_level: Level, colors: Colors) -> Result<()> { + /// The debug log is opened later and provided by [`Console::set_debug_log`]. + pub fn setup_global_trace(&self, console_trace_level: Level, colors: Colors) { // Show time relative to the start of the program. let uptime = tracing_subscriber::fmt::time::uptime(); let stderr_colors = colors @@ -278,10 +262,17 @@ impl Console { .with(debug_log_layer) .with(console_layer) .init(); - Ok(()) } } +pub fn enable_console_colors(colors: Colors) { + if let Some(colors) = colors.forced_value() { + ::console::set_colors_enabled(colors); + ::console::set_colors_enabled_stderr(colors); + } + // Otherwise, let the console crate decide, based on isatty, etc. +} + impl Default for Console { fn default() -> Self { Self::new() @@ -370,53 +361,53 @@ struct LabModel { } impl nutmeg::Model for LabModel { + #[allow(clippy::cast_precision_loss)] fn render(&mut self, width: usize) -> String { let mut s = String::with_capacity(1024); if let Some(walk_tree) = &mut self.walk_tree { s += &walk_tree.render(width); } - for copy_model in self.copy_models.iter_mut() { + for copy_model in &mut self.copy_models { if !s.is_empty() { - s.push('\n') + s.push('\n'); } s.push_str(©_model.render(width)); } - for sm in self.scenario_models.iter_mut() { + for sm in &mut self.scenario_models { + if !s.is_empty() { + s.push('\n'); + } s.push_str(&sm.render(width)); - s.push('\n'); } if let Some(lab_start_time) = self.lab_start_time { + if !s.is_empty() { + s.push('\n'); + } let elapsed = lab_start_time.elapsed(); - write!( - s, + s += &format!( "{}/{} mutants tested", style(self.mutants_done).cyan(), style(self.n_mutants).cyan(), - ) - .unwrap(); + ); if self.mutants_missed > 0 { - write!( - s, + s += &format!( ", {} {}", style(self.mutants_missed).cyan(), style("MISSED").red() - ) - .unwrap(); + ); } if self.timeouts > 0 { - write!( - s, + s += &format!( ", {} {}", style(self.timeouts).cyan(), style("timeout").red() - ) - .unwrap(); + ); } if self.mutants_caught > 0 { - write!(s, ", {} caught", style(self.mutants_caught).cyan()).unwrap(); + s.push_str(&format!(", {} caught", style(self.mutants_caught).cyan())); } if self.unviable > 0 { - write!(s, ", {} unviable", style(self.unviable).cyan()).unwrap(); + s.push_str(&format!(", {} unviable", style(self.unviable).cyan())); } // Maybe don't report these, because they're uninteresting? // if self.successes > 0 { @@ -425,25 +416,21 @@ impl nutmeg::Model for LabModel { // if self.failures > 0 { // write!(s, ", {} failures", self.failures).unwrap(); // } - write!(s, ", {} elapsed", style_duration(elapsed)).unwrap(); + s.push_str(&format!(", {} elapsed", style_duration(elapsed))); if self.mutants_done > 2 { - let done = self.mutants_done as u64; - let remain = self.n_mutants as u64 - done; - let mut remaining_secs = lab_start_time.elapsed().as_secs() * remain / done; - if remaining_secs > 300 { - remaining_secs = (remaining_secs + 30) / 60 * 60; + let done = self.mutants_done as f64; + let remain = self.n_mutants as f64 - done; + let mut remaining_secs = + self.mutants_start_time.unwrap().elapsed().as_secs_f64() * remain / done; + if remaining_secs > 300.0 { + // Round up to minutes + remaining_secs = ((remaining_secs + 30.0) / 60.0).ceil() * 60.0; } - write!( - s, + s += &format!( ", about {} remaining", - style_duration(Duration::from_secs(remaining_secs)) - ) - .unwrap(); + style_duration(Duration::from_secs_f64(remaining_secs.ceil())) + ); } - writeln!(s).unwrap(); - } - while s.ends_with('\n') { - s.pop(); } s } @@ -497,21 +484,15 @@ struct ScenarioModel { } impl ScenarioModel { - fn new( - dir: &Utf8Path, - scenario: &Scenario, - start: Instant, - log_file: File, - ) -> Result { - let log_tail = TailFile::new(log_file).context("Failed to open log file")?; - Ok(ScenarioModel { + fn new(dir: &Utf8Path, scenario: &Scenario, start: Instant, log_file: File) -> ScenarioModel { + ScenarioModel { dir: dir.to_owned(), name: style_scenario(scenario, true), phase: None, phase_start: start, - log_tail, + log_tail: TailFile::new(log_file), previous_phase_durations: Vec::new(), - }) + } } fn phase_started(&mut self, phase: Phase) { @@ -547,7 +528,11 @@ impl nutmeg::Model for ScenarioModel { // parts.push(prs.join(" + ")); let mut s = parts.join(" "); if let Ok(last_line) = self.log_tail.last_line() { - write!(s, "\n{:8} {}", style("└").cyan(), style(last_line).dim()).unwrap(); + s.push_str(&format!( + "\n{:8} {}", + style("└").cyan(), + style(last_line).dim() + )); } s } @@ -573,7 +558,7 @@ impl CopyModel { /// /// `bytes_copied` is the total bytes copied so far. fn bytes_copied(&mut self, bytes_copied: u64) { - self.bytes_copied = bytes_copied + self.bytes_copied = bytes_copied; } } diff --git a/src/copy_tree.rs b/src/copy_tree.rs index ea759fd1..37661d7c 100644 --- a/src/copy_tree.rs +++ b/src/copy_tree.rs @@ -2,8 +2,6 @@ //! Copy a source tree, with some exclusions, to a new temporary directory. -use std::fs::FileType; - use anyhow::Context; use camino::{Utf8Path, Utf8PathBuf}; use ignore::WalkBuilder; @@ -11,9 +9,17 @@ use path_slash::PathExt; use tempfile::TempDir; use tracing::{debug, warn}; -use crate::check_interrupted; -use crate::Console; -use crate::Result; +use crate::{check_interrupted, Console, Result}; + +#[cfg(unix)] +mod unix; +#[cfg(unix)] +use unix::copy_symlink; + +#[cfg(windows)] +mod windows; +#[cfg(windows)] +use windows::copy_symlink; /// Filenames excluded from being copied with the source. static SOURCE_EXCLUDE: &[&str] = &[ @@ -32,7 +38,7 @@ static SOURCE_EXCLUDE: &[&str] = &[ /// If `git` is true, ignore files that are excluded by all the various `.gitignore` /// files. /// -/// Regardless, anything matching [SOURCE_EXCLUDE] is excluded. +/// Regardless, anything matching [`SOURCE_EXCLUDE`] is excluded. pub fn copy_tree( from_path: &Utf8Path, name_base: &str, @@ -51,16 +57,17 @@ pub fn copy_tree( .try_into() .context("Convert path to UTF-8")?; console.start_copy(dest); - for entry in WalkBuilder::new(from_path) + let mut walk_builder = WalkBuilder::new(from_path); + walk_builder .standard_filters(gitignore) - .hidden(false) - .ignore(false) - .require_git(false) + .hidden(false) // copy hidden files + .ignore(false) // don't use .ignore + .require_git(true) // stop at git root; only read gitignore files inside git trees .filter_entry(|entry| { !SOURCE_EXCLUDE.contains(&entry.file_name().to_string_lossy().as_ref()) - }) - .build() - { + }); + debug!(?walk_builder); + for entry in walk_builder.build() { check_interrupted()?; let entry = entry?; let relative_path = entry @@ -106,29 +113,38 @@ pub fn copy_tree( Ok(temp_dir) } -#[cfg(unix)] -fn copy_symlink(_ft: FileType, src_path: &Utf8Path, dest_path: &Utf8Path) -> Result<()> { - let link_target = std::fs::read_link(src_path) - .with_context(|| format!("Failed to read link {src_path:?}"))?; - std::os::unix::fs::symlink(link_target, dest_path) - .with_context(|| format!("Failed to create symlink {dest_path:?}",))?; - Ok(()) -} +#[cfg(test)] +mod test { + use std::fs::{create_dir, write}; -#[cfg(windows)] -#[mutants::skip] // Mutant tests run on Linux -fn copy_symlink(ft: FileType, src_path: &Utf8Path, dest_path: &Utf8Path) -> Result<()> { - use std::os::windows::fs::FileTypeExt; - let link_target = - std::fs::read_link(src_path).with_context(|| format!("read link {src_path:?}"))?; - if ft.is_symlink_dir() { - std::os::windows::fs::symlink_dir(link_target, dest_path) - .with_context(|| format!("create symlink {dest_path:?}"))?; - } else if ft.is_symlink_file() { - std::os::windows::fs::symlink_file(link_target, dest_path) - .with_context(|| format!("create symlink {dest_path:?}"))?; - } else { - anyhow::bail!("Unknown symlink type: {:?}", ft); + use camino::Utf8PathBuf; + use tempfile::TempDir; + + use crate::console::Console; + use crate::Result; + + use super::copy_tree; + + /// Test for regression of + #[test] + fn copy_tree_with_parent_ignoring_star() -> Result<()> { + let tmp_dir = TempDir::new().unwrap(); + let tmp = tmp_dir.path(); + write(tmp.join(".gitignore"), "*\n")?; + + let a = Utf8PathBuf::try_from(tmp.join("a")).unwrap(); + create_dir(&a)?; + write(a.join("Cargo.toml"), "[package]\nname = a")?; + let src = a.join("src"); + create_dir(&src)?; + write(src.join("main.rs"), "fn main() {}")?; + + let dest_tmpdir = copy_tree(&a, "a", true, &Console::new())?; + let dest = dest_tmpdir.path(); + assert!(dest.join("Cargo.toml").is_file()); + assert!(dest.join("src").is_dir()); + assert!(dest.join("src/main.rs").is_file()); + + Ok(()) } - Ok(()) } diff --git a/src/copy_tree/unix.rs b/src/copy_tree/unix.rs new file mode 100644 index 00000000..9d35bc38 --- /dev/null +++ b/src/copy_tree/unix.rs @@ -0,0 +1,14 @@ +use std::fs::FileType; + +use anyhow::Context; +use camino::Utf8Path; + +use crate::Result; + +pub(super) fn copy_symlink(_ft: FileType, src_path: &Utf8Path, dest_path: &Utf8Path) -> Result<()> { + let link_target = std::fs::read_link(src_path) + .with_context(|| format!("Failed to read link {src_path:?}"))?; + std::os::unix::fs::symlink(link_target, dest_path) + .with_context(|| format!("Failed to create symlink {dest_path:?}",))?; + Ok(()) +} diff --git a/src/copy_tree/windows.rs b/src/copy_tree/windows.rs new file mode 100644 index 00000000..19b91470 --- /dev/null +++ b/src/copy_tree/windows.rs @@ -0,0 +1,22 @@ +use std::fs::FileType; +use std::os::windows::fs::FileTypeExt; + +use anyhow::Context; +use camino::Utf8Path; + +use crate::Result; +#[mutants::skip] // Mutant tests run on Linux +pub(super) fn copy_symlink(ft: FileType, src_path: &Utf8Path, dest_path: &Utf8Path) -> Result<()> { + let link_target = + std::fs::read_link(src_path).with_context(|| format!("read link {src_path:?}"))?; + if ft.is_symlink_dir() { + std::os::windows::fs::symlink_dir(link_target, dest_path) + .with_context(|| format!("create symlink {dest_path:?}"))?; + } else if ft.is_symlink_file() { + std::os::windows::fs::symlink_file(link_target, dest_path) + .with_context(|| format!("create symlink {dest_path:?}"))?; + } else { + anyhow::bail!("Unknown symlink type: {:?}", ft); + } + Ok(()) +} diff --git a/src/fnvalue.rs b/src/fnvalue.rs index c5e1baab..1408b596 100644 --- a/src/fnvalue.rs +++ b/src/fnvalue.rs @@ -2,6 +2,8 @@ //! Mutations of replacing a function body with a value of a (hopefully) appropriate type. +#![warn(clippy::pedantic)] + use std::iter; use itertools::Itertools; @@ -25,8 +27,7 @@ pub(crate) fn return_type_replacements( } /// Generate some values that we hope are reasonable replacements for a type. -/// -/// This is really the heart of cargo-mutants. +#[allow(clippy::too_many_lines)] fn type_replacements(type_: &Type, error_exprs: &[Expr]) -> impl Iterator { // This could probably change to run from some configuration rather than // hardcoding various types, which would make it easier to support tree-specific @@ -292,7 +293,7 @@ fn known_container(path: &Path) -> Option<(&Ident, &Type)> { /// Match known simple collections that can be empty or constructed from an /// iterator. /// -/// Returns the short name (like "VecDeque") and the inner type. +/// Returns the short name (like `"VecDeque"`) and the inner type. fn known_collection(path: &Path) -> Option<(&Ident, &Type)> { let last = path.segments.last()?; if ![ @@ -445,7 +446,7 @@ mod test { #[test] fn recurse_into_result_bool() { check_replacements( - parse_quote! {-> std::result::Result }, + &parse_quote! {-> std::result::Result }, &[], &["Ok(true)", "Ok(false)"], ); @@ -454,7 +455,7 @@ mod test { #[test] fn recurse_into_result_result_bool_with_error_values() { check_replacements( - parse_quote! {-> std::result::Result> }, + &parse_quote! {-> std::result::Result> }, &[parse_quote! { anyhow!("mutated") }], &[ "Ok(Ok(true))", @@ -467,43 +468,43 @@ mod test { #[test] fn u16_replacements() { - check_replacements(parse_quote! { -> u16 }, &[], &["0", "1"]); + check_replacements(&parse_quote! { -> u16 }, &[], &["0", "1"]); } #[test] fn isize_replacements() { - check_replacements(parse_quote! { -> isize }, &[], &["0", "1", "-1"]); + check_replacements(&parse_quote! { -> isize }, &[], &["0", "1", "-1"]); } #[test] fn nonzero_integer_replacements() { check_replacements( - parse_quote! { -> std::num::NonZeroIsize }, + &parse_quote! { -> std::num::NonZeroIsize }, &[], &["1", "-1"], ); - check_replacements(parse_quote! { -> std::num::NonZeroUsize }, &[], &["1"]); + check_replacements(&parse_quote! { -> std::num::NonZeroUsize }, &[], &["1"]); - check_replacements(parse_quote! { -> std::num::NonZeroU32 }, &[], &["1"]); + check_replacements(&parse_quote! { -> std::num::NonZeroU32 }, &[], &["1"]); } #[test] fn unit_replacement() { - check_replacements(parse_quote! { -> () }, &[], &["()"]); + check_replacements(&parse_quote! { -> () }, &[], &["()"]); } #[test] fn result_unit_replacement() { - check_replacements(parse_quote! { -> Result<(), Error> }, &[], &["Ok(())"]); + check_replacements(&parse_quote! { -> Result<(), Error> }, &[], &["Ok(())"]); - check_replacements(parse_quote! { -> Result<()> }, &[], &["Ok(())"]); + check_replacements(&parse_quote! { -> Result<()> }, &[], &["Ok(())"]); } #[test] fn http_response_replacement() { check_replacements( - parse_quote! { -> HttpResponse }, + &parse_quote! { -> HttpResponse }, &[], &["HttpResponse::Ok().finish()"], ); @@ -512,7 +513,7 @@ mod test { #[test] fn option_usize_replacement() { check_replacements( - parse_quote! { -> Option }, + &parse_quote! { -> Option }, &[], &["None", "Some(0)", "Some(1)"], ); @@ -521,7 +522,7 @@ mod test { #[test] fn box_usize_replacement() { check_replacements( - parse_quote! { -> Box }, + &parse_quote! { -> Box }, &[], &["Box::new(0)", "Box::new(1)"], ); @@ -530,7 +531,7 @@ mod test { #[test] fn box_unrecognized_type_replacement() { check_replacements( - parse_quote! { -> Box }, + &parse_quote! { -> Box }, &[], &["Box::new(Default::default())"], ); @@ -539,7 +540,7 @@ mod test { #[test] fn vec_string_replacement() { check_replacements( - parse_quote! { -> std::vec::Vec }, + &parse_quote! { -> std::vec::Vec }, &[], &["vec![]", "vec![String::new()]", r#"vec!["xyzzy".into()]"#], ); @@ -547,18 +548,18 @@ mod test { #[test] fn float_replacement() { - check_replacements(parse_quote! { -> f32 }, &[], &["0.0", "1.0", "-1.0"]); + check_replacements(&parse_quote! { -> f32 }, &[], &["0.0", "1.0", "-1.0"]); } #[test] fn ref_replacement_recurses() { - check_replacements(parse_quote! { -> &bool }, &[], &["&true", "&false"]); + check_replacements(&parse_quote! { -> &bool }, &[], &["&true", "&false"]); } #[test] fn array_replacement() { check_replacements( - parse_quote! { -> [u8; 256] }, + &parse_quote! { -> [u8; 256] }, &[], &["[0; 256]", "[1; 256]"], ); @@ -569,7 +570,7 @@ mod test { // Also checks that it matches the path, even using an atypical path. // TODO: Ideally this would be fully qualified like `alloc::sync::Arc::new(String::new())`. check_replacements( - parse_quote! { -> alloc::sync::Arc }, + &parse_quote! { -> alloc::sync::Arc }, &[], &["Arc::new(String::new())", r#"Arc::new("xyzzy".into())"#], ); @@ -580,7 +581,7 @@ mod test { // Also checks that it matches the path, even using an atypical path. // TODO: Ideally this would be fully qualified like `alloc::sync::Rc::new(String::new())`. check_replacements( - parse_quote! { -> alloc::sync::Rc }, + &parse_quote! { -> alloc::sync::Rc }, &[], &["Rc::new(String::new())", r#"Rc::new("xyzzy".into())"#], ); @@ -652,7 +653,7 @@ mod test { #[test] fn btreeset_replacement() { check_replacements( - parse_quote! { -> std::collections::BTreeSet }, + &parse_quote! { -> std::collections::BTreeSet }, &[], &[ "BTreeSet::new()", @@ -665,7 +666,7 @@ mod test { #[test] fn cow_generates_borrowed_and_owned() { check_replacements( - parse_quote! { -> Cow<'static, str> }, + &parse_quote! { -> Cow<'static, str> }, &[], &[ r#"Cow::Borrowed("")"#, @@ -681,7 +682,7 @@ mod test { // This looks like something that holds a &str, and maybe can be constructed // from a &str, but we don't know anything else about it, so we just guess. check_replacements( - parse_quote! { -> UnknownContainer<'static, str> }, + &parse_quote! { -> UnknownContainer<'static, str> }, &[], &[ "UnknownContainer::new()", @@ -698,16 +699,16 @@ mod test { #[test] fn tuple_combinations() { check_replacements( - parse_quote! { -> (bool, usize) }, + &parse_quote! { -> (bool, usize) }, &[], &["(true, 0)", "(true, 1)", "(false, 0)", "(false, 1)"], - ) + ); } #[test] fn tuple_combination_longer() { check_replacements( - parse_quote! { -> (bool, Option) }, + &parse_quote! { -> (bool, Option) }, &[], &[ "(true, None)", @@ -717,13 +718,13 @@ mod test { "(false, Some(String::new()))", r#"(false, Some("xyzzy".into()))"#, ], - ) + ); } #[test] fn iter_replacement() { check_replacements( - parse_quote! { -> impl Iterator }, + &parse_quote! { -> impl Iterator }, &[], &[ "::std::iter::empty()", @@ -755,7 +756,7 @@ mod test { #[test] fn slice_replacement() { check_replacements( - parse_quote! { -> [u8] }, + &parse_quote! { -> [u8] }, &[], &[ "Vec::leak(Vec::new())", @@ -768,7 +769,7 @@ mod test { #[test] fn btreemap_replacement() { check_replacements( - parse_quote! { -> BTreeMap }, + &parse_quote! { -> BTreeMap }, &[], &[ "BTreeMap::new()", @@ -780,9 +781,9 @@ mod test { ); } - fn check_replacements(return_type: ReturnType, error_exprs: &[Expr], expected: &[&str]) { + fn check_replacements(return_type: &ReturnType, error_exprs: &[Expr], expected: &[&str]) { assert_eq!( - return_type_replacements(&return_type, error_exprs) + return_type_replacements(return_type, error_exprs) .into_iter() .map(|t| t.to_pretty_string()) .collect_vec(), diff --git a/src/in_diff.rs b/src/in_diff.rs index ab9a30e7..84efe18e 100644 --- a/src/in_diff.rs +++ b/src/in_diff.rs @@ -51,7 +51,7 @@ pub fn diff_filter(mutants: Vec, diff_text: &str) -> Result> } } let mut matched: Vec = Vec::with_capacity(mutants.len()); - 'mutant: for mutant in mutants.into_iter() { + 'mutant: for mutant in mutants { let path = mutant.source_file.path(); if let Some(lines_changed) = lines_changed_by_path.get(path) { // We could do be smarter about searching for an intersection of ranges, rather @@ -206,8 +206,8 @@ fn partial_new_file<'d>(patch: &Patch<'d>) -> Vec<(usize, &'d str)> { } } debug_assert_eq!( - lineno, - (hunk.new_range.start + hunk.new_range.count) as usize, + Ok(lineno), + (hunk.new_range.start + hunk.new_range.count).try_into(), "Wrong number of resulting lines?" ); } @@ -271,7 +271,7 @@ index eb42779..a0091b7 100644 #[test] fn affected_lines_from_single_insertion() { - let orig_lines = (1..=4).map(|i| format!("line {}\n", i)).collect_vec(); + let orig_lines = (1..=4).map(|i| format!("line {i}\n")).collect_vec(); for i in 1..=5 { let mut new = orig_lines.clone(); let new_value = "new line\n".to_owned(); @@ -291,7 +291,7 @@ index eb42779..a0091b7 100644 #[test] fn affected_lines_from_single_deletion() { - let orig_lines = (1..=5).map(|i| format!("line {}\n", i)).collect_vec(); + let orig_lines = (1..=5).map(|i| format!("line {i}\n")).collect_vec(); for i in 1..=5 { let mut new = orig_lines.clone(); new.remove(i - 1); @@ -312,7 +312,7 @@ index eb42779..a0091b7 100644 #[test] fn affected_lines_from_double_deletion() { - let orig_lines = (1..=5).map(|i| format!("line {}\n", i)).collect_vec(); + let orig_lines = (1..=5).map(|i| format!("line {i}\n")).collect_vec(); for i in 1..=4 { let mut new = orig_lines.clone(); new.remove(i - 1); @@ -332,7 +332,7 @@ index eb42779..a0091b7 100644 #[test] fn affected_lines_from_replacement() { - let orig_lines = (1..=5).map(|i| format!("line {}\n", i)).collect_vec(); + let orig_lines = (1..=5).map(|i| format!("line {i}\n")).collect_vec(); for i in 1..=5 { let insertion = ["new 1\n".to_owned(), "new 2\n".to_owned()]; let new = orig_lines[..(i - 1)] diff --git a/src/lab.rs b/src/lab.rs index cd487a3e..81873a79 100644 --- a/src/lab.rs +++ b/src/lab.rs @@ -203,7 +203,7 @@ impl Lab<'_> { Worker { build_dir, output_mutex: &self.output_mutex, - jobserver: &self.jobserver, + jobserver: self.jobserver.as_ref(), options: self.options, console: self.console, } @@ -217,7 +217,7 @@ impl Lab<'_> { struct Worker<'a> { build_dir: &'a BuildDir, output_mutex: &'a Mutex, - jobserver: &'a Option, + jobserver: Option<&'a jobserver::Client>, options: &'a Options, console: &'a Console, } @@ -262,7 +262,7 @@ impl Worker<'_> { .start_scenario(scenario)?; let dir = self.build_dir.path(); self.console - .scenario_started(dir, scenario, scenario_output.open_log_read()?)?; + .scenario_started(dir, scenario, scenario_output.open_log_read()?); if let Some(mutant) = scenario.mutant() { let mutated_code = mutant.mutated_code(); diff --git a/src/main.rs b/src/main.rs index 4c193d02..74629093 100644 --- a/src/main.rs +++ b/src/main.rs @@ -4,6 +4,9 @@ //! //! See for the manual and more information. +#![warn(clippy::pedantic)] +#![allow(clippy::module_name_repetitions, clippy::needless_raw_string_hashes)] + mod build_dir; mod cargo; mod config; @@ -49,6 +52,7 @@ use clap::builder::Styles; use clap::{ArgAction, CommandFactory, Parser, ValueEnum}; use clap_complete::{generate, Shell}; use color_print::cstr; +use console::enable_console_colors; use output::{load_previously_caught, OutputDir}; use tracing::{debug, info}; @@ -102,6 +106,7 @@ pub enum BaselineStrategy { /// Find inadequately-tested code that can be removed without any tests failing. /// /// See for more information. +#[allow(clippy::struct_excessive_bools)] #[derive(Parser, PartialEq, Debug)] #[command( author, @@ -269,7 +274,12 @@ pub struct Args { line_col: bool, /// Create mutants.out within this directory. - #[arg(long, short = 'o', help_heading = "Output")] + #[arg( + long, + short = 'o', + env = "CARGO_MUTANTS_OUTPUT", + help_heading = "Output" + )] output: Option, /// Include only mutants in code touched by this diff. @@ -425,8 +435,8 @@ fn main() -> Result<()> { } let console = Console::new(); - console.setup_global_trace(args.level, args.colors)?; // We don't have Options yet. - console.set_colors_enabled(args.colors); + console.setup_global_trace(args.level, args.colors); // We don't have Options yet. + enable_console_colors(args.colors); interrupt::install_handler(); let start_dir: &Utf8Path = if let Some(manifest_path) = &args.manifest_path { @@ -514,7 +524,7 @@ mod test { let args = super::Args::command(); let mut problems = Vec::new(); for arg in args.get_arguments() { - if let Some(help) = arg.get_help().map(|s| s.to_string()) { + if let Some(help) = arg.get_help().map(ToString::to_string) { if !help.starts_with(char::is_uppercase) { problems.push(format!( "Help for {:?} does not start with a capital letter: {:?}", @@ -538,9 +548,9 @@ mod test { problems.push(format!("No help for {:?}", arg.get_id())); } } - problems.iter().for_each(|s| eprintln!("{s}")); - if !problems.is_empty() { - panic!("Problems with help text"); + for problem in &problems { + eprintln!("{problem}"); } + assert!(problems.is_empty(), "Problems with help text"); } } diff --git a/src/manifest.rs b/src/manifest.rs index 15152aba..50894202 100644 --- a/src/manifest.rs +++ b/src/manifest.rs @@ -1,11 +1,11 @@ -// Copyright 2022-2023 Martin Pool. +// Copyright 2022-2024 Martin Pool. //! Manipulate Cargo manifest and config files. //! //! In particular, when the tree is copied we have to fix up relative paths, so //! that they still work from the new location of the scratch directory. -use std::fs; +use std::fs::{read_to_string, write}; use anyhow::Context; use camino::Utf8Path; @@ -18,12 +18,17 @@ use crate::Result; /// /// `manifest_source_dir` is the directory originally containing the manifest, from /// which the absolute paths are calculated. +#[allow(clippy::module_name_repetitions)] pub fn fix_manifest(manifest_scratch_path: &Utf8Path, source_dir: &Utf8Path) -> Result<()> { - let toml_str = fs::read_to_string(manifest_scratch_path).context("read manifest")?; + let toml_str = read_to_string(manifest_scratch_path).with_context(|| { + format!("failed to read manifest from build directory: {manifest_scratch_path}") + })?; if let Some(changed_toml) = fix_manifest_toml(&toml_str, source_dir)? { let toml_str = toml::to_string_pretty(&changed_toml).context("serialize changed manifest")?; - fs::write(manifest_scratch_path, toml_str.as_bytes()).context("write manifest")?; + write(manifest_scratch_path, toml_str.as_bytes()).with_context(|| { + format!("Failed to write fixed manifest to {manifest_scratch_path}") + })?; } Ok(()) } @@ -101,9 +106,9 @@ fn fix_dependency_table(dependencies: &mut toml::Value, manifest_source_dir: &Ut pub fn fix_cargo_config(build_path: &Utf8Path, source_path: &Utf8Path) -> Result<()> { let config_path = build_path.join(".cargo/config.toml"); if config_path.exists() { - let toml_str = fs::read_to_string(&config_path).context("read .cargo/config.toml")?; + let toml_str = read_to_string(&config_path).context("read .cargo/config.toml")?; if let Some(changed_toml) = fix_cargo_config_toml(&toml_str, source_path)? { - fs::write(build_path.join(&config_path), changed_toml.as_bytes()) + write(build_path.join(&config_path), changed_toml.as_bytes()) .context("write .cargo/config.toml")?; } } diff --git a/src/mutate.rs b/src/mutate.rs index fd5c3525..a21634ef 100644 --- a/src/mutate.rs +++ b/src/mutate.rs @@ -57,6 +57,7 @@ pub struct Mutant { #[derive(Eq, PartialEq, Debug, Serialize)] pub struct Function { /// The function that's being mutated, including any containing namespaces. + #[allow(clippy::struct_field_names)] pub function_name: String, /// The return type of the function, including a leading "-> ", as a fragment of Rust syntax. @@ -84,8 +85,7 @@ impl Mutant { self.styled_parts() .into_iter() .map(|x| x.force_styling(false).to_string()) - .collect::>() - .join("") + .collect::() } pub fn name(&self, show_line_col: bool) -> String { @@ -126,6 +126,7 @@ impl Mutant { fn styled_parts(&self) -> Vec> { // This is like `impl Display for Mutant`, but with colors. // The text content should be the same. + #[allow(clippy::needless_pass_by_value)] // actually is needed for String vs &str? fn s(s: S) -> StyledObject { style(s.to_string()) } @@ -186,7 +187,7 @@ impl Mutant { .to_string() } - /// Apply this mutant to the relevant file within a BuildDir. + /// Apply this mutant to the relevant file within a `BuildDir`. pub fn apply(&self, build_dir: &BuildDir, mutated_code: &str) -> Result<()> { trace!(?self, "Apply mutant"); build_dir.overwrite_file(&self.source_file.tree_relative_path, mutated_code) @@ -236,7 +237,7 @@ impl Serialize for Mutant { let mut ss = serializer.serialize_struct("Mutant", 7)?; ss.serialize_field("package", &self.source_file.package_name)?; ss.serialize_field("file", &self.source_file.tree_relative_slashes())?; - ss.serialize_field("function", &self.function.as_ref().map(|a| a.as_ref()))?; + ss.serialize_field("function", &self.function.as_ref().map(Arc::as_ref))?; ss.serialize_field("span", &self.span)?; ss.serialize_field("replacement", &self.replacement)?; ss.serialize_field("genre", &self.genre)?; @@ -327,21 +328,21 @@ mod test { .unwrap() .mutants; let descriptions = mutants.iter().map(Mutant::describe_change).collect_vec(); - insta::assert_snapshot!( - descriptions.join("\n"), - @r###" - replace controlled_loop with () - replace > with == in controlled_loop - replace > with < in controlled_loop - replace * with + in controlled_loop - replace * with / in controlled_loop - "### + assert_eq!( + descriptions, + [ + "replace controlled_loop with ()", + "replace > with == in controlled_loop", + "replace > with < in controlled_loop", + "replace * with + in controlled_loop", + "replace * with / in controlled_loop", + ] ); } #[test] fn always_skip_constructors_called_new() { - let code = indoc! { r#" + let code = indoc! { r" struct S { x: i32, } @@ -351,7 +352,7 @@ mod test { Self { x } } } - "# }; + " }; let mutants = mutate_source_str(code, &Options::default()).unwrap(); assert_eq!(mutants, []); } @@ -422,6 +423,6 @@ mod test { fn strip_trailing_space(s: &str) -> String { // Split on \n so that we retain empty lines etc - s.split('\n').map(|l| l.trim_end()).join("\n") + s.split('\n').map(str::trim_end).join("\n") } } diff --git a/src/options.rs b/src/options.rs index 0f596736..dfccdc7d 100644 --- a/src/options.rs +++ b/src/options.rs @@ -8,10 +8,14 @@ //! 2. Config options (read from `.cargo/mutants.toml`) //! 3. Built-in defaults +#![warn(clippy::pedantic)] + +use std::env; #[cfg(test)] use std::ffi::OsString; use std::time::Duration; +use camino::Utf8PathBuf; use globset::GlobSet; use regex::RegexSet; use serde::Deserialize; @@ -21,11 +25,12 @@ use tracing::warn; use crate::config::Config; use crate::glob::build_glob_set; -use crate::*; +use crate::{Args, BaselineStrategy, Context, Phase, Result, ValueEnum}; /// Options for mutation testing, based on both command-line arguments and the /// config file. #[derive(Default, Debug, Clone)] +#[allow(clippy::struct_excessive_bools)] pub struct Options { /// Run tests in an unmutated tree? pub baseline: BaselineStrategy, @@ -197,7 +202,7 @@ impl Colors { /// /// Otherwise, return None, meaning we should decide based on the /// detected terminal characteristics. - pub fn forced_value(&self) -> Option { + pub fn forced_value(self) -> Option { // From https://bixense.com/clicolors/ if env::var("NO_COLOR").is_ok_and(|x| x != "0") { Some(false) @@ -213,7 +218,7 @@ impl Colors { } #[mutants::skip] // depends on a real tty etc, hard to test - pub fn active_stdout(&self) -> bool { + pub fn active_stdout(self) -> bool { self.forced_value() .unwrap_or_else(::console::colors_enabled) } @@ -240,7 +245,7 @@ impl Options { args.test_package .iter() .flat_map(|s| s.split(',')) - .map(|s| s.to_string()) + .map(ToString::to_string) .collect(), ) } else if args.test_workspace.is_none() && config.test_workspace == Some(true) { @@ -255,7 +260,7 @@ impl Options { .skip_calls .iter() .flat_map(|s| s.split(',')) - .map(|s| s.to_string()) + .map(ToString::to_string) .chain(config.skip_calls.iter().cloned()) .collect(); if args @@ -297,7 +302,7 @@ impl Options { jobserver_tasks: args.jobserver_tasks, leak_dirs: args.leak_dirs, minimum_test_timeout, - output_in_dir: args.output.clone(), + output_in_dir: args.output.clone().or(config.output.clone()), print_caught: args.caught, print_unviable: args.unviable, profile: args.profile.as_ref().or(config.profile.as_ref()).cloned(), @@ -334,6 +339,8 @@ impl Options { /// If the arguments are invalid. #[cfg(test)] pub fn from_arg_strs, S: Into + Clone>(args: I) -> Options { + use crate::Args; + use clap::Parser; let args = Args::try_parse_from(args).expect("Failed to parse args"); Options::from_args(&args).expect("Build options from args") } @@ -373,11 +380,13 @@ mod test { use std::io::Write; use std::str::FromStr; + use clap::Parser; use indoc::indoc; use rusty_fork::rusty_fork_test; use tempfile::NamedTempFile; use super::*; + use crate::Args; #[test] fn default_options() { @@ -441,10 +450,10 @@ mod test { #[test] fn cli_timeout_multiplier_overrides_config() { - let config = indoc! { r#" + let config = indoc! { r" timeout_multiplier = 1.0 build_timeout_multiplier = 2.0 - "#}; + "}; let mut config_file = NamedTempFile::new().unwrap(); config_file.write_all(config.as_bytes()).unwrap(); let args = Args::parse_from([ @@ -678,9 +687,9 @@ mod test { #[test] fn test_workspace_config_true() { let args = Args::try_parse_from(["mutants"]).unwrap(); - let config = indoc! { r#" + let config = indoc! { r" test_workspace = true - "#}; + "}; let config = Config::from_str(config).unwrap(); let options = Options::new(&args, &config).unwrap(); assert_eq!(options.test_package, TestPackages::Workspace); @@ -689,9 +698,9 @@ mod test { #[test] fn test_workspace_config_false() { let args = Args::try_parse_from(["mutants"]).unwrap(); - let config = indoc! { r#" + let config = indoc! { r" test_workspace = false - "#}; + "}; let config = Config::from_str(config).unwrap(); let options = Options::new(&args, &config).unwrap(); assert_eq!(options.test_package, TestPackages::Mutated); @@ -700,9 +709,9 @@ mod test { #[test] fn test_workspace_args_override_config_true() { let args = Args::try_parse_from(["mutants", "--test-workspace=true"]).unwrap(); - let config = indoc! { r#" + let config = indoc! { r" test_workspace = false - "#}; + "}; let config = Config::from_str(config).unwrap(); let options = Options::new(&args, &config).unwrap(); assert_eq!(options.test_package, TestPackages::Workspace); @@ -711,9 +720,9 @@ mod test { #[test] fn test_workspace_args_override_config_false() { let args = Args::try_parse_from(["mutants", "--test-workspace=false"]).unwrap(); - let config = indoc! { r#" + let config = indoc! { r" test_workspace = true - "#}; + "}; let config = Config::from_str(config).unwrap(); let options = Options::new(&args, &config).unwrap(); assert_eq!(options.test_package, TestPackages::Mutated); @@ -800,10 +809,10 @@ mod test { // You can configure off the default `with_capacity` skip_calls let args = Args::try_parse_from(["mutants"]).unwrap(); let config = Config::from_str( - r#" + r" skip_calls_defaults = false skip_calls = [] - "#, + ", ) .unwrap(); let options = Options::new(&args, &config).unwrap(); diff --git a/src/outcome.rs b/src/outcome.rs index 72413032..90e6c22c 100644 --- a/src/outcome.rs +++ b/src/outcome.rs @@ -3,9 +3,11 @@ //! The outcome of running a single mutation scenario, or a whole lab. use std::fmt; -use std::time::Duration; -use std::time::Instant; +use std::fs::read_to_string; +use std::time::{Duration, Instant}; +use anyhow::Context; +use camino::Utf8PathBuf; use humantime::format_duration; use output::ScenarioOutput; use serde::ser::SerializeStruct; @@ -14,8 +16,8 @@ use serde::Serializer; use tracing::warn; use crate::console::plural; -use crate::process::ProcessStatus; -use crate::*; +use crate::process::Exit; +use crate::{exit_code, output, Options, Result, Scenario}; /// What phase of running a scenario. /// @@ -36,7 +38,7 @@ pub enum Phase { } impl Phase { - pub fn name(&self) -> &'static str { + pub fn name(self) -> &'static str { match self { Phase::Check => "check", Phase::Build => "build", @@ -53,6 +55,7 @@ impl fmt::Display for Phase { /// The outcome from a whole lab run containing multiple mutants. #[derive(Debug, Default, Serialize)] +#[allow(clippy::module_name_repetitions)] pub struct LabOutcome { /// All the scenario outcomes, including baseline builds. pub outcomes: Vec, @@ -140,6 +143,7 @@ impl LabOutcome { /// The result of running one mutation scenario. #[derive(Debug, Clone, Eq, PartialEq)] +#[allow(clippy::module_name_repetitions)] pub struct ScenarioOutcome { /// A file holding the text output from running this test. // TODO: Maybe this should be a log object? @@ -173,9 +177,9 @@ impl Serialize for ScenarioOutcome { impl ScenarioOutcome { pub fn new(scenario_output: &ScenarioOutput, scenario: Scenario) -> ScenarioOutcome { ScenarioOutcome { - output_dir: scenario_output.output_dir.to_owned(), + output_dir: scenario_output.output_dir.clone(), log_path: scenario_output.log_path().to_owned(), - diff_path: scenario_output.diff_path.to_owned(), + diff_path: scenario_output.diff_path.clone(), scenario, phase_results: Vec::new(), } @@ -193,7 +197,7 @@ impl ScenarioOutcome { self.phase_results.last().unwrap().phase } - pub fn last_phase_result(&self) -> ProcessStatus { + pub fn last_phase_result(&self) -> Exit { self.phase_results.last().unwrap().process_status } @@ -284,7 +288,7 @@ pub struct PhaseResult { /// How long did it take? pub duration: Duration, /// Did it succeed? - pub process_status: ProcessStatus, + pub process_status: Exit, /// What command was run, as an argv list. pub argv: Vec, } @@ -311,6 +315,7 @@ impl Serialize for PhaseResult { /// Overall summary outcome for one mutant. #[derive(Debug, Clone, Eq, PartialEq, Serialize, Hash)] +#[allow(clippy::module_name_repetitions)] pub enum SummaryOutcome { Success, CaughtMutant, @@ -324,7 +329,7 @@ pub enum SummaryOutcome { mod test { use std::time::Duration; - use crate::process::ProcessStatus; + use crate::process::Exit; use super::{Phase, PhaseResult, Scenario, ScenarioOutcome}; @@ -339,13 +344,13 @@ mod test { PhaseResult { phase: Phase::Build, duration: Duration::from_secs(2), - process_status: ProcessStatus::Success, + process_status: Exit::Success, argv: vec!["cargo".into(), "build".into()], }, PhaseResult { phase: Phase::Test, duration: Duration::from_secs(3), - process_status: ProcessStatus::Success, + process_status: Exit::Success, argv: vec!["cargo".into(), "test".into()], }, ], @@ -355,7 +360,7 @@ mod test { Some(&PhaseResult { phase: Phase::Build, duration: Duration::from_secs(2), - process_status: ProcessStatus::Success, + process_status: Exit::Success, argv: vec!["cargo".into(), "build".into()], }) ); diff --git a/src/output.rs b/src/output.rs index 1505489a..71fd0616 100644 --- a/src/output.rs +++ b/src/output.rs @@ -2,14 +2,14 @@ //! A `mutants.out` directory holding logs and other output. -use std::collections::hash_map::Entry; -use std::collections::HashMap; -use std::fs::{create_dir, remove_dir_all, rename, write, File, OpenOptions}; +use std::collections::{hash_map::Entry, HashMap}; +use std::fs::{create_dir, read_to_string, remove_dir_all, rename, write, File, OpenOptions}; use std::io::{BufWriter, Write}; use std::path::Path; use std::thread::sleep; use std::time::Duration; +use camino::{Utf8Path, Utf8PathBuf}; use fs2::FileExt; use path_slash::PathExt; use serde::Serialize; @@ -18,7 +18,7 @@ use time::OffsetDateTime; use tracing::{info, trace}; use crate::outcome::{LabOutcome, SummaryOutcome}; -use crate::*; +use crate::{check_interrupted, Context, Mutant, Result, Scenario, ScenarioOutcome}; const OUTDIR_NAME: &str = "mutants.out"; const ROTATED_NAME: &str = "mutants.out.old"; @@ -86,6 +86,7 @@ impl LockFile { /// A `mutants.out` directory holding logs and other output information. #[derive(Debug)] +#[allow(clippy::module_name_repetitions)] pub struct OutputDir { path: Utf8PathBuf, @@ -276,7 +277,7 @@ pub fn load_previously_caught(output_parent_dir: &Utf8Path) -> Result Result Result<()> { - self.message(&format!("mutation diff:\n{}", diff))?; + 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}")) @@ -332,7 +334,7 @@ impl ScenarioOutput { OpenOptions::new() .read(true) .open(&path) - .with_context(|| format!("reopen {} for read", path)) + .with_context(|| format!("reopen {path} for read")) } /// Open a new handle that appends to the log file, so that it can be passed to a subprocess. @@ -341,7 +343,7 @@ impl ScenarioOutput { OpenOptions::new() .append(true) .open(&path) - .with_context(|| format!("reopen {} for append", path)) + .with_context(|| format!("reopen {path} for append")) } /// Write a message, with a marker. @@ -370,6 +372,7 @@ mod test { use tempfile::{tempdir, TempDir}; use super::*; + use crate::workspace::Workspace; fn minimal_source_tree() -> TempDir { let tmp = tempdir().unwrap(); diff --git a/src/package.rs b/src/package.rs index f11d4785..bb2c86e8 100644 --- a/src/package.rs +++ b/src/package.rs @@ -20,6 +20,7 @@ pub struct Package { /// A more specific view of which packages to mutate, after resolving `PackageFilter::Auto`. #[derive(Debug, Clone)] +#[allow(clippy::module_name_repetitions)] pub enum PackageSelection { All, Explicit(Vec), diff --git a/src/path.rs b/src/path.rs index 75eb304b..99027c7c 100644 --- a/src/path.rs +++ b/src/path.rs @@ -28,7 +28,7 @@ pub fn ascent(path: &Utf8Path) -> isize { max_ascent } -/// An extension trait that helps Utf8Path print with forward slashes, +/// An extension trait that helps `Utf8Path` print with forward slashes, /// even on Windows. /// /// This makes the output more consistent across platforms and so easier diff --git a/src/pretty.rs b/src/pretty.rs index ee1cec5c..1e91a112 100644 --- a/src/pretty.rs +++ b/src/pretty.rs @@ -1,4 +1,4 @@ -// Copyright 2021-2023 Martin Pool +// Copyright 2021-2024 Martin Pool //! Convert a token stream back to (reasonably) pretty Rust code in a string. @@ -10,10 +10,10 @@ pub(crate) trait ToPrettyString { fn to_pretty_string(&self) -> String; } -/// Convert a TokenStream representing some code to a reasonably formatted +/// Convert a `TokenStream` representing some code to a reasonably formatted /// string of Rust code. /// -/// [TokenStream] has a `to_string`, but it adds spaces in places that don't +/// `TokenStream` has a `to_string`, but it adds spaces in places that don't /// look idiomatic, so this reimplements it in a way that looks better. /// /// This is probably not correctly formatted for all Rust syntax, and only tries @@ -23,7 +23,7 @@ where T: ToTokens, { fn to_pretty_string(&self) -> String { - use TokenTree::*; + use TokenTree::{Group, Ident, Literal, Punct}; let mut b = String::with_capacity(200); let mut ts = self.to_token_stream().into_iter().peekable(); while let Some(tt) = ts.next() { diff --git a/src/process.rs b/src/process.rs index 25f0600e..439c1f67 100644 --- a/src/process.rs +++ b/src/process.rs @@ -5,19 +5,18 @@ //! On Unix, the subprocess runs as its own process group, so that any //! grandchild processes are also signalled if it's interrupted. -#![allow(clippy::option_map_unit_fn)] // I don't think it's clearer with if/let. +#![warn(clippy::pedantic)] +#![allow(clippy::redundant_else)] use std::ffi::OsStr; -#[cfg(unix)] -use std::os::unix::process::{CommandExt, ExitStatusExt}; use std::process::{Child, Command, Stdio}; use std::thread::sleep; use std::time::{Duration, Instant}; -use anyhow::{bail, Context}; +use anyhow::Context; use camino::Utf8Path; use serde::Serialize; -use tracing::{debug, span, trace, warn, Level}; +use tracing::{debug, span, trace, Level}; use crate::console::Console; use crate::interrupt::check_interrupted; @@ -27,6 +26,16 @@ use crate::Result; /// How frequently to check if a subprocess finished. const WAIT_POLL_INTERVAL: Duration = Duration::from_millis(50); +#[cfg(windows)] +mod windows; +#[cfg(windows)] +use windows::{configure_command, terminate_child}; + +#[cfg(unix)] +mod unix; +#[cfg(unix)] +use unix::{configure_command, terminate_child}; + pub struct Process { child: Child, start: Instant, @@ -41,18 +50,17 @@ impl Process { env: &[(String, String)], cwd: &Utf8Path, timeout: Option, - jobserver: &Option, + jobserver: Option<&jobserver::Client>, scenario_output: &mut ScenarioOutput, console: &Console, - ) -> Result { + ) -> Result { 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; - } else { - console.tick(); - sleep(WAIT_POLL_INTERVAL); } + console.tick(); + sleep(WAIT_POLL_INTERVAL); }; scenario_output.message(&format!("result: {process_status:?}"))?; Ok(process_status) @@ -64,26 +72,27 @@ impl Process { env: &[(String, String)], cwd: &Utf8Path, timeout: Option, - jobserver: &Option, + jobserver: Option<&jobserver::Client>, scenario_output: &mut ScenarioOutput, ) -> Result { let start = Instant::now(); - let quoted_argv = cheap_shell_quote(argv); + let quoted_argv = quote_argv(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]); - child + let mut command = Command::new(&argv[0]); + command .args(&argv[1..]) .envs(os_env) .stdin(Stdio::null()) .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)] - child.process_group(0); - let child = child + if let Some(js) = jobserver { + js.configure(&mut command); + } + configure_command(&mut command); + let child = command .spawn() .with_context(|| format!("failed to spawn {}", argv.join(" ")))?; Ok(Process { @@ -95,28 +104,17 @@ impl Process { /// Check if the child process has finished; if so, return its status. #[mutants::skip] // It's hard to avoid timeouts if this never works... - pub fn poll(&mut self) -> Result> { + pub fn poll(&mut self) -> Result> { if self.timeout.is_some_and(|t| self.start.elapsed() > t) { debug!("timeout, terminating child process...",); self.terminate()?; - Ok(Some(ProcessStatus::Timeout)) + Ok(Some(Exit::Timeout)) } else if let Err(e) = check_interrupted() { debug!("interrupted, terminating child process..."); self.terminate()?; Err(e) } else if let Some(status) = self.child.try_wait()? { - if let Some(code) = status.code() { - if code == 0 { - return Ok(Some(ProcessStatus::Success)); - } else { - return Ok(Some(ProcessStatus::Failure(code as u32))); - } - } - #[cfg(unix)] - if let Some(signal) = status.signal() { - return Ok(Some(ProcessStatus::Signalled(signal as u8))); - } - Ok(Some(ProcessStatus::Other)) + Ok(Some(status.into())) } else { Ok(None) } @@ -126,12 +124,12 @@ impl Process { /// /// Blocks until the subprocess is terminated and then returns the exit status. /// - /// The status might not be Timeout if this raced with a normal exit. + /// The status might not be `Timeout` if this raced with a normal exit. #[mutants::skip] // would leak processes from tests if skipped fn terminate(&mut self) -> Result<()> { let _span = span!(Level::DEBUG, "terminate_child", pid = self.child.id()).entered(); debug!("terminating child process"); - terminate_child_impl(&mut self.child)?; + terminate_child(&mut self.child)?; trace!("wait for child after termination"); match self.child.wait() { Err(err) => debug!(?err, "Failed to wait for child after termination"), @@ -141,93 +139,77 @@ impl Process { } } -#[cfg(unix)] -#[allow(unknown_lints, clippy::needless_pass_by_ref_mut)] // To match Windows -#[mutants::skip] // hard to exercise the ESRCH edge case -fn terminate_child_impl(child: &mut Child) -> Result<()> { - use nix::errno::Errno; - use nix::sys::signal::{killpg, Signal}; - - let pid = nix::unistd::Pid::from_raw(child.id().try_into().unwrap()); - match killpg(pid, Signal::SIGTERM) { - Ok(()) => Ok(()), - Err(Errno::ESRCH) => { - Ok(()) // Probably already gone - } - Err(Errno::EPERM) if cfg!(target_os = "macos") => { - Ok(()) // If the process no longer exists then macos can return EPERM (maybe?) - } - Err(errno) => { - let message = format!("failed to terminate child: {errno}"); - warn!("{}", message); - bail!(message); - } - } -} - -#[cfg(windows)] -#[mutants::skip] // hard to exercise the ESRCH edge case -fn terminate_child_impl(child: &mut Child) -> Result<()> { - child.kill().context("Kill child") -} - /// The result of running a single child process. #[derive(Copy, Clone, Debug, PartialEq, Eq, Serialize)] -pub enum ProcessStatus { +pub enum Exit { /// Exited with status 0. Success, /// Exited with status non-0. - Failure(u32), + Failure(i32), /// Exceeded its timeout, and killed. Timeout, /// Killed by some signal. - Signalled(u8), + #[cfg(unix)] + Signalled(i32), /// Unknown or unexpected situation. Other, } -impl ProcessStatus { - pub fn is_success(&self) -> bool { - *self == ProcessStatus::Success +impl Exit { + pub fn is_success(self) -> bool { + self == Exit::Success } - pub fn is_timeout(&self) -> bool { - *self == ProcessStatus::Timeout + pub fn is_timeout(self) -> bool { + self == Exit::Timeout } - pub fn is_failure(&self) -> bool { - matches!(self, ProcessStatus::Failure(_)) + pub fn is_failure(self) -> bool { + matches!(self, Exit::Failure(_)) } } /// Quote an argv slice in Unix shell style. /// -/// This is not completely guaranteed, but is only for debug logs. -fn cheap_shell_quote, I: IntoIterator>(argv: I) -> String { - argv.into_iter() - .map(|s| { - s.as_ref() - .chars() - .flat_map(|c| match c { - ' ' | '\t' | '\n' | '\r' | '\\' | '\'' | '"' => vec!['\\', c], - _ => vec![c], - }) - .collect::() - }) - .collect::>() - .join(" ") +/// This isn't guaranteed to match the interpretation of a shell or to be safe. +/// It's just for debug logs. +fn quote_argv, I: IntoIterator>(argv: I) -> String { + let mut r = String::new(); + for s in argv { + if !r.is_empty() { + r.push(' '); + } + for c in s.as_ref().chars() { + match c { + '\t' => r.push_str(r"\t"), + '\n' => r.push_str(r"\n"), + '\r' => r.push_str(r"\r"), + ' ' | '\\' | '\'' | '"' => { + r.push('\\'); + r.push(c); + } + _ => r.push(c), + } + } + } + r } #[cfg(test)] mod test { - use super::cheap_shell_quote; + use super::quote_argv; #[test] fn shell_quoting() { - assert_eq!(cheap_shell_quote(["foo".to_string()]), "foo"); + assert_eq!(quote_argv(["foo".to_string()]), "foo"); + assert_eq!( + quote_argv(["foo bar", r"\blah\x", r#""quoted""#]), + r#"foo\ bar \\blah\\x \"quoted\""# + ); + assert_eq!(quote_argv([""]), ""); assert_eq!( - cheap_shell_quote(["foo bar", r#"\blah\t"#, r#""quoted""#]), - r#"foo\ bar \\blah\\t \"quoted\""# + quote_argv(["with whitespace", "\r\n\t\t"]), + r"with\ whitespace \r\n\t\t" ); } } diff --git a/src/process/unix.rs b/src/process/unix.rs new file mode 100644 index 00000000..d5f52aac --- /dev/null +++ b/src/process/unix.rs @@ -0,0 +1,54 @@ +use std::os::unix::process::{CommandExt, ExitStatusExt}; +use std::process::{Child, Command, ExitStatus}; + +use anyhow::bail; +use nix::errno::Errno; +use nix::sys::signal::{killpg, Signal}; +use nix::unistd::Pid; +use tracing::warn; + +use crate::Result; + +use super::Exit; + +#[allow(unknown_lints, clippy::needless_pass_by_ref_mut)] // To match Windows +#[mutants::skip] // hard to exercise the ESRCH edge case +pub(super) fn terminate_child(child: &mut Child) -> Result<()> { + let pid = Pid::from_raw(child.id().try_into().unwrap()); + match killpg(pid, Signal::SIGTERM) { + Ok(()) => Ok(()), + Err(Errno::ESRCH) => { + Ok(()) // Probably already gone + } + Err(Errno::EPERM) if cfg!(target_os = "macos") => { + Ok(()) // If the process no longer exists then macos can return EPERM (maybe?) + } + Err(errno) => { + // TODO: Maybe strerror? + let message = format!("failed to terminate child: error {errno}"); + warn!("{}", message); + bail!(message); + } + } +} + +#[mutants::skip] +pub(super) fn configure_command(command: &mut Command) { + command.process_group(0); +} + +impl From for Exit { + fn from(status: ExitStatus) -> Self { + if let Some(code) = status.code() { + if code == 0 { + Exit::Success + } else { + Exit::Failure(code) + } + } else if let Some(signal) = status.signal() { + return Exit::Signalled(signal); + } else { + Exit::Other + } + } +} diff --git a/src/process/windows.rs b/src/process/windows.rs new file mode 100644 index 00000000..c37950fd --- /dev/null +++ b/src/process/windows.rs @@ -0,0 +1,29 @@ +use std::process::{Child, Command, ExitStatus}; + +use anyhow::Context; + +use crate::Result; + +use super::Exit; + +#[mutants::skip] // hard to exercise the ESRCH edge case +pub(super) fn terminate_child(child: &mut Child) -> Result<()> { + child.kill().context("Kill child") +} + +#[mutants::skip] +pub(super) fn configure_command(_command: &mut Command) {} + +impl From for Exit { + fn from(status: ExitStatus) -> Self { + if let Some(code) = status.code() { + if code == 0 { + Exit::Success + } else { + Exit::Failure(code) + } + } else { + Exit::Other + } + } +} diff --git a/src/source.rs b/src/source.rs index 0240cedd..057cff91 100644 --- a/src/source.rs +++ b/src/source.rs @@ -21,6 +21,7 @@ use crate::span::LineColumn; /// Code is normalized to Unix line endings as it's read in, and modified /// files are written with Unix line endings. #[derive(Clone, Debug, Eq, PartialEq)] +#[allow(clippy::module_name_repetitions)] pub struct SourceFile { /// Which package within the workspace contains this file? pub package_name: String, @@ -30,7 +31,7 @@ pub struct SourceFile { /// Full copy of the unmodified source. /// - /// This is held in an [Arc] so that SourceFiles can be cloned without using excessive + /// This is held in an [Arc] so that `SourceFile`s can be cloned without using excessive /// amounts of memory. pub code: Arc, @@ -40,7 +41,7 @@ pub struct SourceFile { } impl SourceFile { - /// Construct a SourceFile representing a file within a tree. + /// Construct a `SourceFile` representing a file within a tree. /// /// This eagerly loads the text of the file. /// diff --git a/src/span.rs b/src/span.rs index 5dd0b817..6f12fe0e 100644 --- a/src/span.rs +++ b/src/span.rs @@ -3,7 +3,7 @@ //! Locations (line/column) and spans between them in source code. //! //! This is similar to, and can be automatically derived from, -//! [proc_macro2::Span] and [proc_macro2::LineColumn], but is +//! `proc_macro2::Span` and `proc_macro2::LineColumn`, but is //! a bit more convenient for our purposes. use std::fmt; @@ -183,13 +183,13 @@ mod test { #[test] fn linecolumn_debug_form() { let lc = LineColumn { line: 1, column: 2 }; - assert_eq!(format!("{:?}", lc), "LineColumn(1, 2)"); + assert_eq!(format!("{lc:?}"), "LineColumn(1, 2)"); } #[test] fn span_debug_form() { let span = Span::quad(1, 2, 3, 4); - assert_eq!(format!("{:?}", span), "Span(1, 2, 3, 4)"); + assert_eq!(format!("{span:?}"), "Span(1, 2, 3, 4)"); } #[test] @@ -230,7 +230,7 @@ mod test { } #[test] fn span_ops() { - let source = indoc! { r#" + let source = indoc! { r" fn foo() { some(); @@ -238,19 +238,19 @@ mod test { } const BAR: u32 = 32; - "# }; + " }; // typical multi-line case let span = Span::quad(2, 10, 5, 2); assert_eq!(span.extract(source), "{\n some();\n stuff();\n}"); let replaced = span.replace(source, "{ /* body deleted */ }"); assert_eq!( replaced, - indoc! { r#" + indoc! { r" fn foo() { /* body deleted */ } const BAR: u32 = 32; - "# } + " } ); // single-line case @@ -259,7 +259,7 @@ mod test { let replaced = span.replace(source, "69"); assert_eq!( replaced, - indoc! { r#" + indoc! { r" fn foo() { some(); @@ -267,7 +267,7 @@ mod test { } const BAR: u32 = 69; - "# } + " } ); } } diff --git a/src/tail_file.rs b/src/tail_file.rs index ee5e386c..71789f1e 100644 --- a/src/tail_file.rs +++ b/src/tail_file.rs @@ -23,12 +23,12 @@ pub struct TailFile { impl TailFile { /// Watch lines appended to the given file, which should be open for reading. - pub fn new(file: File) -> Result { - Ok(TailFile { + pub fn new(file: File) -> TailFile { + TailFile { file, last_line_seen: String::new(), read_buf: Vec::new(), - }) + } } /// Return the last non-empty, non-whitespace line from this file, or an empty string @@ -67,7 +67,7 @@ mod test { let mut tempfile = tempfile::NamedTempFile::new().unwrap(); let path: Utf8PathBuf = tempfile.path().to_owned().try_into().unwrap(); let reopened = File::open(&path).unwrap(); - let mut tailer = TailFile::new(reopened).unwrap(); + let mut tailer = TailFile::new(reopened); assert_eq!( tailer.last_line().unwrap(), diff --git a/src/timeouts.rs b/src/timeouts.rs index 47d191df..2d191442 100644 --- a/src/timeouts.rs +++ b/src/timeouts.rs @@ -31,17 +31,17 @@ impl Timeouts { baseline.phase_result(Phase::Build).map(|pr| pr.duration), options, ), - test: test_timeout( + test: Some(test_timeout( baseline.phase_result(Phase::Test).map(|pr| pr.duration), options, - ), + )), } } pub fn without_baseline(options: &Options) -> Timeouts { Timeouts { build: build_timeout(None, options), - test: test_timeout(None, options), + test: Some(test_timeout(None, options)), } } } @@ -51,15 +51,15 @@ fn warn_fallback_timeout(phase_name: &str, option: &str) { warn!("An explicit {phase_name} timeout is recommended when using {option}; using {FALLBACK_TIMEOUT_SECS} seconds by default"); } -fn test_timeout(baseline_duration: Option, options: &Options) -> Option { +fn test_timeout(baseline_duration: Option, options: &Options) -> Duration { if let Some(explicit) = options.test_timeout { - Some(explicit) + explicit } else if let Some(baseline_duration) = baseline_duration { let timeout = max( options.minimum_test_timeout, - Duration::from_secs( + Duration::from_secs_f64( (baseline_duration.as_secs_f64() * options.test_timeout_multiplier.unwrap_or(5.0)) - .ceil() as u64, + .ceil(), ), ); if options.show_times { @@ -68,10 +68,13 @@ fn test_timeout(baseline_duration: Option, options: &Options) -> Optio humantime::format_duration(timeout) ); } - Some(timeout) + timeout + } else if options.check_only { + // We won't have run baseline tests, and we won't run any other tests either. + Duration::from_secs(0) } else { warn_fallback_timeout("test", "--baseline=skip"); - Some(Duration::from_secs(FALLBACK_TIMEOUT_SECS)) + Duration::from_secs(FALLBACK_TIMEOUT_SECS) } } @@ -113,7 +116,7 @@ mod test { assert_eq!(options.test_timeout_multiplier, Some(1.5)); assert_eq!( test_timeout(Some(Duration::from_secs(40)), &options), - Some(Duration::from_secs(60)), + Duration::from_secs(60), ); } @@ -124,7 +127,7 @@ mod test { assert_eq!( test_timeout(Some(Duration::from_secs(40)), &options), - Some(Duration::from_secs(60)), + Duration::from_secs(60), ); } @@ -167,7 +170,7 @@ mod test { assert_eq!(options.test_timeout_multiplier, Some(2.0)); assert_eq!( test_timeout(Some(Duration::from_secs(42)), &options), - Some(Duration::from_secs(42 * 2)), + Duration::from_secs(42 * 2), ); } @@ -195,7 +198,7 @@ mod test { assert_eq!(options.test_timeout_multiplier, None); assert_eq!( test_timeout(Some(Duration::from_secs(42)), &options), - Some(Duration::from_secs(42 * 5)), + Duration::from_secs(42 * 5), ); } @@ -240,7 +243,7 @@ mod test { let options = Options::new(&args, &Config::default()).unwrap(); assert_eq!(options.test_timeout_multiplier, None); - assert_eq!(test_timeout(None, &options), Some(Duration::from_secs(300))); + assert_eq!(test_timeout(None, &options), Duration::from_secs(300)); assert_eq!(build_timeout(None, &options), None); } } diff --git a/src/visit.rs b/src/visit.rs index babe4117..83547444 100644 --- a/src/visit.rs +++ b/src/visit.rs @@ -7,10 +7,13 @@ //! e.g. for cargo they are identified from the targets. The tree walker then //! follows `mod` statements to recursively visit other referenced files. +#![warn(clippy::pedantic)] + use std::collections::VecDeque; use std::sync::Arc; use std::vec; +use camino::{Utf8Path, Utf8PathBuf}; use proc_macro2::{Ident, TokenStream}; use quote::{quote, ToTokens}; use syn::ext::IdentExt; @@ -24,7 +27,7 @@ use crate::mutate::Function; use crate::pretty::ToPrettyString; use crate::source::SourceFile; use crate::span::Span; -use crate::*; +use crate::{check_interrupted, Console, Context, Genre, Mutant, Options, Result}; /// Mutants and files discovered in a source tree. /// @@ -44,7 +47,7 @@ impl Discovered { trace!(?name, "skip previously caught mutant"); } !c - }) + }); } } @@ -72,13 +75,13 @@ pub fn walk_tree( // collect any mutants from them, and they don't count as "seen" for // `--list-files`. for mod_namespace in &external_mods { - if let Some(mod_path) = find_mod_source(workspace_dir, &source_file, mod_namespace)? { + if let Some(mod_path) = find_mod_source(workspace_dir, &source_file, mod_namespace) { file_queue.extend(SourceFile::load( workspace_dir, &mod_path, &source_file.package_name, false, - )?) + )?); } } let path = &source_file.tree_relative_path; @@ -184,8 +187,7 @@ impl ModNamespace { fn get_filesystem_name(&self) -> &Utf8Path { self.path_attribute .as_ref() - .map(Utf8PathBuf::as_path) - .unwrap_or(Utf8Path::new(&self.name)) + .map_or(Utf8Path::new(&self.name), Utf8PathBuf::as_path) } } @@ -263,14 +265,14 @@ impl DiscoveryVisitor<'_> { } /// Record that we generated some mutants. - fn collect_mutant(&mut self, span: Span, replacement: TokenStream, genre: Genre) { + fn collect_mutant(&mut self, span: Span, replacement: &TokenStream, genre: Genre) { self.mutants.push(Mutant { source_file: self.source_file.clone(), function: self.fn_stack.last().cloned(), span, replacement: replacement.to_pretty_string(), genre, - }) + }); } fn collect_fn_mutants(&mut self, sig: &Signature, block: &Block) { @@ -299,7 +301,7 @@ impl DiscoveryVisitor<'_> { if orig_block == new_block { debug!("Replacement is the same as the function body; skipping"); } else { - self.collect_mutant(body_span, rep, Genre::FnValue); + self.collect_mutant(body_span, &rep, Genre::FnValue); } } } @@ -527,10 +529,8 @@ impl<'ast> Visit<'ast> for DiscoveryVisitor<'_> { BinOp::Ge(_) => vec![quote! {<}], BinOp::Add(_) => vec![quote! {-}, quote! {*}], BinOp::AddAssign(_) => vec![quote! {-=}, quote! {*=}], - BinOp::Sub(_) => vec![quote! {+}, quote! {/}], - BinOp::SubAssign(_) => vec![quote! {+=}, quote! {/=}], - BinOp::Mul(_) => vec![quote! {+}, quote! {/}], - BinOp::MulAssign(_) => vec![quote! {+=}, quote! {/=}], + BinOp::Sub(_) | BinOp::Mul(_) => vec![quote! {+}, quote! {/}], + BinOp::SubAssign(_) | BinOp::MulAssign(_) => vec![quote! {+=}, quote! {/=}], BinOp::Div(_) => vec![quote! {%}, quote! {*}], BinOp::DivAssign(_) => vec![quote! {%=}, quote! {*=}], BinOp::Rem(_) => vec![quote! {/}, quote! {+}], @@ -555,7 +555,7 @@ impl<'ast> Visit<'ast> for DiscoveryVisitor<'_> { }; replacements .into_iter() - .for_each(|rep| self.collect_mutant(i.op.span().into(), rep, Genre::BinaryOperator)); + .for_each(|rep| self.collect_mutant(i.op.span().into(), &rep, Genre::BinaryOperator)); syn::visit::visit_expr_binary(self, i); } @@ -567,7 +567,7 @@ impl<'ast> Visit<'ast> for DiscoveryVisitor<'_> { } match i.op { UnOp::Not(_) | UnOp::Neg(_) => { - self.collect_mutant(i.op.span().into(), quote! {}, Genre::UnaryOperator); + self.collect_mutant(i.op.span().into(), "e! {}, Genre::UnaryOperator); } _ => { trace!( @@ -596,7 +596,7 @@ fn find_mod_source( tree_root: &Utf8Path, parent: &SourceFile, mod_namespace: &ExternalModRef, -) -> Result> { +) -> Option { // First, work out whether the mod will be a sibling in the same directory, or // in a child directory. // @@ -663,15 +663,14 @@ fn find_mod_source( let full_path = tree_root.join(&relative_path); if full_path.is_file() { trace!("found submodule in {full_path}"); - return Ok(Some(relative_path)); - } else { - tried_paths.push(full_path); + return Some(relative_path); } + tried_paths.push(full_path); } let mod_name = &mod_child.name; let definition_site = parent.format_source_location(mod_child.source_location.start); warn!(?definition_site, %mod_name, ?tried_paths, "referent of mod not found"); - Ok(None) + None } /// True if the signature of a function is such that it should be excluded. @@ -738,15 +737,12 @@ fn path_is(path: &syn::Path, idents: &[&str]) -> bool { /// /// This does not check type arguments. fn path_ends_with(path: &syn::Path, ident: &str) -> bool { - path.segments - .last() - .map(|s| s.ident == ident) - .unwrap_or(false) + path.segments.last().is_some_and(|s| s.ident == ident) } /// True if the attribute contains `mutants::skip`. /// -/// This for example returns true for `#[mutants::skip] or `#[cfg_attr(test, mutants::skip)]`. +/// This for example returns true for `#[mutants::skip]` or `#[cfg_attr(test, mutants::skip)]`. fn attr_is_mutants_skip(attr: &Attribute) -> bool { if path_is(attr.path(), &["mutants", "skip"]) { return true; @@ -757,7 +753,7 @@ fn attr_is_mutants_skip(attr: &Attribute) -> bool { let mut skip = false; if let Err(err) = attr.parse_nested_meta(|meta| { if path_is(&meta.path, &["mutants", "skip"]) { - skip = true + skip = true; } Ok(()) }) { @@ -802,11 +798,12 @@ fn find_path_attribute(attrs: &[Attribute]) -> std::result::Result std::result::Result, String> { let token_string = token_stream.to_string(); let item_mod = syn::parse_str::(&token_string).unwrap_or_else(|err| { @@ -880,13 +877,13 @@ mod test { #[test] fn find_path_attribute_on_module_item() { - let outer = run_find_path_attribute(quote! { + let outer = run_find_path_attribute("e! { #[path = "foo_file.rs"] mod foo; }); assert_eq!(outer, Ok(Some(Utf8PathBuf::from("foo_file.rs")))); - let inner = run_find_path_attribute(quote! { + let inner = run_find_path_attribute("e! { mod foo { #![path = "foo_folder"] @@ -900,26 +897,26 @@ mod test { #[test] fn reject_module_path_absolute() { // dots are valid - let dots = run_find_path_attribute(quote! { + let dots = run_find_path_attribute("e! { #[path = "contains/../dots.rs"] mod dots; }); assert_eq!(dots, Ok(Some(Utf8PathBuf::from("contains/../dots.rs")))); - let dots_inner = run_find_path_attribute(quote! { + let dots_inner = run_find_path_attribute("e! { mod dots_in_path { #![path = "contains/../dots"] } }); assert_eq!(dots_inner, Ok(Some(Utf8PathBuf::from("contains/../dots")))); - let leading_slash = run_find_path_attribute(quote! { + let leading_slash = run_find_path_attribute("e! { #[path = "/leading_slash.rs"] mod dots; }); assert_eq!(leading_slash, Err("/leading_slash.rs".to_owned())); - let allow_other_slashes = run_find_path_attribute(quote! { + let allow_other_slashes = run_find_path_attribute("e! { #[path = "foo/other/slashes/are/allowed.rs"] mod dots; }); @@ -928,7 +925,7 @@ mod test { Ok(Some(Utf8PathBuf::from("foo/other/slashes/are/allowed.rs"))) ); - let leading_slash2 = run_find_path_attribute(quote! { + let leading_slash2 = run_find_path_attribute("e! { #[path = "/leading_slash/../and_dots.rs"] mod dots; }); @@ -978,9 +975,7 @@ mod test { #[test] fn skip_with_capacity_by_default() { - let args = Args::try_parse_from(["mutants"]).unwrap(); - let config = Config::default(); - let options = Options::new(&args, &config).unwrap(); + let options = Options::from_arg_strs(["mutants"]); let mut mutants = mutate_source_str( indoc! {" fn main() { @@ -997,9 +992,7 @@ mod test { #[test] fn mutate_vec_with_capacity_when_default_skips_are_turned_off() { - let args = Args::try_parse_from(["mutants", "--skip-calls-defaults", "false"]).unwrap(); - let config = Config::default(); - let options = Options::new(&args, &config).unwrap(); + let options = Options::from_arg_strs(["mutants", "--skip-calls-defaults", "false"]); let mutants = mutate_source_str( indoc! {" fn main() { diff --git a/src/workspace.rs b/src/workspace.rs index 62228ef8..ee1aef1f 100644 --- a/src/workspace.rs +++ b/src/workspace.rs @@ -22,7 +22,7 @@ use std::process::Command; use anyhow::{anyhow, bail, ensure, Context}; use camino::{Utf8Path, Utf8PathBuf}; -use cargo_metadata::Metadata; +use cargo_metadata::{Metadata, TargetKind}; use itertools::Itertools; use serde_json::Value; use tracing::{debug, debug_span, error, warn}; @@ -306,7 +306,17 @@ fn package_top_sources( fn should_mutate_target(target: &cargo_metadata::Target) -> bool { for kind in &target.kind { - if kind == "bin" || kind == "proc-macro" || kind.ends_with("lib") { + // bin / proc-macro / *lib + if matches!( + kind, + TargetKind::Bin + | TargetKind::ProcMacro + | TargetKind::CDyLib + | TargetKind::DyLib + | TargetKind::Lib + | TargetKind::RLib + | TargetKind::StaticLib + ) { return true; } } diff --git a/tests/build_dir.rs b/tests/build_dir.rs index 3496e6f3..919b802c 100644 --- a/tests/build_dir.rs +++ b/tests/build_dir.rs @@ -1,6 +1,6 @@ -// Copyright 2023 Martin Pool +// Copyright 2023-2024 Martin Pool -use std::fs::write; +use std::fs::{create_dir, write}; mod util; use util::{copy_of_testdata, run}; @@ -10,6 +10,9 @@ fn gitignore_respected_in_copy_by_default() { // Make a tree with a (dumb) gitignore that excludes the source file; when you copy it // to a build directory, the source file should not be there and so the check will fail. let tmp = copy_of_testdata("factorial"); + // There must be something that looks like a `.git` dir, otherwise we don't read + // `.gitignore` files. + create_dir(tmp.path().join(".git")).unwrap(); write(tmp.path().join(".gitignore"), b"src\n").unwrap(); run() .args(["mutants", "--check", "-d"]) diff --git a/tests/check_build.rs b/tests/check_build.rs index 6401c5d8..ecf044f3 100644 --- a/tests/check_build.rs +++ b/tests/check_build.rs @@ -2,6 +2,7 @@ //! Tests for `--check` +use indoc::indoc; use predicates::prelude::*; use pretty_assertions::assert_eq; @@ -16,8 +17,7 @@ fn small_well_tested_tree_check_only() { .current_dir(tmp_src_dir.path()) .assert() .success() - .stdout(predicate::function(|stdout: &str| { - insta::assert_snapshot!(stdout, @r###" + .stdout(indoc! {r" Found 4 mutants to test ok Unmutated baseline ok src/lib.rs:5:5: replace factorial -> u32 with 0 @@ -25,9 +25,8 @@ fn small_well_tested_tree_check_only() { ok src/lib.rs:7:11: replace *= with += in factorial ok src/lib.rs:7:11: replace *= with /= in factorial 4 mutants tested: 4 succeeded - "###); - true - })); + "}) + .stderr(""); let outcomes = outcome_json_counts(&tmp_src_dir); assert_eq!( outcomes, @@ -124,8 +123,7 @@ fn check_tree_with_mutants_skip() { .env_remove("RUST_BACKTRACE") .assert() .success() - .stdout(predicate::function(|stdout: &str| { - insta::assert_snapshot!(stdout, @r###" + .stdout(indoc! { r" Found 5 mutants to test ok Unmutated baseline ok src/lib.rs:15:5: replace controlled_loop with () @@ -134,9 +132,8 @@ fn check_tree_with_mutants_skip() { ok src/lib.rs:21:53: replace * with + in controlled_loop ok src/lib.rs:21:53: replace * with / in controlled_loop 5 mutants tested: 5 succeeded - "###); - true - })); + "}) + .stderr(""); assert_eq!( outcome_json_counts(&tmp_src_dir), serde_json::json!({ diff --git a/tests/config.rs b/tests/config.rs index 504c30ef..57a4b5f1 100644 --- a/tests/config.rs +++ b/tests/config.rs @@ -263,3 +263,51 @@ fn additional_cargo_test_args() { .assert() .success(); } + +#[test] +/// Set the `--output` directory via `output` config directive. +fn output_option_use_config() { + let output_tmpdir = TempDir::new().unwrap(); + let output_via_config = output_tmpdir.path().join("output_via_config"); + let testdata = copy_of_testdata("factorial"); + + let out_path_str = output_via_config + .to_string_lossy() + .escape_default() + .to_string(); + write_config_file(&testdata, &format!("output = \"{out_path_str}\"")); + + assert!( + !testdata.path().join("mutants.out").exists(), + "mutants.out should not be in a clean copy of the test data" + ); + + run() + .arg("mutants") + .args(["--check", "--no-times"]) + .arg("-d") + .arg(testdata.path()) + .assert() + .success(); + + assert!( + !testdata.path().join("mutants.out").exists(), + "mutants.out should not be in the source directory" + ); + let mutants_out = output_via_config.join("mutants.out"); + assert!( + mutants_out.exists(), + "mutants.out is in changed `output` directory" + ); + for name in [ + "mutants.json", + "debug.log", + "outcomes.json", + "missed.txt", + "caught.txt", + "timeout.txt", + "unviable.txt", + ] { + assert!(mutants_out.join(name).is_file(), "{name} is in mutants.out",); + } +} diff --git a/tests/in_place.rs b/tests/in_place.rs index 49bfd6a9..9e248c97 100644 --- a/tests/in_place.rs +++ b/tests/in_place.rs @@ -30,17 +30,18 @@ fn in_place_check_leaves_no_changes() -> Result<()> { "stderr:\n{}", String::from_utf8_lossy(&cmd.get_output().stderr) ); - fn check_file(tmp: &Path, new_name: &str, old_name: &str) -> Result<()> { - let orig_path = Path::new("testdata/small_well_tested"); - println!("comparing {new_name} and {old_name}"); - assert_eq!( - read_to_string(tmp.join(new_name))?.replace("\r\n", "\n"), - read_to_string(orig_path.join(old_name))?.replace("\r\n", "\n"), - "{old_name} should be the same as {new_name}" - ); - Ok(()) - } check_file(tmp_path, "Cargo.toml", "Cargo_test.toml")?; check_file(tmp_path, "src/lib.rs", "src/lib.rs")?; Ok(()) } + +fn check_file(tmp: &Path, new_name: &str, old_name: &str) -> Result<()> { + let orig_path = Path::new("testdata/small_well_tested"); + println!("comparing {new_name} and {old_name}"); + assert_eq!( + read_to_string(tmp.join(new_name))?.replace("\r\n", "\n"), + read_to_string(orig_path.join(old_name))?.replace("\r\n", "\n"), + "{old_name} should be the same as {new_name}" + ); + Ok(()) +} diff --git a/tests/iterate.rs b/tests/iterate.rs index 2252bd9d..e90a4c14 100644 --- a/tests/iterate.rs +++ b/tests/iterate.rs @@ -15,6 +15,7 @@ use tempfile::tempdir; use self::util::run; #[test] +#[allow(clippy::too_many_lines)] // long but pretty straightforward fn iterate() { let temp = tempdir().unwrap(); diff --git a/tests/main.rs b/tests/main.rs index 9637a2ae..f428de98 100644 --- a/tests/main.rs +++ b/tests/main.rs @@ -6,8 +6,6 @@ use std::collections::HashSet; use std::env; use std::fs::{self, create_dir, read_dir, read_to_string}; use std::path::Path; -use std::thread::sleep; -use std::time::Duration; use indoc::indoc; use itertools::Itertools; @@ -18,7 +16,7 @@ use pretty_assertions::assert_eq; use tempfile::TempDir; mod util; -use util::{copy_of_testdata, copy_testdata_to, run, MAIN_BINARY, OUTER_TIMEOUT}; +use util::{copy_of_testdata, copy_testdata_to, run, OUTER_TIMEOUT}; #[test] fn incorrect_cargo_subcommand() { @@ -496,6 +494,46 @@ fn output_option() { } } +#[test] +/// Set the `--output` directory via environment variable `CARGO_MUTANTS_OUTPUT` +fn output_option_use_env() { + let tmp_src_dir = copy_of_testdata("factorial"); + let output_tmpdir = TempDir::new().unwrap(); + let output_via_env = output_tmpdir.path().join("output_via_env"); + assert!( + !tmp_src_dir.path().join("mutants.out").exists(), + "mutants.out should not be in a clean copy of the test data" + ); + run() + .env("CARGO_MUTANTS_OUTPUT", &output_via_env) + .arg("mutants") + .args(["--check", "--no-times"]) + .arg("-d") + .arg(tmp_src_dir.path()) + .assert() + .success(); + assert!( + !tmp_src_dir.path().join("mutants.out").exists(), + "mutants.out should not be in the source directory" + ); + let mutants_out = output_via_env.join("mutants.out"); + assert!( + mutants_out.exists(), + "mutants.out is in $CARGO_MUTANTS_OUTPUT directory" + ); + for name in [ + "mutants.json", + "debug.log", + "outcomes.json", + "missed.txt", + "caught.txt", + "timeout.txt", + "unviable.txt", + ] { + assert!(mutants_out.join(name).is_file(), "{name} is in mutants.out",); + } +} + #[test] fn check_succeeds_in_tree_that_builds_but_fails_tests() { // --check doesn't actually run the tests so won't discover that they fail. @@ -657,90 +695,6 @@ fn timeout_when_unmutated_tree_test_hangs() { )); } -/// If the test hangs and the user (in this case the test suite) interrupts it, then -/// the `cargo test` child should be killed. -/// -/// This is a bit hard to directly observe: the property that we really most care -/// about is that _all_ grandchild processes are also killed and nothing is left -/// behind. (On Unix, this is accomplished by use of a pgroup.) However that's a bit -/// hard to mechanically check without reading and interpreting the process tree, which -/// seems likely to be a bit annoying to do portably and without flakes. -/// (But maybe we still should?) -/// -/// An easier thing to test is that the cargo-mutants process _thinks_ it has killed -/// the children, and we can observe this in the debug log. -/// -/// In this test cargo-mutants has a very long timeout, but the test driver has a -/// short timeout, so it should kill cargo-mutants. -#[test] -#[cfg(unix)] // Should in principle work on Windows, but does not at the moment. -fn interrupt_caught_and_kills_children() { - // Test a tree that has enough tests that we'll probably kill it before it completes. - - use std::process::{Command, Stdio}; - - use nix::libc::pid_t; - use nix::sys::signal::{kill, SIGTERM}; - use nix::unistd::Pid; - - let tmp_src_dir = copy_of_testdata("well_tested"); - // We can't use `assert_cmd` `timeout` here because that sends the child a `SIGKILL`, - // which doesn't give it a chance to clean up. And, `std::process::Command` only - // has an abrupt kill. - - // Drop RUST_BACKTRACE because the traceback mentions "panic" handler functions - // and we want to check that the process does not panic. - - // Skip baseline because firstly it should already pass but more importantly - // #333 exhibited only during non-baseline scenarios. - let args = [ - MAIN_BINARY.to_str().unwrap(), - "mutants", - "--timeout=300", - "--baseline=skip", - "--level=trace", - ]; - - println!("Running: {args:?}"); - let mut child = Command::new(args[0]) - .args(&args[1..]) - .current_dir(&tmp_src_dir) - .stdout(Stdio::piped()) - .stderr(Stdio::piped()) - .env_remove("RUST_BACKTRACE") - .spawn() - .expect("spawn child"); - - sleep(Duration::from_secs(2)); // Let it get started - assert!( - child.try_wait().expect("try to wait for child").is_none(), - "child exited early" - ); - - println!("Sending SIGTERM to cargo-mutants..."); - kill(Pid::from_raw(child.id() as pid_t), SIGTERM).expect("send SIGTERM"); - - println!("Wait for cargo-mutants to exit..."); - let output = child - .wait_with_output() - .expect("wait for child after SIGTERM"); - - let stdout = String::from_utf8_lossy(&output.stdout); - let stderr = String::from_utf8_lossy(&output.stderr); - println!("stdout:\n{stdout}"); - println!("stderr:\n{stderr}"); - - assert!(stderr.contains("interrupted")); - // We used to look here for some other trace messages about how it's interrupted, but - // that seems to be racy: sometimes the parent sees the child interrupted before it - // emits these messages? Anyhow, it's not essential. - - // This shouldn't cause a panic though (#333) - assert!(!stderr.contains("panic")); - // And we don't want duplicate messages about workers failing. - assert!(!stderr.contains("Worker thread failed")); -} - /// A tree that hangs when some functions are mutated does not hang cargo-mutants /// overall, because we impose a timeout. The timeout can be specified on the /// command line, with decimal seconds. diff --git a/tests/unix.rs b/tests/unix.rs new file mode 100644 index 00000000..84157a9f --- /dev/null +++ b/tests/unix.rs @@ -0,0 +1,91 @@ +#![cfg(unix)] + +use std::thread::sleep; +use std::time::Duration; + +mod util; +use util::{copy_of_testdata, MAIN_BINARY}; + +/// If the test hangs and the user (in this case the test suite) interrupts it, then +/// the `cargo test` child should be killed. +/// +/// This is a bit hard to directly observe: the property that we really most care +/// about is that _all_ grandchild processes are also killed and nothing is left +/// behind. (On Unix, this is accomplished by use of a pgroup.) However that's a bit +/// hard to mechanically check without reading and interpreting the process tree, which +/// seems likely to be a bit annoying to do portably and without flakes. +/// (But maybe we still should?) +/// +/// An easier thing to test is that the cargo-mutants process _thinks_ it has killed +/// the children, and we can observe this in the debug log. +/// +/// In this test cargo-mutants has a very long timeout, but the test driver has a +/// short timeout, so it should kill cargo-mutants. +// TODO: An equivalent test on Windows? +#[test] +fn interrupt_caught_and_kills_children() { + // Test a tree that has enough tests that we'll probably kill it before it completes. + + use std::process::{Command, Stdio}; + + use nix::libc::pid_t; + use nix::sys::signal::{kill, SIGTERM}; + use nix::unistd::Pid; + + let tmp_src_dir = copy_of_testdata("well_tested"); + // We can't use `assert_cmd` `timeout` here because that sends the child a `SIGKILL`, + // which doesn't give it a chance to clean up. And, `std::process::Command` only + // has an abrupt kill. + + // Drop RUST_BACKTRACE because the traceback mentions "panic" handler functions + // and we want to check that the process does not panic. + + // Skip baseline because firstly it should already pass but more importantly + // #333 exhibited only during non-baseline scenarios. + let args = [ + MAIN_BINARY.to_str().unwrap(), + "mutants", + "--timeout=300", + "--baseline=skip", + "--level=trace", + ]; + + println!("Running: {args:?}"); + let mut child = Command::new(args[0]) + .args(&args[1..]) + .current_dir(&tmp_src_dir) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .env_remove("RUST_BACKTRACE") + .spawn() + .expect("spawn child"); + + sleep(Duration::from_secs(2)); // Let it get started + assert!( + child.try_wait().expect("try to wait for child").is_none(), + "child exited early" + ); + + println!("Sending SIGTERM to cargo-mutants..."); + kill(Pid::from_raw(child.id() as pid_t), SIGTERM).expect("send SIGTERM"); + + println!("Wait for cargo-mutants to exit..."); + let output = child + .wait_with_output() + .expect("wait for child after SIGTERM"); + + let stdout = String::from_utf8_lossy(&output.stdout); + let stderr = String::from_utf8_lossy(&output.stderr); + println!("stdout:\n{stdout}"); + println!("stderr:\n{stderr}"); + + assert!(stderr.contains("interrupted")); + // We used to look here for some other trace messages about how it's interrupted, but + // that seems to be racy: sometimes the parent sees the child interrupted before it + // emits these messages? Anyhow, it's not essential. + + // This shouldn't cause a panic though (#333) + assert!(!stderr.contains("panic")); + // And we don't want duplicate messages about workers failing. + assert!(!stderr.contains("Worker thread failed")); +} diff --git a/tests/util/mod.rs b/tests/util/mod.rs index 07ec37ec..ae9fe246 100644 --- a/tests/util/mod.rs +++ b/tests/util/mod.rs @@ -89,7 +89,7 @@ pub fn copy_testdata_to>(tree_name: &str, dest: P) { }) .after_entry_copied(|path, _file_type, _stats| { if path.ends_with("Cargo_test.toml") || path.ends_with(".cargo_test") { - renames.push(dest.join(path)) + renames.push(dest.join(path)); } Ok(()) })