Skip to content

Commit

Permalink
feat(max-fail): introduce --max-fail runner option
Browse files Browse the repository at this point in the history
  • Loading branch information
Aidan De Angelis committed Dec 3, 2024
1 parent f0d557a commit 7eb4c44
Show file tree
Hide file tree
Showing 7 changed files with 126 additions and 32 deletions.
43 changes: 37 additions & 6 deletions cargo-nextest/src/dispatch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,9 @@ use nextest_metadata::BuildPlatform;
use nextest_runner::{
cargo_config::{CargoConfigs, EnvironmentMap, TargetTriple},
config::{
get_num_cpus, ConfigExperimental, EarlyProfile, NextestConfig, NextestVersionConfig,
NextestVersionEval, RetryPolicy, TestGroup, TestThreads, ToolConfigFile, VersionOnlyConfig,
get_num_cpus, ConfigExperimental, EarlyProfile, MaxFail, NextestConfig,
NextestVersionConfig, NextestVersionEval, RetryPolicy, TestGroup, TestThreads,
ToolConfigFile, VersionOnlyConfig,
},
double_spawn::DoubleSpawnInfo,
errors::WriteTestListError,
Expand Down Expand Up @@ -845,9 +846,24 @@ pub struct TestRunnerOpts {
fail_fast: bool,

/// Run all tests regardless of failure
#[arg(long, conflicts_with = "no-run", overrides_with = "fail-fast")]
#[arg(
long,
name = "no-fail-fast",
conflicts_with = "no-run",

Check warning on line 852 in cargo-nextest/src/dispatch.rs

View check run for this annotation

Codecov / codecov/patch

cargo-nextest/src/dispatch.rs#L852

Added line #L852 was not covered by tests
overrides_with = "fail-fast"
)]
no_fail_fast: bool,

/// Number of tests that can fail before exiting test run [possible values: integer or "all"]
#[arg(
long,
name = "max-fail",
value_name = "N",
conflicts_with_all = &["no-run", "fail-fast", "no-fail-fast"],
require_equals = true,
)]
max_fail: Option<MaxFail>,

