Record failed tests with --record, and rerun them with --rerun

src/bootstrap/src/core/build_steps/test/failed_tests.rs

@@ -0,0 +1,110 @@
+use std::collections::BTreeSet;
+use std::fs::{self, File};
+use std::io::{BufRead, BufReader, ErrorKind};
+use std::path::{Path, PathBuf};
+
+use crate::core::builder::{Builder, ShouldRun, Step};
+use crate::t;
+
+#[derive(Clone)]
+pub struct RecordFailedTests {
+    failed_tests_path: Option<PathBuf>,
+}
+
+impl RecordFailedTests {
+    pub fn path(&self) -> Option<&Path> {
+        self.failed_tests_path.as_deref()
+    }
+}
+
+/// This step is run as a dependency of most testing steps.
+/// Upon running, a file is created for failed tests to be recorded in if `--record` is passed on
+/// the command line.
+///
+/// This step is the only way to get access to a token type called [`RecordFailedTests`].
+/// Having this token type signifies the fact that a file was created to store failed tests in,
+/// and is required to create a `Renderer`, the type that renders the outputs of tests.
+///
+/// If `--rerun` isn't passed, or we're in dry-run mode, running this step is a no-op,
+/// and the `RecordFailedTest` type doesn't (need to) signify anything.
+#[derive(Clone, Copy, Eq, PartialEq, Hash, Debug)]
+pub struct SetupFailedTestsFile;
+impl Step for SetupFailedTestsFile {
+    type Output = RecordFailedTests;
+
+    fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> {
+        run.never()
+    }
+
+    fn run(self, builder: &Builder<'_>) -> Self::Output {
+        if !builder.config.cmd.record() || builder.config.dry_run() {
+            return RecordFailedTests { failed_tests_path: None };
+        }
+
+        let failed_tests_path = builder.config.record_failed_tests_path.clone();
+        println!(
+            "setting up tracking of failed tests in {} (`--record` was passed)",
+            failed_tests_path.display()
+        );
+        if failed_tests_path.exists() {
+            println!("deleting previously recorded failed tests");
+            t!(fs::remove_file(&failed_tests_path));
+        }
+        RecordFailedTests { failed_tests_path: Some(failed_tests_path) }
+    }
+}
+
+pub fn collect_previously_failed_tests(failed_tests_file_path: &PathBuf) -> Vec<PathBuf> {
+    let mut paths = BTreeSet::new();
+
+    println!(
+        "`--rerun` passed so looking for failed tests in {}",
+        failed_tests_file_path.display()
+    );
+
+    let lines: Vec<String> = match File::open(failed_tests_file_path) {
+        Ok(f) => t!(BufReader::new(f).lines().collect()),
+        Err(e) if e.kind() == ErrorKind::NotFound => {
+            println!(
+                "WARNING: failed tests file doesn't exist: `--rerun` only makes sense after a previous test run with `--record`"
+            );
+            return Vec::new();
+        }
+        Err(e) => t!(Err(e)),
+    };
+
+    const MAX_RERUN_PRINTS: usize = 10;
+
+    for line in lines {
+        let trimmed = line.as_str().trim();
+        let without_revision =
+            trimmed.rsplit_once("#").map(|(before, _)| before).unwrap_or(trimmed);
+        let without_suite_prefix = without_revision
+            .strip_prefix("[")
+            .and_then(|rest| rest.split_once("]"))
+            .map(|(_, after)| after.trim())
+            .unwrap_or(without_revision);
+
+        let failed_test_path = PathBuf::from(without_suite_prefix.to_string());
+        if paths.insert(failed_test_path.clone()) {
+            if paths.len() == 1 {
+                println!("rerunning previously failed tests:");
+            }
+            if paths.len() <= MAX_RERUN_PRINTS {
+                println!("    {}", failed_test_path.display());
+            }
+        }
+    }
+
+    if paths.len() > MAX_RERUN_PRINTS {
+        println!("    and {} more...", paths.len() - MAX_RERUN_PRINTS)
+    }
+
+    if paths.is_empty() {
+        println!(
+            "WARNING: failed tests file doesn't contain any failed tests: `--rerun` only makes sense after a previous test run with `--record`"
+        );
+    }
+
+    paths.into_iter().collect()
+}

src/bootstrap/src/core/config/config.rs

@@ -30,6 +30,7 @@ use tracing::{instrument, span};
 
 use crate::core::build_steps::llvm;
 use crate::core::build_steps::llvm::LLVM_INVALIDATION_PATHS;
+use crate::core::build_steps::test::failed_tests::collect_previously_failed_tests;
 pub use crate::core::config::flags::Subcommand;
 use crate::core::config::flags::{Color, Flags, Warnings};
 use crate::core::config::target_selection::TargetSelectionList;
