diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 56c286ba..955e0440 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -3,7 +3,11 @@ name: Tests permissions: contents: read -on: push +on: + push: + branches: + - main + pull_request: # see https://matklad.github.io/2021/09/04/fast-rust-builds.html env: @@ -43,9 +47,35 @@ jobs: - name: Test run: cargo test --workspace + incremental-mutants: + runs-on: ubuntu-latest + if: github.event_name == 'pull_request' + steps: + - uses: actions/checkout@v3 + with: + fetch-depth: 0 + - name: Relative diff + run: | + git branch -av + git diff origin/${{ github.base_ref }}.. | tee git.diff + - uses: dtolnay/rust-toolchain@master + with: + toolchain: beta + - uses: Swatinem/rust-cache@v2 + - run: cargo install --path . + - name: Mutants + run: | + cargo mutants --no-shuffle --exclude console.rs -j 2 -vV --in-diff git.diff + - name: Archive mutants.out + uses: actions/upload-artifact@v3 + if: always() + with: + name: mutants-incremental.out + path: mutants.out + cargo-mutants: runs-on: ubuntu-latest - needs: build + # needs: [build, incremental-mutants] steps: - uses: actions/checkout@v3 - uses: dtolnay/rust-toolchain@master diff --git a/Cargo.lock b/Cargo.lock index 6f7dd4e0..bbaf6d26 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -11,6 +11,21 @@ dependencies = [ "memchr", ] +[[package]] +name = "android-tzdata" +version = "0.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e999941b234f3131b00bc13c22d06e8c5ff726d1b6318ac7eb276997bbb4fef0" + +[[package]] +name = "android_system_properties" +version = "0.1.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "819e7219dbd41043ac279b19830f2efc897156490d7fd6ea916720117ee66311" +dependencies = [ + "libc", +] + [[package]] name = "anstream" version = "0.5.0" @@ -126,6 +141,12 @@ version = "3.13.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "a3e2c3daef883ecc1b5d58c15adae93470a91d425f3532ba1695849656af3fc1" +[[package]] +name = "bytecount" +version = "0.6.7" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e1e5f035d16fc623ae5f74981db80a439803888314e3a555fd6f04acd51a3205" + [[package]] name = "camino" version = "1.1.6" @@ -158,6 +179,7 @@ dependencies = [ "mutants 0.0.3 (registry+https://github.com/rust-lang/crates.io-index)", "nix", "nutmeg", + "patch", "path-slash", "predicates", "pretty_assertions", @@ -217,6 +239,20 @@ version = "1.0.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "baf1de4339761588bc0619e3cbc0120ee582ebb74b53b4efbf79117bd2da40fd" +[[package]] +name = "chrono" +version = "0.4.31" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7f2c685bad3eb3d45a01354cedb7d5faa66194d1d58ba6e267a8de788f79db38" +dependencies = [ + "android-tzdata", + "iana-time-zone", + "js-sys", + "num-traits", + "wasm-bindgen", + "windows-targets 0.48.5", +] + [[package]] name = "clap" version = "4.4.3" @@ -286,6 +322,12 @@ dependencies = [ "windows-sys 0.45.0", ] +[[package]] +name = "core-foundation-sys" +version = "0.8.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e496a50fda8aacccc86d7529e2c1e0892dbd0f898a6b5645b5561b89c3210efa" + [[package]] name = "cp_r" version = "0.5.1" @@ -470,6 +512,29 @@ version = "0.3.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "443144c8cdadd93ebf52ddb4056d257f5b52c04d3c804e657d19eb73fc33668b" +[[package]] +name = "iana-time-zone" +version = "0.1.58" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8326b86b6cff230b97d0d312a6c40a60726df3332e721f72a1b035f451663b20" +dependencies = [ + "android_system_properties", + "core-foundation-sys", + "iana-time-zone-haiku", + "js-sys", + "wasm-bindgen", + "windows-core", +] + +[[package]] +name = "iana-time-zone-haiku" +version = "0.1.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f31827a206f56af32e590ba56d5d2d085f558508192593743f16b2306495269f" +dependencies = [ + "cc", +] + [[package]] name = "indexmap" version = "2.0.0" @@ -595,6 +660,12 @@ version = "2.6.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "8f232d6ef707e1956a43342693d2a31e72989554d58299d7a88738cc95b0d35c" +[[package]] +name = "minimal-lexical" +version = "0.2.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "68354c5c6bd36d73ff3feceb05efa59b6acb7626617f4962be322a825e61f79a" + [[package]] name = "mutants" version = "0.0.3" @@ -616,6 +687,27 @@ dependencies = [ "libc", ] +[[package]] +name = "nom" +version = "7.1.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d273983c5a657a70a3e8f2a01329822f3b8c8172b73826411a55751e404a0a4a" +dependencies = [ + "memchr", + "minimal-lexical", +] + +[[package]] +name = "nom_locate" +version = "4.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1e3c83c053b0713da60c5b8de47fe8e494fe3ece5267b2f23090a07a053ba8f3" +dependencies = [ + "bytecount", + "memchr", + "nom", +] + [[package]] name = "normalize-line-endings" version = "0.3.0" @@ -688,6 +780,17 @@ dependencies = [ "windows-targets 0.48.5", ] +[[package]] +name = "patch" +version = "0.7.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "15c07fdcdd8b05bdcf2a25bc195b6c34cbd52762ada9dba88bf81e7686d14e7a" +dependencies = [ + "chrono", + "nom", + "nom_locate", +] + [[package]] name = "path-slash" version = "0.2.1" @@ -1280,6 +1383,15 @@ version = "0.4.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "712e227841d057c1ee1cd2fb22fa7e5a5461ae8e48fa2ca79ec42cfc1931183f" +[[package]] +name = "windows-core" +version = "0.51.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f1f8cf84f35d2db49a46868f947758c7a1138116f7fac3bc844f43ade1292e64" +dependencies = [ + "windows-targets 0.48.5", +] + [[package]] name = "windows-sys" version = "0.45.0" diff --git a/Cargo.toml b/Cargo.toml index b8635e3b..5a5fbf5f 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -46,6 +46,7 @@ indoc = "2.0.0" itertools = "0.11" mutants = "0.0.3" nix = "0.27" +patch = "0.7" path-slash = "0.2" quote = "1.0" serde_json = "1" @@ -105,6 +106,8 @@ exclude = [ "testdata/tree/cfg_attr_mutants_skip", "testdata/tree/cfg_attr_test_skip", "testdata/tree/dependency", + "testdata/tree/diff0", + "testdata/tree/diff1", "testdata/tree/error_value", "testdata/tree/everything_skipped", "testdata/tree/factorial", diff --git a/NEWS.md b/NEWS.md index ac1d9f32..38f5725b 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,5 +1,11 @@ # cargo-mutants changelog +## 23.11.1 + +- New `--in-diff FILE` option tests only mutants that are in the diff from the + given file. This is useful to avoid testing mutants from code that has not changed, + either locally or in CI. + ## 23.11.0 - Changed: `cargo mutants` now tries to match the behavior of `cargo test` when run within a workspace. If run in a package directory, it tests only that package. If run in a workspace that is not a package (a "virtual workspace"), it tests the configured default packages, or otherwise all packages. This can all be overridden with the `--package` or `--workspace` options. diff --git a/book/src/SUMMARY.md b/book/src/SUMMARY.md index 95eb27f3..91e104e5 100644 --- a/book/src/SUMMARY.md +++ b/book/src/SUMMARY.md @@ -19,8 +19,10 @@ - [Error values](error-values.md) - [Improving performance](performance.md) - [Parallelism](parallelism.md) + - [Incremental tests of modified code](in-diff.md) - [Integrations](integrations.md) - - [Continuous integration](ci.md) +- [Continuous integration](ci.md) + - [Incremental tests of pull requests](pr-diff.md) - [How it works](how-it-works.md) - [Goals](goals.md) - [Mutations vs coverage](vs-coverage.md) diff --git a/book/src/in-diff.md b/book/src/in-diff.md new file mode 100644 index 00000000..64624fa9 --- /dev/null +++ b/book/src/in-diff.md @@ -0,0 +1,52 @@ +# Incremental tests of modified code + +If you're working on a large project or one with a long test suite, you may not want to test the entire codebase every time you make a change. You can use `cargo-mutants --in-diff` to test only mutants generated from recently changed code. + +The `--in-diff DIFF_FILE` option tests only mutants that overlap with regions changed in the diff. + +The diff is expected to either have a prefix of `b/` on the new filename, which is the format produced by `git diff`, or no prefix. + +Some ways you could use `--in-diff`: + +1. Before submitting code, check your uncommitted changes with `git diff`. +2. In CI, or locally, check the diff between the current branch and the base branch of the pull request. + +Changes to non-Rust files, or files from which no mutants are produced, are ignored. + +`--in-diff` is applied on the output of other filters including `--package` and `--regex`. For example, `cargo mutants --in-diff --package foo` will only test mutants in the `foo` package that overlap with the diff. + +## Caution + +`--in-diff` makes tests faster by covering the mutants that are most likely to be missed in the changed code. However, it's certainly possible that edits in one region cause code in a different region or a different file to no longer be well tested. Incremental tests are helpful for giving faster feedback, but they're not a substitute for a full test run. + +## Example + +In this diff, we've added a new function `two` to `src/lib.rs`, and the existing code is unaltered. With `--in-diff`, `cargo-mutants` will only test mutants that affect the function `two`. + +```diff + +```diff +--- a/src/lib.rs 2023-11-12 13:05:25.774658230 -0800 ++++ b/src/lib.rs 2023-11-12 12:54:04.373806696 -0800 +@@ -2,6 +2,10 @@ + "one".to_owned() + } + ++pub fn two() -> String { ++ format!("{}", 2) ++} ++ + #[cfg(test)] + mod test_super { + use super::*; +@@ -10,4 +14,9 @@ + fn test_one() { + assert_eq!(one(), "one"); + } ++ ++ #[test] ++ fn test_two() { ++ assert_eq!(two(), "2"); ++ } + } +``` diff --git a/book/src/pr-diff.md b/book/src/pr-diff.md new file mode 100644 index 00000000..bd32d2b6 --- /dev/null +++ b/book/src/pr-diff.md @@ -0,0 +1,42 @@ +# Incremental tests of pull requests + +You can use `--in-diff` to test only the code that has changed in a pull request. This can be useful for incremental testing in CI, where you want to test only the code that has changed since the last commit. + +For example, you can use the following workflow to test only the code that has changed in a pull request: + +```yaml +name: Tests + +permissions: + contents: read + +on: + push: + branches: + - main + pull_request: + +jobs: + incremental-mutants: + runs-on: ubuntu-latest + if: github.event_name == 'pull_request' + steps: + - uses: actions/checkout@v3 + with: + fetch-depth: 0 + - name: Relative diff + run: | + git branch -av + git diff origin/${{ github.base_ref }}.. | tee git.diff + - uses: Swatinem/rust-cache@v2 + - run: cargo install cargo-mutants + - name: Mutants + run: | + cargo mutants --no-shuffle -j 2 -vV --in-diff git.diff + - name: Archive mutants.out + uses: actions/upload-artifact@v3 + if: always() + with: + name: mutants-incremental.out + path: mutants.out +``` diff --git a/src/in_diff.rs b/src/in_diff.rs new file mode 100644 index 00000000..87c1e17e --- /dev/null +++ b/src/in_diff.rs @@ -0,0 +1,350 @@ +// Copyright 2023 Martin Pool + +//! Filter mutants to those intersecting a diff on the file tree, +//! for example from uncommitted or unmerged changes. + +use std::collections::HashMap; +use std::sync::Arc; + +use anyhow::{anyhow, bail}; +use camino::Utf8Path; +use indoc::formatdoc; +use itertools::Itertools; +use patch::{Line, Patch}; +use tracing::{trace, warn}; + +use crate::mutate::Mutant; +use crate::source::SourceFile; +use crate::Result; + +/// Return only mutants to functions whose source was touched by this diff. +pub fn diff_filter(mutants: Vec, diff_text: &str) -> Result> { + // Flatten the error to a string because otherwise it references the diff, and can't be returned. + let patches = + Patch::from_multiple(diff_text).map_err(|err| anyhow!("Failed to parse diff: {err}"))?; + check_diff_new_text_matches(&patches, &mutants)?; + let mut lines_changed_by_path: HashMap<&Utf8Path, Vec> = HashMap::new(); + for patch in &patches { + let path = strip_patch_path(&patch.new.path); + if lines_changed_by_path + .insert(path, affected_lines(patch)) + .is_some() + { + bail!("Patch input contains repeated filename: {path:?}"); + } + } + let mut matched: Vec = Vec::with_capacity(mutants.len()); + 'mutant: for mutant in mutants.into_iter() { + 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 + // than probing one line at a time... But, the numbers are likely to be small + // enough that this is tolerable... + // + // We could also search for each unique span in each file, and then include + // every mutant that intersects any of those spans, since commonly there will + // be multiple mutants in the same function. + for line in mutant.span.start.line..=mutant.span.end.line { + if lines_changed.binary_search(&line).is_ok() { + trace!( + ?path, + line, + %mutant, + "diff matched mutant" + ); + matched.push(mutant); + continue 'mutant; + } + } + } + } + Ok(matched) +} + +/// Error if the new text from the diffs doesn't match the source files. +fn check_diff_new_text_matches(patches: &[Patch], mutants: &[Mutant]) -> Result<()> { + let mut source_by_name: HashMap<&Utf8Path, Arc> = HashMap::new(); + for mutant in mutants { + source_by_name + .entry(mutant.source_file.path()) + .or_insert_with(|| Arc::clone(&mutant.source_file)); + } + for patch in patches { + let path = strip_patch_path(&patch.new.path); + if let Some(source_file) = source_by_name.get(&path) { + let reconstructed = partial_new_file(patch); + let lines = source_file.code.lines().collect_vec(); + for (lineno, diff_content) in reconstructed { + let source_content = lines.get(lineno - 1).unwrap_or(&""); + if diff_content != *source_content { + warn!( + ?path, + lineno, + ?diff_content, + ?source_content, + "Diff content doesn't match source file" + ); + bail!(formatdoc! { "\ + Diff content doesn't match source file: {path} line {lineno} + diff has: {diff_content:?} + source has: {source_content:?} + The diff might be out of date with this source tree. + "}); + } + } + } + } + Ok(()) +} + +/// Remove the `b/` prefix commonly found in paths within diffs. +fn strip_patch_path(path: &str) -> &Utf8Path { + let path = Utf8Path::new(path); + path.strip_prefix("b").unwrap_or(path) +} + +/// Given a diff, return the ranges of actually-changed lines, ignoring context lines. +/// +/// Code that's only included as context doesn't need to be tested. +/// +/// This returns a list of line numbers that are either added to the new file, or +/// adjacent to deletions. +/// +/// (A list of ranges would be more concise but this is easier for a first version.) +/// +/// If a line is deleted then the range will span from the line before to the line after. +fn affected_lines(patch: &Patch) -> Vec { + let mut r = 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 + // new file, if there is one, will be marked as affected. + let mut prev_removed = false; + for line in &hunk.lines { + match line { + Line::Remove(_) => { + prev_removed = true; + } + Line::Add(_) | Line::Context(_) => { + if prev_removed { + debug_assert!( + r.last().map_or(true, |last| *last < lineno), + "{lineno} {r:?}" + ); + debug_assert!(lineno >= 1, "{lineno}"); + r.push(lineno); + prev_removed = false; + } + } + } + match line { + Line::Context(_) => { + lineno += 1; + } + Line::Add(_) => { + if r.last().map_or(true, |last| *last < lineno) { + r.push(lineno); + } + lineno += 1; + } + Line::Remove(_) => { + if lineno > 1 && r.last().map_or(true, |last| *last < (lineno - 1)) { + r.push(lineno - 1); + } + } + } + } + } + debug_assert!( + r.iter().tuple_windows().all(|(a, b)| a < b), + "remove_context: line numbers not sorted and unique: {r:?}" + ); + r +} + +/// Recreate a partial view of the new file from a Patch. +/// +/// This contains lines present as adedd or context. Typically not all context +/// will be covered, so the output is a list of line numbers and their text. +fn partial_new_file<'d>(patch: &Patch<'d>) -> Vec<(usize, &'d str)> { + let mut r: Vec<(usize, &'d str)> = Vec::new(); + for hunk in &patch.hunks { + let mut lineno: usize = hunk.new_range.start.try_into().unwrap(); + for line in &hunk.lines { + match line { + Line::Context(text) | Line::Add(text) => { + debug_assert!(lineno >= 1, "{lineno}"); + debug_assert!( + r.last().map_or(true, |last| last.0 < lineno), + "{lineno} {r:?}" + ); + r.push((lineno, text)); + lineno += 1; + } + Line::Remove(_) => {} + } + } + debug_assert_eq!( + lineno, + (hunk.new_range.start + hunk.new_range.count) as usize, + "Wrong number of resulting lines?" + ); + } + r +} + +#[cfg(test)] +mod test_super { + use std::fs::read_to_string; + + use pretty_assertions::assert_eq; + use similar::TextDiff; + + use super::*; + + #[test] + fn patch_parse_error() { + let diff = "not really a diff\n"; + let err = diff_filter(Vec::new(), diff).unwrap_err(); + assert_eq!( + err.to_string(), + "Failed to parse diff: Line 1: Error while parsing: not really a diff\n" + ); + } + + #[test] + fn read_diff_with_empty_mutants() { + let diff = "\ +diff --git a/src/mutate.rs b/src/mutate.rs +index eb42779..a0091b7 100644 +--- a/src/mutate.rs ++++ b/src/mutate.rs +@@ -6,9 +6,7 @@ use std::fmt; + use std::fs; + use std::sync::Arc; + use std::foo; +-use anyhow::ensure; +-use anyhow::Context; +-use anyhow::Result; ++use anyhow::{ensure, Context, Result}; + use serde::ser::{SerializeStruct, Serializer}; + use serde::Serialize; + use similar::TextDiff; +"; + let filtered: Vec = diff_filter(Vec::new(), diff).expect("diff filtered"); + assert_eq!(filtered.len(), 0); + } + + fn make_diff(old: &str, new: &str) -> String { + TextDiff::from_lines(old, new) + .unified_diff() + .context_radius(2) + .header("a/file.rs", "b/file.rs") + .to_string() + } + + #[test] + fn strip_patch_path_prefix() { + assert_eq!(strip_patch_path("b/src/mutate.rs"), "src/mutate.rs"); + } + + #[test] + fn affected_lines_from_single_insertion() { + let orig_lines = (1..=4).map(|i| format!("line {}\n", i)).collect_vec(); + for i in 1..=5 { + let mut new = orig_lines.clone(); + let new_value = "new line\n".to_owned(); + if i < 5 { + new.insert(i - 1, new_value); + } else { + new.push(new_value); + } + let diff = make_diff(&orig_lines.join(""), &new.join("")); + println!("{diff}"); + let patch = Patch::from_single(&diff).unwrap(); + let affected = affected_lines(&patch); + // When we insert a line then only that one line is affected. + assert_eq!(affected, &[i]); + } + } + + #[test] + fn affected_lines_from_single_deletion() { + let orig_lines = (1..=5).map(|i| format!("line {}\n", i)).collect_vec(); + for i in 1..=5 { + let mut new = orig_lines.clone(); + new.remove(i - 1); + let diff = make_diff(&orig_lines.join(""), &new.join("")); + println!("{diff}"); + let patch = Patch::from_single(&diff).unwrap(); + let affected = affected_lines(&patch); + // If line 1 is removed we should see line 1 as affected. If line 2 is removed + // then 1 and 2 are affected, etc. If line 5 is removed, then only 4, the last + // remaining line is affected. + match i { + 1 => assert_eq!(affected, &[1]), + 5 => assert_eq!(affected, &[4]), + i => assert_eq!(affected, &[i - 1, i]), + } + } + } + + #[test] + fn affected_lines_from_double_deletion() { + let orig_lines = (1..=5).map(|i| format!("line {}\n", i)).collect_vec(); + for i in 1..=4 { + let mut new = orig_lines.clone(); + new.remove(i - 1); + new.remove(i - 1); + let diff = make_diff(&orig_lines.join(""), &new.join("")); + println!("{diff}"); + let patch = Patch::from_single(&diff).unwrap(); + let affected = affected_lines(&patch); + match i { + 1 => assert_eq!(affected, &[1]), + 4 => assert_eq!(affected, &[3]), + 2 | 3 => assert_eq!(affected, &[i - 1, i]), + _ => unreachable!(), + } + } + } + + #[test] + fn affected_lines_from_replacement() { + let orig_lines = (1..=5).map(|i| format!("line {}\n", i)).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)] + .iter() + .cloned() + .chain(insertion) + .chain(orig_lines[i..].iter().cloned()) + .collect_vec(); + let diff = make_diff(&orig_lines.join(""), &new.join("")); + println!("{diff}"); + let patch = Patch::from_single(&diff).unwrap(); + let affected = affected_lines(&patch); + if i > 1 { + // The line before the deletion also counts as affected. + assert_eq!(affected, &[i - 1, i, i + 1]); + } else { + assert_eq!(affected, &[i, i + 1]); + } + } + } + + #[test] + fn reconstruct_partial_new_file() { + let old = read_to_string("testdata/tree/diff0/src/lib.rs").unwrap(); + let new = read_to_string("testdata/tree/diff1/src/lib.rs").unwrap(); + let diff = make_diff(&old, &new); + let patch = Patch::from_single(&diff).unwrap(); + let reconstructed = partial_new_file(&patch); + println!("{reconstructed:#?}"); + assert_eq!(reconstructed.len(), 16); + let new_lines = new.lines().collect_vec(); + for (lineno, text) in reconstructed { + assert_eq!(text, new_lines[lineno - 1]); + } + } +} diff --git a/src/list.rs b/src/list.rs index 1b9edcfd..80246121 100644 --- a/src/list.rs +++ b/src/list.rs @@ -4,12 +4,14 @@ use std::fmt; use std::io; +use std::sync::Arc; use serde_json::{json, Value}; use crate::console::style_mutant; +use crate::mutate::Mutant; use crate::path::Utf8PathSlashes; -use crate::visit::Discovered; +use crate::source::SourceFile; use crate::{Options, Result}; /// Convert `fmt::Write` to `io::Write`. @@ -29,13 +31,13 @@ impl fmt::Write for FmtToIoWrite { pub(crate) fn list_mutants( mut out: W, - discovered: Discovered, + mutants: &[Mutant], options: &Options, ) -> Result<()> { if options.emit_json { let mut list: Vec = Vec::new(); - for mutant in discovered.mutants { - let mut obj = serde_json::to_value(&mutant)?; + for mutant in mutants { + let mut obj = serde_json::to_value(mutant)?; if options.emit_diffs { obj.as_object_mut() .unwrap() @@ -45,7 +47,7 @@ pub(crate) fn list_mutants( } out.write_str(&serde_json::to_string_pretty(&list)?)?; } else { - for mutant in &discovered.mutants { + for mutant in mutants { if options.colors { writeln!(out, "{}", style_mutant(mutant))?; } else { @@ -61,13 +63,12 @@ pub(crate) fn list_mutants( pub(crate) fn list_files( mut out: W, - discovered: Discovered, + source_files: &[Arc], options: &Options, ) -> Result<()> { if options.emit_json { let json_list = Value::Array( - discovered - .files + source_files .iter() .map(|source_file| { json!({ @@ -79,7 +80,7 @@ pub(crate) fn list_files( ); writeln!(out, "{}", serde_json::to_string_pretty(&json_list)?)?; } else { - for file in discovered.files { + for file in source_files { writeln!(out, "{}", file.tree_relative_path.to_slash_path())?; } } diff --git a/src/main.rs b/src/main.rs index dafe78b8..ea6cc216 100644 --- a/src/main.rs +++ b/src/main.rs @@ -1,6 +1,6 @@ // Copyright 2021-2023 Martin Pool -//! `cargo-mutants`: Find inadequately-tested code that can be removed without any tests failing. +//! `cargo-mutants`: Find test gaps by inserting bugs. mod build_dir; mod cargo; @@ -8,6 +8,7 @@ mod config; mod console; mod exit_code; mod fnvalue; +mod in_diff; mod interrupt; mod lab; mod list; @@ -25,12 +26,14 @@ mod scenario; mod source; mod textedit; mod visit; -pub mod workspace; +mod workspace; use std::env; +use std::fs::read_to_string; use std::io; use std::process::exit; +use anyhow::Context; use anyhow::Result; use camino::Utf8Path; use camino::Utf8PathBuf; @@ -40,9 +43,9 @@ use clap_complete::{generate, Shell}; use path_slash::PathExt; use tracing::debug; -// Imports of public names from this crate. use crate::build_dir::BuildDir; use crate::console::Console; +use crate::in_diff::diff_filter; use crate::interrupt::check_interrupted; use crate::lab::test_mutants; use crate::list::{list_files, list_mutants, FmtToIoWrite}; @@ -163,6 +166,10 @@ struct Args { #[arg(long, short = 'o')] output: Option, + /// include only mutants in code touched by this diff. + #[arg(long, short = 'D')] + in_diff: Option, + /// only test mutants from these packages. #[arg(id = "package", long, short = 'p')] mutate_packages: Vec, @@ -247,14 +254,22 @@ fn main() -> Result<()> { PackageFilter::Auto(start_dir.to_owned()) }; let discovered = workspace.discover(&package_filter, &options, &console)?; + console.clear(); if args.list_files { - console.clear(); - list_files(FmtToIoWrite::new(io::stdout()), discovered, &options)?; - } else if args.list { - console.clear(); - list_mutants(FmtToIoWrite::new(io::stdout()), discovered, &options)?; + list_files(FmtToIoWrite::new(io::stdout()), &discovered.files, &options)?; + return Ok(()); + } + let mut mutants = discovered.mutants; + if let Some(in_diff) = &args.in_diff { + mutants = diff_filter( + mutants, + &read_to_string(in_diff).context("Failed to read filter diff")?, + )?; + } + if args.list { + list_mutants(FmtToIoWrite::new(io::stdout()), &mutants, &options)?; } else { - let lab_outcome = test_mutants(discovered.mutants, &workspace.dir, options, &console)?; + let lab_outcome = test_mutants(mutants, &workspace.dir, options, &console)?; exit(lab_outcome.exit_code()); } Ok(()) diff --git a/src/snapshots/cargo_mutants__visit__test__expected_mutants_for_own_source_tree.snap b/src/snapshots/cargo_mutants__visit__test__expected_mutants_for_own_source_tree.snap index 73524271..03c93739 100644 --- a/src/snapshots/cargo_mutants__visit__test__expected_mutants_for_own_source_tree.snap +++ b/src/snapshots/cargo_mutants__visit__test__expected_mutants_for_own_source_tree.snap @@ -132,6 +132,20 @@ src/fnvalue.rs: replace path_is_nonzero_unsigned -> bool with true src/fnvalue.rs: replace path_is_nonzero_unsigned -> bool with false src/fnvalue.rs: replace match_first_type_arg -> Option<&'p Type> with None src/fnvalue.rs: replace match_first_type_arg -> Option<&'p Type> with Some(&Default::default()) +src/in_diff.rs: replace diff_filter -> Result> with Ok(vec![]) +src/in_diff.rs: replace diff_filter -> Result> with Ok(vec![Default::default()]) +src/in_diff.rs: replace diff_filter -> Result> with Err(::anyhow::anyhow!("mutated!")) +src/in_diff.rs: replace check_diff_new_text_matches -> Result<()> with Ok(()) +src/in_diff.rs: replace check_diff_new_text_matches -> Result<()> with Err(::anyhow::anyhow!("mutated!")) +src/in_diff.rs: replace strip_patch_path -> &Utf8Path with &Default::default() +src/in_diff.rs: replace affected_lines -> Vec with vec![] +src/in_diff.rs: replace affected_lines -> Vec with vec![0] +src/in_diff.rs: replace affected_lines -> Vec with vec![1] +src/in_diff.rs: replace partial_new_file -> Vec<(usize, &'d str)> with vec![] +src/in_diff.rs: replace partial_new_file -> Vec<(usize, &'d str)> with vec![(0, "")] +src/in_diff.rs: replace partial_new_file -> Vec<(usize, &'d str)> with vec![(0, "xyzzy")] +src/in_diff.rs: replace partial_new_file -> Vec<(usize, &'d str)> with vec![(1, "")] +src/in_diff.rs: replace partial_new_file -> Vec<(usize, &'d str)> with vec![(1, "xyzzy")] src/interrupt.rs: replace install_handler with () src/lab.rs: replace test_mutants -> Result with Ok(Default::default()) src/lab.rs: replace test_mutants -> Result with Err(::anyhow::anyhow!("mutated!")) @@ -288,6 +302,7 @@ src/scenario.rs: replace Scenario::log_file_name_base -> String with String::new src/scenario.rs: replace Scenario::log_file_name_base -> String with "xyzzy".into() src/source.rs: replace SourceFile::tree_relative_slashes -> String with String::new() src/source.rs: replace SourceFile::tree_relative_slashes -> String with "xyzzy".into() +src/source.rs: replace SourceFile::path -> &Utf8Path with &Default::default() src/textedit.rs: replace ::from -> Self with Default::default() src/textedit.rs: replace ::from -> Self with Default::default() src/textedit.rs: replace ::from -> Self with Default::default() diff --git a/src/source.rs b/src/source.rs index bf5ee6e5..d1866442 100644 --- a/src/source.rs +++ b/src/source.rs @@ -55,6 +55,10 @@ impl SourceFile { pub fn tree_relative_slashes(&self) -> String { self.tree_relative_path.to_slash_path() } + + pub fn path(&self) -> &Utf8Path { + self.tree_relative_path.as_path() + } } #[cfg(test)] diff --git a/src/visit.rs b/src/visit.rs index 35a63cc7..4f731d8a 100644 --- a/src/visit.rs +++ b/src/visit.rs @@ -421,7 +421,7 @@ mod test { let discovered = workspace .discover(&PackageFilter::All, &options, &console) .expect("Discover mutants"); - crate::list_mutants(&mut list_output, discovered, &options) + crate::list_mutants(&mut list_output, &discovered.mutants, &options) .expect("Discover mutants in own source tree"); // Strip line numbers so this is not too brittle. diff --git a/src/workspace.rs b/src/workspace.rs index 050f65ff..4a5d15fc 100644 --- a/src/workspace.rs +++ b/src/workspace.rs @@ -120,6 +120,7 @@ impl Workspace { } /// Find packages to mutate, subject to some filtering. + #[allow(dead_code)] pub fn packages(&self, package_filter: &PackageFilter) -> Result>> { Ok(self .package_tops(package_filter)? @@ -212,6 +213,7 @@ impl Workspace { } /// Return all mutants generated from this workspace. + #[allow(dead_code)] // called from tests, for now pub fn mutants( &self, package_filter: &PackageFilter, diff --git a/testdata/tree/diff0/Cargo.toml b/testdata/tree/diff0/Cargo.toml new file mode 100644 index 00000000..3d8b5e13 --- /dev/null +++ b/testdata/tree/diff0/Cargo.toml @@ -0,0 +1,9 @@ +[package] +name = "diff0" +version = "0.1.0" +edition = "2021" +publish = false + +# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html + +[dependencies] diff --git a/testdata/tree/diff0/src/lib.rs b/testdata/tree/diff0/src/lib.rs new file mode 100644 index 00000000..72203d02 --- /dev/null +++ b/testdata/tree/diff0/src/lib.rs @@ -0,0 +1,13 @@ +pub fn one() -> String { + "one".to_owned() +} + +#[cfg(test)] +mod test_super { + use super::*; + + #[test] + fn test_one() { + assert_eq!(one(), "one"); + } +} diff --git a/testdata/tree/diff1/Cargo.toml b/testdata/tree/diff1/Cargo.toml new file mode 100644 index 00000000..3f9f93f4 --- /dev/null +++ b/testdata/tree/diff1/Cargo.toml @@ -0,0 +1,9 @@ +[package] +name = "diff1" +version = "0.1.0" +edition = "2021" +publish = false + +# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html + +[dependencies] diff --git a/testdata/tree/diff1/src/lib.rs b/testdata/tree/diff1/src/lib.rs new file mode 100644 index 00000000..887b26fb --- /dev/null +++ b/testdata/tree/diff1/src/lib.rs @@ -0,0 +1,22 @@ +pub fn one() -> String { + "one".to_owned() +} + +pub fn two() -> String { + format!("{}", 2) +} + +#[cfg(test)] +mod test_super { + use super::*; + + #[test] + fn test_one() { + assert_eq!(one(), "one"); + } + + #[test] + fn test_two() { + assert_eq!(two(), "2"); + } +} diff --git a/tests/cli/in_diff.rs b/tests/cli/in_diff.rs new file mode 100644 index 00000000..bd165c3c --- /dev/null +++ b/tests/cli/in_diff.rs @@ -0,0 +1,98 @@ +// Copyright 2023 Martin Pool + +use std::fs::read_to_string; +use std::io::Write; + +use indoc::indoc; +use similar::TextDiff; +use tempfile::NamedTempFile; + +use super::{copy_of_testdata, run}; + +#[test] +fn diff_trees_well_tested() { + for name in &["diff0", "diff1"] { + let tmp = copy_of_testdata(name); + run() + .args(["mutants", "-d"]) + .arg(tmp.path()) + .assert() + .success(); + } +} + +#[test] +fn list_mutants_changed_in_diff1() { + let src0 = read_to_string("testdata/tree/diff0/src/lib.rs").unwrap(); + let src1 = read_to_string("testdata/tree/diff1/src/lib.rs").unwrap(); + let diff = TextDiff::from_lines(&src0, &src1) + .unified_diff() + .context_radius(2) + .header("a/src/lib.rs", "b/src/lib.rs") + .to_string(); + println!("{diff}"); + + let mut diff_file = NamedTempFile::new().unwrap(); + diff_file.write_all(diff.as_bytes()).unwrap(); + + let tmp = copy_of_testdata("diff1"); + + run() + .args(["mutants", "--no-shuffle", "-d"]) + .arg(tmp.path()) + .arg("--in-diff") + .arg(diff_file.path()) + .assert() + .success(); + + // Between these trees we just added one function; the existing unchanged + // function does not need to be tested. + assert_eq!( + read_to_string(tmp.path().join("mutants.out/caught.txt")).unwrap(), + indoc! { "\ + src/lib.rs:5: replace two -> String with String::new() + src/lib.rs:5: replace two -> String with \"xyzzy\".into() + "} + ); + + let mutants_json: serde_json::Value = + serde_json::from_str(&read_to_string(tmp.path().join("mutants.out/mutants.json")).unwrap()) + .unwrap(); + assert_eq!( + mutants_json + .as_array() + .expect("mutants.json contains an array") + .len(), + 2 + ); +} + +/// If the text in the diff doesn't look like the tree then error out. +#[test] +fn mismatched_diff_causes_error() { + let src0 = read_to_string("testdata/tree/diff0/src/lib.rs").unwrap(); + let src1 = read_to_string("testdata/tree/diff1/src/lib.rs").unwrap(); + let diff = TextDiff::from_lines(&src0, &src1) + .unified_diff() + .context_radius(2) + .header("a/src/lib.rs", "b/src/lib.rs") + .to_string(); + let diff = diff.replace("fn", "FUNCTION"); + println!("{diff}"); + + let mut diff_file = NamedTempFile::new().unwrap(); + diff_file.write_all(diff.as_bytes()).unwrap(); + + let tmp = copy_of_testdata("diff1"); + + run() + .args(["mutants", "--no-shuffle", "-d"]) + .arg(tmp.path()) + .arg("--in-diff") + .arg(diff_file.path()) + .assert() + .failure() + .stderr(predicates::str::contains( + "Diff content doesn't match source file: src/lib.rs", + )); +} diff --git a/tests/cli/main.rs b/tests/cli/main.rs index 7e3c5d1d..3b3bacf9 100644 --- a/tests/cli/main.rs +++ b/tests/cli/main.rs @@ -24,6 +24,7 @@ use tempfile::{tempdir, TempDir}; mod config; mod error_value; +mod in_diff; mod jobs; mod trace; #[cfg(windows)] @@ -93,7 +94,6 @@ fn copy_of_testdata(tree_name: &str) -> TempDir { /// Remove anything that looks like a duration or tree size, since they'll be unpredictable. fn redact_timestamps_sizes(s: &str) -> String { - // TODO: Maybe match the number of digits? let s = DURATION_RE.replace_all(s, "x.xxxs"); SIZE_RE.replace_all(&s, "xxx MB").to_string() } diff --git a/tests/cli/snapshots/cli__list_mutants_in_all_trees_as_json.snap b/tests/cli/snapshots/cli__list_mutants_in_all_trees_as_json.snap index 13ec1b51..493298b9 100644 --- a/tests/cli/snapshots/cli__list_mutants_in_all_trees_as_json.snap +++ b/tests/cli/snapshots/cli__list_mutants_in_all_trees_as_json.snap @@ -149,6 +149,74 @@ expression: buf ] ``` +## testdata/tree/diff0 + +```json +[ + { + "file": "src/lib.rs", + "function": "one", + "genre": "FnValue", + "line": 1, + "package": "diff0", + "replacement": "String::new()", + "return_type": "-> String" + }, + { + "file": "src/lib.rs", + "function": "one", + "genre": "FnValue", + "line": 1, + "package": "diff0", + "replacement": "\"xyzzy\".into()", + "return_type": "-> String" + } +] +``` + +## testdata/tree/diff1 + +```json +[ + { + "file": "src/lib.rs", + "function": "one", + "genre": "FnValue", + "line": 1, + "package": "diff1", + "replacement": "String::new()", + "return_type": "-> String" + }, + { + "file": "src/lib.rs", + "function": "one", + "genre": "FnValue", + "line": 1, + "package": "diff1", + "replacement": "\"xyzzy\".into()", + "return_type": "-> String" + }, + { + "file": "src/lib.rs", + "function": "two", + "genre": "FnValue", + "line": 5, + "package": "diff1", + "replacement": "String::new()", + "return_type": "-> String" + }, + { + "file": "src/lib.rs", + "function": "two", + "genre": "FnValue", + "line": 5, + "package": "diff1", + "replacement": "\"xyzzy\".into()", + "return_type": "-> String" + } +] +``` + ## testdata/tree/error_value ```json diff --git a/tests/cli/snapshots/cli__list_mutants_in_all_trees_as_text.snap b/tests/cli/snapshots/cli__list_mutants_in_all_trees_as_text.snap index 849f1eff..9570825e 100644 --- a/tests/cli/snapshots/cli__list_mutants_in_all_trees_as_text.snap +++ b/tests/cli/snapshots/cli__list_mutants_in_all_trees_as_text.snap @@ -48,6 +48,22 @@ src/lib.rs:1: replace factorial -> u32 with 0 src/lib.rs:1: replace factorial -> u32 with 1 ``` +## testdata/tree/diff0 + +``` +src/lib.rs:1: replace one -> String with String::new() +src/lib.rs:1: replace one -> String with "xyzzy".into() +``` + +## testdata/tree/diff1 + +``` +src/lib.rs:1: replace one -> String with String::new() +src/lib.rs:1: replace one -> String with "xyzzy".into() +src/lib.rs:5: replace two -> String with String::new() +src/lib.rs:5: replace two -> String with "xyzzy".into() +``` + ## testdata/tree/error_value ```