Skip to content

Commit

Permalink
Auto merge of #108659 - ferrocene:pa-test-metrics, r=Mark-Simulacrum
Browse files Browse the repository at this point in the history
Include executed tests in the build metrics (and use a custom test display impl)

The main goal of this PR is to include all tests executed in CI inside the build metrics JSON files. I need this for Ferrocene, and `@Mark-Simulacrum` expressed desire to have this as well to ensure all tests are executed at least once somewhere in CI.

Unfortunately implementing this required rewriting inside of bootstrap all of the code to render the test output to console. libtest supports outputting JSON instead of raw text, which we can indeed use to populate the build metrics. Doing that suppresses the console output though, and compared to rustc and Cargo the console output is not included as a JSON field.

Because of that, this PR had to reimplement both the "pretty" format (one test per line, with `rust.verbose-tests = true`), and the "terse" format (the wall of dots, with `rust.verbose-tests = false`). The current implementation should have the exact same output as libtest, except for the benchmark output. libtest's benchmark output is broken in the "terse" format, so since that's our default I slightly improved how it's rendered.

Also, to bring parity with libtest I had to introduce support for coloring output from bootstrap, using the same dependencies `annotate-snippets` uses. It's now possible to use `builder.color_for_stdout(Color::Red, "text")` and `builder.color_for_stderr(Color::Green, "text")` across all of bootstrap, automatically respecting the `--color` flag and whether the stream is a terminal or not.

I recommend reviewing the PR commit-by-commit.
r? `@Mark-Simulacrum`
  • Loading branch information
bors committed Mar 21, 2023
2 parents a01b4cc + aacbd86 commit 6667682
Show file tree
Hide file tree
Showing 9 changed files with 504 additions and 38 deletions.
22 changes: 22 additions & 0 deletions src/bootstrap/Cargo.lock
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,17 @@ dependencies = [
"memchr",
]

[[package]]
name = "atty"
version = "0.2.14"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "d9b39be18770d11421cdb1b9947a45dd3f37e93092cbf377614828a319d5fee8"
dependencies = [
"hermit-abi",
"libc",
"winapi",
]

