Skip to content

Commit

Permalink
[cargo-nextest] add a new --no-tests option
Browse files Browse the repository at this point in the history
Default to `warn` currently, but can also be `fail` or `pass`. The plan is to
default to `fail` in the future (see #1646).
  • Loading branch information
sunshowers committed Aug 20, 2024
1 parent 4f5c89a commit d2fb356
Show file tree
Hide file tree
Showing 4 changed files with 96 additions and 30 deletions.
60 changes: 52 additions & 8 deletions cargo-nextest/src/dispatch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ use camino::{Utf8Path, Utf8PathBuf};
use clap::{builder::BoolishValueParser, ArgAction, Args, Parser, Subcommand, ValueEnum};
use guppy::graph::PackageGraph;
use itertools::Itertools;
use log::warn;
use nextest_filtering::FilteringExpr;
use nextest_metadata::BuildPlatform;
use nextest_runner::{
Expand Down Expand Up @@ -406,8 +407,7 @@ impl NtrOpts {
&self.run_opts.reporter_opts,
cli_args,
output_writer,
)?;
Ok(0)
)

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

View check run for this annotation

Codecov / codecov/patch

cargo-nextest/src/dispatch.rs#L410

Added line #L410 was not covered by tests
}
}

Expand Down Expand Up @@ -754,6 +754,33 @@ pub struct TestRunnerOpts {
/// Run all tests regardless of failure
#[arg(long, conflicts_with = "no-run", overrides_with = "fail-fast")]
no_fail_fast: bool,

/// Behavior if no tests were run.
///
/// The default is currently `warn`, but it will change to `fail` in the future.
#[arg(
long,
value_enum,
conflicts_with = "no-run",
value_name = "ACTION",
require_equals = true,
env = "NEXTEST_NO_TESTS"
)]
no_tests: Option<NoTestsBehavior>,
}

#[derive(Clone, Copy, Debug, Default, ValueEnum)]
enum NoTestsBehavior {
/// Produce no warning and exit with code 0.
Pass,

/// Produce a warning and exit with code 0.
#[default]
Warn,

/// Produce an error message and exit with code 4.
#[clap(alias = "error")]
Fail,
}

