Skip to content

Commit

Permalink
Auto merge of #117484 - Zalathar:tests, r=cjgillot
Browse files Browse the repository at this point in the history
coverage: Unify `tests/coverage-map` and `tests/run-coverage` into `tests/coverage`

Ever since the introduction of the `coverage-map` suite, it's been awkward to have to manage two separate coverage test directories containing dozens of mostly-identical files.

However, those two suites were separate for good reasons. They have very different requirements (since only one of them requires actually running the test program), running only one suite is noticeably faster than running both, and having separate suites allows them to be blessed separately if desired. So while unifying them was an obvious idea, actually doing so was non-trivial.

---

Nevertheless, this PR finds a way to merge the two suites into one directory while retaining almost all of the developer-experience benefits of having two suites. This required non-trivial implementations of `Step`, but the end result works very smoothly.

---

The first 5 commits are a copy of #117340, which has been closed in favour of this PR.
  • Loading branch information
bors committed Nov 8, 2023
2 parents 0d5ec96 + 4e6f438 commit 91cfcb0
Show file tree
Hide file tree
Showing 206 changed files with 435 additions and 2,385 deletions.
104 changes: 99 additions & 5 deletions src/bootstrap/src/core/build_steps/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1305,6 +1305,47 @@ macro_rules! test_definitions {
};
}

/// Declares an alias for running the [`Coverage`] tests in only one mode.
/// Adapted from [`test_definitions`].
macro_rules! coverage_test_alias {
($name:ident {
alias_and_mode: $alias_and_mode:expr,
default: $default:expr,
only_hosts: $only_hosts:expr $(,)?
}) => {
#[derive(Debug, Copy, 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<'_> {
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 { compiler: self.compiler, target: self.target }
.run_unified_suite(builder, Self::MODE)
}
}
};
}

default_test!(Ui { path: "tests/ui", mode: "ui", suite: "ui" });