[[package]]
name = "autocfg"
version = "1.1.0"
Expand All @@ -36,6 +47,7 @@ dependencies = [
name = "bootstrap"
version = "0.0.0"
dependencies = [
"atty",
"build_helper",
"cc",
"cmake",
Expand All @@ -55,6 +67,7 @@ dependencies = [
"sha2",
"sysinfo",
"tar",
"termcolor",
"toml",
"walkdir",
"windows",
Expand Down Expand Up @@ -636,6 +649,15 @@ dependencies = [
"xattr",
]

[[package]]
name = "termcolor"
version = "1.2.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "be55cf8942feac5c765c2c993422806843c9a9a45d4d5c407ad6dd2ea95eb9b6"
dependencies = [
"winapi-util",
]

[[package]]
name = "thread_local"
version = "1.1.4"
Expand Down
2 changes: 2 additions & 0 deletions src/bootstrap/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ path = "bin/sccache-plus-cl.rs"
test = false

[dependencies]
atty = "0.2.14"
build_helper = { path = "../tools/build_helper" }
cmake = "0.1.38"
filetime = "0.2"
Expand All @@ -45,6 +46,7 @@ serde_derive = "1.0.137"
serde_json = "1.0.2"
sha2 = "0.10"
tar = "0.4"
termcolor = "1.2.0"
toml = "0.5"
ignore = "0.4.10"
opener = "0.5"
Expand Down
6 changes: 6 additions & 0 deletions src/bootstrap/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,9 @@ pub struct Config {
pub patch_binaries_for_nix: bool,
pub stage0_metadata: Stage0Metadata,

pub stdout_is_tty: bool,
pub stderr_is_tty: bool,

pub on_fail: Option<String>,
pub stage: u32,
pub keep_stage: Vec<u32>,
Expand Down Expand Up @@ -825,6 +828,9 @@ impl Config {
config.dist_include_mingw_linker = true;
config.dist_compression_profile = "fast".into();

config.stdout_is_tty = atty::is(atty::Stream::Stdout);
config.stderr_is_tty = atty::is(atty::Stream::Stderr);

// set by build.rs
config.build = TargetSelection::from_user(&env!("BUILD_TRIPLE"));

Expand Down
27 changes: 27 additions & 0 deletions src/bootstrap/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ mod format;
mod install;
mod metadata;
mod native;
mod render_tests;
mod run;
mod sanity;
mod setup;
Expand Down Expand Up @@ -88,6 +89,7 @@ pub use crate::builder::PathSet;
use crate::cache::{Interned, INTERNER};
pub use crate::config::Config;
pub use crate::flags::Subcommand;
use termcolor::{ColorChoice, StandardStream, WriteColor};

const LLVM_TOOLS: &[&str] = &[
"llvm-cov", // used to generate coverage report
Expand Down Expand Up @@ -1582,6 +1584,31 @@ to download LLVM rather than building it.

self.config.ninja_in_file
}

pub fn colored_stdout<R, F: FnOnce(&mut dyn WriteColor) -> R>(&self, f: F) -> R {
self.colored_stream_inner(StandardStream::stdout, self.config.stdout_is_tty, f)
}

pub fn colored_stderr<R, F: FnOnce(&mut dyn WriteColor) -> R>(&self, f: F) -> R {
self.colored_stream_inner(StandardStream::stderr, self.config.stderr_is_tty, f)
}

fn colored_stream_inner<R, F, C>(&self, constructor: C, is_tty: bool, f: F) -> R
where
C: Fn(ColorChoice) -> StandardStream,
F: FnOnce(&mut dyn WriteColor) -> R,
{
let choice = match self.config.color {
flags::Color::Always => ColorChoice::Always,
flags::Color::Never => ColorChoice::Never,
flags::Color::Auto if !is_tty => ColorChoice::Never,
flags::Color::Auto => ColorChoice::Auto,
};
let mut stream = constructor(choice);
let result = f(&mut stream);
stream.reset().unwrap();
result
}
}

#[cfg(unix)]
Expand Down
44 changes: 39 additions & 5 deletions src/bootstrap/metrics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ impl BuildMetrics {
duration_excluding_children_sec: Duration::ZERO,

children: Vec::new(),
tests: Vec::new(),
});
}

Expand All @@ -72,6 +73,16 @@ impl BuildMetrics {
}
}

pub(crate) fn record_test(&self, name: &str, outcome: TestOutcome) {
let mut state = self.state.borrow_mut();
state
.running_steps
.last_mut()
.unwrap()
.tests
.push(Test { name: name.to_string(), outcome });
}

fn collect_stats(&self, state: &mut MetricsState) {
let step = state.running_steps.last_mut().unwrap();

Expand Down Expand Up @@ -125,6 +136,14 @@ impl BuildMetrics {
}

fn prepare_json_step(&self, step: StepMetrics) -> JsonNode {
let mut children = Vec::new();
children.extend(step.children.into_iter().map(|child| self.prepare_json_step(child)));
children.extend(
step.tests
.into_iter()
.map(|test| JsonNode::Test { name: test.name, outcome: test.outcome }),
);

JsonNode::RustbuildStep {
type_: step.type_,
debug_repr: step.debug_repr,
Expand All @@ -135,11 +154,7 @@ impl BuildMetrics {
/ step.duration_excluding_children_sec.as_secs_f64(),
},

children: step
.children
.into_iter()
.map(|child| self.prepare_json_step(child))
.collect(),
children,
}
}
}
Expand All @@ -161,6 +176,12 @@ struct StepMetrics {
duration_excluding_children_sec: Duration,

children: Vec<StepMetrics>,
tests: Vec<Test>,
}

struct Test {
name: String,
outcome: TestOutcome,
}

#[derive(Serialize, Deserialize)]
Expand Down Expand Up @@ -190,6 +211,19 @@ enum JsonNode {

children: Vec<JsonNode>,
},
Test {
name: String,
#[serde(flatten)]
outcome: TestOutcome,
},
}

#[derive(Serialize, Deserialize)]
#[serde(tag = "outcome", rename_all = "snake_case")]
pub(crate) enum TestOutcome {
Passed,
Failed,
Ignored { ignore_reason: Option<String> },
}

#[derive(Serialize, Deserialize)]
Expand Down
Loading

0 comments on commit 6667682

Please sign in to comment.