[cargo-nextest] add a new --no-tests option

Default to `warn` currently, but can also be `fail` or `pass`. The plan is to
default to `fail` in the future (see #1646).

Author: sunshowers

cargo-nextest/src/dispatch.rs

@@ -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::{
@@ -406,8 +407,7 @@ impl NtrOpts {
             &self.run_opts.reporter_opts,
             cli_args,
             output_writer,
-        )?;
-        Ok(0)
+        )
     }
 }
 
@@ -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 there are no tests to 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 {
+    /// Silently 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 {
@@ -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)?;
 
@@ -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);
             }
         };
 
@@ -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),
             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),
+                    Some(NoTestsBehavior::Warn) => {
+                        warn!("no tests to run");
+                        Ok(0)
+                    }
+                    Some(NoTestsBehavior::Fail) => {
+                        Err(ExpectedError::NoTestsRun { is_default: false })
+                    }
+                    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 to run -- this will become an error in the future\n\
+                             (hint: use `--no-tests` to customize)"
+                        );
+                        Ok(0)
+                    }
+                }
             }
             FinalRunStats::Canceled(RunStatsFailureKind::SetupScript)
             | FinalRunStats::Failed(RunStatsFailureKind::SetupScript) => {

cargo-nextest/src/errors.rs

@@ -184,6 +184,12 @@ pub enum ExpectedError {
     SetupScriptFailed,
     #[error("test run failed")]
     TestRunFailed,
+    #[error("no tests to 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 {
@@ -406,6 +412,7 @@ impl ExpectedError {
             }
             Self::SetupScriptFailed => NextestExitCode::SETUP_SCRIPT_FAILED,
             Self::TestRunFailed => NextestExitCode::TEST_RUN_FAILED,
+            Self::NoTestsRun { .. } => NextestExitCode::NO_TESTS_RUN,
             Self::ArchiveCreateError { .. } => NextestExitCode::ARCHIVE_CREATION_FAILED,
             Self::WriteTestListError { .. } | Self::WriteEventError { .. } => {
                 NextestExitCode::WRITE_OUTPUT_ERROR
@@ -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)"
+                } else {
+                    ""
+                };
+                log::error!("no tests to run{hint_str}");
+                None
+            }
             Self::ShowTestGroupsError { err } => {
                 log::error!("{err}");
                 err.source()

integration-tests/tests/integration/main.rs

@@ -228,6 +228,87 @@ fn test_list_target_after_build() {
     check_list_full_output(&output.stdout, Some(BuildPlatform::Target));
 }
 
+#[test]
+fn test_run_no_tests() {
+    set_env_vars();
+
+    let p = TempProject::new().unwrap();
+    build_tests(&p);
+
+    let output = CargoNextestCli::new()
+        .args([
+            "--manifest-path",
+            p.manifest_path().as_str(),
+            "run",
+            "-E",
+            "none()",
+        ])
+        .output();
+
+    let stderr = output.stderr_as_str();
+    assert!(
+        stderr.contains("warning: no tests to run -- this will become an error in the future"),
+        "stderr contains no tests message"
+    );
+
+    let output = CargoNextestCli::new()
+        .args([
+            "--manifest-path",
+            p.manifest_path().as_str(),
+            "run",
+            "-E",
+            "none()",
+            "--no-tests=warn",
+        ])
+        .output();
+
+    let stderr = output.stderr_as_str();
+    assert!(
+        stderr.contains("warning: no tests to run"),
+        "stderr contains no tests message"
+    );
+
+    let output = CargoNextestCli::new()
+        .args([
+            "--manifest-path",
+            p.manifest_path().as_str(),
+            "run",
+            "-E",
+            "none()",
+            "--no-tests=fail",
+        ])
+        .unchecked(true)
+        .output();
+    assert_eq!(
+        output.exit_status.code(),
+        Some(NextestExitCode::NO_TESTS_RUN),
+        "correct exit code for command\n{output}"
+    );
+
+    let stderr = output.stderr_as_str();
+    assert!(
+        stderr.contains("error: no tests to run"),
+        "stderr contains no tests message: {output}"
+    );
+
+    let output = CargoNextestCli::new()
+        .args([
+            "--manifest-path",
+            p.manifest_path().as_str(),
+            "run",
+            "-E",
+            "none()",
+            "--no-tests=pass",
+        ])
+        .output();
+
+    let stderr = output.stderr_as_str();
+    assert!(
+        !stderr.contains("no tests to run"),
+        "no tests message does not error out"
+    );
+}
+
 #[test]
 fn test_run() {
     set_env_vars();

nextest-metadata/src/exit_codes.rs

@@ -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 selected to 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;
 

nextest-runner/src/reporter.rs

@@ -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();
+            }
+            ReporterStderrImpl::TerminalWithoutBar | ReporterStderrImpl::Buffer(_) => {}
+        }
+    }
+}
+
 /// Functionality to report test results to stderr, JUnit, and/or structured,
 /// machine-readable results to stdout
 pub struct TestReporter<'a> {
@@ -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
     // ---
@@ -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)?;
             }
         }
@@ -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,
@@ -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)
-            )?;
-        }
         _ => {}
     }
 
@@ -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);

nextest-runner/src/runner.rs

@@ -1649,7 +1649,7 @@ pub enum FinalRunStats {
     /// The test run was successful, or is successful so far.
     Success,
 
-    /// The test run was successful, or is successful so far, but no tests were run.
+    /// The test run was successful, or is successful so far, but no tests were selected to run.
     NoTestsRun,
 
     /// The test run was canceled.