No build timeout by default

NEWS.md

@@ -4,6 +4,8 @@
 
 - Fixed: The auto-set timeout for building mutants is now 2 times the baseline build time times the number of jobs, with a minimum of 20 seconds. This was changed because builds of mutants contend with each other for access to CPUs and may be slower than the baseline build.
 
+- 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`.
+
 ## 24.5.0
 
 - Fixed: Follow `path` attributes on `mod` statements.

book/src/timeouts.md

@@ -22,32 +22,31 @@ continue to the next mutant.
 By default, the timeouts are set automatically, relative to the times taken to
 build and test the unmodified tree (baseline).
 
-The default timeouts are:
-
-- `cargo build`/`cargo check`: 2 times the baseline build time times the number of jobs (with a minimum of 20 seconds)
-- `cargo test`: 5 times baseline test time (with a minimum of 20 seconds)
-
-The build timeout scales with the number of jobs to reflect that cargo often spawns many jobs, and so builds run in parallel are likely to take longer than the baseline, which has no external parallelism.
+The default test timeout is 5 times the baseline test time, with a minimum of 20 seconds.
 
 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 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.
+You can set an explicit timeouts with the `--timeout` option, also measured in seconds.
+
+You can also set the test timeout as a multiple of the duration of the baseline test, with the `--timeout-multiplier` option and the `timeout_multiplier` configuration key.
+The multiplier only has an effect if the baseline is not skipped and if `--timeout` is not specified.
+
+## Build timeouts
+
+`const` expressions may be evaluated at compile time. In the same way that mutations can cause tests to hang, mutations to const code may potentially cause the compiler to enter an infinite loop.
+
+rustc imposes a time limit on evaluation of const expressions. This is controlled by the `long_running_const_eval` lint, which by default will interrupt compilation: as a result the mutants will be seen as unviable.
+
+If this lint is configured off in your program, or if you use the `--cap-lints=true` option to turn off all lints, then the compiler may hang when constant expressions are mutated.
+
+In this case you can use the `--build-timeout` or `--build-timeout-multiplier` options, or their corresponding configuration keys, to impose a limit on overall build time. However, because build time can be quite variable there's some risk of this causing builds to be flaky, and so it's off by default.
 
-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.
+You might also choose to skip mutants that can cause long-running const evaluation.
 
 ## 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.
+explicit timeouts is provided in these cases, then there is no build timeout and the test timeout default of 300 seconds will be used.

src/cargo.rs

@@ -17,7 +17,7 @@ pub fn run_cargo(
     build_dir: &BuildDir,
     packages: Option<&[&Package]>,
     phase: Phase,
-    timeout: Duration,
+    timeout: Option<Duration>,
     log_file: &mut LogFile,
     options: &Options,
     console: &Console,

src/process.rs

@@ -33,7 +33,7 @@ const WAIT_POLL_INTERVAL: Duration = Duration::from_millis(50);
 pub struct Process {
     child: Popen,
     start: Instant,
-    timeout: Duration,
+    timeout: Option<Duration>,
 }
 
 impl Process {
@@ -43,7 +43,7 @@ impl Process {
         argv: &[String],
         env: &[(String, String)],
         cwd: &Utf8Path,
-        timeout: Duration,
+        timeout: Option<Duration>,
         log_file: &mut LogFile,
         console: &Console,
     ) -> Result<ProcessStatus> {
@@ -65,7 +65,7 @@ impl Process {
         argv: &[String],
         env: &[(String, String)],
         cwd: &Utf8Path,
-        timeout: Duration,
+        timeout: Option<Duration>,
         log_file: &mut LogFile,
     ) -> Result<Process> {
         let start = Instant::now();
@@ -99,9 +99,8 @@ impl Process {
     /// Check if the child process has finished; if so, return its status.
     #[mutants::skip] // It's hard to avoid timeouts if this never works...
     pub fn poll(&mut self) -> Result<Option<ProcessStatus>> {
-        let elapsed = self.start.elapsed();
-        if elapsed > self.timeout {
-            debug!(?elapsed, "timeout, terminating child process...",);
+        if self.timeout.map_or(false, |t| self.start.elapsed() > t) {
+            debug!("timeout, terminating child process...",);
             self.terminate()?;
             Ok(Some(ProcessStatus::Timeout))
         } else if let Err(e) = check_interrupted() {

src/timeouts.rs

@@ -13,15 +13,15 @@ use crate::{
 
 #[derive(Debug, Copy, Clone)]
 pub struct Timeouts {
-    pub build: Duration,
-    pub test: Duration,
+    pub build: Option<Duration>,
+    pub test: Option<Duration>,
 }
 
 impl Timeouts {
     pub fn for_baseline(options: &Options) -> Timeouts {
         Timeouts {
-            test: options.test_timeout.unwrap_or(Duration::MAX),
-            build: options.build_timeout.unwrap_or(Duration::MAX),
+            test: options.test_timeout,
+            build: None,
         }
     }
 
@@ -51,67 +51,51 @@ 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");
 }
 
-fn phase_timeout(
-    phase: Phase,
-    explicit_timeout: Option<Duration>,
-    baseline_duration: Option<Duration>,
-    minimum: Duration,
-    multiplier: f64,
-    options: &Options,
-) -> Duration {
-    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),
+fn test_timeout(baseline_duration: Option<Duration>, options: &Options) -> Option<Duration> {
+    if let Some(explicit) = options.test_timeout {
+        Some(explicit)
+    } else if let Some(baseline_duration) = baseline_duration {
+        let timeout = max(
+            options.minimum_test_timeout,
+            Duration::from_secs(
+                (baseline_duration.as_secs_f64() * options.test_timeout_multiplier.unwrap_or(5.0))
+                    .ceil() as u64,
+            ),
+        );
+        if options.show_times {
+            info!(
+                "Auto-set test timeout to {}",
+                humantime::format_duration(timeout)
             );
+        }
+        Some(timeout)
+    } else {
+        warn_fallback_timeout("test", "--baseline=skip");
+        Some(Duration::from_secs(FALLBACK_TIMEOUT_SECS))
+    }
+}
+
+fn build_timeout(baseline_duration: Option<Duration>, options: &Options) -> Option<Duration> {
+    if let Some(t) = options.build_timeout {
+        Some(t)
+    } else if let Some(baseline) = baseline_duration {
+        if let Some(multiplier) = options.build_timeout_multiplier {
+            let timeout = Duration::from_secs_f64(baseline.as_secs_f64() * multiplier);
             if options.show_times {
                 info!(
-                    "Auto-set {} timeout to {}",
-                    phase.name(),
+                    "Auto-set build timeout to {}",
                     humantime::format_duration(timeout)
                 );
             }
-            timeout
-        }
-        None => {
-            warn_fallback_timeout(phase.name(), "--baseline=skip");
-            Duration::from_secs(FALLBACK_TIMEOUT_SECS)
+            Some(timeout)
+        } else {
+            None
         }
+    } else {
+        None
     }
 }
 
-fn test_timeout(baseline_duration: Option<Duration>, 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<Duration>, options: &Options) -> Duration {
-    phase_timeout(
-        Phase::Build,
-        options.build_timeout,
-        baseline_duration,
-        Duration::from_secs(20),
-        options
-            .build_timeout_multiplier
-            .unwrap_or(2.0 * options.jobs.unwrap_or(1) as f64),
-        options,
-    )
-}
-
 #[cfg(test)]
 mod test {
     use std::str::FromStr;
@@ -130,7 +114,7 @@ mod test {
         assert_eq!(options.test_timeout_multiplier, Some(1.5));
         assert_eq!(
             test_timeout(Some(Duration::from_secs(40)), &options),
-            Duration::from_secs(60),
+            Some(Duration::from_secs(60)),
         );
     }
 
@@ -141,7 +125,7 @@ mod test {
 
         assert_eq!(
             test_timeout(Some(Duration::from_secs(40)), &options),
-            Duration::from_secs(60),
+            Some(Duration::from_secs(60)),
         );
     }
 
@@ -153,28 +137,18 @@ mod test {
         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 mutant_build_timeout_with_multiple_jobs() {
-        let args = Args::parse_from(["mutants", "--jobs=4"]);
-        let options = Options::new(&args, &Config::default()).unwrap();
-        assert_eq!(
-            build_timeout(Some(Duration::from_secs(40)), &options),
-            Duration::from_secs(40 * 2 * 4),
+            Some(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 args = Args::parse_from(["mutants", "--build-timeout-multiplier", "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),
+            Some(Duration::from_secs(40 * 5))
         );
     }
 
@@ -190,7 +164,7 @@ mod test {
         assert_eq!(options.test_timeout_multiplier, Some(2.0));
         assert_eq!(
             test_timeout(Some(Duration::from_secs(42)), &options),
-            Duration::from_secs(42 * 2),
+            Some(Duration::from_secs(42 * 2)),
         );
     }
 
@@ -206,7 +180,7 @@ mod test {
         assert_eq!(options.build_timeout_multiplier, Some(2.0));
         assert_eq!(
             build_timeout(Some(Duration::from_secs(42)), &options),
-            Duration::from_secs(42 * 2),
+            Some(Duration::from_secs(42 * 2)),
         );
     }
 
@@ -218,7 +192,7 @@ mod test {
         assert_eq!(options.test_timeout_multiplier, None);
         assert_eq!(
             test_timeout(Some(Duration::from_secs(42)), &options),
-            Duration::from_secs(42 * 5),
+            Some(Duration::from_secs(42 * 5)),
         );
     }
 
@@ -228,10 +202,7 @@ mod test {
         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),
-        );
+        assert_eq!(build_timeout(Some(Duration::from_secs(42)), &options), None,);
     }
 
     #[test]
@@ -251,24 +222,22 @@ mod test {
     }
 
     #[test]
-    fn 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"]);
+    fn no_default_build_timeout() {
+        let args = Args::parse_from(["mutants"]);
         let options = Options::new(&args, &Config::default()).unwrap();
 
-        assert_eq!(options.test_timeout_multiplier, None);
-        assert_eq!(test_timeout(None, &options), Duration::from_secs(300),);
+        assert_eq!(options.build_timeout, None);
     }
 
     #[test]
-    fn build_timeout_multiplier_default_with_baseline_skip() {
+    fn 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),);
+        assert_eq!(options.test_timeout_multiplier, None);
+        assert_eq!(test_timeout(None, &options), Some(Duration::from_secs(300)));
+        assert_eq!(build_timeout(None, &options), None);
     }
 }

tests/main.rs

@@ -164,9 +164,6 @@ fn test_small_well_tested_tree_with_baseline_skip() {
             predicate::str::contains(
                 "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
@@ -748,7 +745,7 @@ fn mutants_causing_tests_to_hang_are_stopped_by_manual_timeout() {
     // Also test that it accepts decimal seconds
     run()
         .arg("mutants")
-        .args(["-t", "8.1"])
+        .args(["-t", "8.1", "--build-timeout-multiplier=3"])
         .current_dir(tmp_src_dir.path())
         .env_remove("RUST_BACKTRACE")
         .timeout(OUTER_TIMEOUT)