impl TestRunnerOpts {
Expand Down Expand Up @@ -1563,7 +1590,7 @@ impl App {
reporter_opts: &TestReporterOpts,
cli_args: Vec<String>,
output_writer: &mut OutputWriter,
) -> Result<()> {
) -> Result<i32> {
let (version_only_config, config) = self.base.load_config()?;
let profile = self.base.load_profile(profile_name, &config)?;

Expand Down Expand Up @@ -1638,7 +1665,7 @@ impl App {
Some(runner_builder) => runner_builder,
None => {
// This means --no-run was passed in. Exit.
return Ok(());
return Ok(0);

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

View check run for this annotation

Codecov / codecov/patch

cargo-nextest/src/dispatch.rs#L1668

Added line #L1668 was not covered by tests
}
};

Expand All @@ -1656,15 +1683,32 @@ impl App {
// Write and flush the event.
reporter.report_event(event)
})?;
reporter.finish();
self.base
.check_version_config_final(version_only_config.nextest_version())?;

match run_stats.summarize_final() {
FinalRunStats::Success => Ok(()),
FinalRunStats::Success => Ok(0),

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

View check run for this annotation

Codecov / codecov/patch

cargo-nextest/src/dispatch.rs#L1691

Added line #L1691 was not covered by tests
FinalRunStats::NoTestsRun => {
// This currently does not exit with a non-zero code, but will in the future:
// https://github.com/nextest-rs/nextest/issues/1639
Ok(())
match runner_opts.no_tests {
Some(NoTestsBehavior::Pass) => Ok(0),

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

View check run for this annotation

Codecov / codecov/patch

cargo-nextest/src/dispatch.rs#L1693-L1694

Added lines #L1693 - L1694 were not covered by tests
Some(NoTestsBehavior::Warn) => {
warn!("no tests were run");
Ok(0)

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

View check run for this annotation

Codecov / codecov/patch

cargo-nextest/src/dispatch.rs#L1696-L1697

Added lines #L1696 - L1697 were not covered by tests
}
Some(NoTestsBehavior::Fail) => {
Err(ExpectedError::NoTestsRun { is_default: false })

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

View check run for this annotation

Codecov / codecov/patch

cargo-nextest/src/dispatch.rs#L1700

Added line #L1700 was not covered by tests
}
None => {
// This currently does not exit with a non-zero code, but will in the
// future: https://github.com/nextest-rs/nextest/issues/1639
warn!(
"no tests were run -- this will become an error in the future\n\
(hint: use `--no-tests` to customize this behavior)"

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

View check run for this annotation

Codecov / codecov/patch

cargo-nextest/src/dispatch.rs#L1705-L1707

Added lines #L1705 - L1707 were not covered by tests
);
Ok(0)

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

View check run for this annotation

Codecov / codecov/patch

cargo-nextest/src/dispatch.rs#L1709

Added line #L1709 was not covered by tests
}
}
}
FinalRunStats::Canceled(RunStatsFailureKind::SetupScript)
| FinalRunStats::Failed(RunStatsFailureKind::SetupScript) => {
Expand Down
16 changes: 16 additions & 0 deletions cargo-nextest/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,12 @@ pub enum ExpectedError {
SetupScriptFailed,
#[error("test run failed")]
TestRunFailed,
#[error("no tests were run")]
NoTestsRun {
/// The no-tests-run error was chosen because it was the default (we show a hint in this
/// case)
is_default: bool,
},
#[cfg(feature = "self-update")]
#[error("failed to parse --version")]
UpdateVersionParseError {
Expand Down Expand Up @@ -406,6 +412,7 @@ impl ExpectedError {
}
Self::SetupScriptFailed => NextestExitCode::SETUP_SCRIPT_FAILED,
Self::TestRunFailed => NextestExitCode::TEST_RUN_FAILED,
Self::NoTestsRun { .. } => NextestExitCode::NO_TESTS_RUN,

Check warning on line 415 in cargo-nextest/src/errors.rs

View check run for this annotation

Codecov / codecov/patch

cargo-nextest/src/errors.rs#L415

Added line #L415 was not covered by tests
Self::ArchiveCreateError { .. } => NextestExitCode::ARCHIVE_CREATION_FAILED,
Self::WriteTestListError { .. } | Self::WriteEventError { .. } => {
NextestExitCode::WRITE_OUTPUT_ERROR
Expand Down Expand Up @@ -731,6 +738,15 @@ impl ExpectedError {
log::error!("test run failed");
None
}
Self::NoTestsRun { is_default } => {
let hint_str = if *is_default {
"\n(hint: use `--no-tests` to customize this behavior)"

Check warning on line 743 in cargo-nextest/src/errors.rs

View check run for this annotation

Codecov / codecov/patch

cargo-nextest/src/errors.rs#L741-L743

Added lines #L741 - L743 were not covered by tests
} else {
""

Check warning on line 745 in cargo-nextest/src/errors.rs

View check run for this annotation

Codecov / codecov/patch

cargo-nextest/src/errors.rs#L745

Added line #L745 was not covered by tests
};
log::error!("no tests were run{hint_str}");
None

Check warning on line 748 in cargo-nextest/src/errors.rs

View check run for this annotation

Codecov / codecov/patch

cargo-nextest/src/errors.rs#L747-L748

Added lines #L747 - L748 were not covered by tests
}
Self::ShowTestGroupsError { err } => {
log::error!("{err}");
err.source()
Expand Down
8 changes: 8 additions & 0 deletions nextest-metadata/src/exit_codes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,14 @@ impl NextestExitCode {
/// An error was encountered while attempting to double-spawn a nextest process.
pub const DOUBLE_SPAWN_ERROR: i32 = 70;

/// No tests were run, but no other errors occurred.
///
/// This is an advisory exit code generated if nextest is run with `--no-tests=fail` (soon to
/// become the default). See [discussion #1646] for more.
///
/// [discussion #1646]: https://github.com/nextest-rs/nextest/discussions/1646
pub const NO_TESTS_RUN: i32 = 4;

/// One or more tests failed.
pub const TEST_RUN_FAILED: i32 = 100;

Expand Down
42 changes: 20 additions & 22 deletions nextest-runner/src/reporter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -342,6 +342,17 @@ enum ReporterStderrImpl<'a> {
Buffer(&'a mut Vec<u8>),
}

impl ReporterStderrImpl<'_> {
fn finish_and_clear_bar(&self) {
match self {
ReporterStderrImpl::TerminalWithBar(bar) => {
bar.finish_and_clear();
}

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

View check run for this annotation

Codecov / codecov/patch

nextest-runner/src/reporter.rs#L348-L350

Added lines #L348 - L350 were not covered by tests
ReporterStderrImpl::TerminalWithoutBar | ReporterStderrImpl::Buffer(_) => {}
}
}
}

/// Functionality to report test results to stderr, JUnit, and/or structured,
/// machine-readable results to stdout
pub struct TestReporter<'a> {
Expand All @@ -364,6 +375,11 @@ impl<'a> TestReporter<'a> {
self.write_event(event)
}

/// Mark the reporter done.
pub fn finish(&mut self) {
self.stderr.finish_and_clear_bar();
}

// ---
// Helper methods
// ---
Expand Down Expand Up @@ -1021,8 +1037,7 @@ impl<'a> TestReporterImpl<'a> {
}
}

// Print out warnings at the end, if any. We currently print out warnings in two
// cases:
// Print out warnings at the end, if any.
write_final_warnings(stats_summary, self.cancel_status, &self.styles, writer)?;
}
}
Expand Down Expand Up @@ -1634,7 +1649,6 @@ fn write_final_warnings(
writer: &mut dyn Write,
) -> io::Result<()> {
match final_stats {
// 1. When some tests are not run due to a test failure.
FinalRunStats::Failed(RunStatsFailureKind::Test {
initial_run_count,
not_run,
Expand Down Expand Up @@ -1674,16 +1688,6 @@ fn write_final_warnings(
)?;
}
}

// 2. When no tests are run at all.
FinalRunStats::NoTestsRun => {
writeln!(
writer,
"{}: no tests were run (this will exit with a \
non-zero code in the future)",
"warning".style(styles.skip)
)?;
}
_ => {}
}

Expand Down Expand Up @@ -2406,17 +2410,11 @@ mod tests {
);
assert_eq!(warnings, "warning: 1/1 test was not run due to interrupt\n");

// These warnings are taken care of by cargo-nextest.
let warnings = final_warnings_for(FinalRunStats::NoTestsRun, None);
assert_eq!(
warnings,
"warning: no tests were run (this will exit with a non-zero code in the future)\n"
);

assert_eq!(warnings, "");
let warnings = final_warnings_for(FinalRunStats::NoTestsRun, Some(CancelReason::Signal));
assert_eq!(
warnings,
"warning: no tests were run (this will exit with a non-zero code in the future)\n"
);
assert_eq!(warnings, "");

// No warnings for success.
let warnings = final_warnings_for(FinalRunStats::Success, None);
Expand Down

0 comments on commit d2fb356

Please sign in to comment.