Add timeouts for builds (#345)

Fixes #322

NEWS.md

@@ -2,6 +2,12 @@
 
 ## Unreleased
 
+- 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`.
+
+- 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.

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.

src/config.rs

@@ -47,6 +47,8 @@ pub struct Config {
     pub test_tool: Option<TestTool>,
     /// Timeout multiplier, relative to the baseline 'cargo test'.
     pub timeout_multiplier: Option<f64>,
+    /// Build timeout multiplier, relative to the baseline 'cargo build'.
+    pub build_timeout_multiplier: Option<f64>,
 }
 
 impl Config {

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<Duration>, 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<Duration>,
+    baseline_duration: Option<Duration>,
+    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<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::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<OutputDir>,
     scenario: &Scenario,
     test_packages: &[&Package],
-    test_timeout: Duration,
+    timeouts: Timeouts,
     options: &Options,
     console: &Console,
 ) -> Result<ScenarioOutcome> {
@@ -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),);
+    }
 }

src/main.rs

@@ -289,9 +289,17 @@ pub struct Args {
     timeout: Option<f64>,
 
     /// Test timeout multiplier (relative to base test time).
-    #[arg(long, help_heading = "Execution")]
+    #[arg(long, help_heading = "Execution", conflicts_with = "timeout")]
     timeout_multiplier: Option<f64>,
 
+    /// Maximum run time for cargo build command, in seconds.
+    #[arg(long, help_heading = "Execution")]
+    build_timeout: Option<f64>,
+
+    /// Build timeout multiplier (relative to base build time).
+    #[arg(long, help_heading = "Execution", conflicts_with = "build_timeout")]
+    build_timeout_multiplier: Option<f64>,
+
     /// Print mutations that failed to check or build.
     #[arg(long, short = 'V', help_heading = "Output")]
     unviable: bool,