Improve own test coverage and remove brittle expected_mutants_for_own…

…_source_tree (#296)

This brittle test was hiding some mutants that were not otherwise well
tested: coverage is better with the additions in this PR.

Also, this test needed to be updated on many commits, which was tedious
and in hindsight a bad smell.

Fixes #295

Cargo.toml

@@ -98,6 +98,7 @@ lazy_static = "1.4"
 predicates = "3"
 pretty_assertions = "1"
 regex = "1.5"
+rusty-fork = "0.3"
 walkdir = "2.3"
 
 [workspace]

NEWS.md

@@ -14,6 +14,8 @@
 
 - Changed: Minimum Rust version (to build cargo-mutants, not to use it) increased to 1.74.
 
+- Changed: Removed the count of `failure` from `mutants.out/outcomes.json`: it was already the case that every outcome received some other classification, so the count was always zero.
+
 ## 24.2.1
 
 - New: `--features`, `--no-default-features` and `--all-features` options are passed through to Cargo.

src/cargo.rs

@@ -37,15 +37,11 @@ pub fn run_cargo(
     let process_status = Process::run(&argv, &env, build_dir.path(), timeout, log_file, console)?;
     check_interrupted()?;
     debug!(?process_status, elapsed = ?start.elapsed());
-    if options.test_tool == TestTool::Nextest && phase == Phase::Test {
-        // Nextest returns detailed exit codes. I think we should still treat any non-zero result as just an
-        // error, but we can at least warn if it's unexpected.
-        if let ProcessStatus::Failure(code) = process_status {
-            // TODO: When we build with `nextest test --no-test` then we should also check build
-            // processes.
-            if code != NextestExitCode::TEST_RUN_FAILED as u32 {
-                warn!(%code, "nextest process exited with unexpected code (not TEST_RUN_FAILED)");
-            }
+    if let ProcessStatus::Failure(code) = process_status {
+        if argv[1] == "nextest" && code != NextestExitCode::TEST_RUN_FAILED as u32 {
+            // Nextest returns detailed exit codes. I think we should still treat any non-zero result as just an
+            // error, but we can at least warn if it's unexpected.
+            warn!(%code, "nextest process exited with unexpected code (not TEST_RUN_FAILED)");
         }
     }
     Ok(PhaseResult {
@@ -158,6 +154,7 @@ mod test {
     use std::sync::Arc;
 
     use pretty_assertions::assert_eq;
+    use rusty_fork::rusty_fork_test;
 
     use super::*;
 
@@ -294,4 +291,27 @@ mod test {
             ]
         );
     }
+
+    rusty_fork_test! {
+        #[test]
+        fn rustflags_with_no_environment_variables() {
+            env::remove_var("RUSTFLAGS");
+            env::remove_var("CARGO_ENCODED_RUSTFLAGS");
+            assert_eq!(rustflags(), "--cap-lints=allow");
+        }
+
+        #[test]
+        fn rustflags_added_to_existing_encoded_rustflags() {
+            env::set_var("RUSTFLAGS", "--something\x1f--else");
+            env::remove_var("CARGO_ENCODED_RUSTFLAGS");
+            assert_eq!(rustflags(), "--something\x1f--else\x1f--cap-lints=allow");
+        }
+
+        #[test]
+        fn rustflags_added_to_existing_rustflags() {
+            env::set_var("RUSTFLAGS", "-Dwarnings");
+            env::remove_var("CARGO_ENCODED_RUSTFLAGS");
+            assert_eq!(rustflags(), "-Dwarnings\x1f--cap-lints=allow");
+        }
+    }
 }

src/config.rs

@@ -11,6 +11,7 @@
 use std::default::Default;
 use std::fs::read_to_string;
 use std::path::Path;
+use std::str::FromStr;
 
 use anyhow::Context;
 use camino::Utf8Path;
@@ -65,3 +66,11 @@ impl Config {
         }
     }
 }
+
+impl FromStr for Config {
+    type Err = anyhow::Error;
+
+    fn from_str(s: &str) -> Result<Self> {
+        toml::de::from_str(s).with_context(|| "parse toml")
+    }
+}

src/console.rs

@@ -142,12 +142,6 @@ impl Console {
         self.message(&s);
     }
 
-    pub fn build_dirs_start(&self, _n: usize) {
-        // self.message(&format!("Make {n} more build directories...\n"));
-    }
-
-    pub fn build_dirs_finished(&self) {}
-
     pub fn start_copy(&self) {
         self.view.update(|model| {
             assert!(model.copy_model.is_none());

src/fnvalue.rs

@@ -1,4 +1,4 @@
-// Copyright 2021-2023 Martin Pool
+// Copyright 2021-2024 Martin Pool
 
 //! Mutations of replacing a function body with a value of a (hopefully) appropriate type.
 
@@ -242,16 +242,19 @@ fn path_ends_with(path: &Path, ident: &str) -> bool {
 fn match_impl_iterator(TypeImplTrait { bounds, .. }: &TypeImplTrait) -> Option<&Type> {
     for bound in bounds {
         if let TypeParamBound::Trait(TraitBound { path, .. }) = bound {
-            if path.segments.len() == 1 && path.segments[0].ident == "Iterator" {
-                if let PathArguments::AngleBracketed(AngleBracketedGenericArguments {
-                    args, ..
-                }) = &path.segments[0].arguments
-                {
-                    if let Some(GenericArgument::AssocType(AssocType { ident, ty, .. })) =
-                        args.first()
+            if let Some(last_segment) = path.segments.last() {
+                if last_segment.ident == "Iterator" {
+                    if let PathArguments::AngleBracketed(AngleBracketedGenericArguments {
+                        args,
+                        ..
+                    }) = &last_segment.arguments
                     {
-                        if ident == "Item" {
-                            return Some(ty);
+                        if let Some(GenericArgument::AssocType(AssocType { ident, ty, .. })) =
+                            args.first()
+                        {
+                            if ident == "Item" {
+                                return Some(ty);
+                            }
                         }
                     }
                 }
@@ -288,6 +291,8 @@ 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.
 fn known_collection(path: &Path) -> Option<(&Ident, &Type)> {
     let last = path.segments.last()?;
     if ![
@@ -432,6 +437,7 @@ mod test {
     use pretty_assertions::assert_eq;
     use syn::{parse_quote, Expr, ReturnType};
 
+    use crate::fnvalue::match_impl_iterator;
     use crate::pretty::ToPrettyString;
 
     use super::{known_map, return_type_replacements};
@@ -580,6 +586,69 @@ mod test {
         );
     }
 
+    #[test]
+    fn match_known_collection() {
+        assert_eq!(
+            super::known_collection(&parse_quote! { std::collections::VecDeque<String> }),
+            Some((&parse_quote! { VecDeque }, &parse_quote! { String }))
+        );
+
+        assert_eq!(
+            super::known_collection(&parse_quote! { std::collections::BinaryHeap<(u32, u32)> }),
+            Some((&parse_quote! { BinaryHeap }, &parse_quote! { (u32, u32) }))
+        );
+
+        assert_eq!(
+            super::known_collection(&parse_quote! { LinkedList<[u8; 256]> }),
+            Some((&parse_quote! { LinkedList }, &parse_quote! { [u8; 256] }))
+        );
+
+        assert_eq!(super::known_collection(&parse_quote! { Arc<String> }), None);
+
+        // This might be a collection, and is handled generically, but it's not a specifically known
+        // collection type. (Maybe we shouldn't bother knowing specific types?)
+        assert_eq!(
+            super::known_collection(&parse_quote! { Wibble<&str> }),
+            None
+        );
+    }
+
+    #[test]
+    fn match_known_map() {
+        assert_eq!(
+            super::known_map(&parse_quote! { std::collections::BTreeMap<String, usize> }),
+            Some((
+                &parse_quote! { BTreeMap },
+                &parse_quote! { String },
+                &parse_quote! { usize }
+            ))
+        );
+
+        assert_eq!(
+            super::known_map(&parse_quote! { std::collections::HashMap<(usize, usize), bool> }),
+            Some((
+                &parse_quote! { HashMap },
+                &parse_quote! { (usize, usize) },
+                &parse_quote! { bool }
+            ))
+        );
+
+        assert_eq!(
+            super::known_map(&parse_quote! { Option<(usize, usize)> }),
+            None
+        );
+
+        assert_eq!(
+            super::known_map(&parse_quote! { MyMap<String, usize> }),
+            None,
+        );
+
+        assert_eq!(
+            super::known_map(&parse_quote! { Pair<String, usize> }),
+            None,
+        );
+    }
+
     #[test]
     fn btreeset_replacement() {
         check_replacements(
@@ -664,6 +733,25 @@ mod test {
         );
     }
 
+    #[test]
+    fn impl_matches_iterator() {
+        assert_eq!(
+            match_impl_iterator(&parse_quote! { impl std::iter::Iterator<Item = String> }),
+            Some(&parse_quote! { String })
+        );
+        assert_eq!(
+            match_impl_iterator(&parse_quote! { impl Iterator<Item = String> }),
+            Some(&parse_quote! { String })
+        );
+        // Strange, maybe it's a type defined in this crate, but we don't know what to
+        // do with it.
+        assert_eq!(match_impl_iterator(&parse_quote! { impl Iterator }), None);
+        assert_eq!(
+            match_impl_iterator(&parse_quote! { impl Borrow<String> }),
+            None
+        );
+    }
+
     #[test]
     fn slice_replacement() {
         check_replacements(

src/in_diff.rs

@@ -117,7 +117,7 @@ fn strip_patch_path(path: &str) -> &Utf8Path {
 ///
 /// If a line is deleted then the range will span from the line before to the line after.
 fn affected_lines(patch: &Patch) -> Vec<usize> {
-    let mut r = Vec::new();
+    let mut affected_lines = Vec::new();
     for hunk in &patch.hunks {
         let mut lineno: usize = hunk.new_range.start.try_into().unwrap();
         // True if the previous line was deleted. If set, then the next line that exists in the
@@ -131,11 +131,11 @@ fn affected_lines(patch: &Patch) -> Vec<usize> {
                 Line::Add(_) | Line::Context(_) => {
                     if prev_removed {
                         debug_assert!(
-                            r.last().map_or(true, |last| *last < lineno),
-                            "{lineno} {r:?}"
+                            affected_lines.last().map_or(true, |last| *last < lineno),
+                            "{lineno} {affected_lines:?}"
                         );
                         debug_assert!(lineno >= 1, "{lineno}");
-                        r.push(lineno);
+                        affected_lines.push(lineno);
                         prev_removed = false;
                     }
                 }
@@ -145,24 +145,28 @@ fn affected_lines(patch: &Patch) -> Vec<usize> {
                     lineno += 1;
                 }
                 Line::Add(_) => {
-                    if r.last().map_or(true, |last| *last < lineno) {
-                        r.push(lineno);
+                    if affected_lines.last().map_or(true, |last| *last != lineno) {
+                        affected_lines.push(lineno);
                     }
                     lineno += 1;
                 }
                 Line::Remove(_) => {
-                    if lineno > 1 && r.last().map_or(true, |last| *last < (lineno - 1)) {
-                        r.push(lineno - 1);
+                    if lineno > 1
+                        && affected_lines
+                            .last()
+                            .map_or(true, |last| *last != (lineno - 1))
+                    {
+                        affected_lines.push(lineno - 1);
                     }
                 }
             }
         }
     }
     debug_assert!(
-        r.iter().tuple_windows().all(|(a, b)| a < b),
-        "remove_context: line numbers not sorted and unique: {r:?}"
+        affected_lines.iter().tuple_windows().all(|(a, b)| a < b),
+        "remove_context: line numbers not sorted and unique: {affected_lines:?}"
     );
-    r
+    affected_lines
 }
 
 /// Recreate a partial view of the new file from a Patch.