Skip to content

Commit

Permalink
rustbuild: Add ./x.py test --no-fail-fast
Browse files Browse the repository at this point in the history
This option forwards to each `cargo test` invocation, and applies the
same logic across all test steps to keep going after failures.  At the
end, a brief summary line reports how many commands failed, if any.

Note that if a test program fails to even start at all, or if an
auxiliary build command related to testing fails, these are still left
to stop everything right away.

Fixes #40219.
  • Loading branch information
cuviper committed Jun 2, 2017
1 parent d7798c3 commit 617aea4
Show file tree
Hide file tree
Showing 5 changed files with 115 additions and 31 deletions.
61 changes: 46 additions & 15 deletions src/bootstrap/check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,28 @@ impl fmt::Display for TestKind {
}
}

fn try_run(build: &Build, cmd: &mut Command) {
if build.flags.cmd.no_fail_fast() {
if !build.try_run(cmd) {
let failures = build.delayed_failures.get();
build.delayed_failures.set(failures + 1);
}
} else {
build.run(cmd);
}
}

fn try_run_quiet(build: &Build, cmd: &mut Command) {
if build.flags.cmd.no_fail_fast() {
if !build.try_run_quiet(cmd) {
let failures = build.delayed_failures.get();
build.delayed_failures.set(failures + 1);
}
} else {
build.run_quiet(cmd);
}
}

/// Runs the `linkchecker` tool as compiled in `stage` by the `host` compiler.
///
/// This tool in `src/tools` will verify the validity of all our links in the
Expand All @@ -67,8 +89,8 @@ pub fn linkcheck(build: &Build, host: &str) {
let compiler = Compiler::new(0, host);

let _time = util::timeit();
build.run(build.tool_cmd(&compiler, "linkchecker")
.arg(build.out.join(host).join("doc")));
try_run(build, build.tool_cmd(&compiler, "linkchecker")
.arg(build.out.join(host).join("doc")));
}

/// Runs the `cargotest` tool as compiled in `stage` by the `host` compiler.
Expand All @@ -87,10 +109,10 @@ pub fn cargotest(build: &Build, stage: u32, host: &str) {
let _time = util::timeit();
let mut cmd = Command::new(build.tool(&Compiler::new(0, host), "cargotest"));
build.prepare_tool_cmd(compiler, &mut cmd);
build.run(cmd.arg(&build.cargo)
.arg(&out_dir)
.env("RUSTC", build.compiler_path(compiler))
.env("RUSTDOC", build.rustdoc(compiler)))
try_run(build, cmd.arg(&build.cargo)
.arg(&out_dir)
.env("RUSTC", build.compiler_path(compiler))
.env("RUSTDOC", build.rustdoc(compiler)));
}