default_test!(RunPassValgrind {
Expand Down Expand Up @@ -1349,13 +1390,66 @@ host_test!(RunMakeFullDeps {

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

default_test!(CoverageMap {
path: "tests/coverage-map",
mode: "coverage-map",
suite: "coverage-map"
/// Custom test step that is responsible for running the coverage tests
/// in multiple different modes.
///
/// Each individual mode also has its own alias that will run the tests in
/// just that mode.
#[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)]
pub struct Coverage {
pub compiler: Compiler,
pub target: TargetSelection,
}

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

fn run_unified_suite(&self, builder: &Builder<'_>, mode: &'static str) {
builder.ensure(Compiletest {
compiler: self.compiler,
target: self.target,
mode,
suite: Self::SUITE,
path: Self::PATH,
compare_mode: None,
})
}
}

impl Step for Coverage {
type Output = ();
const DEFAULT: bool = false;
const ONLY_HOSTS: bool = false;

fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> {
run.suite_path(Self::PATH)
}

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

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

fn run(self, builder: &Builder<'_>) {
self.run_unified_suite(builder, CoverageMap::MODE);
self.run_unified_suite(builder, RunCoverage::MODE);
}
}

// Aliases for running the coverage tests in only one mode.
coverage_test_alias!(CoverageMap {
alias_and_mode: "coverage-map",
default: true,
only_hosts: false,
});
coverage_test_alias!(RunCoverage {
alias_and_mode: "run-coverage",
default: true,
only_hosts: true,
});

host_test!(RunCoverage { path: "tests/run-coverage", mode: "run-coverage", suite: "run-coverage" });
host_test!(RunCoverageRustdoc {
path: "tests/run-coverage-rustdoc",
mode: "run-coverage",
Expand Down
1 change: 1 addition & 0 deletions src/bootstrap/src/core/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -727,6 +727,7 @@ impl<'a> Builder<'a> {
test::Tidy,
test::Ui,
test::RunPassValgrind,
test::Coverage,
test::CoverageMap,
test::RunCoverage,
test::MirOpt,
Expand Down
12 changes: 11 additions & 1 deletion src/tools/compiletest/src/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -78,14 +78,23 @@ impl Default for Mode {
}

impl Mode {
pub fn disambiguator(self) -> &'static str {
pub fn aux_dir_disambiguator(self) -> &'static str {
// Pretty-printing tests could run concurrently, and if they do,
// they need to keep their output segregated.
match self {
Pretty => ".pretty",
_ => "",
}
}

pub fn output_dir_disambiguator(self) -> &'static str {
// Coverage tests use the same test files for multiple test modes,
// so each mode should have a separate output directory.
match self {
CoverageMap | RunCoverage => self.to_str(),
_ => "",
}
}
}

string_enum! {
Expand Down Expand Up @@ -699,6 +708,7 @@ pub fn output_testname_unique(
let mode = config.compare_mode.as_ref().map_or("", |m| m.to_str());
let debugger = config.debugger.as_ref().map_or("", |m| m.to_str());
PathBuf::from(&testpaths.file.file_stem().unwrap())
.with_extra_extension(config.mode.output_dir_disambiguator())
.with_extra_extension(revision.unwrap_or(""))
.with_extra_extension(mode)
.with_extra_extension(debugger)
Expand Down
8 changes: 4 additions & 4 deletions src/tools/compiletest/src/runtest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2193,7 +2193,7 @@ impl<'test> TestCx<'test> {
|| self.is_vxworks_pure_static()
|| self.config.target.contains("bpf")
|| !self.config.target_cfg().dynamic_linking
|| self.config.mode == RunCoverage
|| matches!(self.config.mode, CoverageMap | RunCoverage)
{
// We primarily compile all auxiliary libraries as dynamic libraries
// to avoid code size bloat and large binaries as much as possible
Expand Down Expand Up @@ -2481,9 +2481,9 @@ impl<'test> TestCx<'test> {
RunCoverage => {
rustc.arg("-Cinstrument-coverage");
// Coverage reports are sometimes sensitive to optimizations,
// and the current snapshots assume no optimization unless
// and the current snapshots assume `opt-level=2` unless
// overridden by `compile-flags`.
rustc.arg("-Copt-level=0");
rustc.arg("-Copt-level=2");
}
RunPassValgrind | Pretty | DebugInfo | Codegen | Rustdoc | RustdocJson | RunMake
| CodegenUnits | JsDocTest | Assembly => {
Expand Down Expand Up @@ -2720,7 +2720,7 @@ impl<'test> TestCx<'test> {
fn aux_output_dir_name(&self) -> PathBuf {
self.output_base_dir()
.join("auxiliary")
.with_extra_extension(self.config.mode.disambiguator())
.with_extra_extension(self.config.mode.aux_dir_disambiguator())
}

/// Generates a unique name for the test, such as `testname.revision.mode`.
Expand Down
13 changes: 0 additions & 13 deletions tests/coverage-map/README.md

This file was deleted.

15 changes: 0 additions & 15 deletions tests/coverage-map/if.cov-map

This file was deleted.

9 changes: 0 additions & 9 deletions tests/coverage-map/if.rs

This file was deleted.

63 changes: 0 additions & 63 deletions tests/coverage-map/status-quo/overflow.rs

This file was deleted.

23 changes: 0 additions & 23 deletions tests/coverage-map/status-quo/sort_groups.rs

This file was deleted.

16 changes: 16 additions & 0 deletions tests/coverage/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
The tests in this directory are shared by two different test modes, and can be
run in multiple different ways:

- `./x.py test coverage-map` (compiles to LLVM IR and checks coverage mappings)
- `./x.py test run-coverage` (runs a test binary and checks its coverage report)
- `./x.py test coverage` (runs both `coverage-map` and `run-coverage`)

## Maintenance note

These tests can be sensitive to small changes in MIR spans or MIR control flow,
especially in HIR-to-MIR lowering or MIR optimizations.

If you haven't touched the coverage code directly, and the tests still pass in
`run-coverage` mode, then it should usually be OK to just re-bless the mappings
as necessary with `./x.py test coverage-map --bless`, without worrying too much
about the exact changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
8 changes: 8 additions & 0 deletions tests/coverage/issue-85461.cov-map
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
Function name: issue_85461::main
Raw bytes (9): 0x[01, 01, 00, 01, 01, 08, 01, 03, 02]
Number of files: 1
- file 0 => global file 1
Number of expressions: 0
Number of file 0 mappings: 1
- Code(Counter(0)) at (prev + 8, 1) to (start + 3, 2)

File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
Loading

0 comments on commit 91cfcb0

Please sign in to comment.