Skip to content

Commit

Permalink
Rollup merge of #112300 - Zalathar:run-coverage, r=wesleywiser
Browse files Browse the repository at this point in the history
Convert `run-make/coverage-reports` tests to use a custom compiletest mode

I was frustrated by the fact that most of the coverage tests are glued together with makefiles and shell scripts, so I tried my hand at converting most of them over to a newly-implemented `run-coverage` mode/suite in compiletest.

This ~~*mostly*~~ resolves #85009, ~~though I've left a small number of the existing tests as-is because they would require more work to fix/support~~.

---

I had time to go back and add support for the more troublesome tests that I had initially skipped over, so this PR now manages to completely get rid of `run-make/coverage-reports`.

---

The patches are arranged as follows:

- Declare the new mode/suite in bootstrap
- Small changes to compiletest that will be used by the new mode
- Implement the new mode in compiletest
- Migrate most of the tests over
- Add more code to bootstrap and compiletest to support the remaining tests
- Migrate the remaining tests (with some temporary hacks to avoid re-blessing them)
- Remove the temporary hacks and re-bless the migrated tests
- Remove the unused remnants of `run-make/coverage-reports`
  • Loading branch information
matthiaskrgr authored Jun 29, 2023
2 parents 93a97c7 + 7b4e75b commit f00db43
Show file tree
Hide file tree
Showing 109 changed files with 523 additions and 356 deletions.
2 changes: 2 additions & 0 deletions src/bootstrap/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -686,6 +686,7 @@ impl<'a> Builder<'a> {
test::Tidy,
test::Ui,
test::RunPassValgrind,
test::RunCoverage,
test::MirOpt,
test::Codegen,
test::CodegenUnits,
Expand All @@ -694,6 +695,7 @@ impl<'a> Builder<'a> {
test::Debuginfo,
test::UiFullDeps,
test::Rustdoc,
test::RunCoverageRustdoc,
test::Pretty,
test::Crate,
test::CrateLibrustc,
Expand Down
20 changes: 16 additions & 4 deletions src/bootstrap/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1319,6 +1319,13 @@ host_test!(RunMakeFullDeps {

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

host_test!(RunCoverage { path: "tests/run-coverage", mode: "run-coverage", suite: "run-coverage" });
host_test!(RunCoverageRustdoc {
path: "tests/run-coverage-rustdoc",
mode: "run-coverage",
suite: "run-coverage-rustdoc"
});

// For the mir-opt suite we do not use macros, as we need custom behavior when blessing.
#[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)]
pub struct MirOpt {
Expand Down Expand Up @@ -1503,6 +1510,7 @@ note: if you're sure you want to do this, please open an issue as to why. In the
|| (mode == "ui" && is_rustdoc)
|| mode == "js-doc-test"
|| mode == "rustdoc-json"
|| suite == "run-coverage-rustdoc"
{
cmd.arg("--rustdoc-path").arg(builder.rustdoc(compiler));
}
Expand All @@ -1516,7 +1524,7 @@ note: if you're sure you want to do this, please open an issue as to why. In the
.arg(builder.ensure(tool::JsonDocLint { compiler: json_compiler, target }));
}

if mode == "run-make" {
if mode == "run-make" || mode == "run-coverage" {
let rust_demangler = builder
.ensure(tool::RustDemangler {
compiler,
Expand Down Expand Up @@ -1703,17 +1711,21 @@ note: if you're sure you want to do this, please open an issue as to why. In the
add_link_lib_path(vec![llvm_libdir.trim().into()], &mut cmd);
}

// Only pass correct values for these flags for the `run-make` suite as it
// requires that a C++ compiler was configured which isn't always the case.
if !builder.config.dry_run() && matches!(suite, "run-make" | "run-make-fulldeps") {
if !builder.config.dry_run()
&& (matches!(suite, "run-make" | "run-make-fulldeps") || mode == "run-coverage")
{
// The llvm/bin directory contains many useful cross-platform
// tools. Pass the path to run-make tests so they can use them.
// (The run-coverage tests also need these tools to process
// coverage reports.)
let llvm_bin_path = llvm_config
.parent()
.expect("Expected llvm-config to be contained in directory");
assert!(llvm_bin_path.is_dir());
cmd.arg("--llvm-bin-dir").arg(llvm_bin_path);
}

if !builder.config.dry_run() && matches!(suite, "run-make" | "run-make-fulldeps") {
// If LLD is available, add it to the PATH
if builder.config.lld_enabled {
let lld_install_root =
Expand Down
3 changes: 3 additions & 0 deletions src/tools/compiletest/src/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ string_enum! {
JsDocTest => "js-doc-test",
MirOpt => "mir-opt",
Assembly => "assembly",
RunCoverage => "run-coverage",
}
}

Expand Down Expand Up @@ -626,6 +627,7 @@ pub const UI_EXTENSIONS: &[&str] = &[
UI_STDERR_64,
UI_STDERR_32,
UI_STDERR_16,
UI_COVERAGE,
];
pub const UI_STDERR: &str = "stderr";
pub const UI_STDOUT: &str = "stdout";
Expand All @@ -635,6 +637,7 @@ pub const UI_RUN_STDOUT: &str = "run.stdout";
pub const UI_STDERR_64: &str = "64bit.stderr";
pub const UI_STDERR_32: &str = "32bit.stderr";
pub const UI_STDERR_16: &str = "16bit.stderr";
pub const UI_COVERAGE: &str = "coverage";

/// Absolute path to the directory where all output for all tests in the given
/// `relative_dir` group should reside. Example:
Expand Down
48 changes: 40 additions & 8 deletions src/tools/compiletest/src/header.rs
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ pub struct TestProps {
// customized normalization rules
pub normalize_stdout: Vec<(String, String)>,
pub normalize_stderr: Vec<(String, String)>,
pub failure_status: i32,
pub failure_status: Option<i32>,
// For UI tests, allows compiler to exit with arbitrary failure status
pub dont_check_failure_status: bool,
// Whether or not `rustfix` should apply the `CodeSuggestion`s of this test and compile the
Expand Down Expand Up @@ -257,7 +257,7 @@ impl TestProps {
check_test_line_numbers_match: false,
normalize_stdout: vec![],
normalize_stderr: vec![],
failure_status: -1,
failure_status: None,
dont_check_failure_status: false,
run_rustfix: false,
rustfix_only_machine_applicable: false,
Expand Down Expand Up @@ -428,7 +428,7 @@ impl TestProps {
.parse_name_value_directive(ln, FAILURE_STATUS)
.and_then(|code| code.trim().parse::<i32>().ok())
{
self.failure_status = code;
self.failure_status = Some(code);
}

config.set_name_directive(
Expand Down Expand Up @@ -491,11 +491,8 @@ impl TestProps {
});
}

if self.failure_status == -1 {
self.failure_status = 1;
}
if self.should_ice {
self.failure_status = 101;
self.failure_status = Some(101);
}

if config.mode == Mode::Incremental {
Expand Down Expand Up @@ -615,10 +612,25 @@ pub fn line_directive<'line>(
}

fn iter_header<R: Read>(testfile: &Path, rdr: R, it: &mut dyn FnMut(Option<&str>, &str, usize)) {
iter_header_extra(testfile, rdr, &[], it)
}

fn iter_header_extra(
testfile: &Path,
rdr: impl Read,
extra_directives: &[&str],
it: &mut dyn FnMut(Option<&str>, &str, usize),
) {
if testfile.is_dir() {
return;
}

// Process any extra directives supplied by the caller (e.g. because they
// are implied by the test mode), with a dummy line number of 0.
for directive in extra_directives {
it(None, directive, 0);
}

let comment = if testfile.extension().map(|e| e == "rs") == Some(true) { "//" } else { "#" };

let mut rdr = BufReader::new(rdr);
Expand Down Expand Up @@ -897,7 +909,27 @@ pub fn make_test_description<R: Read>(
let mut ignore_message = None;
let mut should_fail = false;

iter_header(path, src, &mut |revision, ln, line_number| {
let extra_directives: &[&str] = match config.mode {
// The run-coverage tests are treated as having these extra directives,
// without needing to specify them manually in every test file.
// (Some of the comments below have been copied over from
// `tests/run-make/coverage-reports/Makefile`, which no longer exists.)
Mode::RunCoverage => {
&[
"needs-profiler-support",
// FIXME(mati865): MinGW GCC miscompiles compiler-rt profiling library but with Clang it works
// properly. Since we only have GCC on the CI ignore the test for now.
"ignore-windows-gnu",
// FIXME(pietroalbini): this test currently does not work on cross-compiled
// targets because remote-test is not capable of sending back the *.profraw
// files generated by the LLVM instrumentation.
"ignore-cross-compile",
]
}
_ => &[],
};

iter_header_extra(path, src, extra_directives, &mut |revision, ln, line_number| {
if revision.is_some() && revision != cfg {
return;
}
Expand Down
4 changes: 3 additions & 1 deletion src/tools/compiletest/src/header/needs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ pub(super) fn handle_needs(
},
Need {
name: "needs-profiler-support",
condition: std::env::var_os("RUSTC_PROFILER_SUPPORT").is_some(),
condition: cache.profiler_support,
ignore_reason: "ignored when profiler support is disabled",
},
Need {
Expand Down Expand Up @@ -195,6 +195,7 @@ pub(super) struct CachedNeedsConditions {
sanitizer_memtag: bool,
sanitizer_shadow_call_stack: bool,
sanitizer_safestack: bool,
profiler_support: bool,
xray: bool,
rust_lld: bool,
i686_dlltool: bool,
Expand Down Expand Up @@ -232,6 +233,7 @@ impl CachedNeedsConditions {
sanitizer_memtag: util::MEMTAG_SUPPORTED_TARGETS.contains(target),
sanitizer_shadow_call_stack: util::SHADOWCALLSTACK_SUPPORTED_TARGETS.contains(target),
sanitizer_safestack: util::SAFESTACK_SUPPORTED_TARGETS.contains(target),
profiler_support: std::env::var_os("RUSTC_PROFILER_SUPPORT").is_some(),
xray: util::XRAY_SUPPORTED_TARGETS.contains(target),

// For tests using the `needs-rust-lld` directive (e.g. for `-Zgcc-ld=lld`), we need to find
Expand Down
Loading

0 comments on commit f00db43

Please sign in to comment.