diff --git a/NEWS.md b/NEWS.md index 479df323..196d91fa 100644 --- a/NEWS.md +++ b/NEWS.md @@ -6,6 +6,8 @@ - Changed: No build timeouts by default. Previously, cargo-mutants set a default build timeout based on the baseline build, but experience showed that this would sometimes make builds flaky, because build times can be quite variable. If mutants cause builds to hang, then you can still set a timeout using `--build-timeout` or `--build-timeout-multiplier`. +- Changed: cargo-mutants no longer passes `--cap-lints=allow` to rustc. This was previously done so that mutants would not unnecessarily be unviable due to triggering compiler warnings in trees configured to deny some lints, but it had the undesirable effect of disabling rustc's detection of long running const evaluations. If your tree treats some lints as errors then the previous behavior can be restored with `--cap-lints=true` (or the equivalent config option), or you can use `cfg_attr` and a feature flag to accept those warnings when testing under cargo-mutants. + ## 24.5.0 - Fixed: Follow `path` attributes on `mod` statements. diff --git a/book/src/SUMMARY.md b/book/src/SUMMARY.md index 7b37b414..c2035616 100644 --- a/book/src/SUMMARY.md +++ b/book/src/SUMMARY.md @@ -20,6 +20,7 @@ - [Using nextest](nextest.md) - [Baseline tests](baseline.md) - [Testing in-place](in-place.md) + - [Strict lints](lints.md) - [Generating mutants](mutants.md) - [Error values](error-values.md) - [Improving performance](performance.md) diff --git a/book/src/lints.md b/book/src/lints.md new file mode 100644 index 00000000..739f1878 --- /dev/null +++ b/book/src/lints.md @@ -0,0 +1,12 @@ +# Strict lints + +Because cargo-mutants builds versions of your tree with many heuristically injected errors, it may not work well in trees that are configured to treat warnings as errors. + +For example, mutants that delete code are likely to cause some parameters to be seen as unused, which will cause problems with trees that configure `#[deny(unused)]`. This will manifest as an excessive number of mutants being reported as "unviable". + +There are a few possible solutions: + +1. Define a feature flag for mutation testing, and use `cfg_attr` to enable strict warnings only when not testing mutants. +2. Use the `cargo mutants --cap-lints=true` command line option, or the `cap_lints = true` config option. + +`--cap_lints=true` also disables rustc's detection of long-running const expression evaluation, so may cause some builds to fail. If that happens in your tree, you can set a [build timeout](timeouts.md). diff --git a/src/cargo.rs b/src/cargo.rs index f7f60254..d2ede3d9 100644 --- a/src/cargo.rs +++ b/src/cargo.rs @@ -26,7 +26,7 @@ pub fn run_cargo( let start = Instant::now(); let argv = cargo_argv(build_dir.path(), packages, phase, options); let env = vec![ - ("CARGO_ENCODED_RUSTFLAGS".to_owned(), rustflags()), + ("CARGO_ENCODED_RUSTFLAGS".to_owned(), rustflags(options)), // The tests might use Insta , and we don't want it to write // updates to the source tree, and we *certainly* don't want it to write // updates and then let the test pass. @@ -140,7 +140,7 @@ fn cargo_argv( /// /// See /// -fn rustflags() -> String { +fn rustflags(options: &Options) -> String { let mut rustflags: Vec = if let Some(rustflags) = env::var_os("CARGO_ENCODED_RUSTFLAGS") { rustflags @@ -163,7 +163,9 @@ fn rustflags() -> String { // TODO: build.rustflags config value. Vec::new() }; - rustflags.push("--cap-lints=allow".to_owned()); + if options.cap_lints { + rustflags.push("--cap-lints=warn".to_owned()); + } // debug!("adjusted rustflags: {:?}", rustflags); rustflags.join("\x1f") } @@ -291,6 +293,17 @@ mod test { ); } + #[test] + fn cap_lints_passed_to_cargo() { + let args = Args::try_parse_from(["mutants", "--cap-lints=true"].as_slice()).unwrap(); + let options = Options::from_args(&args).unwrap(); + let build_dir = Utf8Path::new("/tmp/buildXYZ"); + assert_eq!( + cargo_argv(build_dir, None, Phase::Check, &options)[1..], + ["check", "--tests", "--workspace",] + ); + } + #[test] fn feature_args_passed_to_cargo() { let args = Args::try_parse_from( @@ -316,21 +329,34 @@ mod test { fn rustflags_with_no_environment_variables() { env::remove_var("RUSTFLAGS"); env::remove_var("CARGO_ENCODED_RUSTFLAGS"); - assert_eq!(rustflags(), "--cap-lints=allow"); + assert_eq!( + rustflags(&Options { + cap_lints: true, + ..Default::default() + }), + "--cap-lints=warn" + ); } #[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"); + let options = Options { + cap_lints: true, + ..Default::default() + }; + assert_eq!(rustflags(&options), "--something\x1f--else\x1f--cap-lints=warn"); } #[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"); + assert_eq!(rustflags(&Options { + cap_lints: true, + ..Default::default() + }), "-Dwarnings\x1f--cap-lints=warn"); } } } diff --git a/src/config.rs b/src/config.rs index 96cdd71b..5209f3b5 100644 --- a/src/config.rs +++ b/src/config.rs @@ -27,6 +27,8 @@ use crate::Result; #[derive(Debug, Default, Clone, Deserialize)] #[serde(default, deny_unknown_fields)] pub struct Config { + /// Pass `--cap-lints` to rustc. + pub cap_lints: bool, /// Generate these error values from functions returning Result. pub error_values: Vec, /// Generate mutants from source files matching these globs. diff --git a/src/main.rs b/src/main.rs index 47133dd9..18b11417 100644 --- a/src/main.rs +++ b/src/main.rs @@ -116,6 +116,10 @@ pub struct Args { #[arg(long, value_enum, default_value_t = BaselineStrategy::Run, help_heading = "Execution")] baseline: BaselineStrategy, + /// Turn off all rustc lints, so that denied warnings won't make mutants unviable. + #[arg(long, action = ArgAction::Set, help_heading = "Build")] + cap_lints: Option, + /// Print mutants that were caught by tests. #[arg(long, short = 'v', help_heading = "Output")] caught: bool, diff --git a/src/options.rs b/src/options.rs index 6902eccd..170851a7 100644 --- a/src/options.rs +++ b/src/options.rs @@ -24,6 +24,9 @@ pub struct Options { /// Run tests in an unmutated tree? pub baseline: BaselineStrategy, + /// Turn off all lints. + pub cap_lints: bool, + /// Don't run the tests, just see if each mutant builds. pub check_only: bool, @@ -196,6 +199,7 @@ impl Options { &config.additional_cargo_test_args, ), baseline: args.baseline, + cap_lints: args.cap_lints.unwrap_or(config.cap_lints), check_only: args.check, colors: args.colors, emit_json: args.json, @@ -270,6 +274,7 @@ mod test { let options = Options::new(&args, &Config::default()).unwrap(); assert!(!options.check_only); assert_eq!(options.test_tool, TestTool::Cargo); + assert!(!options.cap_lints); } #[test] @@ -358,9 +363,10 @@ mod test { } #[test] - fn test_tool_from_config() { + fn from_config() { let config = indoc! { r#" test_tool = "nextest" + cap_lints = true "#}; let mut config_file = NamedTempFile::new().unwrap(); config_file.write_all(config.as_bytes()).unwrap(); @@ -368,6 +374,7 @@ mod test { let config = Config::read_file(config_file.path()).unwrap(); let options = Options::new(&args, &config).unwrap(); assert_eq!(options.test_tool, TestTool::Nextest); + assert!(options.cap_lints); } #[test] diff --git a/tests/main.rs b/tests/main.rs index 03a0cf5d..3b0b84a3 100644 --- a/tests/main.rs +++ b/tests/main.rs @@ -743,14 +743,22 @@ fn interrupt_caught_and_kills_children() { fn mutants_causing_tests_to_hang_are_stopped_by_manual_timeout() { let tmp_src_dir = copy_of_testdata("hang_when_mutated"); // Also test that it accepts decimal seconds - run() + let out = run() .arg("mutants") - .args(["-t", "8.1", "--build-timeout-multiplier=3"]) + .args(["-t", "8.1", "--build-timeout=15.5"]) .current_dir(tmp_src_dir.path()) .env_remove("RUST_BACKTRACE") .timeout(OUTER_TIMEOUT) .assert() .code(3); // exit_code::TIMEOUT + println!( + "output:\n{}", + String::from_utf8_lossy(&out.get_output().stdout) + ); + let unviable_txt = read_to_string(tmp_src_dir.path().join("mutants.out/unviable.txt")) + .expect("read timeout.txt"); + let caught_txt = read_to_string(tmp_src_dir.path().join("mutants.out/caught.txt")) + .expect("read timeout.txt"); let timeout_txt = read_to_string(tmp_src_dir.path().join("mutants.out/timeout.txt")) .expect("read timeout.txt"); assert!( @@ -758,10 +766,9 @@ fn mutants_causing_tests_to_hang_are_stopped_by_manual_timeout() { "expected text not found in:\n{timeout_txt}" ); assert!( - timeout_txt.contains("replace should_stop_const -> bool with false"), - "expected text not found in:\n{timeout_txt}" + unviable_txt.contains("replace should_stop_const -> bool with false"), + "expected text not found in:\n{unviable_txt}" ); - let caught_txt = read_to_string(tmp_src_dir.path().join("mutants.out/caught.txt")).unwrap(); assert!( caught_txt.contains("replace should_stop -> bool with true"), "expected text not found in:\n{caught_txt}" @@ -775,7 +782,7 @@ fn mutants_causing_tests_to_hang_are_stopped_by_manual_timeout() { .expect("read outcomes.json") .parse() .expect("parse outcomes.json"); - assert_eq!(outcomes_json["timeout"], 2); + assert_eq!(outcomes_json["timeout"], 1); let phases_for_const_fn = outcomes_json["outcomes"] .as_array() @@ -793,11 +800,15 @@ fn mutants_causing_tests_to_hang_are_stopped_by_manual_timeout() { } #[test] -fn mutants_causing_check_to_timeout_are_stopped_by_manual_timeout() { +fn hang_avoided_by_build_timeout_with_cap_lints() { let tmp_src_dir = copy_of_testdata("hang_when_mutated"); run() .arg("mutants") - .args(["--check", "--build-timeout=4"]) + .args([ + "--build-timeout-multiplier=4", + "--regex=const", + "--cap-lints=true", + ]) .current_dir(tmp_src_dir.path()) .env_remove("RUST_BACKTRACE") .timeout(OUTER_TIMEOUT) @@ -811,6 +822,20 @@ fn mutants_causing_check_to_timeout_are_stopped_by_manual_timeout() { ); } +#[test] +fn constfn_mutation_passes_check() { + let tmp_src_dir = copy_of_testdata("hang_when_mutated"); + let cmd = run() + .arg("mutants") + .args(["--check", "--build-timeout=4"]) + .current_dir(tmp_src_dir.path()) + .env_remove("RUST_BACKTRACE") + .timeout(OUTER_TIMEOUT) + .assert() + .code(0); + println!("{}", String::from_utf8_lossy(&cmd.get_output().stdout)); +} + #[test] fn log_file_names_are_short_and_dont_collide() { // The "well_tested" tree can generate multiple mutants from single lines. They get distinct file names.