Skip to content

bootstrap: Consolidate coverage test suite steps into a single step #135097

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Jan 7, 2025
Merged

bootstrap: Consolidate coverage test suite steps into a single step #135097

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
186 changes: 68 additions & 118 deletions src/bootstrap/src/core/build_steps/test.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ use crate::utils::helpers::{
linker_args, linker_flags, t, target_supports_cranelift_backend, up_to_date,
};
use crate::utils::render_tests::{add_flags_and_try_run_tests, try_run_tests};
use crate::{CLang, DocTests, GitRepo, Mode, envify};
use crate::{CLang, DocTests, GitRepo, Mode, PathSet, envify};

const ADB_TEST_DIR: &str = "/data/local/tmp/work";

Expand Down Expand Up @@ -1185,53 +1185,6 @@ macro_rules! test {
};
}

/// Declares an alias for running the [`Coverage`] tests in only one mode.
/// Adapted from [`test`].
macro_rules! coverage_test_alias {
(
$( #[$attr:meta] )* // allow docstrings and attributes
$name:ident {
alias_and_mode: $alias_and_mode:expr, // &'static str
default: $default:expr, // bool
only_hosts: $only_hosts:expr // bool
$( , )? // optional trailing comma
}
) => {
$( #[$attr] )*
#[derive(Debug, Clone, PartialEq, Eq, Hash)]
pub struct $name {
pub compiler: Compiler,
pub target: TargetSelection,
}

impl $name {
const MODE: &'static str = $alias_and_mode;
}

impl Step for $name {
type Output = ();
const DEFAULT: bool = $default;
const ONLY_HOSTS: bool = $only_hosts;

fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> {
// Register the mode name as a command-line alias.
// This allows `x test coverage-map` and `x test coverage-run`.
run.alias($alias_and_mode)
}

fn make_run(run: RunConfig<'_>) {
let compiler = run.builder.compiler(run.builder.top_stage, run.build_triple());

run.builder.ensure($name { compiler, target: run.target });
}

fn run(self, builder: &Builder<'_>) {
Coverage::run_coverage_tests(builder, self.compiler, self.target, Self::MODE);
}
}
};
}

#[derive(Debug, Copy, Clone, Hash, PartialEq, Eq, Ord, PartialOrd)]
pub struct RunMakeSupport {
pub compiler: Compiler,
Expand Down Expand Up @@ -1473,99 +1426,96 @@ impl Step for RunMake {

test!(Assembly { path: "tests/assembly", mode: "assembly", suite: "assembly", default: true });

/// Coverage tests are a bit more complicated than other test suites, because
/// we want to run the same set of test files in multiple different modes,
/// in a way that's convenient and flexible when invoked manually.
///
/// This combined step runs the specified tests (or all of `tests/coverage`)
/// in both "coverage-map" and "coverage-run" modes.
///
/// Used by:
/// - `x test coverage`
/// - `x test tests/coverage`
/// - `x test tests/coverage/trivial.rs` (etc)
///
/// (Each individual mode also has its own step that will run the tests in
/// just that mode.)
#[derive(Debug, Clone, PartialEq, Eq, Hash)]
/// Runs the coverage test suite at `tests/coverage` in some or all of the
/// coverage test modes.
#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)]
pub struct Coverage {
pub compiler: Compiler,
pub target: TargetSelection,
pub mode: &'static str,
}

impl Coverage {
const PATH: &'static str = "tests/coverage";
const SUITE: &'static str = "coverage";

/// Runs the coverage test suite (or a user-specified subset) in one mode.
///
/// This same function is used by the multi-mode step ([`Coverage`]) and by
/// the single-mode steps ([`CoverageMap`] and [`CoverageRun`]), to help
/// ensure that they all behave consistently with each other, regardless of
/// how the coverage tests have been invoked.
fn run_coverage_tests(
builder: &Builder<'_>,
compiler: Compiler,
target: TargetSelection,
mode: &'static str,
) {
// Like many other test steps, we delegate to a `Compiletest` step to
// actually run the tests. (See `test_definitions!`.)
builder.ensure(Compiletest {
compiler,
target,
mode,
suite: Self::SUITE,
path: Self::PATH,
compare_mode: None,
});
}
const ALL_MODES: &[&str] = &["coverage-map", "coverage-run"];
}

impl Step for Coverage {
type Output = ();
/// We rely on the individual CoverageMap/CoverageRun steps to run themselves.
const DEFAULT: bool = false;
/// When manually invoked, try to run as much as possible.
const DEFAULT: bool = true;
/// Compiletest will automatically skip the "coverage-run" tests if necessary.
const ONLY_HOSTS: bool = false;

fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> {
// Take responsibility for command-line paths within `tests/coverage`.
run.suite_path(Self::PATH)
fn should_run(mut run: ShouldRun<'_>) -> ShouldRun<'_> {
// Support various invocation styles, including:
// - `./x test coverage`
// - `./x test tests/coverage/trivial.rs`
// - `./x test coverage-map`
// - `./x test coverage-run -- tests/coverage/trivial.rs`
run = run.suite_path(Self::PATH);
for mode in Self::ALL_MODES {
run = run.alias(mode);
}
run
}

fn make_run(run: RunConfig<'_>) {
let compiler = run.builder.compiler(run.builder.top_stage, run.build_triple());
let target = run.target;

// List of (coverage) test modes that the coverage test suite will be
// run in. It's OK for this to contain duplicates, because the call to
// `Builder::ensure` below will take care of deduplication.
let mut modes = vec![];

// From the pathsets that were selected on the command-line (or by default),
// determine which modes to run in.
for path in &run.paths {
match path {
PathSet::Set(_) => {
for mode in Self::ALL_MODES {
if path.assert_single_path().path == Path::new(mode) {
modes.push(mode);
break;
}
}
}
PathSet::Suite(_) => {
modes.extend(Self::ALL_MODES);
break;
}
}
}

// Skip any modes that were explicitly skipped/excluded on the command-line.
// FIXME(Zalathar): Integrate this into central skip handling somehow?
modes.retain(|mode| !run.builder.config.skip.iter().any(|skip| skip == Path::new(mode)));

// FIXME(Zalathar): Make these commands skip all coverage tests, as expected:
// - `./x test --skip=tests`
// - `./x test --skip=tests/coverage`
// - `./x test --skip=coverage`
// Skip handling currently doesn't have a way to know that skipping the coverage
// suite should also skip the `coverage-map` and `coverage-run` aliases.

run.builder.ensure(Coverage { compiler, target: run.target });
for mode in modes {
run.builder.ensure(Coverage { compiler, target, mode });
}
}

fn run(self, builder: &Builder<'_>) {
// Run the specified coverage tests (possibly all of them) in both modes.
Self::run_coverage_tests(builder, self.compiler, self.target, CoverageMap::MODE);
Self::run_coverage_tests(builder, self.compiler, self.target, CoverageRun::MODE);
}
}

coverage_test_alias! {
/// Runs the `tests/coverage` test suite in "coverage-map" mode only.
/// Used by `x test` and `x test coverage-map`.
CoverageMap {
alias_and_mode: "coverage-map",
default: true,
only_hosts: false,
}
}
coverage_test_alias! {
/// Runs the `tests/coverage` test suite in "coverage-run" mode only.
/// Used by `x test` and `x test coverage-run`.
CoverageRun {
alias_and_mode: "coverage-run",
default: true,
// Compiletest knows how to automatically skip these tests when cross-compiling,
// but skipping the whole step here makes it clearer that they haven't run at all.
only_hosts: true,
let Self { compiler, target, mode } = self;
// Like other compiletest suite test steps, delegate to an internal
// compiletest task to actually run the tests.
builder.ensure(Compiletest {
compiler,
target,
mode,
suite: Self::SUITE,
path: Self::PATH,
compare_mode: None,
});
}
}

Expand Down
2 changes: 0 additions & 2 deletions src/bootstrap/src/core/builder/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -943,8 +943,6 @@ impl<'a> Builder<'a> {
test::Ui,
test::Crashes,
test::Coverage,
test::CoverageMap,
test::CoverageRun,
test::MirOpt,
test::Codegen,
test::CodegenUnits,
Expand Down
33 changes: 33 additions & 0 deletions src/bootstrap/src/core/builder/tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -828,3 +828,36 @@ fn test_test_compiler() {

assert_eq!((compiler, cranelift, gcc), (true, false, false));
}

#[test]
fn test_test_coverage() {
struct Case {
cmd: &'static [&'static str],
expected: &'static [&'static str],
}
let cases = &[
Case { cmd: &["test"], expected: &["coverage-map", "coverage-run"] },
Case { cmd: &["test", "coverage"], expected: &["coverage-map", "coverage-run"] },
Case { cmd: &["test", "coverage-map"], expected: &["coverage-map"] },
Case { cmd: &["test", "coverage-run"], expected: &["coverage-run"] },
Case { cmd: &["test", "coverage", "--skip=coverage"], expected: &[] },
Case { cmd: &["test", "coverage", "--skip=tests/coverage"], expected: &[] },
Case { cmd: &["test", "coverage", "--skip=coverage-map"], expected: &["coverage-run"] },
Case { cmd: &["test", "coverage", "--skip=coverage-run"], expected: &["coverage-map"] },
Case { cmd: &["test", "--skip=coverage-map", "--skip=coverage-run"], expected: &[] },
Case { cmd: &["test", "coverage", "--skip=tests"], expected: &[] },
];

for &Case { cmd, expected } in cases {
// Print each test case so that if one fails, the most recently printed
// case is the one that failed.
println!("Testing case: {cmd:?}");
let cmd = cmd.iter().copied().map(str::to_owned).collect::<Vec<_>>();
let config = configure_with_args(&cmd, &[TEST_TRIPLE_1], &[TEST_TRIPLE_1]);
let mut cache = run_build(&config.paths.clone(), config);

let modes =
cache.all::<test::Coverage>().iter().map(|(step, ())| step.mode).collect::<Vec<_>>();
assert_eq!(modes, expected);
}
}
Loading