Skip to content

Commit

Permalink
Add timeouts for builds
Browse files Browse the repository at this point in the history
  • Loading branch information
zaneduffield committed Apr 21, 2024
1 parent c93ff14 commit 9cf2216
Show file tree
Hide file tree
Showing 14 changed files with 373 additions and 77 deletions.
2 changes: 2 additions & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@

- New: Generate mutations that delete the `!` and `-` unary operators.

- New: `--build-timeout` and `--build-timeout-multiplier` options for setting timeouts for the `build` and `check` cargo phases.

## 24.3.0

- Fixed: `cargo install cargo-mutants` without `--locked` was failing due to breaking API changes in some unstable dependencies.
Expand Down
31 changes: 19 additions & 12 deletions book/src/timeouts.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,21 +16,28 @@ 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.
2 changes: 2 additions & 0 deletions src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
147 changes: 124 additions & 23 deletions src/lab.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
)?;
Expand All @@ -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 {
Expand Down Expand Up @@ -134,7 +142,7 @@ pub fn test_mutants(
&output_mutex,
&Scenario::Mutant(mutant),
&[&package],
test_timeout,
timeouts,
&options,
console,
)?;
Expand Down Expand Up @@ -190,33 +198,75 @@ 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 {
#[derive(Copy, Clone)]
struct Timeouts {
build: Duration,
test: Duration,
}

fn phase_timeout(
explicit_timeout: Option<Duration>,
check_only_timeout: Duration,
baseline_duration: Option<Duration>,
minimum: Duration,
multiplier: f64,
phase_name: &str,
options: &Options,
) -> Duration {
if let Some(timeout) = explicit_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,
),
check_only_timeout
} else if let Some(baseline_duration) = baseline_duration {
let mut timeout = max(
minimum,
Duration::from_secs_f64(baseline_duration.as_secs_f64() * multiplier),
);

// Round large timeouts to a nice even number, but round smaller timeouts to 2DP
timeout = if timeout.as_secs() >= 1 {
Duration::from_secs(timeout.as_secs_f64().round() as u64)
} else {
Duration::from_secs_f64((timeout.as_secs_f64() * 100.0).round() / 100.0)
};

if options.show_times {
info!(
"Auto-set test timeout to {}",
"Auto-set {phase_name} timeout to {}",
humantime::format_duration(timeout)
);
}
timeout
} else {
warn!("An explicit timeout is recommended when using --baseline=skip; using 300 seconds by default");
warn!("An explicit {phase_name} 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(
options.test_timeout,
Duration::ZERO,
baseline_duration,
options.minimum_test_timeout,
options.test_timeout_multiplier.unwrap_or(5.0),
"test",
options,
)
}

fn build_timeout(baseline_duration: Option<Duration>, options: &Options) -> Duration {
phase_timeout(
options.build_timeout,
Duration::MAX,
baseline_duration,
Duration::ZERO,
options.build_timeout_multiplier.unwrap_or(2.0),
"build",
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
Expand All @@ -226,7 +276,7 @@ fn test_scenario(
output_mutex: &Mutex<OutputDir>,
scenario: &Scenario,
test_packages: &[&Package],
test_timeout: Duration,
timeouts: Timeouts,
options: &Options,
console: &Console,
) -> Result<ScenarioOutcome> {
Expand Down Expand Up @@ -255,8 +305,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,
Expand Down Expand Up @@ -306,6 +356,18 @@ mod test {
);
}

#[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 timeout_multiplier_from_config() {
let args = Args::parse_from(["mutants"]);
Expand All @@ -322,6 +384,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"]);
Expand All @@ -334,6 +412,18 @@ 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_multiplier_default_with_baseline_skip() {
// The --baseline option is not used to set the timeout but it's
Expand All @@ -344,4 +434,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),);
}
}
8 changes: 8 additions & 0 deletions src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -290,6 +290,14 @@ pub struct Args {
#[arg(long, help_heading = "Execution")]
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")]
build_timeout_multiplier: Option<f64>,

/// print mutations that failed to check or build.
#[arg(long, short = 'V', help_heading = "Output")]
unviable: bool,
Expand Down
14 changes: 14 additions & 0 deletions src/options.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<f64>,

/// 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<Duration>,

/// The time multiplier for build tasks, if set (relative to baseline build duration).
pub build_timeout_multiplier: Option<f64>,

/// The minimum test timeout, as a floor on the autoset value.
pub minimum_test_timeout: Duration,

Expand Down Expand Up @@ -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| {
Expand Down
17 changes: 17 additions & 0 deletions testdata/hang_when_mutated/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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);
}
}
Loading

0 comments on commit 9cf2216

Please sign in to comment.