Auto merge of rust-lang#117484 - Zalathar:tests, r=cjgillot

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 rust-lang#117340, which has been closed in favour of this PR.

Author: bors

src/bootstrap/src/core/build_steps/test.rs

@@ -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 {
@@ -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",

src/bootstrap/src/core/builder.rs

@@ -727,6 +727,7 @@ impl<'a> Builder<'a> {
                 test::Tidy,
                 test::Ui,
                 test::RunPassValgrind,
+                test::Coverage,
                 test::CoverageMap,
                 test::RunCoverage,
                 test::MirOpt,

src/tools/compiletest/src/common.rs

@@ -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! {
@@ -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)

src/tools/compiletest/src/runtest.rs

@@ -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
@@ -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 => {
@@ -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`.

tests/coverage-map/README.md

@@ -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)
+