/// Behavior if there are no tests to run [default: fail]
#[arg(
long,
Expand Down Expand Up @@ -887,10 +903,17 @@ impl TestRunnerOpts {
if let Some(retries) = self.retries {
builder.set_retries(RetryPolicy::new_without_delay(retries));
}
if self.no_fail_fast {
builder.set_fail_fast(false);
if let Some(max_fail) = self.max_fail {
match max_fail {
MaxFail::Count(n) => {

Check warning on line 908 in cargo-nextest/src/dispatch.rs

View check run for this annotation

Codecov / codecov/patch

cargo-nextest/src/dispatch.rs#L906-L908

Added lines #L906 - L908 were not covered by tests
builder.set_max_fail(n);
}
MaxFail::All => {}

Check warning on line 911 in cargo-nextest/src/dispatch.rs

View check run for this annotation

Codecov / codecov/patch

cargo-nextest/src/dispatch.rs#L911

Added line #L911 was not covered by tests
}
} else if self.no_fail_fast {

Check warning on line 913 in cargo-nextest/src/dispatch.rs

View check run for this annotation

Codecov / codecov/patch

cargo-nextest/src/dispatch.rs#L913

Added line #L913 was not covered by tests
// Ignore --fail-fast
} else if self.fail_fast {
builder.set_fail_fast(true);
builder.set_max_fail(1);

Check warning on line 916 in cargo-nextest/src/dispatch.rs

View check run for this annotation

Codecov / codecov/patch

cargo-nextest/src/dispatch.rs#L916

Added line #L916 was not covered by tests
}
if let Some(test_threads) = self.test_threads {
builder.set_test_threads(test_threads);
Expand Down Expand Up @@ -2420,6 +2443,7 @@ mod tests {
"cargo nextest run --no-run --no-fail-fast",
ArgumentConflict,
),
("cargo nextest run --no-run --max-fail=3", ArgumentConflict),
(
"cargo nextest run --no-run --failure-output immediate",
ArgumentConflict,
Expand All @@ -2437,6 +2461,13 @@ mod tests {
ArgumentConflict,
),
// ---
// --max-fail and these options conflict
// ---
(
"cargo nextest run --max-fail=3 --no-fail-fast",
ArgumentConflict,
),
// ---
// Reuse build options conflict with cargo options
// ---
(
Expand Down
37 changes: 37 additions & 0 deletions nextest-runner/src/config/max_fail.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
use crate::errors::MaxFailParseError;
use std::{fmt, str::FromStr};

/// Type for the max-fail flag
#[derive(Clone, Copy, Debug, Eq, PartialEq)]
pub enum MaxFail {
/// Allow a specific number of tests to fail before exiting.
Count(usize),

/// Run all tests. Equivalent to --no-fast-fail.
All,
}

impl FromStr for MaxFail {
type Err = MaxFailParseError;

fn from_str(s: &str) -> Result<Self, Self::Err> {
if s.to_lowercase() == "all" {
return Ok(Self::All);

Check warning on line 19 in nextest-runner/src/config/max_fail.rs

View check run for this annotation

Codecov / codecov/patch

nextest-runner/src/config/max_fail.rs#L19

Added line #L19 was not covered by tests
}

match s.parse::<isize>() {
Err(e) => Err(MaxFailParseError::new(format!("Error: {e} parsing {s}"))),

Check warning on line 23 in nextest-runner/src/config/max_fail.rs

View check run for this annotation

Codecov / codecov/patch

nextest-runner/src/config/max_fail.rs#L23

Added line #L23 was not covered by tests
Ok(j) if j <= 0 => Err(MaxFailParseError::new("max-fail may not be <= 0")),
Ok(j) => Ok(MaxFail::Count(j as usize)),
}
}
}

impl fmt::Display for MaxFail {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
match self {
Self::All => write!(f, "all"),
Self::Count(n) => write!(f, "{n}"),

Check warning on line 34 in nextest-runner/src/config/max_fail.rs

View check run for this annotation

Codecov / codecov/patch

nextest-runner/src/config/max_fail.rs#L31-L34

Added lines #L31 - L34 were not covered by tests
}
}

Check warning on line 36 in nextest-runner/src/config/max_fail.rs

View check run for this annotation

Codecov / codecov/patch

nextest-runner/src/config/max_fail.rs#L36

Added line #L36 was not covered by tests
}
2 changes: 2 additions & 0 deletions nextest-runner/src/config/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ mod archive;
mod config_impl;
mod helpers;
mod identifier;
mod max_fail;
mod nextest_version;
mod overrides;
mod retry_policy;
Expand All @@ -36,6 +37,7 @@ mod track_default;
pub use archive::*;
pub use config_impl::*;
pub use identifier::*;
pub use max_fail::*;
pub use nextest_version::*;
pub use overrides::*;
pub use retry_policy::*;
Expand Down
16 changes: 16 additions & 0 deletions nextest-runner/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -554,6 +554,22 @@ pub enum ToolConfigFileParseError {
},
}

/// Error returned while parsing a [`MaxFail`](crate::config::MaxFail) input.
#[derive(Clone, Debug, Error)]
#[error("unrecognized value for max-fail: {input}\n(hint: expected either an integer or \"all\")")]
pub struct MaxFailParseError {
/// The input that failed to parse.
pub input: String,
}

impl MaxFailParseError {
pub(crate) fn new(input: impl Into<String>) -> Self {
Self {
input: input.into(),
}
}
}

/// Error returned while parsing a [`TestThreads`](crate::config::TestThreads) value.
#[derive(Clone, Debug, Error)]
#[error(
Expand Down
5 changes: 3 additions & 2 deletions nextest-runner/src/reporter/displayer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1982,14 +1982,15 @@ fn write_final_warnings(
if cancel_status == Some(CancelReason::TestFailure) {
writeln!(
writer,
"{}: {}/{} {} {} not run due to {} (run with {} to run all tests)",
"{}: {}/{} {} {} not run due to {} (run with {} to run all tests, or run with {})",
"warning".style(styles.skip),
not_run.style(styles.count),
initial_run_count.style(styles.count),
plural::tests_plural_if(initial_run_count != 1 || not_run != 1),
plural::were_plural_if(initial_run_count != 1 || not_run != 1),
CancelReason::TestFailure.to_static_str().style(styles.skip),
"--no-fail-fast".style(styles.count),
"--max-fail".style(styles.count),

Check warning on line 1993 in nextest-runner/src/reporter/displayer.rs

View check run for this annotation

Codecov / codecov/patch

nextest-runner/src/reporter/displayer.rs#L1993

Added line #L1993 was not covered by tests
)?;
} else {
let due_to_reason = match cancel_status {
Expand Down Expand Up @@ -2650,7 +2651,7 @@ mod tests {
assert_eq!(
warnings,
"warning: 1/3 tests were not run due to test failure \
(run with --no-fail-fast to run all tests)\n"
(run with --no-fail-fast to run all tests, or run with --max-fail)\n"
);

let warnings = final_warnings_for(
Expand Down
50 changes: 27 additions & 23 deletions nextest-runner/src/runner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ impl Iterator for BackoffIter {
pub struct TestRunnerBuilder {
capture_strategy: CaptureStrategy,
retries: Option<RetryPolicy>,
fail_fast: Option<bool>,
max_fail: Option<usize>,
test_threads: Option<TestThreads>,
}

Expand All @@ -158,9 +158,9 @@ impl TestRunnerBuilder {
self
}

/// Sets the fail-fast value for this test runner.
pub fn set_fail_fast(&mut self, fail_fast: bool) -> &mut Self {
self.fail_fast = Some(fail_fast);
/// Sets the max-fail value for this test runner.
pub fn set_max_fail(&mut self, max_fail: usize) -> &mut Self {
self.max_fail = Some(max_fail);

Check warning on line 163 in nextest-runner/src/runner.rs

View check run for this annotation

Codecov / codecov/patch

nextest-runner/src/runner.rs#L162-L163

Added lines #L162 - L163 were not covered by tests
self
}

Expand All @@ -187,7 +187,7 @@ impl TestRunnerBuilder {
.unwrap_or_else(|| profile.test_threads())
.compute(),
};
let fail_fast = self.fail_fast.unwrap_or_else(|| profile.fail_fast());
let max_fail = self.max_fail.or_else(|| profile.fail_fast().then_some(1));

let runtime = Runtime::new().map_err(TestRunnerBuildError::TokioRuntimeCreate)?;
let _guard = runtime.enter();
Expand All @@ -202,7 +202,7 @@ impl TestRunnerBuilder {
cli_args,
test_threads,
force_retries: self.retries,
fail_fast,
max_fail,
test_list,
double_spawn,
target_runner,
Expand Down Expand Up @@ -308,7 +308,7 @@ struct TestRunnerInner<'a> {
test_threads: usize,
// This is Some if the user specifies a retry policy over the command-line.
force_retries: Option<RetryPolicy>,
fail_fast: bool,
max_fail: Option<usize>,
test_list: &'a TestList<'a>,
double_spawn: DoubleSpawnInfo,
target_runner: TargetRunner,
Expand Down Expand Up @@ -337,7 +337,7 @@ impl<'a> TestRunnerInner<'a> {
self.profile.name(),
self.cli_args.clone(),
self.test_list.run_count(),
self.fail_fast,
self.max_fail,
);

// Send the initial event.
Expand Down Expand Up @@ -1618,26 +1618,28 @@ pub struct RunStats {
impl RunStats {
/// Returns true if there are any failures recorded in the stats.
pub fn has_failures(&self) -> bool {
self.setup_scripts_failed > 0
|| self.setup_scripts_exec_failed > 0
|| self.setup_scripts_timed_out > 0
|| self.failed > 0
|| self.exec_failed > 0
|| self.timed_out > 0
self.failed_setup_script_count() > 0 || self.failed_count() > 0
}

/// Returns count of setup scripts that did not pass.
pub fn failed_setup_script_count(&self) -> usize {
self.setup_scripts_failed + self.setup_scripts_exec_failed + self.setup_scripts_timed_out
}

/// Returns count of tests that did not pass.
pub fn failed_count(&self) -> usize {
self.failed + self.exec_failed + self.timed_out
}

/// Summarizes the stats as an enum at the end of a test run.
pub fn summarize_final(&self) -> FinalRunStats {
// Check for failures first. The order of setup scripts vs tests should not be important,
// though we don't assert that here.
if self.setup_scripts_failed > 0
|| self.setup_scripts_exec_failed > 0
|| self.setup_scripts_timed_out > 0
{
if self.failed_setup_script_count() > 0 {
FinalRunStats::Failed(RunStatsFailureKind::SetupScript)
} else if self.setup_scripts_initial_count > self.setup_scripts_finished_count {
FinalRunStats::Cancelled(RunStatsFailureKind::SetupScript)
} else if self.failed > 0 || self.exec_failed > 0 || self.timed_out > 0 {
} else if self.failed_count() > 0 {
FinalRunStats::Failed(RunStatsFailureKind::Test {
initial_run_count: self.initial_run_count,
not_run: self.initial_run_count.saturating_sub(self.finished_count),
Expand Down Expand Up @@ -1869,7 +1871,7 @@ struct CallbackContext<'a, F> {
cli_args: Vec<String>,
stopwatch: StopwatchStart,
run_stats: RunStats,
fail_fast: bool,
max_fail: Option<usize>,
running_setup_script: Option<ContextSetupScript<'a>>,
running_tests: BTreeMap<TestInstanceId<'a>, ContextTestInstance<'a>>,
cancel_state: Option<CancelReason>,
Expand All @@ -1886,7 +1888,7 @@ where
profile_name: &str,
cli_args: Vec<String>,
initial_run_count: usize,
fail_fast: bool,
max_fail: Option<usize>,
) -> Self {
Self {
callback,
Expand All @@ -1898,7 +1900,7 @@ where
initial_run_count,
..RunStats::default()
},
fail_fast,
max_fail,
running_setup_script: None,
running_tests: BTreeMap::new(),
cancel_state: None,
Expand Down Expand Up @@ -2078,7 +2080,9 @@ where
self.run_stats.on_test_finished(&run_statuses);

// should this run be cancelled because of a failure?
let fail_cancel = self.fail_fast && !run_statuses.last_status().result.is_success();
let fail_cancel = self
.max_fail
.map_or(false, |mf| self.run_stats.failed_count() >= mf);

self.callback(TestEventKind::TestFinished {
test_instance,
Expand Down
5 changes: 4 additions & 1 deletion site/src/docs/running.md
Original file line number Diff line number Diff line change
Expand Up @@ -184,8 +184,11 @@ cargo nextest run -E 'platform(host)'

## Other runner options

`--max-fail=N` <!-- md:version 0.9.86 -->
: Number of tests that can fail before aborting the test run. Useful for uncovering multiple issues without having to run the whole test suite.

`--no-fail-fast`
: Do not exit the test run on the first failure. Most useful for CI scenarios.
: Do not exit the test run on the first failure. Most useful for CI scenarios. Equivalent to `--max-fail=all`

`-j`, `--test-threads`
: Number of tests to run simultaneously. Note that this is separate from the number of build jobs to run simultaneously, which is specified by `--build-jobs`.
Expand Down

0 comments on commit 7eb4c44

Please sign in to comment.