From cbb3121a513303f7dd061176242cd54ed58a3ee2 Mon Sep 17 00:00:00 2001 From: Martin Pool Date: Mon, 18 Nov 2024 07:53:45 -0800 Subject: [PATCH 01/51] Release 24.11.1 --- Cargo.lock | 2 +- Cargo.toml | 2 +- NEWS.md | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index b52e3d3c..08755c72 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -216,7 +216,7 @@ dependencies = [ [[package]] name = "cargo-mutants" -version = "24.11.0" +version = "24.11.1" dependencies = [ "anyhow", "assert_cmd", diff --git a/Cargo.toml b/Cargo.toml index cc8a3222..d5dd85dd 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "cargo-mutants" -version = "24.11.0" +version = "24.11.1" edition = "2021" authors = ["Martin Pool"] license = "MIT" diff --git a/NEWS.md b/NEWS.md index c58471b4..0f59e2b3 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,6 +1,6 @@ # cargo-mutants changelog -## Unreleased +## 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`. From d796f97d8f3623a40469541ad4700cdd953877c2 Mon Sep 17 00:00:00 2001 From: Martin Pool Date: Fri, 22 Nov 2024 07:45:58 -0800 Subject: [PATCH 02/51] ci: cargo update and test in the test job One fewer set of jobs to schedule and launch --- .github/workflows/tests.yml | 22 +++------------------- 1 file changed, 3 insertions(+), 19 deletions(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index e33590a1..0dfa8476 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -115,6 +115,9 @@ jobs: - name: Check clippy # TODO: Deny warnings run: cargo clippy --all-targets --all-features + - run: cargo update + - name: Test after cargo update + run: cargo test --workspace minimal-versions: needs: [quick-test] @@ -134,24 +137,6 @@ jobs: 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: @@ -303,7 +288,6 @@ jobs: quick-test, test, minimal-versions, - maximal-versions, tests-from-tarball, install, pr-mutants, From e3b31670bc254f13c431a2511a22a741a3070c71 Mon Sep 17 00:00:00 2001 From: Martin Pool Date: Fri, 22 Nov 2024 07:48:04 -0800 Subject: [PATCH 03/51] ci: Also run minimal versions in the tests job Might also be faster and need fewer jobs --- .github/workflows/tests.yml | 25 ++++++------------------- 1 file changed, 6 insertions(+), 19 deletions(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 0dfa8476..6b3c6cb5 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -118,24 +118,12 @@ jobs: - run: cargo update - name: Test after cargo update run: cargo test --workspace - - 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 + - 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 tests-from-tarball: needs: [quick-test] @@ -287,7 +275,6 @@ jobs: [ quick-test, test, - minimal-versions, tests-from-tarball, install, pr-mutants, From f8e2cf15f945c6d45c70d504b9401c834b744b60 Mon Sep 17 00:00:00 2001 From: Martin Pool Date: Fri, 22 Nov 2024 18:00:21 -0800 Subject: [PATCH 04/51] clippy: silence warning about too many bools in Args --- src/main.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/main.rs b/src/main.rs index 4c193d02..a67aeb28 100644 --- a/src/main.rs +++ b/src/main.rs @@ -102,6 +102,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, From a33c5fbb5ffa4c2c569281d7b76e970c6a0e1741 Mon Sep 17 00:00:00 2001 From: Martin Pool Date: Fri, 22 Nov 2024 18:01:50 -0800 Subject: [PATCH 05/51] clippy: add semicolons --- src/console.rs | 16 ++++++++-------- src/fnvalue.rs | 4 ++-- src/visit.rs | 8 ++++---- tests/util/mod.rs | 2 +- 4 files changed, 15 insertions(+), 15 deletions(-) diff --git a/src/console.rs b/src/console.rs index c6d6c1c0..9125633f 100644 --- a/src/console.rs +++ b/src/console.rs @@ -172,7 +172,7 @@ impl Console { .iter_mut() .find(|m| m.dest == dest) .expect("copy in progress") - .bytes_copied(total_bytes) + .bytes_copied(total_bytes); }); } @@ -186,7 +186,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 +199,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 +219,7 @@ impl Console { } pub fn clear(&self) { - self.view.clear() + self.view.clear(); } pub fn message(&self, message: &str) { @@ -231,7 +231,7 @@ impl Console { } pub fn tick(&self) { - self.view.update(|_| ()) + self.view.update(|_| ()); } /// Return a tracing `MakeWriter` that will send messages via nutmeg to the console. @@ -377,7 +377,7 @@ impl nutmeg::Model for LabModel { } for copy_model in self.copy_models.iter_mut() { if !s.is_empty() { - s.push('\n') + s.push('\n'); } s.push_str(©_model.render(width)); } @@ -573,7 +573,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/fnvalue.rs b/src/fnvalue.rs index c5e1baab..e8fa12b3 100644 --- a/src/fnvalue.rs +++ b/src/fnvalue.rs @@ -701,7 +701,7 @@ mod test { parse_quote! { -> (bool, usize) }, &[], &["(true, 0)", "(true, 1)", "(false, 0)", "(false, 1)"], - ) + ); } #[test] @@ -717,7 +717,7 @@ mod test { "(false, Some(String::new()))", r#"(false, Some("xyzzy".into()))"#, ], - ) + ); } #[test] diff --git a/src/visit.rs b/src/visit.rs index babe4117..21f755ba 100644 --- a/src/visit.rs +++ b/src/visit.rs @@ -44,7 +44,7 @@ impl Discovered { trace!(?name, "skip previously caught mutant"); } !c - }) + }); } } @@ -78,7 +78,7 @@ pub fn walk_tree( &mod_path, &source_file.package_name, false, - )?) + )?); } } let path = &source_file.tree_relative_path; @@ -270,7 +270,7 @@ impl DiscoveryVisitor<'_> { span, replacement: replacement.to_pretty_string(), genre, - }) + }); } fn collect_fn_mutants(&mut self, sig: &Signature, block: &Block) { @@ -757,7 +757,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(()) }) { 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(()) }) From 79bd104d148fe3f7544183120fd60e850ae76fc5 Mon Sep 17 00:00:00 2001 From: Martin Pool Date: Fri, 22 Nov 2024 18:10:04 -0800 Subject: [PATCH 06/51] clippy: remove unneccessary Option --- src/timeouts.rs | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/src/timeouts.rs b/src/timeouts.rs index 47d191df..d0f159cd 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,10 @@ fn test_timeout(baseline_duration: Option, options: &Options) -> Optio humantime::format_duration(timeout) ); } - Some(timeout) + timeout } else { warn_fallback_timeout("test", "--baseline=skip"); - Some(Duration::from_secs(FALLBACK_TIMEOUT_SECS)) + Duration::from_secs(FALLBACK_TIMEOUT_SECS) } } @@ -113,7 +113,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 +124,7 @@ mod test { assert_eq!( test_timeout(Some(Duration::from_secs(40)), &options), - Some(Duration::from_secs(60)), + Duration::from_secs(60), ); } @@ -167,7 +167,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 +195,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 +240,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); } } From 97354b66a97383602403ff93f07dd7cbac8cec50 Mon Sep 17 00:00:00 2001 From: Martin Pool Date: Fri, 22 Nov 2024 18:13:06 -0800 Subject: [PATCH 07/51] clippy: pedantic in visit.rs --- src/visit.rs | 69 +++++++++++++++++++++++----------------------------- 1 file changed, 31 insertions(+), 38 deletions(-) diff --git a/src/visit.rs b/src/visit.rs index 21f755ba..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. /// @@ -72,7 +75,7 @@ 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, @@ -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,7 +265,7 @@ 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(), @@ -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; @@ -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() { From a4eccfc6d833feb4a49c01d04b0bae5b6c3b80dd Mon Sep 17 00:00:00 2001 From: Martin Pool Date: Fri, 22 Nov 2024 18:16:16 -0800 Subject: [PATCH 08/51] clippy: pedantic in fnvalue.rs --- src/fnvalue.rs | 67 +++++++++++++++++++++++++------------------------- 1 file changed, 34 insertions(+), 33 deletions(-) diff --git a/src/fnvalue.rs b/src/fnvalue.rs index e8fa12b3..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,7 +699,7 @@ mod test { #[test] fn tuple_combinations() { check_replacements( - parse_quote! { -> (bool, usize) }, + &parse_quote! { -> (bool, usize) }, &[], &["(true, 0)", "(true, 1)", "(false, 0)", "(false, 1)"], ); @@ -707,7 +708,7 @@ mod test { #[test] fn tuple_combination_longer() { check_replacements( - parse_quote! { -> (bool, Option) }, + &parse_quote! { -> (bool, Option) }, &[], &[ "(true, None)", @@ -723,7 +724,7 @@ mod test { #[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(), From a6c73006aa5f34377146d62cf59d68d1013e1065 Mon Sep 17 00:00:00 2001 From: Martin Pool Date: Fri, 22 Nov 2024 18:20:10 -0800 Subject: [PATCH 09/51] clippy: pedantic in options.rs --- src/options.rs | 43 ++++++++++++++++++++++++++----------------- 1 file changed, 26 insertions(+), 17 deletions(-) diff --git a/src/options.rs b/src/options.rs index 0f596736..5e62e9b5 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 @@ -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(); From c537e59aa517fd2b1d948c67deeaacfdb8d9e39c Mon Sep 17 00:00:00 2001 From: Martin Pool Date: Fri, 22 Nov 2024 18:22:23 -0800 Subject: [PATCH 10/51] clippy: Option<&> rather than &Option --- src/cargo.rs | 2 +- src/lab.rs | 4 ++-- src/process.rs | 12 ++++++------ 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/cargo.rs b/src/cargo.rs index 5360017d..ec39b6ae 100644 --- a/src/cargo.rs +++ b/src/cargo.rs @@ -23,7 +23,7 @@ use crate::Result; #[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, diff --git a/src/lab.rs b/src/lab.rs index cd487a3e..2aa8e780 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, } diff --git a/src/process.rs b/src/process.rs index 25f0600e..2c7f816e 100644 --- a/src/process.rs +++ b/src/process.rs @@ -5,7 +5,8 @@ //! 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. +// #![allow(clippy::option_map_unit_fn)] // I don't think it's clearer with if/let. +#![warn(clippy::pedantic)] use std::ffi::OsStr; #[cfg(unix)] @@ -41,7 +42,7 @@ impl Process { env: &[(String, String)], cwd: &Utf8Path, timeout: Option, - jobserver: &Option, + jobserver: Option<&jobserver::Client>, scenario_output: &mut ScenarioOutput, console: &Console, ) -> Result { @@ -49,10 +50,9 @@ impl Process { 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,7 +64,7 @@ impl Process { env: &[(String, String)], cwd: &Utf8Path, timeout: Option, - jobserver: &Option, + jobserver: Option<&jobserver::Client>, scenario_output: &mut ScenarioOutput, ) -> Result { let start = Instant::now(); From 3744fc465141e7a076f268dcf783a401addd7a7c Mon Sep 17 00:00:00 2001 From: Martin Pool Date: Fri, 22 Nov 2024 18:27:29 -0800 Subject: [PATCH 11/51] clippy: cleanup process.rs --- src/process.rs | 26 ++++++++++++++++---------- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/src/process.rs b/src/process.rs index 2c7f816e..f09ac920 100644 --- a/src/process.rs +++ b/src/process.rs @@ -5,8 +5,8 @@ //! 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)] @@ -80,7 +80,9 @@ impl Process { .stdout(scenario_output.open_log_append()?) .stderr(scenario_output.open_log_append()?) .current_dir(cwd); - jobserver.as_ref().map(|js| js.configure(&mut child)); + if let Some(js) = jobserver { + js.configure(&mut child); + } #[cfg(unix)] child.process_group(0); let child = child @@ -109,12 +111,16 @@ impl Process { if code == 0 { return Ok(Some(ProcessStatus::Success)); } else { - return Ok(Some(ProcessStatus::Failure(code as u32))); + return Ok(Some(ProcessStatus::Failure( + code.try_into().context("Read exit code as u32")?, + ))); } } #[cfg(unix)] if let Some(signal) = status.signal() { - return Ok(Some(ProcessStatus::Signalled(signal as u8))); + return Ok(Some(ProcessStatus::Signalled( + signal.try_into().context("Read signal as u8")?, + ))); } Ok(Some(ProcessStatus::Other)) } else { @@ -187,15 +193,15 @@ pub enum ProcessStatus { } impl ProcessStatus { - pub fn is_success(&self) -> bool { - *self == ProcessStatus::Success + pub fn is_success(self) -> bool { + self == ProcessStatus::Success } - pub fn is_timeout(&self) -> bool { - *self == ProcessStatus::Timeout + pub fn is_timeout(self) -> bool { + self == ProcessStatus::Timeout } - pub fn is_failure(&self) -> bool { + pub fn is_failure(self) -> bool { matches!(self, ProcessStatus::Failure(_)) } } @@ -226,7 +232,7 @@ mod test { fn shell_quoting() { assert_eq!(cheap_shell_quote(["foo".to_string()]), "foo"); assert_eq!( - cheap_shell_quote(["foo bar", r#"\blah\t"#, r#""quoted""#]), + cheap_shell_quote(["foo bar", r"\blah\t", r#""quoted""#]), r#"foo\ bar \\blah\\t \"quoted\""# ); } From 41edeea654fd6afe5f6174ae209ce99b141df865 Mon Sep 17 00:00:00 2001 From: Martin Pool Date: Fri, 22 Nov 2024 18:29:20 -0800 Subject: [PATCH 12/51] refactor: rename ProcessStatus to Exit --- src/cargo.rs | 4 ++-- src/outcome.rs | 14 +++++++------- src/process.rs | 24 ++++++++++++------------ 3 files changed, 21 insertions(+), 21 deletions(-) diff --git a/src/cargo.rs b/src/cargo.rs index ec39b6ae..0d765945 100644 --- a/src/cargo.rs +++ b/src/cargo.rs @@ -16,7 +16,7 @@ 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. @@ -56,7 +56,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 { diff --git a/src/outcome.rs b/src/outcome.rs index 72413032..0ca5cab3 100644 --- a/src/outcome.rs +++ b/src/outcome.rs @@ -14,7 +14,7 @@ use serde::Serializer; use tracing::warn; use crate::console::plural; -use crate::process::ProcessStatus; +use crate::process::Exit; use crate::*; /// What phase of running a scenario. @@ -193,7 +193,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 +284,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, } @@ -324,7 +324,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 +339,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 +355,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/process.rs b/src/process.rs index f09ac920..3078ea33 100644 --- a/src/process.rs +++ b/src/process.rs @@ -45,7 +45,7 @@ impl Process { 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()? { @@ -97,11 +97,11 @@ 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()?; @@ -109,20 +109,20 @@ impl Process { } else if let Some(status) = self.child.try_wait()? { if let Some(code) = status.code() { if code == 0 { - return Ok(Some(ProcessStatus::Success)); + return Ok(Some(Exit::Success)); } else { - return Ok(Some(ProcessStatus::Failure( + return Ok(Some(Exit::Failure( code.try_into().context("Read exit code as u32")?, ))); } } #[cfg(unix)] if let Some(signal) = status.signal() { - return Ok(Some(ProcessStatus::Signalled( + return Ok(Some(Exit::Signalled( signal.try_into().context("Read signal as u8")?, ))); } - Ok(Some(ProcessStatus::Other)) + Ok(Some(Exit::Other)) } else { Ok(None) } @@ -179,7 +179,7 @@ fn terminate_child_impl(child: &mut Child) -> Result<()> { /// 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. @@ -192,17 +192,17 @@ pub enum ProcessStatus { Other, } -impl ProcessStatus { +impl Exit { pub fn is_success(self) -> bool { - self == ProcessStatus::Success + self == Exit::Success } pub fn is_timeout(self) -> bool { - self == ProcessStatus::Timeout + self == Exit::Timeout } pub fn is_failure(self) -> bool { - matches!(self, ProcessStatus::Failure(_)) + matches!(self, Exit::Failure(_)) } } From 4b5c2caf3d41267f97c05e2b08eddecf2b312947 Mon Sep 17 00:00:00 2001 From: Martin Pool Date: Fri, 22 Nov 2024 18:34:27 -0800 Subject: [PATCH 13/51] clippy: cargo.rs --- src/cargo.rs | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/src/cargo.rs b/src/cargo.rs index 0d765945..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}; @@ -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()]); From b9bbee1132ca43e71ac36273c231b18751ab2d36 Mon Sep 17 00:00:00 2001 From: Martin Pool Date: Sat, 23 Nov 2024 08:58:51 -0800 Subject: [PATCH 14/51] clippy --- src/console.rs | 31 ++++++++++++++++--------------- src/copy_tree.rs | 2 +- src/in_diff.rs | 14 +++++++------- src/main.rs | 13 +++++++------ src/manifest.rs | 1 + src/mutate.rs | 4 ++-- 6 files changed, 34 insertions(+), 31 deletions(-) diff --git a/src/console.rs b/src/console.rs index 9125633f..c9c35d9f 100644 --- a/src/console.rs +++ b/src/console.rs @@ -43,14 +43,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())); @@ -93,7 +85,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, @@ -227,7 +221,7 @@ impl Console { // stderr... // self.view.clear(); - print!("{}", message); + print!("{message}"); } pub fn tick(&self) { @@ -254,8 +248,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 +272,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() @@ -375,13 +376,13 @@ impl nutmeg::Model for LabModel { 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_str(©_model.render(width)); } - for sm in self.scenario_models.iter_mut() { + for sm in &mut self.scenario_models { s.push_str(&sm.render(width)); s.push('\n'); } diff --git a/src/copy_tree.rs b/src/copy_tree.rs index ea759fd1..185de9ad 100644 --- a/src/copy_tree.rs +++ b/src/copy_tree.rs @@ -32,7 +32,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, 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/main.rs b/src/main.rs index a67aeb28..5585d857 100644 --- a/src/main.rs +++ b/src/main.rs @@ -49,6 +49,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}; @@ -426,8 +427,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 { @@ -515,7 +516,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: {:?}", @@ -539,9 +540,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..e6a30962 100644 --- a/src/manifest.rs +++ b/src/manifest.rs @@ -18,6 +18,7 @@ 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")?; if let Some(changed_toml) = fix_manifest_toml(&toml_str, source_dir)? { diff --git a/src/mutate.rs b/src/mutate.rs index fd5c3525..e49ae707 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 { From 7bb9ca3dac395e94aaa18603ddedcd2c44a90b30 Mon Sep 17 00:00:00 2001 From: Martin Pool Date: Sat, 23 Nov 2024 09:08:41 -0800 Subject: [PATCH 15/51] clippy --- src/mutate.rs | 28 ++++++++++++++-------------- src/outcome.rs | 9 ++++++--- src/output.rs | 10 ++++++---- src/package.rs | 1 + src/path.rs | 2 +- src/pretty.rs | 4 ++-- src/source.rs | 5 +++-- src/span.rs | 18 +++++++++--------- 8 files changed, 42 insertions(+), 35 deletions(-) diff --git a/src/mutate.rs b/src/mutate.rs index e49ae707..b087e014 100644 --- a/src/mutate.rs +++ b/src/mutate.rs @@ -186,7 +186,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 +236,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 +327,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 +351,7 @@ mod test { Self { x } } } - "# }; + " }; let mutants = mutate_source_str(code, &Options::default()).unwrap(); assert_eq!(mutants, []); } @@ -422,6 +422,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/outcome.rs b/src/outcome.rs index 0ca5cab3..5247bbfc 100644 --- a/src/outcome.rs +++ b/src/outcome.rs @@ -36,7 +36,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 +53,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 +141,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 +175,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(), } @@ -311,6 +313,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, diff --git a/src/output.rs b/src/output.rs index 1505489a..2509d32a 100644 --- a/src/output.rs +++ b/src/output.rs @@ -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. 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..2779dffa 100644 --- a/src/pretty.rs +++ b/src/pretty.rs @@ -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 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; - "# } + " } ); } } From b6573478df91292b3c2d1a57d89a1d56ddff733b Mon Sep 17 00:00:00 2001 From: Martin Pool Date: Sat, 23 Nov 2024 09:19:06 -0800 Subject: [PATCH 16/51] fix: Don't emit warning about timesout with --check --- src/timeouts.rs | 3 +++ tests/check_build.rs | 17 +++++++---------- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/timeouts.rs b/src/timeouts.rs index d0f159cd..2d191442 100644 --- a/src/timeouts.rs +++ b/src/timeouts.rs @@ -69,6 +69,9 @@ fn test_timeout(baseline_duration: Option, options: &Options) -> Durat ); } 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"); Duration::from_secs(FALLBACK_TIMEOUT_SECS) 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!({ From 499c420a8ed5bb335cb1178925d3b38e45fca4ff Mon Sep 17 00:00:00 2001 From: Martin Pool Date: Sat, 23 Nov 2024 09:23:18 -0800 Subject: [PATCH 17/51] clippy: Turn on most pedantic warnings --- src/main.rs | 3 +++ tests/in_place.rs | 21 +++++++++++---------- tests/iterate.rs | 1 + 3 files changed, 15 insertions(+), 10 deletions(-) diff --git a/src/main.rs b/src/main.rs index 5585d857..c9fe8a21 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; 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(); From 87c1cd283452cb5a6bb01f054aed06bee6d0eb8e Mon Sep 17 00:00:00 2001 From: Martin Pool Date: Sat, 23 Nov 2024 09:26:06 -0800 Subject: [PATCH 18/51] clippy: remove glob imports --- src/outcome.rs | 8 +++++--- src/output.rs | 9 +++++---- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/src/outcome.rs b/src/outcome.rs index 5247bbfc..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; @@ -15,7 +17,7 @@ use tracing::warn; use crate::console::plural; use crate::process::Exit; -use crate::*; +use crate::{exit_code, output, Options, Result, Scenario}; /// What phase of running a scenario. /// diff --git a/src/output.rs b/src/output.rs index 2509d32a..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"; @@ -372,6 +372,7 @@ mod test { use tempfile::{tempdir, TempDir}; use super::*; + use crate::workspace::Workspace; fn minimal_source_tree() -> TempDir { let tmp = tempdir().unwrap(); From 3437a801ff3bb398371ba1ff969061c12d7a64dd Mon Sep 17 00:00:00 2001 From: Martin Pool Date: Sat, 23 Nov 2024 09:26:48 -0800 Subject: [PATCH 19/51] clippy: rm glob use --- src/pretty.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/pretty.rs b/src/pretty.rs index 2779dffa..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. @@ -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() { From bdb09c36ce2c25aa18a690cc29af015815a2c3eb Mon Sep 17 00:00:00 2001 From: Martin Pool Date: Sat, 23 Nov 2024 09:29:42 -0800 Subject: [PATCH 20/51] clippy: rm unnecessary Results --- src/console.rs | 27 +++++++-------------------- src/lab.rs | 2 +- src/tail_file.rs | 8 ++++---- 3 files changed, 12 insertions(+), 25 deletions(-) diff --git a/src/console.rs b/src/console.rs index c9c35d9f..b04e66d6 100644 --- a/src/console.rs +++ b/src/console.rs @@ -9,7 +9,6 @@ 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; @@ -22,7 +21,7 @@ use crate::options::Colors; use crate::outcome::{LabOutcome, SummaryOutcome}; use crate::scenario::Scenario; use crate::tail_file::TailFile; -use crate::{Mutant, Options, Phase, Result, ScenarioOutcome}; +use crate::{Mutant, Options, Phase, ScenarioOutcome}; /// An interface to the console for the rest of cargo-mutants. /// @@ -62,18 +61,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. @@ -498,21 +491,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) { diff --git a/src/lab.rs b/src/lab.rs index 2aa8e780..81873a79 100644 --- a/src/lab.rs +++ b/src/lab.rs @@ -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/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(), From 067722cd3a31811c51d7bb3876e3e9f80b2958bc Mon Sep 17 00:00:00 2001 From: Martin Pool Date: Sat, 23 Nov 2024 10:55:01 -0800 Subject: [PATCH 21/51] clippy: silence false positive? --- src/mutate.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/mutate.rs b/src/mutate.rs index b087e014..a21634ef 100644 --- a/src/mutate.rs +++ b/src/mutate.rs @@ -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()) } From 79cb7a75bff0895d0b6efae20e0e84e162391185 Mon Sep 17 00:00:00 2001 From: Martin Pool Date: Sat, 23 Nov 2024 11:20:52 -0800 Subject: [PATCH 22/51] Better error on failing to read/write scratch manifest --- src/manifest.rs | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/src/manifest.rs b/src/manifest.rs index 15152aba..09d578c4 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; @@ -19,11 +19,15 @@ use crate::Result; /// `manifest_source_dir` is the directory originally containing the manifest, from /// which the absolute paths are calculated. 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 +105,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")?; } } From f3be0b7723924cb2a3b12f53a4c3d56f98155a1e Mon Sep 17 00:00:00 2001 From: Martin Pool Date: Sun, 24 Nov 2024 07:59:41 -0800 Subject: [PATCH 23/51] refactor: split out os-specific code --- src/copy_tree.rs | 43 ++++++++++------------------------------ src/copy_tree/unix.rs | 14 +++++++++++++ src/copy_tree/windows.rs | 22 ++++++++++++++++++++ 3 files changed, 47 insertions(+), 32 deletions(-) create mode 100644 src/copy_tree/unix.rs create mode 100644 src/copy_tree/windows.rs diff --git a/src/copy_tree.rs b/src/copy_tree.rs index ea759fd1..72a421b1 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] = &[ @@ -105,30 +111,3 @@ pub fn copy_tree( debug!(?total_bytes, ?total_files, temp_dir = ?temp_dir.path(), "Copied source 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(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); - } - 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..44f65f67 --- /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 +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(()) +} From b87dab84945592ead5b4de05045a69214ac848b9 Mon Sep 17 00:00:00 2001 From: Martin Pool Date: Sun, 24 Nov 2024 08:18:40 -0800 Subject: [PATCH 24/51] fix: visibility of Windows impl --- src/copy_tree/windows.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/copy_tree/windows.rs b/src/copy_tree/windows.rs index 44f65f67..19b91470 100644 --- a/src/copy_tree/windows.rs +++ b/src/copy_tree/windows.rs @@ -6,7 +6,7 @@ use camino::Utf8Path; use crate::Result; #[mutants::skip] // Mutant tests run on Linux -fn copy_symlink(ft: FileType, src_path: &Utf8Path, dest_path: &Utf8Path) -> 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!("read link {src_path:?}"))?; if ft.is_symlink_dir() { From c95bc57ae6d0311774f0c492915052b78fa5c63e Mon Sep 17 00:00:00 2001 From: Martin Pool Date: Sun, 24 Nov 2024 08:19:49 -0800 Subject: [PATCH 25/51] fix: Don't read .gitignore from above the git root Turns out `ignore` crate will do this unless `require_git` is set. Fixes #450 --- src/copy_tree.rs | 42 +++++++++++++++++++++++++++++++++++++++--- 1 file changed, 39 insertions(+), 3 deletions(-) diff --git a/src/copy_tree.rs b/src/copy_tree.rs index 72a421b1..32333e28 100644 --- a/src/copy_tree.rs +++ b/src/copy_tree.rs @@ -59,9 +59,9 @@ pub fn copy_tree( console.start_copy(dest); for entry in WalkBuilder::new(from_path) .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()) }) @@ -111,3 +111,39 @@ pub fn copy_tree( debug!(?total_bytes, ?total_files, temp_dir = ?temp_dir.path(), "Copied source tree"); Ok(temp_dir) } + +#[cfg(test)] +mod test { + use std::fs::{create_dir, write}; + + 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(()) + } +} From c1fadcb0571d10c698746e9a095c26f11718e83c Mon Sep 17 00:00:00 2001 From: Martin Pool Date: Sun, 24 Nov 2024 08:23:17 -0800 Subject: [PATCH 26/51] fix: Test must create a .git dir for ignores to work --- tests/build_dir.rs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) 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"]) From 9d67f08a2d04ac02ce1dfd8c9ceef6851a510108 Mon Sep 17 00:00:00 2001 From: Martin Pool Date: Sun, 24 Nov 2024 08:23:45 -0800 Subject: [PATCH 27/51] internal: Log debug form of ignore::WalkBuilder --- src/copy_tree.rs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/copy_tree.rs b/src/copy_tree.rs index 32333e28..a02d2ff2 100644 --- a/src/copy_tree.rs +++ b/src/copy_tree.rs @@ -57,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) // 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 From 8fb0c1f2691095400d92a292bbfeed01d4063ac2 Mon Sep 17 00:00:00 2001 From: Martin Pool Date: Sun, 24 Nov 2024 08:30:45 -0800 Subject: [PATCH 28/51] doc: gitignore behavior --- book/src/build-dirs.md | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) 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. From 24537c7adba12364d794c44887a1a5029e19c503 Mon Sep 17 00:00:00 2001 From: Martin Pool Date: Sun, 24 Nov 2024 08:56:45 -0800 Subject: [PATCH 29/51] News for #450 --- NEWS.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/NEWS.md b/NEWS.md index 0f59e2b3..2e59637b 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,5 +1,11 @@ # cargo-mutants changelog +## Unreleased + +- 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`. From ab716606900be118bd3c681b8e9bd69ca9982ad0 Mon Sep 17 00:00:00 2001 From: Martin Pool Date: Sun, 24 Nov 2024 09:14:33 -0800 Subject: [PATCH 30/51] refactor: split per-os process impls --- src/process.rs | 48 ++++++++++++------------------------------ src/process/unix.rs | 31 +++++++++++++++++++++++++++ src/process/windows.rs | 10 +++++++++ 3 files changed, 54 insertions(+), 35 deletions(-) create mode 100644 src/process/unix.rs create mode 100644 src/process/windows.rs diff --git a/src/process.rs b/src/process.rs index 25f0600e..fb897229 100644 --- a/src/process.rs +++ b/src/process.rs @@ -8,16 +8,14 @@ #![allow(clippy::option_map_unit_fn)] // I don't think it's clearer with if/let. 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 +25,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::terminate_child_impl; + +#[cfg(unix)] +mod unix; +#[cfg(unix)] +use unix::terminate_child_impl; + pub struct Process { child: Child, start: Instant, @@ -126,7 +134,7 @@ 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(); @@ -141,36 +149,6 @@ 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 { diff --git a/src/process/unix.rs b/src/process/unix.rs new file mode 100644 index 00000000..21a7545c --- /dev/null +++ b/src/process/unix.rs @@ -0,0 +1,31 @@ +use std::os::unix::process::{CommandExt, ExitStatusExt}; +use std::process::Child; + +use anyhow::{bail, Context}; +use nix::errno::Errno; +use nix::sys::signal::{killpg, Signal}; +use nix::unistd::Pid; +use trace::warn; + +use crate::Result; + +#[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_impl(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); + } + } +} diff --git a/src/process/windows.rs b/src/process/windows.rs new file mode 100644 index 00000000..2a9bb2ba --- /dev/null +++ b/src/process/windows.rs @@ -0,0 +1,10 @@ +use std::process::Child; + +use anyhow::Context; + +use crate::Result; + +#[mutants::skip] // hard to exercise the ESRCH edge case +pub(super) fn terminate_child_impl(child: &mut Child) -> Result<()> { + child.kill().context("Kill child") +} From 90177098dde8696918e606b73c2c85f11f0d061b Mon Sep 17 00:00:00 2001 From: Martin Pool Date: Sun, 24 Nov 2024 09:15:35 -0800 Subject: [PATCH 31/51] lint: ProcessStatus::Signalled is never constructed on Windows --- src/process.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/process.rs b/src/process.rs index fb897229..38b8340c 100644 --- a/src/process.rs +++ b/src/process.rs @@ -159,6 +159,7 @@ pub enum ProcessStatus { /// Exceeded its timeout, and killed. Timeout, /// Killed by some signal. + #[cfg(unix)] Signalled(u8), /// Unknown or unexpected situation. Other, From bb486736f1032dc83d35456f8464674738748c8e Mon Sep 17 00:00:00 2001 From: Martin Pool Date: Sun, 24 Nov 2024 09:16:51 -0800 Subject: [PATCH 32/51] chore: Release 24.11.2 --- Cargo.lock | 2 +- Cargo.toml | 2 +- NEWS.md | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 08755c72..623e5f90 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -216,7 +216,7 @@ dependencies = [ [[package]] name = "cargo-mutants" -version = "24.11.1" +version = "24.11.2" dependencies = [ "anyhow", "assert_cmd", diff --git a/Cargo.toml b/Cargo.toml index d5dd85dd..22587b6d 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "cargo-mutants" -version = "24.11.1" +version = "24.11.2" edition = "2021" authors = ["Martin Pool"] license = "MIT" diff --git a/NEWS.md b/NEWS.md index 2e59637b..1834115f 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,6 +1,6 @@ # cargo-mutants changelog -## Unreleased +## 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. From 18d0a4a0fde2a8804ee1c2039c53ecdd878f4d68 Mon Sep 17 00:00:00 2001 From: Martin Pool Date: Sun, 24 Nov 2024 09:26:49 -0800 Subject: [PATCH 33/51] refactor: more Process platform separation --- src/process.rs | 30 +++++++++--------------------- src/process/unix.rs | 29 +++++++++++++++++++++++++---- src/process/windows.rs | 17 ++++++++++++++++- 3 files changed, 50 insertions(+), 26 deletions(-) diff --git a/src/process.rs b/src/process.rs index 38b8340c..c6a38b54 100644 --- a/src/process.rs +++ b/src/process.rs @@ -28,12 +28,12 @@ const WAIT_POLL_INTERVAL: Duration = Duration::from_millis(50); #[cfg(windows)] mod windows; #[cfg(windows)] -use windows::terminate_child_impl; +use windows::{configure_command, interpret_exit, terminate_child}; #[cfg(unix)] mod unix; #[cfg(unix)] -use unix::terminate_child_impl; +use unix::{configure_command, interpret_exit, terminate_child}; pub struct Process { child: Child, @@ -80,18 +80,17 @@ impl Process { 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 + jobserver.as_ref().map(|js| js.configure(&mut command)); + configure_command(&mut command); + let child = command .spawn() .with_context(|| format!("failed to spawn {}", argv.join(" ")))?; Ok(Process { @@ -113,18 +112,7 @@ impl 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(interpret_exit(status))) } else { Ok(None) } @@ -139,7 +127,7 @@ impl Process { 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"), diff --git a/src/process/unix.rs b/src/process/unix.rs index 21a7545c..89d00124 100644 --- a/src/process/unix.rs +++ b/src/process/unix.rs @@ -1,17 +1,19 @@ use std::os::unix::process::{CommandExt, ExitStatusExt}; -use std::process::Child; +use std::process::{Child, Command, ExitStatus}; -use anyhow::{bail, Context}; +use anyhow::bail; use nix::errno::Errno; use nix::sys::signal::{killpg, Signal}; use nix::unistd::Pid; -use trace::warn; +use tracing::warn; use crate::Result; +use super::ProcessStatus; + #[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_impl(child: &mut Child) -> Result<()> { +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(()), @@ -29,3 +31,22 @@ pub(super) fn terminate_child_impl(child: &mut Child) -> Result<()> { } } } + +#[mutants::skip] +pub(super) fn configure_command(command: &mut Command) { + command.process_group(0); +} + +pub(super) fn interpret_exit(status: ExitStatus) -> ProcessStatus { + if let Some(code) = status.code() { + if code == 0 { + ProcessStatus::Success + } else { + ProcessStatus::Failure(code as u32) + } + } else if let Some(signal) = status.signal() { + return ProcessStatus::Signalled(signal as u8); + } else { + ProcessStatus::Other + } +} diff --git a/src/process/windows.rs b/src/process/windows.rs index 2a9bb2ba..690a73a0 100644 --- a/src/process/windows.rs +++ b/src/process/windows.rs @@ -5,6 +5,21 @@ use anyhow::Context; use crate::Result; #[mutants::skip] // hard to exercise the ESRCH edge case -pub(super) fn terminate_child_impl(child: &mut Child) -> Result<()> { +pub(super) fn terminate_child(child: &mut Child) -> Result<()> { child.kill().context("Kill child") } + +#[mutants::skip] +pub(super) fn configure_command(command: &mut Command) {} + +pub(super) fn interpret_exit(status: ExitStatus) -> ProcessStatus { + if let Some(code) = status.code() { + if code == 0 { + ProcessStatus::Success + } else { + ProcessStatus::Failure(code as u32) + } + } else { + ProcessStatus::Other + } +} From 3b4a1a2a2f809dde0738ce9c2a5e12cdcece1e1d Mon Sep 17 00:00:00 2001 From: Martin Pool Date: Sun, 24 Nov 2024 09:29:51 -0800 Subject: [PATCH 34/51] fix Windows imports --- src/process/windows.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/process/windows.rs b/src/process/windows.rs index 690a73a0..284fc151 100644 --- a/src/process/windows.rs +++ b/src/process/windows.rs @@ -1,9 +1,11 @@ -use std::process::Child; +use std::process::{Child, Command, ExitStatus}; use anyhow::Context; use crate::Result; +use super::ProcessStatus; + #[mutants::skip] // hard to exercise the ESRCH edge case pub(super) fn terminate_child(child: &mut Child) -> Result<()> { child.kill().context("Kill child") From 0bafd24333ebcef3a5f4b81c5e9399ffb9affa39 Mon Sep 17 00:00:00 2001 From: Martin Pool Date: Sun, 24 Nov 2024 09:33:18 -0800 Subject: [PATCH 35/51] rename ProcessStatus to Exit --- src/cargo.rs | 4 ++-- src/outcome.rs | 14 +++++++------- src/process.rs | 16 ++++++++-------- src/process/unix.rs | 12 ++++++------ src/process/windows.rs | 12 ++++++------ 5 files changed, 29 insertions(+), 29 deletions(-) diff --git a/src/cargo.rs b/src/cargo.rs index 5360017d..1cf59e67 100644 --- a/src/cargo.rs +++ b/src/cargo.rs @@ -16,7 +16,7 @@ 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. @@ -56,7 +56,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 { diff --git a/src/outcome.rs b/src/outcome.rs index 72413032..0ca5cab3 100644 --- a/src/outcome.rs +++ b/src/outcome.rs @@ -14,7 +14,7 @@ use serde::Serializer; use tracing::warn; use crate::console::plural; -use crate::process::ProcessStatus; +use crate::process::Exit; use crate::*; /// What phase of running a scenario. @@ -193,7 +193,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 +284,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, } @@ -324,7 +324,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 +339,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 +355,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/process.rs b/src/process.rs index c6a38b54..0582edd8 100644 --- a/src/process.rs +++ b/src/process.rs @@ -52,7 +52,7 @@ impl Process { jobserver: &Option, 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()? { @@ -102,11 +102,11 @@ 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()?; @@ -139,7 +139,7 @@ impl Process { /// 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. @@ -153,17 +153,17 @@ pub enum ProcessStatus { Other, } -impl ProcessStatus { +impl Exit { pub fn is_success(&self) -> bool { - *self == ProcessStatus::Success + *self == Exit::Success } pub fn is_timeout(&self) -> bool { - *self == ProcessStatus::Timeout + *self == Exit::Timeout } pub fn is_failure(&self) -> bool { - matches!(self, ProcessStatus::Failure(_)) + matches!(self, Exit::Failure(_)) } } diff --git a/src/process/unix.rs b/src/process/unix.rs index 89d00124..998af0c0 100644 --- a/src/process/unix.rs +++ b/src/process/unix.rs @@ -9,7 +9,7 @@ use tracing::warn; use crate::Result; -use super::ProcessStatus; +use super::Exit; #[allow(unknown_lints, clippy::needless_pass_by_ref_mut)] // To match Windows #[mutants::skip] // hard to exercise the ESRCH edge case @@ -37,16 +37,16 @@ pub(super) fn configure_command(command: &mut Command) { command.process_group(0); } -pub(super) fn interpret_exit(status: ExitStatus) -> ProcessStatus { +pub(super) fn interpret_exit(status: ExitStatus) -> Exit { if let Some(code) = status.code() { if code == 0 { - ProcessStatus::Success + Exit::Success } else { - ProcessStatus::Failure(code as u32) + Exit::Failure(code as u32) } } else if let Some(signal) = status.signal() { - return ProcessStatus::Signalled(signal as u8); + return Exit::Signalled(signal as u8); } else { - ProcessStatus::Other + Exit::Other } } diff --git a/src/process/windows.rs b/src/process/windows.rs index 284fc151..00b800bf 100644 --- a/src/process/windows.rs +++ b/src/process/windows.rs @@ -4,7 +4,7 @@ use anyhow::Context; use crate::Result; -use super::ProcessStatus; +use super::Exit; #[mutants::skip] // hard to exercise the ESRCH edge case pub(super) fn terminate_child(child: &mut Child) -> Result<()> { @@ -12,16 +12,16 @@ pub(super) fn terminate_child(child: &mut Child) -> Result<()> { } #[mutants::skip] -pub(super) fn configure_command(command: &mut Command) {} +pub(super) fn configure_command(_command: &mut Command) {} -pub(super) fn interpret_exit(status: ExitStatus) -> ProcessStatus { +pub(super) fn interpret_exit(status: ExitStatus) -> Exit { if let Some(code) = status.code() { if code == 0 { - ProcessStatus::Success + Exit::Success } else { - ProcessStatus::Failure(code as u32) + Exit::Failure(code as u32) } } else { - ProcessStatus::Other + Exit::Other } } From e1b8f646c6c6d0044f922d22caccf0125eb31fc9 Mon Sep 17 00:00:00 2001 From: Martin Pool Date: Sun, 24 Nov 2024 09:38:36 -0800 Subject: [PATCH 36/51] refactor: impl From for Exit --- src/process.rs | 6 +++--- src/process/unix.rs | 20 +++++++++++--------- src/process/windows.rs | 16 +++++++++------- 3 files changed, 23 insertions(+), 19 deletions(-) diff --git a/src/process.rs b/src/process.rs index 0582edd8..eaa6da4f 100644 --- a/src/process.rs +++ b/src/process.rs @@ -28,12 +28,12 @@ const WAIT_POLL_INTERVAL: Duration = Duration::from_millis(50); #[cfg(windows)] mod windows; #[cfg(windows)] -use windows::{configure_command, interpret_exit, terminate_child}; +use windows::{configure_command, terminate_child}; #[cfg(unix)] mod unix; #[cfg(unix)] -use unix::{configure_command, interpret_exit, terminate_child}; +use unix::{configure_command, terminate_child}; pub struct Process { child: Child, @@ -112,7 +112,7 @@ impl Process { self.terminate()?; Err(e) } else if let Some(status) = self.child.try_wait()? { - Ok(Some(interpret_exit(status))) + Ok(Some(status.into())) } else { Ok(None) } diff --git a/src/process/unix.rs b/src/process/unix.rs index 998af0c0..5243456f 100644 --- a/src/process/unix.rs +++ b/src/process/unix.rs @@ -37,16 +37,18 @@ pub(super) fn configure_command(command: &mut Command) { command.process_group(0); } -pub(super) fn interpret_exit(status: ExitStatus) -> Exit { - if let Some(code) = status.code() { - if code == 0 { - Exit::Success +impl From for Exit { + fn from(status: ExitStatus) -> Self { + if let Some(code) = status.code() { + if code == 0 { + Exit::Success + } else { + Exit::Failure(code as u32) + } + } else if let Some(signal) = status.signal() { + return Exit::Signalled(signal as u8); } else { - Exit::Failure(code as u32) + Exit::Other } - } else if let Some(signal) = status.signal() { - return Exit::Signalled(signal as u8); - } else { - Exit::Other } } diff --git a/src/process/windows.rs b/src/process/windows.rs index 00b800bf..9bb913bf 100644 --- a/src/process/windows.rs +++ b/src/process/windows.rs @@ -14,14 +14,16 @@ pub(super) fn terminate_child(child: &mut Child) -> Result<()> { #[mutants::skip] pub(super) fn configure_command(_command: &mut Command) {} -pub(super) fn interpret_exit(status: ExitStatus) -> Exit { - if let Some(code) = status.code() { - if code == 0 { - Exit::Success +impl From for Exit { + fn from(status: ExitStatus) -> Self { + if let Some(code) = status.code() { + if code == 0 { + Exit::Success + } else { + Exit::Failure(code as u32) + } } else { - Exit::Failure(code as u32) + Exit::Other } - } else { - Exit::Other } } From a4a774dd3be95dac2394a4f6206fcccdac186637 Mon Sep 17 00:00:00 2001 From: Martin Pool Date: Sun, 24 Nov 2024 09:42:13 -0800 Subject: [PATCH 37/51] refactor: cheap_shell_quotes for clarity/efficiency --- src/process.rs | 26 ++++++++++++++------------ 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/src/process.rs b/src/process.rs index eaa6da4f..cc3fe4f3 100644 --- a/src/process.rs +++ b/src/process.rs @@ -171,18 +171,20 @@ impl Exit { /// /// 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(" ") + 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' | '\n' | '\r' | '\\' | '\'' | '"' => r.push('\\'), + _ => (), + } + r.push(c); + } + } + r } #[cfg(test)] From 87b8e0ccca0e1e5a35622eb252975c0a1c21ac84 Mon Sep 17 00:00:00 2001 From: Martin Pool Date: Sun, 24 Nov 2024 09:47:11 -0800 Subject: [PATCH 38/51] better argv quoting --- src/process.rs | 31 +++++++++++++++++++++---------- 1 file changed, 21 insertions(+), 10 deletions(-) diff --git a/src/process.rs b/src/process.rs index cc3fe4f3..17bbfb44 100644 --- a/src/process.rs +++ b/src/process.rs @@ -76,7 +76,7 @@ impl Process { 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))); @@ -169,8 +169,9 @@ impl Exit { /// 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 { +/// 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() { @@ -178,10 +179,15 @@ fn cheap_shell_quote, I: IntoIterator>(argv: I) -> Strin } for c in s.as_ref().chars() { match c { - ' ' | '\t' | '\n' | '\r' | '\\' | '\'' | '"' => r.push('\\'), - _ => (), + '\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.push(c); } } r @@ -189,14 +195,19 @@ fn cheap_shell_quote, I: IntoIterator>(argv: I) -> Strin #[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!( - cheap_shell_quote(["foo bar", r#"\blah\t"#, r#""quoted""#]), - r#"foo\ bar \\blah\\t \"quoted\""# + quote_argv(["foo bar", r#"\blah\x"#, r#""quoted""#]), + r#"foo\ bar \\blah\\x \"quoted\""# + ); + assert_eq!(quote_argv([""]), ""); + assert_eq!( + quote_argv(["with whitespace", "\r\n\t\t"]), + r#"with\ whitespace \r\n\t\t"# ); } } From 37c4af20cf5d61b2c701c3d9562febcda381f814 Mon Sep 17 00:00:00 2001 From: Martin Pool Date: Sun, 24 Nov 2024 10:12:19 -0800 Subject: [PATCH 39/51] ci: Don't mutate windows.rs --- .github/workflows/tests.yml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 0a6afe78..13b449a7 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -197,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 - name: Archive mutants.out uses: actions/upload-artifact@v4 if: always() @@ -234,7 +235,7 @@ 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 - name: Archive mutants.out uses: actions/upload-artifact@v4 if: always() From bffa2f6144ce935dbee17e05c03f6a06df3cd0c3 Mon Sep 17 00:00:00 2001 From: Martin Pool Date: Sun, 24 Nov 2024 11:09:22 -0800 Subject: [PATCH 40/51] mutants: skip windows.rs and console.rs --- .github/workflows/tests.yml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 13b449a7..3ec1bf7f 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -197,7 +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=windows.rs --exclude=console.rs - name: Archive mutants.out uses: actions/upload-artifact@v4 if: always() @@ -236,6 +236,7 @@ jobs: cargo mutants --no-shuffle -vV --shard ${{ matrix.shard }}/10 --test-tool ${{ matrix.test_tool }} --baseline=skip --timeout=500 --build-timeout=500 --in-place --exclude=windows.rs + --exclude=console.rs - name: Archive mutants.out uses: actions/upload-artifact@v4 if: always() From a16a43d222e9044b361f93fe68e902f52da697ed Mon Sep 17 00:00:00 2001 From: Martin Pool Date: Sun, 24 Nov 2024 11:22:47 -0800 Subject: [PATCH 41/51] Split out Unix-only tests --- tests/main.rs | 88 +------------------------------------------------ tests/unix.rs | 91 +++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 92 insertions(+), 87 deletions(-) create mode 100644 tests/unix.rs diff --git a/tests/main.rs b/tests/main.rs index 9637a2ae..fd1bd89d 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() { @@ -657,90 +655,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")); +} From da80657d93f8cdda73302f30aaca3f3a266f70dd Mon Sep 17 00:00:00 2001 From: Martin Pool Date: Sun, 24 Nov 2024 11:42:46 -0800 Subject: [PATCH 42/51] ui: Estimate time remaining without counting the baseline Fixes #414 --- NEWS.md | 4 ++++ src/console.rs | 20 ++++++++++---------- 2 files changed, 14 insertions(+), 10 deletions(-) diff --git a/NEWS.md b/NEWS.md index 1834115f..6a3449f1 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,5 +1,9 @@ # cargo-mutants changelog +## 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. diff --git a/src/console.rs b/src/console.rs index c6d6c1c0..9f7da814 100644 --- a/src/console.rs +++ b/src/console.rs @@ -427,18 +427,18 @@ impl nutmeg::Model for LabModel { // } write!(s, ", {} elapsed", style_duration(elapsed)).unwrap(); 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)) + ); } writeln!(s).unwrap(); } From 551791b8d487dd5e1bd9bb4dac130cc0732cb3ea Mon Sep 17 00:00:00 2001 From: Martin Pool Date: Sun, 24 Nov 2024 11:51:39 -0800 Subject: [PATCH 43/51] refactor: Use push_str not write! to build console output --- src/console.rs | 58 +++++++++++++++++++++++--------------------------- 1 file changed, 27 insertions(+), 31 deletions(-) diff --git a/src/console.rs b/src/console.rs index 9f7da814..57102df1 100644 --- a/src/console.rs +++ b/src/console.rs @@ -3,7 +3,6 @@ //! 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}; @@ -19,10 +18,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, Result}; /// An interface to the console for the rest of cargo-mutants. /// @@ -113,14 +112,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 +129,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'); @@ -382,41 +379,40 @@ impl nutmeg::Model for LabModel { s.push_str(©_model.render(width)); } for sm in self.scenario_models.iter_mut() { + 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,7 +421,7 @@ 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 f64; let remain = self.n_mutants as f64 - done; @@ -437,13 +433,9 @@ impl nutmeg::Model for LabModel { } s += &format!( ", about {} remaining", - style_duration(Duration::from_secs_f64(remaining_secs)) + style_duration(Duration::from_secs_f64(remaining_secs.ceil())) ); } - writeln!(s).unwrap(); - } - while s.ends_with('\n') { - s.pop(); } s } @@ -547,7 +539,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 } From 21260b9770e3ef94654233ec144e97b43513b209 Mon Sep 17 00:00:00 2001 From: Oleksii Karpenko Date: Mon, 25 Nov 2024 21:42:46 +0200 Subject: [PATCH 44/51] feat: add output directory config via environment variable and config file --- book/src/mutants-out.md | 3 ++- src/config.rs | 4 +++- src/main.rs | 7 +++++- src/options.rs | 2 +- tests/config.rs | 47 +++++++++++++++++++++++++++++++++++++++++ tests/main.rs | 40 +++++++++++++++++++++++++++++++++++ 6 files changed, 99 insertions(+), 4 deletions(-) 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/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/main.rs b/src/main.rs index c9fe8a21..74629093 100644 --- a/src/main.rs +++ b/src/main.rs @@ -274,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. diff --git a/src/options.rs b/src/options.rs index 5e62e9b5..dfccdc7d 100644 --- a/src/options.rs +++ b/src/options.rs @@ -302,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(), diff --git a/tests/config.rs b/tests/config.rs index 504c30ef..3ce44852 100644 --- a/tests/config.rs +++ b/tests/config.rs @@ -263,3 +263,50 @@ 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"); + write_config_file( + &testdata, + &format!("output = \"{}\"\n", output_via_config.to_str().unwrap()), + ); + + 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/main.rs b/tests/main.rs index fd1bd89d..f428de98 100644 --- a/tests/main.rs +++ b/tests/main.rs @@ -494,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. From 0be02e54544ef11a24ff8346923bb452f187e269 Mon Sep 17 00:00:00 2001 From: Oleksii Karpenko Date: Mon, 25 Nov 2024 22:27:18 +0200 Subject: [PATCH 45/51] Fix windows path issue --- tests/config.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/config.rs b/tests/config.rs index 3ce44852..3a37530a 100644 --- a/tests/config.rs +++ b/tests/config.rs @@ -273,7 +273,7 @@ fn output_option_use_config() { let testdata = copy_of_testdata("factorial"); write_config_file( &testdata, - &format!("output = \"{}\"\n", output_via_config.to_str().unwrap()), + &format!("output = \"{}\"\n", output_via_config.display()), ); assert!( From f387980f352b838e814526796605b66d6ad71408 Mon Sep 17 00:00:00 2001 From: Oleksii Karpenko Date: Tue, 26 Nov 2024 01:32:20 +0200 Subject: [PATCH 46/51] Fix unescaped path in toml --- tests/config.rs | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/tests/config.rs b/tests/config.rs index 3a37530a..57a4b5f1 100644 --- a/tests/config.rs +++ b/tests/config.rs @@ -269,12 +269,13 @@ fn additional_cargo_test_args() { 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"); - write_config_file( - &testdata, - &format!("output = \"{}\"\n", output_via_config.display()), - ); + + 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(), From a2a9a98e9d8a5713eaef0baa835082e2269ea807 Mon Sep 17 00:00:00 2001 From: Martin Pool Date: Tue, 3 Dec 2024 21:46:48 +0000 Subject: [PATCH 47/51] Bump MSRV to 1.78 --- .github/workflows/tests.yml | 2 +- Cargo.toml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 3ec1bf7f..59bd82f8 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -89,7 +89,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 diff --git a/Cargo.toml b/Cargo.toml index 22587b6d..122ab44b 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -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" From 07241f5be94b37af447574c7092c34c78b603055 Mon Sep 17 00:00:00 2001 From: Paolo Barbolini Date: Wed, 4 Dec 2024 05:28:37 +0100 Subject: [PATCH 48/51] Bump nix to 0.29 --- Cargo.lock | 10 +++++----- Cargo.toml | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 623e5f90..421ea209 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -239,7 +239,7 @@ dependencies = [ "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", @@ -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" @@ -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", diff --git a/Cargo.toml b/Cargo.toml index 122ab44b..5b709554 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -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" From 23534898327450eaad1af1cfc0f30f570b2891b2 Mon Sep 17 00:00:00 2001 From: Paolo Barbolini Date: Wed, 4 Dec 2024 05:30:45 +0100 Subject: [PATCH 49/51] Bump itertools to 0.13 --- Cargo.lock | 6 +++--- Cargo.toml | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 623e5f90..e1501192 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -235,7 +235,7 @@ 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)", @@ -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", ] diff --git a/Cargo.toml b/Cargo.toml index 122ab44b..dec0f99c 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -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" From 02c47b3a12feb0f80bf306feb917d3dede2a6f64 Mon Sep 17 00:00:00 2001 From: Paolo Barbolini Date: Wed, 4 Dec 2024 05:43:11 +0100 Subject: [PATCH 50/51] Upgrade cargo_metadata to 0.19 --- Cargo.lock | 44 ++++++++++++++++++++++---------------------- Cargo.toml | 4 ++-- src/workspace.rs | 14 ++++++++++++-- 3 files changed, 36 insertions(+), 26 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 623e5f90..c3cd7d7b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -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", @@ -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]] @@ -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 122ab44b..db9ab64b 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -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", @@ -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" 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; } } From 5ceb45acf423de00fd0220320898ac65407521bc Mon Sep 17 00:00:00 2001 From: Paolo Barbolini Date: Thu, 5 Dec 2024 12:58:34 +0100 Subject: [PATCH 51/51] feat: add git-repository-url and edit-url-template to mdbook html output --- book/book.toml | 2 ++ 1 file changed, 2 insertions(+) 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