From f2be45ab1dc357f7a224018d8bd767df915ed3ad Mon Sep 17 00:00:00 2001 From: Martin Pool Date: Mon, 1 May 2023 09:34:13 -0700 Subject: [PATCH 01/12] Add a command line option to generate error values --- NEWS.md | 3 +++ src/main.rs | 5 +++++ src/options.rs | 7 ++++++- src/visit.rs | 29 ++++++++++++++++++++++------- 4 files changed, 36 insertions(+), 8 deletions(-) diff --git a/NEWS.md b/NEWS.md index b55fbe3c..34ce895a 100644 --- a/NEWS.md +++ b/NEWS.md @@ -11,6 +11,9 @@ - Minimum supported Rust version increased to 1.65 due to changes in dependencies. +- New `--error` option, to cause functions returning `Result` to be mutated to return the + specified error. + ## 1.2.2 Released 2023-04-01 diff --git a/src/main.rs b/src/main.rs index f4318bc4..1947ad8a 100644 --- a/src/main.rs +++ b/src/main.rs @@ -93,6 +93,11 @@ struct Args { #[arg(long, short = 'd')] dir: Option, + /// return this error values from functions returning Result: + /// for example, `anyhow!("mutated")`. + #[arg(long)] + error: Vec, + /// regex for mutations to examine, matched against the names shown by `--list`. #[arg(long = "re", short = 'F')] examine_re: Vec, diff --git a/src/options.rs b/src/options.rs index 02cb0418..07d9fb5c 100644 --- a/src/options.rs +++ b/src/options.rs @@ -15,7 +15,8 @@ use tracing::warn; use crate::{config::Config, *}; -/// Options for running experiments. +/// Options for mutation testing, based on both command-line arguments and the +/// config file. #[derive(Default, Debug, Clone)] pub struct Options { /// Don't run the tests, just see if each mutant builds. @@ -71,6 +72,9 @@ pub struct Options { /// Run this many `cargo build` or `cargo test` tasks in parallel. pub jobs: Option, + + /// Insert these values as errors from functions returning `Result`. + pub error_values: Vec, } impl Options { @@ -101,6 +105,7 @@ impl Options { .chain(config.additional_cargo_test_args.iter().cloned()) .collect(), check_only: args.check, + error_values: args.error.clone(), examine_names: Some( RegexSet::new(args.examine_re.iter().chain(config.examine_re.iter())) .context("Compiling examine_re regex")?, diff --git a/src/visit.rs b/src/visit.rs index 8d4993ba..3986fc88 100644 --- a/src/visit.rs +++ b/src/visit.rs @@ -44,7 +44,7 @@ pub fn walk_tree(tool: &dyn Tool, root: &Utf8Path, options: &Options) -> Result< while let Some(source_file) = file_queue.pop_front() { check_interrupted()?; let package_name = source_file.package_name.clone(); - let (mut file_mutants, more_files) = walk_file(root, Arc::clone(&source_file))?; + let (mut file_mutants, more_files) = walk_file(root, Arc::clone(&source_file), options)?; // We'll still walk down through files that don't match globs, so that // we have a chance to find modules underneath them. However, we won't // collect any mutants from them, and they don't count as "seen" for @@ -87,6 +87,7 @@ pub fn walk_tree(tool: &dyn Tool, root: &Utf8Path, options: &Options) -> Result< fn walk_file( root: &Utf8Path, source_file: Arc, + options: &Options, ) -> Result<(Vec, Vec)> { let _span = debug_span!("source_file", path = source_file.tree_relative_slashes()).entered(); debug!("visit source file"); @@ -98,6 +99,7 @@ fn walk_file( more_files: Vec::new(), mutants: Vec::new(), namespace_stack: Vec::new(), + options, }; visitor.visit_file(&syn_file); Ok((visitor.mutants, visitor.more_files)) @@ -105,7 +107,7 @@ fn walk_file( /// `syn` visitor that recursively traverses the syntax tree, accumulating places /// that could be mutated. -struct DiscoveryVisitor { +struct DiscoveryVisitor<'o> { /// All the mutants generated by visiting the file. mutants: Vec, @@ -120,13 +122,16 @@ struct DiscoveryVisitor { /// Files discovered by `mod` statements. more_files: Vec, + + /// Global options. + options: &'o Options, } -impl DiscoveryVisitor { +impl<'o> DiscoveryVisitor<'o> { fn collect_fn_mutants(&mut self, return_type: &syn::ReturnType, span: &proc_macro2::Span) { let full_function_name = Arc::new(self.namespace_stack.join("::")); let return_type_str = Arc::new(return_type_to_string(return_type)); - let mut new_mutants = return_value_replacements(return_type) + let mut new_mutants = return_value_replacements(return_type, self.options) .into_iter() .map(|replacement| Mutant { source_file: Arc::clone(&self.source_file), @@ -162,7 +167,7 @@ impl DiscoveryVisitor { } } -impl<'ast> Visit<'ast> for DiscoveryVisitor { +impl<'ast> Visit<'ast> for DiscoveryVisitor<'_> { /// Visit top-level `fn foo()`. fn visit_item_fn(&mut self, i: &'ast ItemFn) { // TODO: Filter out more inapplicable fns. @@ -297,7 +302,11 @@ impl<'ast> Visit<'ast> for DiscoveryVisitor { } } -fn return_value_replacements(return_type: &syn::ReturnType) -> Vec> { +/// Generate replacement text for a function based on its return type. +fn return_value_replacements( + return_type: &syn::ReturnType, + options: &Options, +) -> Vec> { let mut reps: Vec> = Vec::new(); match return_type { syn::ReturnType::Default => reps.push(Cow::Borrowed("()")), @@ -317,9 +326,15 @@ fn return_value_replacements(return_type: &syn::ReturnType) -> Vec Date: Tue, 2 May 2023 07:05:05 -0700 Subject: [PATCH 02/12] Add testdata for error handling --- .vscode/settings.json | 3 ++ Cargo.toml | 1 + testdata/tree/error_handling/Cargo.toml | 10 ++++++ testdata/tree/error_handling/README.md | 12 +++++++ testdata/tree/error_handling/src/lib.rs | 36 +++++++++++++++++++ ...li__list_mutants_in_all_trees_as_json.snap | 15 ++++++++ ...li__list_mutants_in_all_trees_as_text.snap | 6 ++++ 7 files changed, 83 insertions(+) create mode 100644 .vscode/settings.json create mode 100644 testdata/tree/error_handling/Cargo.toml create mode 100644 testdata/tree/error_handling/README.md create mode 100644 testdata/tree/error_handling/src/lib.rs diff --git a/.vscode/settings.json b/.vscode/settings.json new file mode 100644 index 00000000..78648816 --- /dev/null +++ b/.vscode/settings.json @@ -0,0 +1,3 @@ +{ + "rust-analyzer.linkedProjects": ["./testdata/tree/error_handling/Cargo.toml"] +} diff --git a/Cargo.toml b/Cargo.toml index 465d8234..e3c6c961 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -89,6 +89,7 @@ exclude = [ "testdata/tree/cfg_attr_mutants_skip", "testdata/tree/cfg_attr_test_skip", "testdata/tree/dependency", + "testdata/tree/error_handling", "testdata/tree/everything_skipped", "testdata/tree/factorial", "testdata/tree/fails_without_feature", diff --git a/testdata/tree/error_handling/Cargo.toml b/testdata/tree/error_handling/Cargo.toml new file mode 100644 index 00000000..7504c18c --- /dev/null +++ b/testdata/tree/error_handling/Cargo.toml @@ -0,0 +1,10 @@ +[package] +name = "cargo-mutants-testdata-small-well-tested" +description = "A small tree with one function with good coverage" +version = "0.0.0" +edition = "2018" +authors = ["Martin Pool"] +publish = false + +[lib] +doctest = false diff --git a/testdata/tree/error_handling/README.md b/testdata/tree/error_handling/README.md new file mode 100644 index 00000000..c067d3ae --- /dev/null +++ b/testdata/tree/error_handling/README.md @@ -0,0 +1,12 @@ +# `error_handling` tree + +This tree contains a function that can return an error. +The tests ignore the error case, so would fail to detect +a bug that causes an error to be returned. + +(It's probably pretty likely that many Rust tests will `unwrap` +or `?` on the error and so implicitly catch it, but it's still +possible.) + +With cargo-mutants `--error` option, we generate a mutant that +returns an error and so catch the missing coverage. diff --git a/testdata/tree/error_handling/src/lib.rs b/testdata/tree/error_handling/src/lib.rs new file mode 100644 index 00000000..8f98395e --- /dev/null +++ b/testdata/tree/error_handling/src/lib.rs @@ -0,0 +1,36 @@ +use std::result::Result; + +pub fn even_is_ok(n: u32) -> Result { + if n % 2 == 0 { + Ok(n) + } else { + Err("number is odd") + } +} + +#[cfg(test)] +mod test { + use super::*; + + // These would be better tests but for the sake of the test let's + // assume nobody wrote them yet. + + // #[test] + // fn test_even_is_ok() { + // assert_eq!(even_is_ok(2), Ok(2)); + // assert_eq!(even_is_ok(3), Err("number is odd")); + // } + + // #[test] + // fn test_even_with_unwrap() { + // assert_eq!(even_is_ok(2).unwrap(), 2); + // } + + #[test] + fn bad_test_ignores_error_results() { + // A bit contrived but does the job: never checks that + // the code passes on values that it should accept. + assert!(even_is_ok(1).is_err()); + assert!(even_is_ok(3).is_err()); + } +} 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 69bfa266..80285e7a 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 @@ -98,6 +98,21 @@ expression: buf ] ``` +## testdata/tree/error_handling + +```json +[ + { + "package": "cargo-mutants-testdata-small-well-tested", + "file": "src/lib.rs", + "line": 3, + "function": "even_is_ok", + "return_type": "-> Result", + "replacement": "Ok(Default::default())" + } +] +``` + ## testdata/tree/everything_skipped ```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 64f98335..81aaf4f4 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 @@ -43,6 +43,12 @@ src/lib.rs:17: replace double -> usize with Default::default() src/lib.rs:1: replace factorial -> u32 with Default::default() ``` +## testdata/tree/error_handling + +``` +src/lib.rs:3: replace even_is_ok -> Result with Ok(Default::default()) +``` + ## testdata/tree/everything_skipped ``` From da49aa40f52c882dac81e2947e15a8cd9cb1d4a8 Mon Sep 17 00:00:00 2001 From: Martin Pool Date: Tue, 2 May 2023 07:15:09 -0700 Subject: [PATCH 03/12] Add 'error_values' config key --- src/config.rs | 2 ++ src/options.rs | 26 +++++++++++++------------- 2 files changed, 15 insertions(+), 13 deletions(-) diff --git a/src/config.rs b/src/config.rs index 5dfb9216..620c573e 100644 --- a/src/config.rs +++ b/src/config.rs @@ -24,6 +24,8 @@ use crate::Result; #[derive(Debug, Default, Clone, Deserialize)] #[serde(default, deny_unknown_fields)] pub struct Config { + /// Generate these error values from functions returning Result. + pub error_values: Vec, /// Generate mutants from source files matching these globs. pub examine_globs: Vec, /// Exclude mutants from source files matching these globs. diff --git a/src/options.rs b/src/options.rs index 07d9fb5c..dcd02313 100644 --- a/src/options.rs +++ b/src/options.rs @@ -77,6 +77,13 @@ pub struct Options { pub error_values: Vec, } +fn join_slices(a: &[String], b: &[String]) -> Vec { + let mut v = Vec::with_capacity(a.len() + b.len()); + v.extend_from_slice(a); + v.extend_from_slice(b); + v +} + impl Options { /// Build options by merging command-line args and config file. pub(crate) fn new(args: &Args, config: &Config) -> Result { @@ -92,20 +99,13 @@ impl Options { ); Ok(Options { - additional_cargo_args: args - .cargo_arg - .iter() - .cloned() - .chain(config.additional_cargo_args.iter().cloned()) - .collect(), - additional_cargo_test_args: args - .cargo_test_args - .iter() - .cloned() - .chain(config.additional_cargo_test_args.iter().cloned()) - .collect(), + additional_cargo_args: join_slices(&args.cargo_arg, &config.additional_cargo_args), + additional_cargo_test_args: join_slices( + &args.cargo_test_args, + &config.additional_cargo_test_args, + ), check_only: args.check, - error_values: args.error.clone(), + error_values: join_slices(&args.error, &config.error_values), examine_names: Some( RegexSet::new(args.examine_re.iter().chain(config.examine_re.iter())) .context("Compiling examine_re regex")?, From 483671868c465b0ecb8484591604cea30c27c62e Mon Sep 17 00:00:00 2001 From: Martin Pool Date: Tue, 2 May 2023 07:15:44 -0700 Subject: [PATCH 04/12] Configure error values in test tree --- testdata/tree/error_handling/.cargo/mutants.toml | 3 +++ .../snapshots/cli__list_mutants_in_all_trees_as_json.snap | 8 ++++++++ .../snapshots/cli__list_mutants_in_all_trees_as_text.snap | 1 + 3 files changed, 12 insertions(+) create mode 100644 testdata/tree/error_handling/.cargo/mutants.toml diff --git a/testdata/tree/error_handling/.cargo/mutants.toml b/testdata/tree/error_handling/.cargo/mutants.toml new file mode 100644 index 00000000..7952c150 --- /dev/null +++ b/testdata/tree/error_handling/.cargo/mutants.toml @@ -0,0 +1,3 @@ +# This crate uses just `&'static str` as an error type. + +error_values = ['"injected"'] 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 80285e7a..81265436 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 @@ -109,6 +109,14 @@ expression: buf "function": "even_is_ok", "return_type": "-> Result", "replacement": "Ok(Default::default())" + }, + { + "package": "cargo-mutants-testdata-small-well-tested", + "file": "src/lib.rs", + "line": 3, + "function": "even_is_ok", + "return_type": "-> Result", + "replacement": "Err(\"injected\")" } ] ``` 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 81aaf4f4..b172d21c 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 @@ -47,6 +47,7 @@ src/lib.rs:1: replace factorial -> u32 with Default::default() ``` src/lib.rs:3: replace even_is_ok -> Result with Ok(Default::default()) +src/lib.rs:3: replace even_is_ok -> Result with Err("injected") ``` ## testdata/tree/everything_skipped From 83624d0e0097bab15d61a02849a51d6cd2ae401e Mon Sep 17 00:00:00 2001 From: Martin Pool Date: Tue, 2 May 2023 07:24:36 -0700 Subject: [PATCH 05/12] Test error values --- .vscode/settings.json | 4 +--- Cargo.toml | 2 +- testdata/tree/error_handling/Cargo.toml | 10 ---------- .../.cargo/mutants.toml | 0 testdata/tree/error_value/Cargo.toml | 9 +++++++++ .../{error_handling => error_value}/README.md | 2 +- .../src/lib.rs | 0 tests/cli/main.rs | 20 +++++++++++++++++++ ..._error_value_catches_untested_ok_case.snap | 10 ++++++++++ ...li__list_mutants_in_all_trees_as_json.snap | 6 +++--- ...li__list_mutants_in_all_trees_as_text.snap | 2 +- 11 files changed, 46 insertions(+), 19 deletions(-) delete mode 100644 testdata/tree/error_handling/Cargo.toml rename testdata/tree/{error_handling => error_value}/.cargo/mutants.toml (100%) create mode 100644 testdata/tree/error_value/Cargo.toml rename testdata/tree/{error_handling => error_value}/README.md (94%) rename testdata/tree/{error_handling => error_value}/src/lib.rs (100%) create mode 100644 tests/cli/snapshots/cli__error_value_catches_untested_ok_case.snap diff --git a/.vscode/settings.json b/.vscode/settings.json index 78648816..0967ef42 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -1,3 +1 @@ -{ - "rust-analyzer.linkedProjects": ["./testdata/tree/error_handling/Cargo.toml"] -} +{} diff --git a/Cargo.toml b/Cargo.toml index e3c6c961..62207c71 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -89,7 +89,7 @@ exclude = [ "testdata/tree/cfg_attr_mutants_skip", "testdata/tree/cfg_attr_test_skip", "testdata/tree/dependency", - "testdata/tree/error_handling", + "testdata/tree/error_value", "testdata/tree/everything_skipped", "testdata/tree/factorial", "testdata/tree/fails_without_feature", diff --git a/testdata/tree/error_handling/Cargo.toml b/testdata/tree/error_handling/Cargo.toml deleted file mode 100644 index 7504c18c..00000000 --- a/testdata/tree/error_handling/Cargo.toml +++ /dev/null @@ -1,10 +0,0 @@ -[package] -name = "cargo-mutants-testdata-small-well-tested" -description = "A small tree with one function with good coverage" -version = "0.0.0" -edition = "2018" -authors = ["Martin Pool"] -publish = false - -[lib] -doctest = false diff --git a/testdata/tree/error_handling/.cargo/mutants.toml b/testdata/tree/error_value/.cargo/mutants.toml similarity index 100% rename from testdata/tree/error_handling/.cargo/mutants.toml rename to testdata/tree/error_value/.cargo/mutants.toml diff --git a/testdata/tree/error_value/Cargo.toml b/testdata/tree/error_value/Cargo.toml new file mode 100644 index 00000000..83d71864 --- /dev/null +++ b/testdata/tree/error_value/Cargo.toml @@ -0,0 +1,9 @@ +[package] +name = "cargo-mutants-testdata-error-value" +version = "0.0.0" +edition = "2021" +authors = ["Martin Pool"] +publish = false + +[lib] +doctest = false diff --git a/testdata/tree/error_handling/README.md b/testdata/tree/error_value/README.md similarity index 94% rename from testdata/tree/error_handling/README.md rename to testdata/tree/error_value/README.md index c067d3ae..6f27c26c 100644 --- a/testdata/tree/error_handling/README.md +++ b/testdata/tree/error_value/README.md @@ -1,4 +1,4 @@ -# `error_handling` tree +# `error_value` tree This tree contains a function that can return an error. The tests ignore the error case, so would fail to detect diff --git a/testdata/tree/error_handling/src/lib.rs b/testdata/tree/error_value/src/lib.rs similarity index 100% rename from testdata/tree/error_handling/src/lib.rs rename to testdata/tree/error_value/src/lib.rs diff --git a/tests/cli/main.rs b/tests/cli/main.rs index 967e6839..cd3eb630 100644 --- a/tests/cli/main.rs +++ b/tests/cli/main.rs @@ -851,6 +851,26 @@ fn small_well_tested_mutants_with_cargo_arg_release() { .unwrap(); } +#[test] +fn error_value_catches_untested_ok_case() { + // By default this tree should fail because it's configured to + // generate an error value, and the tests forgot to check that + // the code under test does return Ok. + let tmp_src_dir = copy_of_testdata("error_value"); + run() + .arg("mutants") + .args(["-v", "-V", "--no-times", "--no-shuffle"]) + .arg("-d") + .arg(tmp_src_dir.path()) + .assert() + .code(2) + .stderr("") + .stdout(predicate::function(|stdout| { + insta::assert_snapshot!(stdout); + true + })); +} + #[test] /// The `--output` directory creates the named directory if necessary, and then /// creates `mutants.out` within it. `mutants.out` is not created in the diff --git a/tests/cli/snapshots/cli__error_value_catches_untested_ok_case.snap b/tests/cli/snapshots/cli__error_value_catches_untested_ok_case.snap new file mode 100644 index 00000000..6f60cabf --- /dev/null +++ b/tests/cli/snapshots/cli__error_value_catches_untested_ok_case.snap @@ -0,0 +1,10 @@ +--- +source: tests/cli/main.rs +expression: stdout +--- +Found 2 mutants to test +Unmutated baseline ... ok +src/lib.rs:3: replace even_is_ok -> Result with Ok(Default::default()) ... caught +src/lib.rs:3: replace even_is_ok -> Result with Err("injected") ... NOT CAUGHT +2 mutants tested: 1 missed, 1 caught + 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 81265436..16c96501 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 @@ -98,12 +98,12 @@ expression: buf ] ``` -## testdata/tree/error_handling +## testdata/tree/error_value ```json [ { - "package": "cargo-mutants-testdata-small-well-tested", + "package": "cargo-mutants-testdata-error-value", "file": "src/lib.rs", "line": 3, "function": "even_is_ok", @@ -111,7 +111,7 @@ expression: buf "replacement": "Ok(Default::default())" }, { - "package": "cargo-mutants-testdata-small-well-tested", + "package": "cargo-mutants-testdata-error-value", "file": "src/lib.rs", "line": 3, "function": "even_is_ok", 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 b172d21c..92afd72f 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 @@ -43,7 +43,7 @@ src/lib.rs:17: replace double -> usize with Default::default() src/lib.rs:1: replace factorial -> u32 with Default::default() ``` -## testdata/tree/error_handling +## testdata/tree/error_value ``` src/lib.rs:3: replace even_is_ok -> Result with Ok(Default::default()) From c7652958f04aedfb103ae5b6e7718031022202ef Mon Sep 17 00:00:00 2001 From: Martin Pool Date: Fri, 5 May 2023 06:35:17 -0700 Subject: [PATCH 06/12] Suggest using fully-qualified ::anyhow::anyhow --- src/main.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main.rs b/src/main.rs index 1947ad8a..077e7016 100644 --- a/src/main.rs +++ b/src/main.rs @@ -94,7 +94,7 @@ struct Args { dir: Option, /// return this error values from functions returning Result: - /// for example, `anyhow!("mutated")`. + /// for example, `::anyhow::anyhow!("mutated")`. #[arg(long)] error: Vec, From 58f5ff2d8986de6e4566dfa3dab2180d63eb5650 Mon Sep 17 00:00:00 2001 From: Martin Pool Date: Fri, 5 May 2023 06:35:26 -0700 Subject: [PATCH 07/12] Turn on error mutants for this tree --- .cargo/mutants.toml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.cargo/mutants.toml b/.cargo/mutants.toml index b5ea9aa8..3455e814 100644 --- a/.cargo/mutants.toml +++ b/.cargo/mutants.toml @@ -1,3 +1,4 @@ # cargo-mutants configuration -exclude_globs = [ "src/console.rs" ] +error_values = ["::anyhow::anyhow!(\"mutated\")"] +exclude_globs = ["src/console.rs"] From bbc7483db49c57d5cda83cd4e365d1843ba642f8 Mon Sep 17 00:00:00 2001 From: Martin Pool Date: Fri, 5 May 2023 06:46:22 -0700 Subject: [PATCH 08/12] Move error_value tests to their own file --- tests/cli/error_value.rs | 29 +++++++++++++++++++ tests/cli/main.rs | 21 +------------- ...error_value_catches_untested_ok_case.snap} | 3 +- 3 files changed, 32 insertions(+), 21 deletions(-) create mode 100644 tests/cli/error_value.rs rename tests/cli/snapshots/{cli__error_value_catches_untested_ok_case.snap => cli__error_value__error_value_catches_untested_ok_case.snap} (85%) diff --git a/tests/cli/error_value.rs b/tests/cli/error_value.rs new file mode 100644 index 00000000..9166a655 --- /dev/null +++ b/tests/cli/error_value.rs @@ -0,0 +1,29 @@ +// Copyright 2023 Martin Pool + +//! Tests for error value mutations, from `--error-value` etc. + +use std::env; + +use predicates::prelude::*; + +use super::{copy_of_testdata, run}; + +#[test] +fn error_value_catches_untested_ok_case() { + // By default this tree should fail because it's configured to + // generate an error value, and the tests forgot to check that + // the code under test does return Ok. + let tmp_src_dir = copy_of_testdata("error_value"); + run() + .arg("mutants") + .args(["-v", "-V", "--no-times", "--no-shuffle"]) + .arg("-d") + .arg(tmp_src_dir.path()) + .assert() + .code(2) + .stderr("") + .stdout(predicate::function(|stdout| { + insta::assert_snapshot!(stdout); + true + })); +} diff --git a/tests/cli/main.rs b/tests/cli/main.rs index cd3eb630..92cf69a4 100644 --- a/tests/cli/main.rs +++ b/tests/cli/main.rs @@ -22,6 +22,7 @@ use subprocess::{Popen, PopenConfig, Redirection}; use tempfile::{tempdir, TempDir}; mod config; +mod error_value; mod jobs; mod trace; #[cfg(windows)] @@ -851,26 +852,6 @@ fn small_well_tested_mutants_with_cargo_arg_release() { .unwrap(); } -#[test] -fn error_value_catches_untested_ok_case() { - // By default this tree should fail because it's configured to - // generate an error value, and the tests forgot to check that - // the code under test does return Ok. - let tmp_src_dir = copy_of_testdata("error_value"); - run() - .arg("mutants") - .args(["-v", "-V", "--no-times", "--no-shuffle"]) - .arg("-d") - .arg(tmp_src_dir.path()) - .assert() - .code(2) - .stderr("") - .stdout(predicate::function(|stdout| { - insta::assert_snapshot!(stdout); - true - })); -} - #[test] /// The `--output` directory creates the named directory if necessary, and then /// creates `mutants.out` within it. `mutants.out` is not created in the diff --git a/tests/cli/snapshots/cli__error_value_catches_untested_ok_case.snap b/tests/cli/snapshots/cli__error_value__error_value_catches_untested_ok_case.snap similarity index 85% rename from tests/cli/snapshots/cli__error_value_catches_untested_ok_case.snap rename to tests/cli/snapshots/cli__error_value__error_value_catches_untested_ok_case.snap index 6f60cabf..512debea 100644 --- a/tests/cli/snapshots/cli__error_value_catches_untested_ok_case.snap +++ b/tests/cli/snapshots/cli__error_value__error_value_catches_untested_ok_case.snap @@ -1,5 +1,6 @@ --- -source: tests/cli/main.rs +source: tests/cli/error_value.rs +assertion_line: 26 expression: stdout --- Found 2 mutants to test From dc33888da6652d7a827f65638bfc4be256dad566 Mon Sep 17 00:00:00 2001 From: Martin Pool Date: Fri, 5 May 2023 06:53:02 -0700 Subject: [PATCH 09/12] Add --no-config and use it in error value tests --- NEWS.md | 2 + src/main.rs | 13 +++++- tests/cli/error_value.rs | 45 +++++++++++++++++++ ...th_error_value_from_command_line_list.snap | 7 +++ ..._file_so_error_value_is_not_generated.snap | 9 ++++ 5 files changed, 74 insertions(+), 2 deletions(-) create mode 100644 tests/cli/snapshots/cli__error_value__list_mutants_with_error_value_from_command_line_list.snap create mode 100644 tests/cli/snapshots/cli__error_value__no_config_option_disables_config_file_so_error_value_is_not_generated.snap diff --git a/NEWS.md b/NEWS.md index 34ce895a..bf37d0fb 100644 --- a/NEWS.md +++ b/NEWS.md @@ -14,6 +14,8 @@ - New `--error` option, to cause functions returning `Result` to be mutated to return the specified error. +- New `--no-config` option, to disable reading `.cargo/mutants.toml`. + ## 1.2.2 Released 2023-04-01 diff --git a/src/main.rs b/src/main.rs index 077e7016..ea489362 100644 --- a/src/main.rs +++ b/src/main.rs @@ -145,6 +145,10 @@ struct Args { #[arg(long)] list_files: bool, + /// don't read .cargo/mutants.toml. + #[arg(long)] + no_config: bool, + /// don't copy the /target directory, and don't build the source tree first. #[arg(long)] no_copy_target: bool, @@ -219,8 +223,13 @@ fn main() -> Result<()> { }; let tool = CargoTool::new(); let source_tree_root = tool.find_root(source_path)?; - let config = config::Config::read_tree_config(&source_tree_root)?; - debug!(?config); + let config; + if args.no_config { + config = config::Config::default(); + } else { + config = config::Config::read_tree_config(&source_tree_root)?; + debug!(?config); + } let options = Options::new(&args, &config)?; debug!(?options); if args.list_files { diff --git a/tests/cli/error_value.rs b/tests/cli/error_value.rs index 9166a655..8210b3f2 100644 --- a/tests/cli/error_value.rs +++ b/tests/cli/error_value.rs @@ -27,3 +27,48 @@ fn error_value_catches_untested_ok_case() { true })); } + +#[test] +fn no_config_option_disables_config_file_so_error_value_is_not_generated() { + // In this case, the config file is not loaded. Error values are not + // generated by default (because we don't know what a good value for + // this tree would be), so no mutants are caught. + let tmp_src_dir = copy_of_testdata("error_value"); + run() + .arg("mutants") + .args(["-v", "-V", "--no-times", "--no-shuffle", "--no-config"]) + .arg("-d") + .arg(tmp_src_dir.path()) + .assert() + .code(0) + .stderr("") + .stdout(predicate::function(|stdout| { + insta::assert_snapshot!(stdout); + true + })); +} + +#[test] +fn list_mutants_with_error_value_from_command_line_list() { + // This is not a good error mutant for this tree, which uses + // anyhow, but it's a good test of the command line option. + let tmp_src_dir = copy_of_testdata("error_value"); + run() + .arg("mutants") + .args([ + "--no-times", + "--no-shuffle", + "--no-config", + "--list", + "--error=::eyre::eyre!(\"mutant\")", + ]) + .arg("-d") + .arg(tmp_src_dir.path()) + .assert() + .code(0) + .stderr("") + .stdout(predicate::function(|stdout| { + insta::assert_snapshot!(stdout); + true + })); +} diff --git a/tests/cli/snapshots/cli__error_value__list_mutants_with_error_value_from_command_line_list.snap b/tests/cli/snapshots/cli__error_value__list_mutants_with_error_value_from_command_line_list.snap new file mode 100644 index 00000000..d5f0b4ad --- /dev/null +++ b/tests/cli/snapshots/cli__error_value__list_mutants_with_error_value_from_command_line_list.snap @@ -0,0 +1,7 @@ +--- +source: tests/cli/error_value.rs +expression: stdout +--- +src/lib.rs:3: replace even_is_ok -> Result with Ok(Default::default()) +src/lib.rs:3: replace even_is_ok -> Result with Err(::eyre::eyre!("mutant")) + diff --git a/tests/cli/snapshots/cli__error_value__no_config_option_disables_config_file_so_error_value_is_not_generated.snap b/tests/cli/snapshots/cli__error_value__no_config_option_disables_config_file_so_error_value_is_not_generated.snap new file mode 100644 index 00000000..bfd3f593 --- /dev/null +++ b/tests/cli/snapshots/cli__error_value__no_config_option_disables_config_file_so_error_value_is_not_generated.snap @@ -0,0 +1,9 @@ +--- +source: tests/cli/error_value.rs +expression: stdout +--- +Found 1 mutant to test +Unmutated baseline ... ok +src/lib.rs:3: replace even_is_ok -> Result with Ok(Default::default()) ... caught +1 mutant tested: 1 caught + From 0c691c36bfbb9bcaf62a54995b2fd98da455774f Mon Sep 17 00:00:00 2001 From: Martin Pool Date: Fri, 5 May 2023 07:01:18 -0700 Subject: [PATCH 10/12] Warn if --error starts with Err( --- src/options.rs | 14 +++++++++++--- tests/cli/error_value.rs | 25 +++++++++++++++++++++++++ 2 files changed, 36 insertions(+), 3 deletions(-) diff --git a/src/options.rs b/src/options.rs index dcd02313..92d66e07 100644 --- a/src/options.rs +++ b/src/options.rs @@ -91,14 +91,13 @@ impl Options { warn!("--no-copy-target is deprecated and has no effect; target/ is never copied"); } - // If there's a let minimum_test_timeout = Duration::from_secs_f64( args.minimum_test_timeout .or(config.minimum_test_timeout) .unwrap_or(20f64), ); - Ok(Options { + let options = Options { additional_cargo_args: join_slices(&args.cargo_arg, &config.additional_cargo_args), additional_cargo_test_args: join_slices( &args.cargo_test_args, @@ -128,7 +127,16 @@ impl Options { show_all_logs: args.all_logs, test_timeout: args.timeout.map(Duration::from_secs_f64), minimum_test_timeout, - }) + }; + options.error_values.iter().for_each(|e| { + if e.starts_with("Err(") { + warn!( + "error_value option gives the value of the error, and probably should not start with Err(: got {}", + e + ); + } + }); + Ok(options) } } diff --git a/tests/cli/error_value.rs b/tests/cli/error_value.rs index 8210b3f2..99596a7d 100644 --- a/tests/cli/error_value.rs +++ b/tests/cli/error_value.rs @@ -72,3 +72,28 @@ fn list_mutants_with_error_value_from_command_line_list() { true })); } + +#[test] +fn warn_if_error_value_starts_with_err() { + // Users might misunderstand what should be passed to --error, + // so give a warning. + let tmp_src_dir = copy_of_testdata("error_value"); + run() + .arg("mutants") + .args([ + "--no-times", + "--no-shuffle", + "--no-config", + "--list", + "--error=Err(anyhow!(\"mutant\"))", + ]) + .arg("-d") + .arg(tmp_src_dir.path()) + .assert() + .code(0) + .stderr(predicate::str::is_empty()) + .stdout(predicate::str::contains( + "error_value option gives the value of the error, and probably should not start with Err(: got Err(anyhow!(\"mutant\"))" + )); + // don't care about stdout here +} From 61713aab20cf27fd4ffca668efd8d3b56ae4b1e4 Mon Sep 17 00:00:00 2001 From: Martin Pool Date: Fri, 5 May 2023 07:53:45 -0700 Subject: [PATCH 11/12] Scrub the book --- book/src/SUMMARY.md | 6 ++-- book/src/attrs.md | 51 +++++++++++++++++++++++++++++ book/src/cargo-args.md | 14 ++++++-- book/src/config.md | 25 --------------- book/src/controlling.md | 32 +++++++++++------- book/src/filter_mutants.md | 41 ++++++++++++++++------- book/src/parallelism.md | 34 +++++++++++++------- book/src/performance.md | 1 + book/src/skip.md | 66 ++++++-------------------------------- book/src/skip_files.md | 20 ++++++++++-- 10 files changed, 166 insertions(+), 124 deletions(-) create mode 100644 book/src/attrs.md delete mode 100644 book/src/config.md diff --git a/book/src/SUMMARY.md b/book/src/SUMMARY.md index e13ef3b0..7d9e910e 100644 --- a/book/src/SUMMARY.md +++ b/book/src/SUMMARY.md @@ -8,13 +8,13 @@ - [Exit codes](exit-codes.md) - [The `mutants.out` directory](mutants-out.md) - [Skipping untestable code](skip.md) + - [Skipping functions with an attribute](attrs.md) + - [Filtering files](skip_files.md) + - [Filtering functions and mutants](filter_mutants.md) - [Controlling cargo-mutants](controlling.md) - [Listing and previewing mutations](list.md) - - [Filtering files](skip_files.md) - - [Filtering mutants](filter_mutants.md) - [Workspaces and packages](workspaces.md) - [Passing options to Cargo](cargo-args.md) - - [The `mutants.toml` config file](config.md) - [Improving performance](performance.md) - [Parallelism](parallelism.md) - [Integrations](integrations.md) diff --git a/book/src/attrs.md b/book/src/attrs.md new file mode 100644 index 00000000..1f4777fa --- /dev/null +++ b/book/src/attrs.md @@ -0,0 +1,51 @@ +# Skipping functions with an attribute + +To mark functions as skipped, so they are not mutated: + +1. Add a Cargo dependency on the [mutants](https://crates.io/crates/mutants) + crate, version "0.0.3" or later. (This must be a regular `dependency` not a + `dev-dependency`, because the annotation will be on non-test code.) + +2. Mark functions with `#[mutants::skip]` or other attributes containing + `mutants::skip` (e.g. `#[cfg_attr(test, mutants::skip)]`). + +The `mutants` create is tiny and the attribute has no effect on the compiled +code. It only flags the function for cargo-mutants. However, you can avoid the +dependency by using the slightly longer `#[cfg_attr(test, mutants::skip)]` form. + +**Note:** Currently, `cargo-mutants` does not (yet) evaluate attributes like +`cfg_attr`, it only looks for the sequence `mutants::skip` in the attribute. + +You may want to also add a comment explaining why the function is skipped. + +For example: + +```rust +use std::time::{Duration, Instant}; + +/// Returns true if the program should stop +#[cfg_attr(test, mutants::skip)] // Returning false would cause a hang +fn should_stop() -> bool { + true +} + +pub fn controlled_loop() { + let start = Instant::now(); + for i in 0.. { + println!("{}", i); + if should_stop() { + break; + } + if start.elapsed() > Duration::from_secs(60 * 5) { + panic!("timed out"); + } + } +} + +mod test { + #[test] + fn controlled_loop_terminates() { + super::controlled_loop() + } +} +``` diff --git a/book/src/cargo-args.md b/book/src/cargo-args.md index 7832e5d0..0d393856 100644 --- a/book/src/cargo-args.md +++ b/book/src/cargo-args.md @@ -18,18 +18,28 @@ For example cargo mutants -- --cargo-arg=--release ``` +or in `.cargo/mutants.toml`: + +```toml +additional_cargo_args = ["--all-features"] +``` + ## Arguments to `cargo test` Command-line options following a `--` delimiter are passed through to `cargo test`. For example, this can be used to pass `--all-targets` which (unobviously) excludes doctests. (If the doctests are numerous and slow, and not relied upon to catch bugs, this can improve performance.) -These options can also be configured statically with the `additional_cargo_test_args` key in `.cargo/mutants.toml`. - ```shell cargo mutants -- --all-targets ``` +These options can also be configured statically with the `additional_cargo_test_args` key in `.cargo/mutants.toml`: + +```toml +additional_cargo_test_args = ["--jobs=1"] +``` + ## Arguments to test binaries You can use a second double-dash to pass options through to the test targets: diff --git a/book/src/config.md b/book/src/config.md deleted file mode 100644 index affc72a3..00000000 --- a/book/src/config.md +++ /dev/null @@ -1,25 +0,0 @@ -# Config file - -cargo-mutants looks for a `.cargo/mutants.toml` file in the root of the source -directory. If a config file exists, the values are appended to the corresponding -command-line arguments. (This may cause problems if you use `--` twice on the -command line to pass arguments to the inner test binary.) - -Configured exclusions may be particularly when there are modules that are -inherently hard to test, and the project has made a decision to accept lower -test coverage for them. - -Since Rust does not currently allow attributes such as `#[mutants::skip]` on `mod` statements or at module scope this is the only way to skip an entire module. - -The following configuration options are supported: - -```toml -exclude_globs = ["src/main.rs", "src/cache/*.rs"] # same as -e -examine_globs = ["src/important/*.rs"] # same as -f, test *only* these files - -exclude_re = ["impl Debug"] # same as -E -examine_re = ["impl Serialize", "impl Deserialize"] # same as -F, test *only* matches - -additional_cargo_args = ["--all-features"] -additional_cargo_test_args = ["--jobs=1"] -``` diff --git a/book/src/controlling.md b/book/src/controlling.md index c68e5187..fba9a824 100644 --- a/book/src/controlling.md +++ b/book/src/controlling.md @@ -1,16 +1,30 @@ # Controlling cargo-mutants -`cargo mutants` takes various options to control how it runs. These options are shown in `cargo mutants --help` and are described in detail in this section. +`cargo mutants` takes various options to control how it runs. + +These options, can, in general, be passed on the command line, set in a `.cargo/mutants.toml` +file in the source tree, or passed in `CARGO_MUTANTS_` environment variables. Not every +method of setting an option is available for every option, however, as some would not +make sense or be useful. + +For options that take a list of values, values from the configuration file are appended +to values from the command line. + +For options that take a single value, the value from the command line takes precedence. + +`--no-config` can be used to disable reading the configuration file. + +## Execution order By default, mutants are run in a randomized order, so as to surface results from different parts of the codebase earlier. This can be disabled with -`--no-shuffle`, in which case mutants will run in the same order shown by -`--list`: in order by file name and within each file in the order they appear in +`--no-shuffle`, in which case mutants will run in order by file name and within each file in the order they appear in the source. ## Source directory location -`-d`, `--dir`: Test the Rust tree in the given directory, rather than the default directory. +`-d`, `--dir`: Test the Rust tree in the given directory, rather than the source tree +enclosing the working directory where cargo-mutants is launched. ## Console output @@ -20,10 +34,6 @@ the source. `--no-times`: Don't print elapsed times. -## Environment variables - -A few options that may be useful to set globally can be configured through environment -variables: - -* `CARGO_MUTANTS_JOBS` -* `CARGO_MUTANTS_TRACE_LEVEL` +`-L`, `--level`, and `$CARGO_MUTANTS_TRACE_LEVEL`: set the verbosity of trace +output to stdout. The default is `info`, and it can be increased to `debug` or +`trace`. diff --git a/book/src/filter_mutants.md b/book/src/filter_mutants.md index 6fb10495..707d6913 100644 --- a/book/src/filter_mutants.md +++ b/book/src/filter_mutants.md @@ -1,23 +1,33 @@ # Filtering functions and mutants -The `#[mutants::skip]` attributes let you permanently mark a function as skipped -in the source tree. You can also narrow down which functions or mutants are -tested just for a single run of `cargo mutants`. +You can also filter mutants by name, using the `--re` and `--exclude-re` command line +options and the corresponding `examine_re` and `exclude_re` config file options. -Two options filter mutants by the full name of the mutant, which includes the -function name, file name, and a description of the change. +These options are useful if you want to run cargo-mutants just once, focusing on a subset of functions or mutants. -Mutant names are shown by `cargo mutants --list`, and the same command can be -used to preview the effect of filters. +These options filter mutants by the full name of the mutant, which includes the +function name, file name, and a description of the change, as shown in list. -`-F REGEX`, `--re REGEX`: Only test mutants whose full name matches the given regex. +For example, one mutant name might be: -`-E REGEX`, `--exclude-re REGEX`: Exclude mutants whose full name matches -the given regex. +```text +src/outcome.rs:157: replace ::serialize -> Result with Ok(Default::default()) +``` -These options may be repeated. +Within this name, your regex can match any substring, including for example: -The regex matches a substring and can be anchored with `^` and `$`. +- The filename +- The trait, `impl Serialize` +- The struct name, `ScenarioOutcome` +- The function name, `serialize` +- The mutated return value, `with Ok(Defualt::default())`, or any part of it. + +Mutants can also be filtered by name in the `.cargo/mutants.toml` file, for example: + +Regexes from the config file are appended to regexes from the command line. + +The regex matches a substring, but can be anchored with `^` and `$` to require that +it match the whole name. The regex syntax is defined by the [`regex`](https://docs.rs/regex/latest/regex/) crate. @@ -32,3 +42,10 @@ Examples: - `-F 'impl Serialize' -F 'impl Deserialize'` -- test implementations of these two traits. + +Or in `.cargo/mutants.toml`: + +```toml +exclude_re = ["impl Debug"] # same as -E +examine_re = ["impl Serialize", "impl Deserialize"] # same as -F, test *only* matches +``` diff --git a/book/src/parallelism.md b/book/src/parallelism.md index 464cfe4f..bd7da2e9 100644 --- a/book/src/parallelism.md +++ b/book/src/parallelism.md @@ -1,20 +1,30 @@ # Parallelism -The `--jobs` or `-j` option allows to test multiple mutants in parallel, by spawning several Cargo processes. This can give 25-50% performance improvements, depending on the tree under test and the hardware resources available. +After the initial test of the unmutated tree, cargo-mutants can test multiple +mutants in parallel. This can give significant performance improvements, +depending on the tree under test and the hardware resources available. -It's common that for some periods of its execution, a single Cargo build or test job can't use all the available CPU cores. Running multiple jobs in parallel makes use of resources that would otherwise be idle. +Even though cargo builds, rustc, and Rust's test framework launch multiple +processes or threads, they typically can't use all available CPU cores all the +time, and many `cargo test` runs will end up using only one core waiting for the +last task to complete. Running multiple jobs in parallel makes use of resources +that would otherwise be idle. -However, running many jobs simultaneously may also put high demands on the -system's RAM (by running more compile/link/test tasks simultaneously), IO -bandwidth, and cooling (by fully using all cores). +By default, only one job is run at a time. -The best setting will depend on many factors including the behavior of your program's test suite, the amount of memory on your system, and your system's behavior under high thermal load. +To run more, use the `--jobs` or `-j` option, or set the `CARGO_MUTANTS_JOBS` +environment variable. -The default is currently to run only one job at a time. Setting this higher than the number of CPU cores is unlikely to be helpful. +Setting this higher than the number of CPU cores is unlikely to be helpful. -`-j 4` may be a good starting point, even if you have many more CPU cores. Start -there and watch memory and CPU usage, and tune towards a setting where all cores -are always utilized without memory usage going too high, and without thermal -issues. +The best setting will depend on many factors including the behavior of your +program's test suite, the amount of memory on your system, and your system's +behavior under high thermal load. -Because tests may be slower with high parallelism, you may see some spurious timeouts, and you may need to set `--timeout` manually to allow enough safety margin. +`-j 4` may be a good starting point. Start there and watch memory and CPU usage, +and tune towards a setting where all cores are fully utilized without apparent +thrashing, memory exhaustion, or thermal issues. + +Because tests may be slower with high parallelism, you may see some spurious +timeouts, and you may need to set `--timeout` manually to allow enough safety +margin. diff --git a/book/src/performance.md b/book/src/performance.md index 90ad7ab7..2982a29e 100644 --- a/book/src/performance.md +++ b/book/src/performance.md @@ -31,6 +31,7 @@ cargo-mutants causes the Rust toolchain (and, often, the program under test) to ```shell sudo mkdir /ram sudo mount -t tmpfs /ram /ram # or put this in fstab, or just change /tmp +sudo chmod 1777 /ram env TMPDIR=/ram cargo mutants ``` diff --git a/book/src/skip.md b/book/src/skip.md index a2766aa1..6f648866 100644 --- a/book/src/skip.md +++ b/book/src/skip.md @@ -7,63 +7,17 @@ Some functions may be inherently hard to cover with tests, for example if: * The function has side effects or performance characteristics that are hard to test. * You've decided the function is not important to test. -## Skipping function with an attribute +There are three ways to skip mutating some code: -To mark functions so they are not mutated: +1. [Marking the function with an attribute](attrs.md) within the source file. +2. [Filtering by path](skip_files.md) in the config file or command line. +3. [Filtering by function and mutant name](filter_mutants.md) in the config file or command line. -1. Add a Cargo dependency on the [mutants](https://crates.io/crates/mutants) - crate, version "0.0.3" or later. (This must be a regular `dependency` not a - `dev-dependency`, because the annotation will be on non-test code.) +The results of all these filters can be previewed using the `--list` option. -2. Mark functions with `#[mutants::skip]` or other attributes containing - `mutants::skip` (e.g. `#[cfg_attr(test, mutants::skip)]`). +## Which filtering method to use? -The `mutants` create is tiny and the attribute has no effect on the compiled -code. It only flags the function for cargo-mutants. - -**Note:** Currently, `cargo-mutants` does not (yet) evaluate attributes like -`cfg_attr`, it only looks for the sequence `mutants::skip` in the attribute. - -You may want to also add a comment explaining why the function is skipped. - -**TODO**: Explain why `cfg_attr`. - -For example: - -```rust -use std::time::{Duration, Instant}; - -/// If mutated to return false, the program will spin forever. -#[cfg_attr(test, mutants::skip)] // causes a hang -fn should_stop() -> bool { - true -} - -pub fn controlled_loop() { - let start = Instant::now(); - for i in 0.. { - println!("{}", i); - if should_stop() { - break; - } - if start.elapsed() > Duration::from_secs(60 * 5) { - panic!("timed out"); - } - } -} - -mod test { - #[test] - fn controlled_loop_terminates() { - super::controlled_loop() - } -} -``` - -## Skipping files - -**Note:** Rust's "inner macro attributes" feature is currently unstable, so -`#![mutants::skip]` can't be used in module scope or on a `mod` statement. - -However, you can use the `exclude_globs` key in -[`.cargo/mutants.toml`](config.md), or the `--exclude` command-line option, to exclude files. +* If some particular functions are hard to test with cargo-mutants, use an attribute, so that the skip is visible in the code. +* If a whole module is untestable, use a filter by path in the config file, so that the filter's stored in the source tree and covers any new code in that module. +* If you want to permanently ignore a class of functions, such as `Debug` implementations, use a regex filter in the config file. +* If you want to run cargo-mutants just once, focusing on a subset of files, functions, or mutants, use command line options to filter by name or path. diff --git a/book/src/skip_files.md b/book/src/skip_files.md index 3a968022..0639f34d 100644 --- a/book/src/skip_files.md +++ b/book/src/skip_files.md @@ -2,13 +2,21 @@ Two options (each with short and long names) control which files are mutated: -`-f GLOB`, `--file GLOB`: Mutate only functions in files matching -the glob. +- `-f GLOB`, `--file GLOB`: Mutate only functions in files matching the glob. -`-e GLOB`, `--exclude GLOB`: Exclude files that match the glob. +- `-e GLOB`, `--exclude GLOB`: Exclude files that match the glob. These options may be repeated. +Files may also be filtered with the `exclude_globs` and `examine_globs` options in `.cargo/mutants.toml`, for example: + +```toml +exclude_globs = ["src/main.rs", "src/cache/*.rs"] # same as -e +examine_globs = ["src/important/*.rs"] # same as -f, test *only* these files +``` + +Globs from the config file are appended to globs from the command line. + If any `-f` options are given, only source files that match are considered; otherwise all files are considered. This list is then further reduced by exclusions. @@ -30,6 +38,12 @@ valid), and `mod` statements in them are followed to discover other source files. So, for example, you can exclude `src/main.rs` but still test mutants in other files referenced by `mod` statements in `main.rs`. +Since Rust does not currently allow attributes such as `#[mutants::skip]` on `mod` statements or at module scope filtering by filename is the only way to skip an entire module. + +Exclusions in the config file may be particularly useful when there are modules that are +inherently hard to automatically test, and the project has made a decision to accept lower +test coverage for them. + The results of filters can be previewed with the `--list-files` and `--list` options. From e291a01c6ec4e15819df2d1e41a455fe0ce37fc0 Mon Sep 17 00:00:00 2001 From: Martin Pool Date: Fri, 5 May 2023 08:50:12 -0700 Subject: [PATCH 12/12] Docs for error-value --- book/src/SUMMARY.md | 2 ++ book/src/error-values.md | 32 ++++++++++++++++++++++++++++++++ book/src/mutants.md | 27 +++++++++++++++++++++++++++ 3 files changed, 61 insertions(+) create mode 100644 book/src/error-values.md create mode 100644 book/src/mutants.md diff --git a/book/src/SUMMARY.md b/book/src/SUMMARY.md index 7d9e910e..12367b86 100644 --- a/book/src/SUMMARY.md +++ b/book/src/SUMMARY.md @@ -15,6 +15,8 @@ - [Listing and previewing mutations](list.md) - [Workspaces and packages](workspaces.md) - [Passing options to Cargo](cargo-args.md) +- [Generating mutants](mutants.md) + - [Error values](error-values.md) - [Improving performance](performance.md) - [Parallelism](parallelism.md) - [Integrations](integrations.md) diff --git a/book/src/error-values.md b/book/src/error-values.md new file mode 100644 index 00000000..c888cb6d --- /dev/null +++ b/book/src/error-values.md @@ -0,0 +1,32 @@ +# Generating error values + +cargo-mutants can be configured to generate mutants that return an error value from functions that return a Result. + +This will flag cases where no test fails if the function returns an error: that might happen if there are _only_ tests for the error cases and not for the Ok case. + +Since crates can choose to use any type for their error values, +cargo-mutants must be told how to construct an appropriate error. + +The `--error` command line option and the `error_value` configuration option specify an error value to use. + +These options can be repeated or combined, which might be useful +if there are multiple error types in the crate. On any one mutation site, probably only one of the error values will be viable, and cargo-mutants will discover that and use it. + +The error value can be any Rust expression that evaluates to a value of the error type. It should not include the `Err` wrapper, because cargo-mutants will add that. + +For example, if your crate uses `anyhow::Error` as its error type, you might use `--error '::anyhow::anyhow!("error")'`. + +If you have your own error type, you might use `--error 'crate::MyError::Generic'`. + +Since the correct error type is a property of the source tree, the configuration should typically go into `.cargo/mutants.toml` rather than being specified on the command line: + +```toml +error_values = ["::anyhow::anyhow!(\"mutated\")"] +``` + +To see only the mutants generated by this configuration, you +can use a command like this: + +```sh +cargo r mutants -F anyhow -vV -j4 +``` diff --git a/book/src/mutants.md b/book/src/mutants.md new file mode 100644 index 00000000..8b4f6513 --- /dev/null +++ b/book/src/mutants.md @@ -0,0 +1,27 @@ +# Generating mutants + +cargo mutants generates mutants by inspecting the existing +source code and applying a set of rules to generate new code +that is likely to compile but have different behavior. + +In the current release, the only mutation pattern is to +replace function bodies with a value of the same type. +This checks that the tests: + +1. Observe any side effects of the original function. +2. Distinguish return values. + +More mutation rules will be added in future releases. + +| Return type | Mutation pattern | +| ----------- | ---------------- | +| `()` | `()` (return unit, with no side effects) | +| `bool` | `true`, `false` | +| `String` | `String::new()`, `"xyzzy".into()` | +| `Result` | `Ok(Default::default())`, [and an error if configured](error-values.md) | +| (any other) | `Default::default()` (and hope the type implements `Default`) | + +Some of these values may not be valid for all types: for example, returning +`Default::default()` will work for many types, but not all. In this case the +mutant is said to be "unviable": by default these are counted but not printed, +although they can be shown with `--unviable`.