/// Runs `cargo test` for `cargo` packaged with Rust.
Expand All @@ -107,6 +129,9 @@ pub fn cargo(build: &Build, stage: u32, host: &str) {

let mut cargo = build.cargo(compiler, Mode::Tool, host, "test");
cargo.arg("--manifest-path").arg(build.src.join("src/tools/cargo/Cargo.toml"));
if build.flags.cmd.no_fail_fast() {
cargo.arg("--no-fail-fast");
}

// Don't build tests dynamically, just a pain to work with
cargo.env("RUSTC_NO_PREFER_DYNAMIC", "1");
Expand All @@ -115,7 +140,7 @@ pub fn cargo(build: &Build, stage: u32, host: &str) {
// available.
cargo.env("CFG_DISABLE_CROSS_TESTS", "1");

build.run(cargo.env("PATH", newpath));
try_run(build, cargo.env("PATH", newpath));
}

/// Runs the `tidy` tool as compiled in `stage` by the `host` compiler.
Expand All @@ -135,7 +160,7 @@ pub fn tidy(build: &Build, host: &str) {
if build.config.quiet_tests {
cmd.arg("--quiet");
}
build.run(&mut cmd);
try_run(build, &mut cmd);
}

fn testdir(build: &Build, host: &str) -> PathBuf {
Expand Down Expand Up @@ -286,7 +311,7 @@ pub fn compiletest(build: &Build,
build.ci_env.force_coloring_in_ci(&mut cmd);

let _time = util::timeit();
build.run(&mut cmd);
try_run(build, &mut cmd);
}

/// Run `rustdoc --test` for all documentation in `src/doc`.
Expand Down Expand Up @@ -362,9 +387,9 @@ fn markdown_test(build: &Build, compiler: &Compiler, markdown: &Path) {
cmd.arg("--test-args").arg(test_args);

if build.config.quiet_tests {
build.run_quiet(&mut cmd);
try_run_quiet(build, &mut cmd);
} else {
build.run(&mut cmd);
try_run(build, &mut cmd);
}
}

Expand Down Expand Up @@ -419,6 +444,9 @@ pub fn krate(build: &Build,
cargo.arg("--manifest-path")
.arg(build.src.join(path).join("Cargo.toml"))
.arg("--features").arg(features);
if test_kind.subcommand() == "test" && build.flags.cmd.no_fail_fast() {
cargo.arg("--no-fail-fast");
}

match krate {
Some(krate) => {
Expand Down Expand Up @@ -478,7 +506,7 @@ pub fn krate(build: &Build,
krate_remote(build, &compiler, target, mode);
} else {
cargo.args(&build.flags.cmd.test_args());
build.run(&mut cargo);
try_run(build, &mut cargo);
}
}

Expand All @@ -499,7 +527,7 @@ fn krate_emscripten(build: &Build,
if build.config.quiet_tests {
cmd.arg("--quiet");
}
build.run(&mut cmd);
try_run(build, &mut cmd);
}
}

Expand All @@ -521,7 +549,7 @@ fn krate_remote(build: &Build,
cmd.arg("--quiet");
}
cmd.args(&build.flags.cmd.test_args());
build.run(&mut cmd);
try_run(build, &mut cmd);
}
}

Expand Down Expand Up @@ -637,6 +665,9 @@ pub fn bootstrap(build: &Build) {
.current_dir(build.src.join("src/bootstrap"))
.env("CARGO_TARGET_DIR", build.out.join("bootstrap"))
.env("RUSTC", &build.rustc);
if build.flags.cmd.no_fail_fast() {
cmd.arg("--no-fail-fast");
}
cmd.arg("--").args(&build.flags.cmd.test_args());
build.run(&mut cmd);
try_run(build, &mut cmd);
}
14 changes: 13 additions & 1 deletion src/bootstrap/flags.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ pub enum Subcommand {
Test {
paths: Vec<PathBuf>,
test_args: Vec<String>,
no_fail_fast: bool,
},
Bench {
paths: Vec<PathBuf>,
Expand Down Expand Up @@ -141,7 +142,10 @@ To learn more about a subcommand, run `./x.py <subcommand> -h`");

// Some subcommands get extra options
match subcommand.as_str() {
"test" => { opts.optmulti("", "test-args", "extra arguments", "ARGS"); },
"test" => {
opts.optflag("", "no-fail-fast", "Run all tests regardless of failure");
opts.optmulti("", "test-args", "extra arguments", "ARGS");
},
"bench" => { opts.optmulti("", "test-args", "extra arguments", "ARGS"); },
_ => { },
};
Expand Down Expand Up @@ -263,6 +267,7 @@ Arguments:
Subcommand::Test {
paths: paths,
test_args: matches.opt_strs("test-args"),
no_fail_fast: matches.opt_present("no-fail-fast"),
}
}
"bench" => {
Expand Down Expand Up @@ -342,6 +347,13 @@ impl Subcommand {
_ => Vec::new(),
}
}

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

fn split(s: Vec<String>) -> Vec<String> {
Expand Down
21 changes: 20 additions & 1 deletion src/bootstrap/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ extern crate toml;
#[cfg(unix)]
extern crate libc;

use std::cell::Cell;
use std::cmp;
use std::collections::HashMap;
use std::env;
Expand All @@ -88,7 +89,7 @@ use std::io::Read;
use std::path::{PathBuf, Path};
use std::process::Command;

use build_helper::{run_silent, run_suppressed, output, mtime};
use build_helper::{run_silent, run_suppressed, try_run_silent, try_run_suppressed, output, mtime};

use util::{exe, libdir, add_lib_path, OutputFolder, CiEnv};

Expand Down Expand Up @@ -180,6 +181,7 @@ pub struct Build {
is_sudo: bool,
src_is_git: bool,
ci_env: CiEnv,
delayed_failures: Cell<usize>,
}

#[derive(Debug)]
Expand Down Expand Up @@ -274,6 +276,7 @@ impl Build {
is_sudo: is_sudo,
src_is_git: src_is_git,
ci_env: CiEnv::current(),
delayed_failures: Cell::new(0),
}
}

Expand Down Expand Up @@ -784,6 +787,22 @@ impl Build {
run_suppressed(cmd)
}

/// Runs a command, printing out nice contextual information if it fails.
/// Exits if the command failed to execute at all, otherwise returns its
/// `status.success()`.
fn try_run(&self, cmd: &mut Command) -> bool {
self.verbose(&format!("running: {:?}", cmd));
try_run_silent(cmd)
}

/// Runs a command, printing out nice contextual information if it fails.
/// Exits if the command failed to execute at all, otherwise returns its
/// `status.success()`.
fn try_run_quiet(&self, cmd: &mut Command) -> bool {
self.verbose(&format!("running: {:?}", cmd));
try_run_suppressed(cmd)
}

/// Prints a message if this build is configured in verbose mode.
fn verbose(&self, msg: &str) {
if self.flags.verbose() || self.config.verbose() {
Expand Down
12 changes: 10 additions & 2 deletions src/bootstrap/step.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@

use std::collections::{BTreeMap, HashSet, HashMap};
use std::mem;
use std::process;

use check::{self, TestKind};
use compile;
Expand Down Expand Up @@ -1174,8 +1175,8 @@ invalid rule dependency graph detected, was a rule added and maybe typo'd?
let (kind, paths) = match self.build.flags.cmd {
Subcommand::Build { ref paths } => (Kind::Build, &paths[..]),
Subcommand::Doc { ref paths } => (Kind::Doc, &paths[..]),
Subcommand::Test { ref paths, test_args: _ } => (Kind::Test, &paths[..]),
Subcommand::Bench { ref paths, test_args: _ } => (Kind::Bench, &paths[..]),
Subcommand::Test { ref paths, .. } => (Kind::Test, &paths[..]),
Subcommand::Bench { ref paths, .. } => (Kind::Bench, &paths[..]),
Subcommand::Dist { ref paths } => (Kind::Dist, &paths[..]),
Subcommand::Install { ref paths } => (Kind::Install, &paths[..]),
Subcommand::Clean => panic!(),
Expand Down Expand Up @@ -1268,6 +1269,13 @@ invalid rule dependency graph detected, was a rule added and maybe typo'd?
self.build.verbose(&format!("executing step {:?}", step));
(self.rules[step.name].run)(step);
}

// Check for postponed failures from `test --no-fail-fast`.
let failures = self.build.delayed_failures.get();
if failures > 0 {
println!("\n{} command(s) did not execute successfully.\n", failures);
process::exit(1);
}
}

/// From the top level targets `steps` generate a topological ordering of
Expand Down
38 changes: 26 additions & 12 deletions src/build_helper/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,35 +42,49 @@ pub fn run(cmd: &mut Command) {
}

pub fn run_silent(cmd: &mut Command) {
if !try_run_silent(cmd) {
std::process::exit(1);
}
}

pub fn try_run_silent(cmd: &mut Command) -> bool {
let status = match cmd.status() {
Ok(status) => status,
Err(e) => fail(&format!("failed to execute command: {:?}\nerror: {}",
cmd, e)),
};
if !status.success() {
fail(&format!("command did not execute successfully: {:?}\n\
expected success, got: {}",
cmd,
status));
println!("\n\ncommand did not execute successfully: {:?}\n\
expected success, got: {}\n\n",
cmd,
status);
}
status.success()
}

pub fn run_suppressed(cmd: &mut Command) {
if !try_run_suppressed(cmd) {
std::process::exit(1);
}
}

pub fn try_run_suppressed(cmd: &mut Command) -> bool {
let output = match cmd.output() {
Ok(status) => status,
Err(e) => fail(&format!("failed to execute command: {:?}\nerror: {}",
cmd, e)),
};
if !output.status.success() {
fail(&format!("command did not execute successfully: {:?}\n\
expected success, got: {}\n\n\
stdout ----\n{}\n\
stderr ----\n{}\n",
cmd,
output.status,
String::from_utf8_lossy(&output.stdout),
String::from_utf8_lossy(&output.stderr)));
println!("\n\ncommand did not execute successfully: {:?}\n\
expected success, got: {}\n\n\
stdout ----\n{}\n\
stderr ----\n{}\n\n",
cmd,
output.status,
String::from_utf8_lossy(&output.stdout),
String::from_utf8_lossy(&output.stderr));
}
output.status.success()
}

pub fn gnu_target(target: &str) -> String {
Expand Down

0 comments on commit 617aea4

Please sign in to comment.