From 282611f6ccb1de31c510d324369f4c587102a649 Mon Sep 17 00:00:00 2001 From: Zane Duffield Date: Sat, 20 Apr 2024 11:36:22 +1000 Subject: [PATCH 1/3] Add timeouts for builds --- NEWS.md | 2 + book/src/timeouts.md | 38 +++- src/config.rs | 2 + src/lab.rs | 206 +++++++++++++++--- src/main.rs | 8 + src/options.rs | 37 ++++ testdata/hang_when_mutated/src/lib.rs | 17 ++ tests/main.rs | 61 ++++-- ...st__list_mutants_in_all_trees_as_json.snap | 94 +++++--- ...st__list_mutants_in_all_trees_as_text.snap | 17 +- ..._with_short_build_timeout__caught.txt.snap | 5 + ..._with_short_build_timeout__missed.txt.snap | 5 + ...with_short_build_timeout__timeout.txt.snap | 70 ++++++ ...ith_short_build_timeout__unviable.txt.snap | 5 + 14 files changed, 470 insertions(+), 97 deletions(-) create mode 100644 tests/snapshots/main__well_tested_tree_with_short_build_timeout__caught.txt.snap create mode 100644 tests/snapshots/main__well_tested_tree_with_short_build_timeout__missed.txt.snap create mode 100644 tests/snapshots/main__well_tested_tree_with_short_build_timeout__timeout.txt.snap create mode 100644 tests/snapshots/main__well_tested_tree_with_short_build_timeout__unviable.txt.snap diff --git a/NEWS.md b/NEWS.md index 6327f306..b86fc70a 100644 --- a/NEWS.md +++ b/NEWS.md @@ -2,6 +2,8 @@ ## Unreleased +- New: `--build-timeout` and `--build-timeout-multiplier` options for setting timeouts for the `build` and `check` cargo phases. + ## 24.4.0 - Changes: Baselines and mutants are now built with `cargo test --no-run` rather than `cargo build --tests` as previously. This avoids wasted build effort if the `dev` and `test` Cargo profiles are not the same, and may better distinguish build failures from test failures. With `--test-tool=nextest`, the corresponding `cargo nextest run --no-run` is used. diff --git a/book/src/timeouts.md b/book/src/timeouts.md index 2edef0e5..0c20741e 100644 --- a/book/src/timeouts.md +++ b/book/src/timeouts.md @@ -16,21 +16,35 @@ file](filter_mutants.md). ## Timeouts -To avoid hangs, cargo-mutants will kill the test suite after a timeout and +To avoid hangs, cargo-mutants will kill the build or test after a timeout and continue to the next mutant. -By default, the timeout is set automatically. cargo-mutants measures the time to -run the test suite in the unmodified tree, and then sets a timeout for mutated -tests at 5x the time to run tests with no mutations, and a minimum of 20 -seconds. +By default, the timeouts are set automatically, relative to the times taken to +build and test the unmodified tree (baseline). -The minimum of 20 seconds can be overridden by the -`CARGO_MUTANTS_MINIMUM_TEST_TIMEOUT` environment variable, measured in seconds. +The default timeouts are: +- `cargo build`/`cargo check`: 2 times the baseline build time +- `cargo test`: 5 times baseline test time (with a minimum of 20 seconds) -You can also set an explicit timeout with the `--timeout` option, also measured -in seconds. If this option is specified then the timeout is also applied to the -unmutated tests. +The minimum of 20 seconds for the test timeout can be overridden by the +`--minimum-test-timeout` option or the `CARGO_MUTANTS_MINIMUM_TEST_TIMEOUT` +environment variable, measured in seconds. -You can set a timeout multiplier that is relative to the duration of the unmutated tests with `--timeout-multiplier` or setting `timeout_multiplier` in `.cargo/mutants.toml` (`timeout-multiplier = 1.5`). This option is only applied if the baseline is not skipped and no `--timeout` option is specified, otherwise it is ignored. +You can set explicit timeouts with the `--build-timeout`, and `--timeout` +options, also measured in seconds. If these options are specified then they +are applied to the baseline build and test as well. -The timeout does not apply to `cargo check` or `cargo build`, only `cargo test`. +You can set timeout multipliers that are relative to the duration of the +baseline build or test with `--build-timeout-multiplier` and +`--timeout-multiplier`, respectively. Additionally, these can be configured +with `build_timeout_multiplier` and `timeout_multiplier` in +`.cargo/mutants.toml` (e.g. `timeout-multiplier = 1.5`). These options are only +applied if the baseline is not skipped and no corresponding +`--build-timeout`/`--timeout` option is specified, otherwise they are ignored. + +## Exceptions + +The multiplier timeout options cannot be used when the baseline is skipped +(`--baseline=skip`), or when the build is in-place (`--in-place`). If no +explicit timeouts is provided in these cases, then a default of 300 seconds +will be used. diff --git a/src/config.rs b/src/config.rs index 6b2d25f6..96cdd71b 100644 --- a/src/config.rs +++ b/src/config.rs @@ -47,6 +47,8 @@ pub struct Config { pub test_tool: Option, /// Timeout multiplier, relative to the baseline 'cargo test'. pub timeout_multiplier: Option, + /// Build timeout multiplier, relative to the baseline 'cargo build'. + pub build_timeout_multiplier: Option, } impl Config { diff --git a/src/lab.rs b/src/lab.rs index cdc2fd91..5adc8c6e 100644 --- a/src/lab.rs +++ b/src/lab.rs @@ -65,7 +65,10 @@ pub fn test_mutants( &output_mutex, &Scenario::Baseline, &all_packages, - options.test_timeout.unwrap_or(Duration::MAX), + Timeouts { + test: options.test_timeout.unwrap_or(Duration::MAX), + build: options.build_timeout.unwrap_or(Duration::MAX), + }, &options, console, )?; @@ -86,12 +89,17 @@ pub fn test_mutants( BaselineStrategy::Skip => None, }; let mut build_dirs = vec![build_dir]; - let baseline_test_duration = baseline_outcome - .as_ref() - .and_then(|so| so.phase_result(Phase::Test)) - .map(|pr| pr.duration); - let test_timeout = test_timeout(baseline_test_duration, &options); + let baseline_duration_by_phase = |phase| { + baseline_outcome + .as_ref() + .and_then(|so| so.phase_result(phase)) + .map(|pr| pr.duration) + }; + let timeouts = Timeouts { + build: build_timeout(baseline_duration_by_phase(Phase::Build), &options), + test: test_timeout(baseline_duration_by_phase(Phase::Test), &options), + }; let jobs = max(1, min(options.jobs.unwrap_or(1), mutants.len())); for i in 1..jobs { @@ -134,7 +142,7 @@ pub fn test_mutants( &output_mutex, &Scenario::Mutant(mutant), &[&package], - test_timeout, + timeouts, &options, console, )?; @@ -190,33 +198,78 @@ pub fn test_mutants( Ok(lab_outcome) } -fn test_timeout(baseline_test_duration: Option, options: &Options) -> Duration { - if let Some(timeout) = options.test_timeout { - timeout - } else if options.check_only { - Duration::ZERO - } else if let Some(baseline_test_duration) = baseline_test_duration { - let timeout = max( - options.minimum_test_timeout, - Duration::from_secs( - (baseline_test_duration.as_secs_f64() - * options.test_timeout_multiplier.unwrap_or(5.0)) - .round() as u64, - ), - ); - if options.show_times { - info!( - "Auto-set test timeout to {}", - humantime::format_duration(timeout) +#[derive(Copy, Clone)] +struct Timeouts { + build: Duration, + test: Duration, +} + +fn phase_timeout( + phase: Phase, + explicit_timeout: Option, + baseline_duration: Option, + minimum: Duration, + multiplier: f64, + options: &Options, +) -> Duration { + const FALLBACK_TIMEOUT_SECS: u64 = 300; + fn warn_fallback_timeout(phase_name: &str, option: &str) { + warn!("An explicit {phase_name} timeout is recommended when using {option}; using {FALLBACK_TIMEOUT_SECS} seconds by default"); + } + + if let Some(timeout) = explicit_timeout { + return timeout; + } + + match baseline_duration { + Some(_) if options.in_place && phase != Phase::Test => { + warn_fallback_timeout(phase.name(), "--in-place"); + Duration::from_secs(FALLBACK_TIMEOUT_SECS) + } + Some(baseline_duration) => { + let timeout = max( + minimum, + Duration::from_secs((baseline_duration.as_secs_f64() * multiplier).ceil() as u64), ); + + if options.show_times { + info!( + "Auto-set {} timeout to {}", + phase.name(), + humantime::format_duration(timeout) + ); + } + timeout + } + None => { + warn_fallback_timeout(phase.name(), "--baseline=skip"); + Duration::from_secs(FALLBACK_TIMEOUT_SECS) } - timeout - } else { - warn!("An explicit timeout is recommended when using --baseline=skip; using 300 seconds by default"); - Duration::from_secs(300) } } +fn test_timeout(baseline_duration: Option, options: &Options) -> Duration { + phase_timeout( + Phase::Test, + options.test_timeout, + baseline_duration, + options.minimum_test_timeout, + options.test_timeout_multiplier.unwrap_or(5.0), + options, + ) +} + +fn build_timeout(baseline_duration: Option, options: &Options) -> Duration { + phase_timeout( + Phase::Build, + options.build_timeout, + baseline_duration, + Duration::ZERO, + options.build_timeout_multiplier.unwrap_or(2.0), + options, + ) +} + /// Test various phases of one scenario in a build dir. /// /// The [BuildDir] is passed as mutable because it's for the exclusive use of this function for the @@ -226,7 +279,7 @@ fn test_scenario( output_mutex: &Mutex, scenario: &Scenario, test_packages: &[&Package], - test_timeout: Duration, + timeouts: Timeouts, options: &Options, console: &Console, ) -> Result { @@ -255,8 +308,8 @@ fn test_scenario( for &phase in phases { console.scenario_phase_started(scenario, phase); let timeout = match phase { - Phase::Test => test_timeout, - _ => Duration::MAX, + Phase::Test => timeouts.test, + Phase::Build | Phase::Check => timeouts.build, }; let phase_result = run_cargo( build_dir, @@ -306,6 +359,40 @@ mod test { ); } + #[test] + fn test_timeout_unaffected_by_in_place_build() { + let args = Args::parse_from(["mutants", "--timeout-multiplier", "1.5", "--in-place"]); + let options = Options::new(&args, &Config::default()).unwrap(); + + assert_eq!( + test_timeout(Some(Duration::from_secs(40)), &options), + Duration::from_secs(60), + ); + } + + #[test] + fn build_timeout_multiplier_from_option() { + let args = Args::parse_from(["mutants", "--build-timeout-multiplier", "1.5"]); + let options = Options::new(&args, &Config::default()).unwrap(); + + assert_eq!(options.build_timeout_multiplier, Some(1.5)); + assert_eq!( + build_timeout(Some(Duration::from_secs(40)), &options), + Duration::from_secs(60), + ); + } + + #[test] + fn build_timeout_is_affected_by_in_place_build() { + let args = Args::parse_from(["mutants", "--build-timeout-multiplier", "1.5", "--in-place"]); + let options = Options::new(&args, &Config::default()).unwrap(); + + assert_eq!( + build_timeout(Some(Duration::from_secs(40)), &options), + Duration::from_secs(300), + ); + } + #[test] fn timeout_multiplier_from_config() { let args = Args::parse_from(["mutants"]); @@ -322,6 +409,22 @@ mod test { ); } + #[test] + fn build_timeout_multiplier_from_config() { + let args = Args::parse_from(["mutants"]); + let config = Config::from_str(indoc! {r#" + build_timeout_multiplier = 2.0 + "#}) + .unwrap(); + let options = Options::new(&args, &config).unwrap(); + + assert_eq!(options.build_timeout_multiplier, Some(2.0)); + assert_eq!( + build_timeout(Some(Duration::from_secs(42)), &options), + Duration::from_secs(42 * 2), + ); + } + #[test] fn timeout_multiplier_default() { let args = Args::parse_from(["mutants"]); @@ -334,6 +437,34 @@ mod test { ); } + #[test] + fn build_timeout_multiplier_default() { + let args = Args::parse_from(["mutants"]); + let options = Options::new(&args, &Config::default()).unwrap(); + + assert_eq!(options.build_timeout_multiplier, None); + assert_eq!( + build_timeout(Some(Duration::from_secs(42)), &options), + Duration::from_secs(42 * 2), + ); + } + + #[test] + fn timeout_from_option() { + let args = Args::parse_from(["mutants", "--timeout=8"]); + let options = Options::new(&args, &Config::default()).unwrap(); + + assert_eq!(options.test_timeout, Some(Duration::from_secs(8))); + } + + #[test] + fn build_timeout_from_option() { + let args = Args::parse_from(["mutants", "--build-timeout=4"]); + let options = Options::new(&args, &Config::default()).unwrap(); + + assert_eq!(options.build_timeout, Some(Duration::from_secs(4))); + } + #[test] fn timeout_multiplier_default_with_baseline_skip() { // The --baseline option is not used to set the timeout but it's @@ -344,4 +475,15 @@ mod test { assert_eq!(options.test_timeout_multiplier, None); assert_eq!(test_timeout(None, &options), Duration::from_secs(300),); } + + #[test] + fn build_timeout_multiplier_default_with_baseline_skip() { + // The --baseline option is not used to set the timeout but it's + // indicative of the realistic situation. + let args = Args::parse_from(["mutants", "--baseline", "skip"]); + let options = Options::new(&args, &Config::default()).unwrap(); + + assert_eq!(options.build_timeout_multiplier, None); + assert_eq!(build_timeout(None, &options), Duration::from_secs(300),); + } } diff --git a/src/main.rs b/src/main.rs index a002eca5..bc96de1d 100644 --- a/src/main.rs +++ b/src/main.rs @@ -292,6 +292,14 @@ pub struct Args { #[arg(long, help_heading = "Execution")] timeout_multiplier: Option, + /// Maximum run time for cargo build command, in seconds. + #[arg(long, help_heading = "Execution")] + build_timeout: Option, + + /// Build timeout multiplier (relative to base build time). + #[arg(long, help_heading = "Execution")] + build_timeout_multiplier: Option, + /// Print mutations that failed to check or build. #[arg(long, short = 'V', help_heading = "Output")] unviable: bool, diff --git a/src/options.rs b/src/options.rs index d9a4d4cb..822b0272 100644 --- a/src/options.rs +++ b/src/options.rs @@ -46,6 +46,16 @@ pub struct Options { /// The time multiplier for test tasks, if set (relative to baseline test duration). pub test_timeout_multiplier: Option, + /// The time limit for build tasks, if set. + /// + /// If this is not set by the user it's None, in which case there is no time limit + /// on the baseline build, and then the mutated builds get a multiple of the time + /// taken by the baseline build. + pub build_timeout: Option, + + /// The time multiplier for build tasks, if set (relative to baseline build duration). + pub build_timeout_multiplier: Option, + /// The minimum test timeout, as a floor on the autoset value. pub minimum_test_timeout: Duration, @@ -212,6 +222,10 @@ impl Options { show_all_logs: args.all_logs, test_timeout: args.timeout.map(Duration::from_secs_f64), test_timeout_multiplier: config.timeout_multiplier.or(args.timeout_multiplier), + build_timeout: args.build_timeout.map(Duration::from_secs_f64), + build_timeout_multiplier: config + .build_timeout_multiplier + .or(args.build_timeout_multiplier), test_tool: args.test_tool.or(config.test_tool).unwrap_or_default(), }; options.error_values.iter().for_each(|e| { @@ -280,6 +294,29 @@ mod test { assert_eq!(options.baseline, BaselineStrategy::Run); } + #[test] + fn options_from_timeout_args() { + let args = Args::parse_from(["mutants", "--timeout=2.0"]); + let options = Options::new(&args, &Config::default()).unwrap(); + assert_eq!(options.test_timeout, Some(Duration::from_secs(2))); + + let args = Args::parse_from(["mutants", "--timeout-multiplier=2.5"]); + let options = Options::new(&args, &Config::default()).unwrap(); + assert_eq!(options.test_timeout_multiplier, Some(2.5)); + + let args = Args::parse_from(["mutants", "--minimum-test-timeout=60.0"]); + let options = Options::new(&args, &Config::default()).unwrap(); + assert_eq!(options.minimum_test_timeout, Duration::from_secs(60)); + + let args = Args::parse_from(["mutants", "--build-timeout=3.0"]); + let options = Options::new(&args, &Config::default()).unwrap(); + assert_eq!(options.build_timeout, Some(Duration::from_secs(3))); + + let args = Args::parse_from(["mutants", "--build-timeout-multiplier=3.5"]); + let options = Options::new(&args, &Config::default()).unwrap(); + assert_eq!(options.build_timeout_multiplier, Some(3.5)); + } + #[test] fn test_tool_from_config() { let config = indoc! { r#" diff --git a/testdata/hang_when_mutated/src/lib.rs b/testdata/hang_when_mutated/src/lib.rs index 8136dd5a..3d4ced90 100644 --- a/testdata/hang_when_mutated/src/lib.rs +++ b/testdata/hang_when_mutated/src/lib.rs @@ -8,6 +8,18 @@ use std::time::{Duration, Instant}; static TRIGGER: AtomicBool = AtomicBool::new(false); +const fn should_stop_const() -> bool { + true +} + +/// If `should_stop_const` is mutated to return false, then this const block +/// will hang and block compilation. +pub const VAL: i32 = loop { + if should_stop_const() { + break 1; + } +}; + /// If mutated to return false, the program will spin forever. fn should_stop() -> bool { if TRIGGER.load(Ordering::Relaxed) { @@ -44,4 +56,9 @@ mod test { // then the trigger is true and the loop terminates. assert_eq!(super::controlled_loop(), 2); } + + #[test] + fn val_is_correct() { + assert_eq!(super::VAL, 1); + } } diff --git a/tests/main.rs b/tests/main.rs index 9514f9df..04b59f8a 100644 --- a/tests/main.rs +++ b/tests/main.rs @@ -162,8 +162,11 @@ fn test_small_well_tested_tree_with_baseline_skip() { })) .stderr( predicate::str::contains( - "An explicit timeout is recommended when using --baseline=skip", + "An explicit test timeout is recommended when using --baseline=skip", ) + .and(predicate::str::contains( + "An explicit build timeout is recommended when using --baseline=skip", + )) .and(predicate::str::contains("Unmutated baseline in").not()), ); assert!(!tmp_src_dir @@ -734,33 +737,32 @@ fn interrupt_caught_and_kills_children() { /// * `should_stop` could change to always return `false`, in which case /// the loop will never stop, but the test should eventually be killed /// by a timeout. +/// +/// * `should_stop_const` could change to always return `false`, in which +/// case the loop in the block for the const `VAL` will never stop, but +/// the build should eventually be killed by a timeout. #[test] 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() .arg("mutants") - .args([ - "-t", - "8.1", - "-v", - "--line-col=false", - "--", - "--", - "--nocapture", - ]) + .args(["-t", "8.1"]) .current_dir(tmp_src_dir.path()) .env_remove("RUST_BACKTRACE") .timeout(OUTER_TIMEOUT) .assert() - .code(3) // exit_code::TIMEOUT - ; + .code(3); // exit_code::TIMEOUT let timeout_txt = read_to_string(tmp_src_dir.path().join("mutants.out/timeout.txt")) .expect("read timeout.txt"); assert!( timeout_txt.contains("replace should_stop -> bool with false"), "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}" + ); 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"), @@ -775,7 +777,40 @@ 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"], 1); + assert_eq!(outcomes_json["timeout"], 2); + + let phases_for_const_fn = outcomes_json["outcomes"] + .as_array() + .unwrap() + .iter() + .filter(|outcome| { + outcome["scenario"]["Mutant"]["function"]["function_name"] == "should_stop_const" + }) + .flat_map(|outcome| outcome["phase_results"].as_array()) + .next() + .expect("Failed to find phase_results for 'should_stop_const' fn"); + + assert_eq!(phases_for_const_fn.len(), 1); + assert_eq!(phases_for_const_fn[0]["phase"], "Build"); +} + +#[test] +fn mutants_causing_check_to_timeout_are_stopped_by_manual_timeout() { + let tmp_src_dir = copy_of_testdata("hang_when_mutated"); + run() + .arg("mutants") + .args(["--check", "--build-timeout=4"]) + .current_dir(tmp_src_dir.path()) + .env_remove("RUST_BACKTRACE") + .timeout(OUTER_TIMEOUT) + .assert() + .code(3); // exit_code::TIMEOUT + let timeout_txt = read_to_string(tmp_src_dir.path().join("mutants.out/timeout.txt")) + .expect("read timeout.txt"); + assert!( + timeout_txt.contains("replace should_stop_const -> bool with false"), + "expected text not found in:\n{timeout_txt}" + ); } #[test] diff --git a/tests/snapshots/list__list_mutants_in_all_trees_as_json.snap b/tests/snapshots/list__list_mutants_in_all_trees_as_json.snap index b858e4f5..5313d553 100644 --- a/tests/snapshots/list__list_mutants_in_all_trees_as_json.snap +++ b/tests/snapshots/list__list_mutants_in_all_trees_as_json.snap @@ -1618,12 +1618,12 @@ expression: buf { "file": "src/lib.rs", "function": { - "function_name": "should_stop", + "function_name": "should_stop_const", "return_type": "-> bool", "span": { "end": { "column": 2, - "line": 18 + "line": 13 }, "start": { "column": 1, @@ -1633,15 +1633,45 @@ expression: buf }, "genre": "FnValue", "package": "cargo-mutants-testdata-hang-when-mutated", + "replacement": "false", + "span": { + "end": { + "column": 9, + "line": 12 + }, + "start": { + "column": 5, + "line": 12 + } + } + }, + { + "file": "src/lib.rs", + "function": { + "function_name": "should_stop", + "return_type": "-> bool", + "span": { + "end": { + "column": 2, + "line": 30 + }, + "start": { + "column": 1, + "line": 23 + } + } + }, + "genre": "FnValue", + "package": "cargo-mutants-testdata-hang-when-mutated", "replacement": "true", "span": { "end": { "column": 10, - "line": 17 + "line": 29 }, "start": { "column": 5, - "line": 13 + "line": 25 } } }, @@ -1653,11 +1683,11 @@ expression: buf "span": { "end": { "column": 2, - "line": 18 + "line": 30 }, "start": { "column": 1, - "line": 11 + "line": 23 } } }, @@ -1667,11 +1697,11 @@ expression: buf "span": { "end": { "column": 10, - "line": 17 + "line": 29 }, "start": { "column": 5, - "line": 13 + "line": 25 } } }, @@ -1683,11 +1713,11 @@ expression: buf "span": { "end": { "column": 2, - "line": 38 + "line": 50 }, "start": { "column": 1, - "line": 20 + "line": 32 } } }, @@ -1697,11 +1727,11 @@ expression: buf "span": { "end": { "column": 20, - "line": 37 + "line": 49 }, "start": { "column": 5, - "line": 26 + "line": 38 } } }, @@ -1713,11 +1743,11 @@ expression: buf "span": { "end": { "column": 2, - "line": 38 + "line": 50 }, "start": { "column": 1, - "line": 20 + "line": 32 } } }, @@ -1727,11 +1757,11 @@ expression: buf "span": { "end": { "column": 20, - "line": 37 + "line": 49 }, "start": { "column": 5, - "line": 26 + "line": 38 } } }, @@ -1743,11 +1773,11 @@ expression: buf "span": { "end": { "column": 2, - "line": 38 + "line": 50 }, "start": { "column": 1, - "line": 20 + "line": 32 } } }, @@ -1757,11 +1787,11 @@ expression: buf "span": { "end": { "column": 29, - "line": 33 + "line": 45 }, "start": { "column": 28, - "line": 33 + "line": 45 } } }, @@ -1773,11 +1803,11 @@ expression: buf "span": { "end": { "column": 2, - "line": 38 + "line": 50 }, "start": { "column": 1, - "line": 20 + "line": 32 } } }, @@ -1787,11 +1817,11 @@ expression: buf "span": { "end": { "column": 29, - "line": 33 + "line": 45 }, "start": { "column": 28, - "line": 33 + "line": 45 } } }, @@ -1803,11 +1833,11 @@ expression: buf "span": { "end": { "column": 2, - "line": 38 + "line": 50 }, "start": { "column": 1, - "line": 20 + "line": 32 } } }, @@ -1817,11 +1847,11 @@ expression: buf "span": { "end": { "column": 54, - "line": 33 + "line": 45 }, "start": { "column": 53, - "line": 33 + "line": 45 } } }, @@ -1833,11 +1863,11 @@ expression: buf "span": { "end": { "column": 2, - "line": 38 + "line": 50 }, "start": { "column": 1, - "line": 20 + "line": 32 } } }, @@ -1847,11 +1877,11 @@ expression: buf "span": { "end": { "column": 54, - "line": 33 + "line": 45 }, "start": { "column": 53, - "line": 33 + "line": 45 } } } diff --git a/tests/snapshots/list__list_mutants_in_all_trees_as_text.snap b/tests/snapshots/list__list_mutants_in_all_trees_as_text.snap index 368bc2bc..01568286 100644 --- a/tests/snapshots/list__list_mutants_in_all_trees_as_text.snap +++ b/tests/snapshots/list__list_mutants_in_all_trees_as_text.snap @@ -135,14 +135,15 @@ src/lib.rs:21:53: replace * with / in controlled_loop ## testdata/hang_when_mutated ``` -src/lib.rs:13:5: replace should_stop -> bool with true -src/lib.rs:13:5: replace should_stop -> bool with false -src/lib.rs:26:5: replace controlled_loop -> usize with 0 -src/lib.rs:26:5: replace controlled_loop -> usize with 1 -src/lib.rs:33:28: replace > with == in controlled_loop -src/lib.rs:33:28: replace > with < in controlled_loop -src/lib.rs:33:53: replace * with + in controlled_loop -src/lib.rs:33:53: replace * with / in controlled_loop +src/lib.rs:12:5: replace should_stop_const -> bool with false +src/lib.rs:25:5: replace should_stop -> bool with true +src/lib.rs:25:5: replace should_stop -> bool with false +src/lib.rs:38:5: replace controlled_loop -> usize with 0 +src/lib.rs:38:5: replace controlled_loop -> usize with 1 +src/lib.rs:45:28: replace > with == in controlled_loop +src/lib.rs:45:28: replace > with < in controlled_loop +src/lib.rs:45:53: replace * with + in controlled_loop +src/lib.rs:45:53: replace * with / in controlled_loop ``` ## testdata/insta diff --git a/tests/snapshots/main__well_tested_tree_with_short_build_timeout__caught.txt.snap b/tests/snapshots/main__well_tested_tree_with_short_build_timeout__caught.txt.snap new file mode 100644 index 00000000..febf2282 --- /dev/null +++ b/tests/snapshots/main__well_tested_tree_with_short_build_timeout__caught.txt.snap @@ -0,0 +1,5 @@ +--- +source: tests/main.rs +expression: content +--- + diff --git a/tests/snapshots/main__well_tested_tree_with_short_build_timeout__missed.txt.snap b/tests/snapshots/main__well_tested_tree_with_short_build_timeout__missed.txt.snap new file mode 100644 index 00000000..febf2282 --- /dev/null +++ b/tests/snapshots/main__well_tested_tree_with_short_build_timeout__missed.txt.snap @@ -0,0 +1,5 @@ +--- +source: tests/main.rs +expression: content +--- + diff --git a/tests/snapshots/main__well_tested_tree_with_short_build_timeout__timeout.txt.snap b/tests/snapshots/main__well_tested_tree_with_short_build_timeout__timeout.txt.snap new file mode 100644 index 00000000..8e86ad9a --- /dev/null +++ b/tests/snapshots/main__well_tested_tree_with_short_build_timeout__timeout.txt.snap @@ -0,0 +1,70 @@ +--- +source: tests/main.rs +expression: content +--- +src/slices.rs:13:5: replace return_mut_slice -> &mut[usize] with Vec::leak(vec![0]) +src/numbers.rs:6:5: replace is_double -> bool with false +src/simple_fns.rs:13:5: replace returns_42u32 -> u32 with 1 +src/arc.rs:4:5: replace return_arc -> Arc with Arc::new(String::new()) +src/nested_function.rs:5:13: replace * with + in has_nested +src/numbers.rs:6:7: replace == with != in is_double +src/nested_function.rs:2:5: replace has_nested -> u32 with 1 +src/slices.rs:14:12: replace *= with /= in return_mut_slice +src/simple_fns.rs:8:5: replace returns_unit with () +src/nested_function.rs:3:9: replace has_nested::inner -> u32 with 1 +src/simple_fns.rs:18:5: replace divisible_by_three -> bool with true +src/simple_fns.rs:8:8: replace += with -= in returns_unit +src/simple_fns.rs:18:7: replace % with + in divisible_by_three +src/numbers.rs:2:9: replace * with + in double_float +src/numbers.rs:6:12: replace * with / in is_double +src/nested_function.rs:3:9: replace has_nested::inner -> u32 with 0 +src/numbers.rs:2:5: replace double_float -> f32 with 1.0 +src/simple_fns.rs:18:5: replace divisible_by_three -> bool with false +src/simple_fns.rs:18:11: replace == with != in divisible_by_three +src/simple_fns.rs:18:7: replace % with / in divisible_by_three +src/slices.rs:14:12: replace *= with += in return_mut_slice +src/static_item.rs:1:33: replace == with != +src/simple_fns.rs:27:5: replace double_string -> String with "xyzzy".into() +src/slices.rs:4:5: replace pad -> &'a[Cow<'static, str>] with Vec::leak(Vec::new()) +src/methods.rs:17:9: replace Foo::double with () +src/nested_function.rs:5:13: replace * with / in has_nested +src/result.rs:6:5: replace simple_result -> Result<&'static str, ()> with Ok("") +src/arc.rs:4:5: replace return_arc -> Arc with Arc::new("xyzzy".into()) +src/simple_fns.rs:8:8: replace += with *= in returns_unit +src/slices.rs:13:5: replace return_mut_slice -> &mut[usize] with Vec::leak(Vec::new()) +src/methods.rs:17:16: replace *= with += in Foo::double +src/numbers.rs:2:5: replace double_float -> f32 with -1.0 +src/slices.rs:4:5: replace pad -> &'a[Cow<'static, str>] with Vec::leak(vec![Cow::Owned("".to_owned())]) +src/slices.rs:4:5: replace pad -> &'a[Cow<'static, str>] with Vec::leak(vec![Cow::Owned("xyzzy".to_owned())]) +src/numbers.rs:2:5: replace double_float -> f32 with 0.0 +src/slices.rs:4:5: replace pad -> &'a[Cow<'static, str>] with Vec::leak(vec![Cow::Borrowed("")]) +src/numbers.rs:2:9: replace * with / in double_float +src/numbers.rs:6:12: replace * with + in is_double +src/methods.rs:23:9: replace ::fmt -> fmt::Result with Ok(Default::default()) +src/traits.rs:5:9: replace Something::is_three -> bool with true +src/slices.rs:5:20: replace < with > in pad +src/result.rs:6:5: replace simple_result -> Result<&'static str, ()> with Ok("xyzzy") +src/result.rs:10:10: replace < with == in error_if_negative +src/numbers.rs:6:5: replace is_double -> bool with true +src/methods.rs:17:16: replace *= with /= in Foo::double +src/struct_with_lifetime.rs:15:9: replace Lex<'buf>::buf_len -> usize with 1 +src/slices.rs:4:5: replace pad -> &'a[Cow<'static, str>] with Vec::leak(vec![Cow::Borrowed("xyzzy")]) +src/nested_function.rs:2:5: replace has_nested -> u32 with 0 +src/sets.rs:4:5: replace make_a_set -> BTreeSet with BTreeSet::from_iter([String::new()]) +src/slices.rs:5:20: replace < with == in pad +src/methods.rs:29:9: replace ::fmt -> fmt::Result with Ok(Default::default()) +src/struct_with_lifetime.rs:15:9: replace Lex<'buf>::buf_len -> usize with 0 +src/simple_fns.rs:27:5: replace double_string -> String with String::new() +src/sets.rs:4:5: replace make_a_set -> BTreeSet with BTreeSet::new() +src/result.rs:10:5: replace error_if_negative -> Result<(), ()> with Ok(()) +src/result.rs:18:5: replace result_with_no_apparent_type_args -> std::fmt::Result with Ok(Default::default()) +src/simple_fns.rs:13:5: replace returns_42u32 -> u32 with 0 +src/sets.rs:4:5: replace make_a_set -> BTreeSet with BTreeSet::from_iter(["xyzzy".into()]) +src/static_item.rs:1:39: replace + with - +src/static_item.rs:1:39: replace + with * +src/traits.rs:5:9: replace Something::is_three -> bool with false +src/traits.rs:5:11: replace == with != in Something::is_three +src/slices.rs:13:5: replace return_mut_slice -> &mut[usize] with Vec::leak(vec![1]) +src/inside_mod.rs:4:13: replace outer::inner::name -> &'static str with "" +src/result.rs:10:10: replace < with > in error_if_negative +src/inside_mod.rs:4:13: replace outer::inner::name -> &'static str with "xyzzy" diff --git a/tests/snapshots/main__well_tested_tree_with_short_build_timeout__unviable.txt.snap b/tests/snapshots/main__well_tested_tree_with_short_build_timeout__unviable.txt.snap new file mode 100644 index 00000000..febf2282 --- /dev/null +++ b/tests/snapshots/main__well_tested_tree_with_short_build_timeout__unviable.txt.snap @@ -0,0 +1,5 @@ +--- +source: tests/main.rs +expression: content +--- + From cb7ac6e30d976fe232f00f73afa9b9f5628259cf Mon Sep 17 00:00:00 2001 From: Zane Duffield Date: Mon, 22 Apr 2024 19:43:31 +1000 Subject: [PATCH 2/3] Make CLI timeout multipliers have precedence over config equivalents --- NEWS.md | 2 ++ src/options.rs | 26 +++++++++++++++++++++++--- 2 files changed, 25 insertions(+), 3 deletions(-) diff --git a/NEWS.md b/NEWS.md index b86fc70a..ae0fc974 100644 --- a/NEWS.md +++ b/NEWS.md @@ -4,6 +4,8 @@ - New: `--build-timeout` and `--build-timeout-multiplier` options for setting timeouts for the `build` and `check` cargo phases. +- Changed: `--timeout-multiplier` now overrides `timeout_multiplier` from `.cargo/mutants.toml`. + ## 24.4.0 - Changes: Baselines and mutants are now built with `cargo test --no-run` rather than `cargo build --tests` as previously. This avoids wasted build effort if the `dev` and `test` Cargo profiles are not the same, and may better distinguish build failures from test failures. With `--test-tool=nextest`, the corresponding `cargo nextest run --no-run` is used. diff --git a/src/options.rs b/src/options.rs index 822b0272..90bef6f9 100644 --- a/src/options.rs +++ b/src/options.rs @@ -221,11 +221,11 @@ impl Options { show_times: !args.no_times, show_all_logs: args.all_logs, test_timeout: args.timeout.map(Duration::from_secs_f64), - test_timeout_multiplier: config.timeout_multiplier.or(args.timeout_multiplier), + test_timeout_multiplier: args.timeout_multiplier.or(config.timeout_multiplier), build_timeout: args.build_timeout.map(Duration::from_secs_f64), - build_timeout_multiplier: config + build_timeout_multiplier: args .build_timeout_multiplier - .or(args.build_timeout_multiplier), + .or(config.build_timeout_multiplier), test_tool: args.test_tool.or(config.test_tool).unwrap_or_default(), }; options.error_values.iter().for_each(|e| { @@ -317,6 +317,26 @@ mod test { assert_eq!(options.build_timeout_multiplier, Some(3.5)); } + #[test] + fn cli_timeout_multiplier_overrides_config() { + let config = indoc! { r#" + timeout_multiplier = 1.0 + build_timeout_multiplier = 2.0 + "#}; + let mut config_file = NamedTempFile::new().unwrap(); + config_file.write_all(config.as_bytes()).unwrap(); + let args = Args::parse_from([ + "mutants", + "--timeout-multiplier=2.0", + "--build-timeout-multiplier=1.0", + ]); + let config = Config::read_file(config_file.path()).unwrap(); + let options = Options::new(&args, &config).unwrap(); + + assert_eq!(options.test_timeout_multiplier, Some(2.0)); + assert_eq!(options.build_timeout_multiplier, Some(1.0)); + } + #[test] fn test_tool_from_config() { let config = indoc! { r#" From da6443d9455d23b26e17fc001b0d493d3c8757bd Mon Sep 17 00:00:00 2001 From: Zane Duffield Date: Mon, 22 Apr 2024 20:21:02 +1000 Subject: [PATCH 3/3] Make CLI timeout options conflict with corresponding timeout-multiplier options --- NEWS.md | 2 ++ src/main.rs | 4 ++-- src/options.rs | 20 ++++++++++++++++++++ 3 files changed, 24 insertions(+), 2 deletions(-) diff --git a/NEWS.md b/NEWS.md index ae0fc974..b2bb48f3 100644 --- a/NEWS.md +++ b/NEWS.md @@ -6,6 +6,8 @@ - Changed: `--timeout-multiplier` now overrides `timeout_multiplier` from `.cargo/mutants.toml`. +- Changed: `--timeout` and `--timeout-multiplier` are now conflicting options. + ## 24.4.0 - Changes: Baselines and mutants are now built with `cargo test --no-run` rather than `cargo build --tests` as previously. This avoids wasted build effort if the `dev` and `test` Cargo profiles are not the same, and may better distinguish build failures from test failures. With `--test-tool=nextest`, the corresponding `cargo nextest run --no-run` is used. diff --git a/src/main.rs b/src/main.rs index bc96de1d..e5ccbcb5 100644 --- a/src/main.rs +++ b/src/main.rs @@ -289,7 +289,7 @@ pub struct Args { timeout: Option, /// Test timeout multiplier (relative to base test time). - #[arg(long, help_heading = "Execution")] + #[arg(long, help_heading = "Execution", conflicts_with = "timeout")] timeout_multiplier: Option, /// Maximum run time for cargo build command, in seconds. @@ -297,7 +297,7 @@ pub struct Args { build_timeout: Option, /// Build timeout multiplier (relative to base build time). - #[arg(long, help_heading = "Execution")] + #[arg(long, help_heading = "Execution", conflicts_with = "build_timeout")] build_timeout_multiplier: Option, /// Print mutations that failed to check or build. diff --git a/src/options.rs b/src/options.rs index 90bef6f9..6902eccd 100644 --- a/src/options.rs +++ b/src/options.rs @@ -337,6 +337,26 @@ mod test { assert_eq!(options.build_timeout_multiplier, Some(1.0)); } + #[test] + fn conflicting_timeout_options() { + let args = Args::try_parse_from(["mutants", "--timeout=1", "--timeout-multiplier=1"]) + .expect_err("--timeout and --timeout-multiplier should conflict"); + let rendered = format!("{}", args.render()); + assert!(rendered.contains("error: the argument '--timeout ' cannot be used with '--timeout-multiplier '")); + } + + #[test] + fn conflicting_build_timeout_options() { + let args = Args::try_parse_from([ + "mutants", + "--build-timeout=1", + "--build-timeout-multiplier=1", + ]) + .expect_err("--build-timeout and --build-timeout-multiplier should conflict"); + let rendered = format!("{}", args.render()); + assert!(rendered.contains("error: the argument '--build-timeout ' cannot be used with '--build-timeout-multiplier '")); + } + #[test] fn test_tool_from_config() { let config = indoc! { r#"