@@ -125,6 +126,7 @@ pub struct Config {
     pub stage0_metadata: build_helper::stage0_parser::Stage0,
     pub android_ndk: Option<PathBuf>,
     pub optimized_compiler_builtins: CompilerBuiltins,
+    pub record_failed_tests_path: PathBuf,
 
     pub stdout_is_tty: bool,
     pub stderr_is_tty: bool,
@@ -507,6 +509,7 @@ impl Config {
             dist_stage: build_dist_stage,
             bench_stage: build_bench_stage,
             patch_binaries_for_nix: build_patch_binaries_for_nix,
+            record_failed_tests_path: build_record_failed_tests_path,
             // This field is only used by bootstrap.py
             metrics: _,
             android_ndk: build_android_ndk,
@@ -1305,6 +1308,19 @@ impl Config {
         );
         let verbose_tests = rust_verbose_tests.unwrap_or(exec_ctx.is_verbose());
 
+        let record_failed_tests_path =
+            out.join(build_record_failed_tests_path.unwrap_or_else(|| "failed-tests".to_string()));
+
+        let paths = {
+            let mut paths = Vec::new();
+            if flags_cmd.rerun() {
+                paths = collect_previously_failed_tests(&record_failed_tests_path);
+            } else {
+                paths.extend(flags_paths);
+            }
+            paths
+        };
+
         Config {
             // tidy-alphabetical-start
             android_ndk: build_android_ndk,
@@ -1435,13 +1451,14 @@ impl Config {
             out,
             patch_binaries_for_nix: build_patch_binaries_for_nix,
             path_modification_cache,
-            paths: flags_paths,
+            paths,
             prefix: install_prefix.map(PathBuf::from),
             print_step_rusage: build_print_step_rusage.unwrap_or(false),
             print_step_timings: build_print_step_timings.unwrap_or(false),
             profiler: build_profiler.unwrap_or(false),
             python: build_python.map(PathBuf::from),
             quiet: flags_quiet,
+            record_failed_tests_path,
             reproducible_artifacts: flags_reproducible_artifact,
             reuse: build_reuse.map(PathBuf::from),
             rust_analyzer_info,

src/bootstrap/src/core/config/flags.rs

@@ -440,6 +440,15 @@ pub enum Subcommand {
         #[arg(long)]
         #[doc(hidden)]
         no_doc: bool,
+
+        /// Record all the failed tests in a file in the build directory.
+        ///
+        /// On subsequent invocations, this set of tests can be rerun by passing `--rerun`
+        #[arg(long)]
+        record: bool,
+        /// Rerun tests that previously failed, and stored with `--record`.
+        #[arg(long)]
+        rerun: bool,
     },
     /// Build and run some test suites *in Miri*
     Miri {
@@ -723,6 +732,20 @@ impl Subcommand {
             _ => false,
         }
     }
+
+    pub fn record(&self) -> bool {
+        match self {
+            Subcommand::Test { record, .. } => *record,
+            _ => false,
+        }
+    }
+
+    pub fn rerun(&self) -> bool {
+        match self {
+            Subcommand::Test { rerun, .. } => *rerun,
+            _ => false,
+        }
+    }
 }
 
 /// Returns the shell completion for a given shell, if the result differs from the current

src/bootstrap/src/core/config/toml/build.rs

@@ -78,6 +78,7 @@ define_config! {
         tidy_extra_checks: Option<String> = "tidy-extra-checks",
         ccache: Option<StringOrBool> = "ccache",
         exclude: Option<Vec<PathBuf>> = "exclude",
+        record_failed_tests_path: Option<String> = "record_failed_tests_path",
     }
 }
 

src/bootstrap/src/utils/change_tracker.rs

@@ -621,6 +621,11 @@ pub const CONFIG_CHANGE_HISTORY: &[ChangeInfo] = &[
         severity: ChangeSeverity::Info,
         summary: "`x.py` stopped accepting partial argument names. Use full names to avoid errors.",
     },
+    ChangeInfo {
+        change_id: 154586,
+        severity: ChangeSeverity::Info,
+        summary: "New option `build.record_failed_tests_path` to store failed tests when passing `--record`. These can be rerun with `--rerun`.",
+    },
     ChangeInfo {
         change_id: 154587,
         severity: ChangeSeverity::Info,

src/bootstrap/src/utils/render_tests.rs

@@ -6,12 +6,14 @@
 //! and rustc) libtest doesn't include the rendered human-readable output as a JSON field. We had
 //! to reimplement all the rendering logic in this module because of that.
 
+use std::fs::File;
 use std::io::{BufRead, BufReader, Read, Write};
 use std::process::ChildStdout;
 use std::time::Duration;
 
 use termcolor::{Color, ColorSpec, WriteColor};
 
+use crate::core::build_steps::test::failed_tests::RecordFailedTests;
 use crate::core::builder::Builder;
 use crate::utils::exec::BootstrapCommand;
 
@@ -20,21 +22,23 @@ const TERSE_TESTS_PER_LINE: usize = 88;
 pub(crate) fn add_flags_and_try_run_tests(
     builder: &Builder<'_>,
     cmd: &mut BootstrapCommand,
+    record_failed_tests: RecordFailedTests,
 ) -> bool {
     if !cmd.get_args().any(|arg| arg == "--") {
         cmd.arg("--");
     }
     cmd.args(["-Z", "unstable-options", "--format", "json"]);
 
-    try_run_tests(builder, cmd, false)
+    try_run_tests(builder, cmd, false, record_failed_tests)
 }
 
 pub(crate) fn try_run_tests(
     builder: &Builder<'_>,
     cmd: &mut BootstrapCommand,
     stream: bool,
+    record_failed_tests: RecordFailedTests,
 ) -> bool {
-    if run_tests(builder, cmd, stream) {
+    if run_tests(builder, cmd, stream, record_failed_tests) {
         return true;
     }
 
@@ -47,7 +51,12 @@ pub(crate) fn try_run_tests(
     false
 }
 
-fn run_tests(builder: &Builder<'_>, cmd: &mut BootstrapCommand, stream: bool) -> bool {
+fn run_tests(
+    builder: &Builder<'_>,
+    cmd: &mut BootstrapCommand,
+    stream: bool,
+    record_failed_tests: RecordFailedTests,
+) -> bool {
     builder.do_if_verbose(|| println!("running: {cmd:?}"));
 
     let Some(mut streaming_command) = cmd.stream_capture_stdout(&builder.config.exec_ctx) else {
@@ -56,7 +65,8 @@ fn run_tests(builder: &Builder<'_>, cmd: &mut BootstrapCommand, stream: bool) ->
 
     // This runs until the stdout of the child is closed, which means the child exited. We don't
     // run this on another thread since the builder is not Sync.
-    let renderer = Renderer::new(streaming_command.stdout.take().unwrap(), builder);
+    let renderer =
+        Renderer::new(streaming_command.stdout.take().unwrap(), builder, record_failed_tests);
     if stream {
         renderer.stream_all();
     } else {
@@ -87,10 +97,30 @@ struct Renderer<'a> {
     ignored_tests: usize,
     terse_tests_in_line: usize,
     ci_latest_logged_percentage: f64,
+
+    failed_tests: Option<File>,
 }
 
 impl<'a> Renderer<'a> {
-    fn new(stdout: ChildStdout, builder: &'a Builder<'a>) -> Self {
+    fn new(
+        stdout: ChildStdout,
+        builder: &'a Builder<'a>,
+        record_failed_tests: RecordFailedTests,
+    ) -> Self {
+        let failed_tests = record_failed_tests.path().and_then(|path| {
+            // create the file (overwriting any previous) to get ready to record new failed tests
+            match File::options().create(true).append(true).truncate(false).open(path) {
+                Ok(f) => Some(f),
+                Err(e) => {
+                    println!(
+                        "Couldn't open file {} to write test failutes to: {e}. (attempted because `--record` was passed). Test failures will not be recorded.",
+                        path.display()
+                    );
+                    None
+                }
+            }
+        });
+
         Self {
             stdout: BufReader::new(stdout),
             benches: Vec::new(),
@@ -102,6 +132,7 @@ impl<'a> Renderer<'a> {
             ignored_tests: 0,
             terse_tests_in_line: 0,
             ci_latest_logged_percentage: 0.0,
+            failed_tests,
         }
     }
 
@@ -268,6 +299,13 @@ impl<'a> Renderer<'a> {
             for failure in &self.failures {
                 println!("    {}", failure.name);
             }
+
+            if self.failed_tests.is_some() {
+                println!(
+                    "This list of test failures was recorded.\nUse `x test --rerun` to retry just these {} failed tests.",
+                    self.failures.len(),
+                )
+            }
         }
 
         if !self.benches.is_empty() {
@@ -360,6 +398,13 @@ impl<'a> Renderer<'a> {
             }
             Message::Test(TestMessage::Failed(outcome)) => {
                 self.render_test_outcome(Outcome::Failed, &outcome);
+                if let Some(failed_tests) = &mut self.failed_tests
+                    && let Err(e) = writeln!(failed_tests, "{}", outcome.name)
+                {
+                    eprintln!(
+                        "failed to write test failure to file: {e} (attempted because `--record` was passed)"
+                    );
+                }
                 self.failures.push(outcome);
             }
             Message::Test(TestMessage::Timeout { name }) => {

src/etc/completions/x.fish

@@ -465,6 +465,8 @@ complete -c x -n "__fish_x_using_subcommand test" -l rustfix-coverage -d 'enable
 complete -c x -n "__fish_x_using_subcommand test" -l no-capture -d 'don\'t capture stdout/stderr of tests'
 complete -c x -n "__fish_x_using_subcommand test" -l bypass-ignore-backends -d 'Ignore `//@ ignore-backends` directives'
 complete -c x -n "__fish_x_using_subcommand test" -l no-doc -d 'Deprecated. Use `--all-targets` or `--tests` instead'
+complete -c x -n "__fish_x_using_subcommand test" -l record -d 'Record all the failed tests in a file in the build directory'
+complete -c x -n "__fish_x_using_subcommand test" -l rerun -d 'Rerun tests that previously failed, and stored with `--record`'
 complete -c x -n "__fish_x_using_subcommand test" -s v -l verbose -d 'use verbose output (-vv for very verbose)'
 complete -c x -n "__fish_x_using_subcommand test" -s q -l quiet -d 'use quiet output'
 complete -c x -n "__fish_x_using_subcommand test" -s i -l incremental -d 'use incremental compilation'
@@ -520,6 +522,8 @@ complete -c x -n "__fish_x_using_subcommand t" -l rustfix-coverage -d 'enable th
 complete -c x -n "__fish_x_using_subcommand t" -l no-capture -d 'don\'t capture stdout/stderr of tests'
 complete -c x -n "__fish_x_using_subcommand t" -l bypass-ignore-backends -d 'Ignore `//@ ignore-backends` directives'
 complete -c x -n "__fish_x_using_subcommand t" -l no-doc -d 'Deprecated. Use `--all-targets` or `--tests` instead'
+complete -c x -n "__fish_x_using_subcommand t" -l record -d 'Record all the failed tests in a file in the build directory'
+complete -c x -n "__fish_x_using_subcommand t" -l rerun -d 'Rerun tests that previously failed, and stored with `--record`'
 complete -c x -n "__fish_x_using_subcommand t" -s v -l verbose -d 'use verbose output (-vv for very verbose)'
 complete -c x -n "__fish_x_using_subcommand t" -s q -l quiet -d 'use quiet output'
 complete -c x -n "__fish_x_using_subcommand t" -s i -l incremental -d 'use incremental compilation'

src/etc/completions/x.ps1

@@ -543,6 +543,8 @@ Register-ArgumentCompleter -Native -CommandName 'x' -ScriptBlock {
             [CompletionResult]::new('--no-capture', '--no-capture', [CompletionResultType]::ParameterName, 'don''t capture stdout/stderr of tests')
             [CompletionResult]::new('--bypass-ignore-backends', '--bypass-ignore-backends', [CompletionResultType]::ParameterName, 'Ignore `//@ ignore-backends` directives')
             [CompletionResult]::new('--no-doc', '--no-doc', [CompletionResultType]::ParameterName, 'Deprecated. Use `--all-targets` or `--tests` instead')
+            [CompletionResult]::new('--record', '--record', [CompletionResultType]::ParameterName, 'Record all the failed tests in a file in the build directory')
+            [CompletionResult]::new('--rerun', '--rerun', [CompletionResultType]::ParameterName, 'Rerun tests that previously failed, and stored with `--record`')
             [CompletionResult]::new('-v', '-v', [CompletionResultType]::ParameterName, 'use verbose output (-vv for very verbose)')
             [CompletionResult]::new('--verbose', '--verbose', [CompletionResultType]::ParameterName, 'use verbose output (-vv for very verbose)')
             [CompletionResult]::new('-q', '-q', [CompletionResultType]::ParameterName, 'use quiet output')
@@ -606,6 +608,8 @@ Register-ArgumentCompleter -Native -CommandName 'x' -ScriptBlock {
             [CompletionResult]::new('--no-capture', '--no-capture', [CompletionResultType]::ParameterName, 'don''t capture stdout/stderr of tests')
             [CompletionResult]::new('--bypass-ignore-backends', '--bypass-ignore-backends', [CompletionResultType]::ParameterName, 'Ignore `//@ ignore-backends` directives')
             [CompletionResult]::new('--no-doc', '--no-doc', [CompletionResultType]::ParameterName, 'Deprecated. Use `--all-targets` or `--tests` instead')
+            [CompletionResult]::new('--record', '--record', [CompletionResultType]::ParameterName, 'Record all the failed tests in a file in the build directory')
+            [CompletionResult]::new('--rerun', '--rerun', [CompletionResultType]::ParameterName, 'Rerun tests that previously failed, and stored with `--record`')
             [CompletionResult]::new('-v', '-v', [CompletionResultType]::ParameterName, 'use verbose output (-vv for very verbose)')
             [CompletionResult]::new('--verbose', '--verbose', [CompletionResultType]::ParameterName, 'use verbose output (-vv for very verbose)')
             [CompletionResult]::new('-q', '-q', [CompletionResultType]::ParameterName, 'use quiet output')