Skip to content

Adds a --failed flag to x test #114775

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 10 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 4 additions & 6 deletions src/bootstrap/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ use std::io::{BufRead, BufReader};
use std::ops::Deref;
use std::path::{Path, PathBuf};
use std::process::Command;
use std::sync::OnceLock;
use std::time::{Duration, Instant};

use crate::cache::{Cache, Interned, INTERNER};
Expand All @@ -29,12 +30,9 @@ use crate::{clean, dist};
use crate::{Build, CLang, DocTests, GitRepo, Mode};

pub use crate::Compiler;
// FIXME:
// - use std::lazy for `Lazy`
// - use std::cell for `OnceCell`
// Once they get stabilized and reach beta.
use clap::ValueEnum;
use once_cell::sync::{Lazy, OnceCell};
// FIXME: use std::lazy for `Lazy` once stabilised and in beta.
use once_cell::sync::Lazy;

pub struct Builder<'a> {
pub build: &'a Build,
Expand Down Expand Up @@ -494,7 +492,7 @@ impl<'a> ShouldRun<'a> {
///
/// [`path`]: ShouldRun::path
pub fn paths(mut self, paths: &[&str]) -> Self {
static SUBMODULES_PATHS: OnceCell<Vec<String>> = OnceCell::new();
static SUBMODULES_PATHS: OnceLock<Vec<String>> = OnceLock::new();

let init_submodules_paths = |src: &PathBuf| {
let file = File::open(src.join(".gitmodules")).unwrap();
Expand Down
2 changes: 2 additions & 0 deletions src/bootstrap/builder/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -576,6 +576,7 @@ mod dist {
config.cmd = Subcommand::Test {
test_args: vec![],
rustc_args: vec![],
failed: false,
no_fail_fast: false,
no_doc: true,
doc: false,
Expand Down Expand Up @@ -649,6 +650,7 @@ mod dist {
test_args: vec![],
rustc_args: vec![],
no_fail_fast: false,
failed: false,
doc: true,
no_doc: false,
skip: vec![],
Expand Down
4 changes: 2 additions & 2 deletions src/bootstrap/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ use std::io::IsTerminal;
use std::path::{Path, PathBuf};
use std::process::Command;
use std::str::FromStr;
use std::sync::OnceLock;

use crate::cache::{Interned, INTERNER};
use crate::cc_detect::{ndk_compiler, Language};
Expand All @@ -25,7 +26,6 @@ pub use crate::flags::Subcommand;
use crate::flags::{Color, Flags, Warnings};
use crate::util::{exe, output, t};
use build_helper::exit;
use once_cell::sync::OnceCell;
use semver::Version;
use serde::{Deserialize, Deserializer};
use serde_derive::Deserialize;
Expand Down Expand Up @@ -1913,7 +1913,7 @@ impl Config {
}

pub(crate) fn download_rustc_commit(&self) -> Option<&str> {
static DOWNLOAD_RUSTC: OnceCell<Option<String>> = OnceCell::new();
static DOWNLOAD_RUSTC: OnceLock<Option<String>> = OnceLock::new();
if self.dry_run() && DOWNLOAD_RUSTC.get().is_none() {
// avoid trying to actually download the commit
return self.download_rustc_commit.as_deref();
Expand Down
6 changes: 3 additions & 3 deletions src/bootstrap/download.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,10 @@ use std::{
io::{BufRead, BufReader, BufWriter, ErrorKind, Write},
path::{Path, PathBuf},
process::{Command, Stdio},
sync::OnceLock,
};

use build_helper::ci::CiEnv;
use once_cell::sync::OnceCell;
use xz2::bufread::XzDecoder;

use crate::{
Expand All @@ -19,7 +19,7 @@ use crate::{
Config,
};

static SHOULD_FIX_BINS_AND_DYLIBS: OnceCell<bool> = OnceCell::new();
static SHOULD_FIX_BINS_AND_DYLIBS: OnceLock<bool> = OnceLock::new();

/// `Config::try_run` wrapper for this module to avoid warnings on `try_run`, since we don't have access to a `builder` yet.
fn try_run(config: &Config, cmd: &mut Command) -> Result<(), ()> {
Expand Down Expand Up @@ -125,7 +125,7 @@ impl Config {
println!("attempting to patch {}", fname.display());

// Only build `.nix-deps` once.
static NIX_DEPS_DIR: OnceCell<PathBuf> = OnceCell::new();
static NIX_DEPS_DIR: OnceLock<PathBuf> = OnceLock::new();
let mut nix_build_succeeded = true;
let nix_deps_dir = NIX_DEPS_DIR.get_or_init(|| {
// Run `nix-build` to "build" each dependency (which will likely reuse
Expand Down
75 changes: 75 additions & 0 deletions src/bootstrap/failed.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
use std::{
collections::HashSet,
fs::File,
io::Read,
path::{Path, PathBuf},
process::Command,
};

use clap::Parser;

use crate::builder::Builder;
/// Runs all the tests that failed on their last run
pub fn failed(builder: &Builder<'_>) {
match get_failed_tests() {
Some(paths) if !paths.is_empty() => {
let mut build: crate::Build = builder.build.clone();
build.config.paths = paths;
build.config.cmd =
crate::flags::Flags::parse_from(["x.py", "test", "--force-rerun"]).cmd;

build.build();
}
Some(_) => {
println!("No tests failed on the previous iteration!")
}
None => {
eprintln!("Please run `x.py test` atleast once before running this command.");
return;
}
}
}

/// Returns a list of paths of tests that failed the last time that they were run
fn get_failed_tests() -> Option<Vec<PathBuf>> {
let contents = {
let path = get_tracker_file_path();
let mut f = File::open(path).ok()?;

let mut buf = String::new();
f.read_to_string(&mut buf)
.expect(&format!("failed to read tracker file, a bug report would be appreciated!"));

buf
};

let failed_tests =
serde_json::from_str::<HashSet<&str>>(&contents).expect("failed to deserialise tracker");

Some(
failed_tests
.iter()
.map(|test| {
test.split(" ")
.filter(|x| {
let mut chars = x.chars();
!matches!(chars.nth(0).unwrap(), '[' if matches!(chars.last().unwrap(), ']'))
})
.collect::<PathBuf>()
})
.collect::<Vec<_>>(),
)
}

pub fn get_tracker_file_path() -> PathBuf {
let toml = Command::new("cargo")
.args(["locate-project", "--message-format=plain"])
.output()
.expect("failed to locate project root")
.stdout;

let root =
Path::new(std::str::from_utf8(&toml).unwrap().trim()).parent().unwrap().to_path_buf();

root.join("src/bootstrap/test.tracker")
}
10 changes: 10 additions & 0 deletions src/bootstrap/flags.rs
Original file line number Diff line number Diff line change
Expand Up @@ -333,6 +333,9 @@ pub enum Subcommand {
/// do not run doc tests
no_doc: bool,
#[arg(long)]
/// rerun failed tests
failed: bool,
#[arg(long)]
/// only run doc tests
doc: bool,
#[arg(long)]
Expand Down Expand Up @@ -458,6 +461,13 @@ impl Subcommand {
}
}

pub fn failed(&self) -> bool {
match *self {
Subcommand::Test { failed, .. } => !failed,
_ => false,
}
}

pub fn doc_tests(&self) -> DocTests {
match *self {
Subcommand::Test { doc, no_doc, .. } => {
Expand Down
12 changes: 9 additions & 3 deletions src/bootstrap/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,13 +25,13 @@ use std::io;
use std::path::{Path, PathBuf};
use std::process::{Command, Stdio};
use std::str;
use std::sync::OnceLock;

use build_helper::ci::{gha, CiEnv};
use build_helper::exit;
use channel::GitInfo;
use config::{DryRun, Target};
use filetime::FileTime;
use once_cell::sync::OnceCell;

use crate::builder::Kind;
use crate::config::{LlvmLibunwind, TargetSelection};
Expand All @@ -50,6 +50,7 @@ mod config;
mod dist;
mod doc;
mod download;
mod failed;
mod flags;
mod format;
mod install;
Expand Down Expand Up @@ -673,11 +674,16 @@ impl Build {
// Download rustfmt early so that it can be used in rust-analyzer configs.
let _ = &builder::Builder::new(&self).initial_rustfmt();

// hardcoded subcommands
// hardcoded subcommands and flags
match &self.config.cmd {
Subcommand::Format { check } => {
return format::format(&builder::Builder::new(&self), *check, &self.config.paths);
}
Subcommand::Test { failed, .. } => {
if *failed {
return failed::failed(&builder::Builder::new(&self));
}
}
Subcommand::Suggest { run } => {
return suggest::suggest(&builder::Builder::new(&self), *run);
}
Expand Down Expand Up @@ -945,7 +951,7 @@ impl Build {

/// Returns the sysroot of the snapshot compiler.
fn rustc_snapshot_sysroot(&self) -> &Path {
static SYSROOT_CACHE: OnceCell<PathBuf> = once_cell::sync::OnceCell::new();
static SYSROOT_CACHE: OnceLock<PathBuf> = OnceLock::new();
SYSROOT_CACHE.get_or_init(|| {
let mut rustc = Command::new(&self.initial_rustc);
rustc.args(&["--print", "sysroot"]);
Expand Down
57 changes: 51 additions & 6 deletions src/bootstrap/render_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,14 @@
//! about the executed tests. Doing so suppresses the human-readable output, and (compared to Cargo
//! 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 crate::builder::Builder;
use std::collections::HashSet;
use std::fs::{File, OpenOptions};
use std::io::{BufRead, BufReader, Read, Write};
use std::process::{ChildStdout, Command, Stdio};
use std::time::Duration;

use crate::{builder::Builder, failed::get_tracker_file_path};

use termcolor::{Color, ColorSpec, WriteColor};

const TERSE_TESTS_PER_LINE: usize = 88;
Expand Down Expand Up @@ -77,19 +80,23 @@ struct Renderer<'a> {
tests_count: Option<usize>,
executed_tests: usize,
terse_tests_in_line: usize,
tracker: HashSet<String>,
}

impl<'a> Renderer<'a> {
fn new(stdout: ChildStdout, builder: &'a Builder<'a>) -> Self {
Self {
let mut renderer = Self {
stdout: BufReader::new(stdout),
benches: Vec::new(),
failures: Vec::new(),
builder,
tests_count: None,
executed_tests: 0,
terse_tests_in_line: 0,
}
tracker: HashSet::with_capacity(256),
};
renderer.init_tracking();
renderer
}

fn render_all(mut self) {
Expand Down Expand Up @@ -182,7 +189,7 @@ impl<'a> Renderer<'a> {
let _ = std::io::stdout().flush();
}

fn render_suite_outcome(&self, outcome: Outcome<'_>, suite: &SuiteOutcome) {
fn render_suite_outcome(&mut self, outcome: Outcome<'_>, suite: &SuiteOutcome) {
// The terse output doesn't end with a newline, so we need to add it ourselves.
if !self.builder.config.verbose_tests {
println!();
Expand All @@ -204,7 +211,7 @@ impl<'a> Renderer<'a> {

println!("\nfailures:");
for failure in &self.failures {
println!(" {}", failure.name);
println!(" {}", &failure.name);
}
}

Expand Down Expand Up @@ -240,6 +247,7 @@ impl<'a> Renderer<'a> {
suite.filtered_out,
Duration::from_secs_f64(suite.exec_time)
);
self.end_tracking();
}

fn render_message(&mut self, message: Message) {
Expand Down Expand Up @@ -273,6 +281,7 @@ impl<'a> Renderer<'a> {
}
Message::Test(TestMessage::Ok(outcome)) => {
self.render_test_outcome(Outcome::Ok, &outcome);
self.remove_failure(&outcome.name);
}
Message::Test(TestMessage::Ignored(outcome)) => {
self.render_test_outcome(
Expand All @@ -282,6 +291,7 @@ impl<'a> Renderer<'a> {
}
Message::Test(TestMessage::Failed(outcome)) => {
self.render_test_outcome(Outcome::Failed, &outcome);
self.track_failure(&outcome.name);
self.failures.push(outcome);
}
Message::Test(TestMessage::Timeout { name }) => {
Expand All @@ -290,6 +300,41 @@ impl<'a> Renderer<'a> {
Message::Test(TestMessage::Started) => {} // Not useful
}
}

/// Initialises the tracking by reading the last state from a file and storing it
fn init_tracking(&mut self) {
let path = get_tracker_file_path();

if !path.exists() {
let mut file = File::create(&path).expect("failed to create test.tracker");
file.write_all(b"[]").unwrap();
} else {
let mut file = File::open(path).expect("failed to open test.tracker");
let mut contents = String::new();
file.read_to_string(&mut contents).unwrap();
self.tracker = serde_json::from_str(&contents).unwrap();
}
}

/// Add a test that failed to the list
fn track_failure(&mut self, name: &str) {
self.tracker.insert(String::from(name));
}

/// Remove a test that no longer fails from the list
fn remove_failure(&mut self, name: &str) {
if self.tracker.contains(name) {
self.tracker.remove(name);
}
}

/// Store the state of the tracker into file for retrieval on next run
fn end_tracking(&mut self) {
let mut file =
OpenOptions::new().write(true).truncate(true).open(get_tracker_file_path()).unwrap();
file.write_all(serde_json::to_string(&self.tracker).unwrap().as_bytes())
.expect("failed to write to tracking file");
}
}

enum Outcome<'a> {
Expand Down
4 changes: 1 addition & 3 deletions src/bootstrap/suggest.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
#![cfg_attr(feature = "build-metrics", allow(unused))]

use std::str::FromStr;

use std::path::PathBuf;
use std::{path::PathBuf, str::FromStr};

use clap::Parser;

Expand Down
Loading