Skip to content

Commit

Permalink
Rollup merge of #119034 - Zalathar:ignore-mode, r=davidtwco
Browse files Browse the repository at this point in the history
Allow coverage tests to ignore test modes, and to enable color in coverage reports

This PR adds two new header directives to compiletest, intended for use by coverage tests (and by #119033 in particular).

The new headers are:

- `// ignore-mode-{mode}` causes a test to not be run in a particular compiletest mode (e.g. `ignore-mode-coverage-run`).
  - This can theoretically be used by any test, but coverage tests are currently the only ones that automatically run in multiple modes, so it's not very useful for other kinds of test.
- `// llvm-cov-flags: --use-color` makes `coverage-run` tests pass the flag `--use-color` when generating coverage reports.
  - For most tests, non-coloured reports are easier to read and more portable across platforms. But for #119033 specifically, we want to test that `llvm-cov` slices up source text correctly, which only happens when colour output is enabled.
  • Loading branch information
compiler-errors committed Jan 5, 2024
2 parents 6bc08a7 + 731ba80 commit d180e91
Show file tree
Hide file tree
Showing 14 changed files with 132 additions and 14 deletions.
2 changes: 1 addition & 1 deletion src/bootstrap/src/core/build_steps/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1976,7 +1976,7 @@ NOTE: if you're sure you want to do this, please open an issue as to why. In the
}

if builder.config.profiler_enabled(target) {
cmd.env("RUSTC_PROFILER_SUPPORT", "1");
cmd.arg("--profiler-support");
}

cmd.env("RUST_TEST_TMPDIR", builder.tempdir());
Expand Down
4 changes: 4 additions & 0 deletions src/tools/compiletest/src/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -387,6 +387,10 @@ pub struct Config {
// Needed both to construct build_helper::git::GitConfig
pub git_repository: String,
pub nightly_branch: String,

/// True if the profiler runtime is enabled for this target.
/// Used by the "needs-profiler-support" header in test files.
pub profiler_support: bool,
}

impl Config {
Expand Down
34 changes: 25 additions & 9 deletions src/tools/compiletest/src/header.rs
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,9 @@ pub struct TestProps {
// Whether to tell `rustc` to remap the "src base" directory to a fake
// directory.
pub remap_src_base: bool,
/// Extra flags to pass to `llvm-cov` when producing coverage reports.
/// Only used by the "coverage-run" test mode.
pub llvm_cov_flags: Vec<String>,
}

mod directives {
Expand Down Expand Up @@ -216,6 +219,7 @@ mod directives {
pub const MIR_UNIT_TEST: &'static str = "unit-test";
pub const REMAP_SRC_BASE: &'static str = "remap-src-base";
pub const COMPARE_OUTPUT_LINES_BY_SUBSET: &'static str = "compare-output-lines-by-subset";
pub const LLVM_COV_FLAGS: &'static str = "llvm-cov-flags";
// This isn't a real directive, just one that is probably mistyped often
pub const INCORRECT_COMPILER_FLAGS: &'static str = "compiler-flags";
}
Expand Down Expand Up @@ -265,6 +269,7 @@ impl TestProps {
stderr_per_bitwidth: false,
mir_unit_test: None,
remap_src_base: false,
llvm_cov_flags: vec![],
}
}

Expand Down Expand Up @@ -321,16 +326,23 @@ impl TestProps {
|r| r,
);

if let Some(flags) = config.parse_name_value_directive(ln, COMPILE_FLAGS) {
self.compile_flags.extend(
flags
.split("'")
.enumerate()
.flat_map(|(i, f)| {
fn split_flags(flags: &str) -> Vec<String> {
// Individual flags can be single-quoted to preserve spaces; see
// <https://github.com/rust-lang/rust/pull/115948/commits/957c5db6>.
flags
.split("'")
.enumerate()
.flat_map(
|(i, f)| {
if i % 2 == 1 { vec![f] } else { f.split_whitespace().collect() }
})
.map(|s| s.to_owned()),
);
},
)
.map(move |s| s.to_owned())
.collect::<Vec<_>>()
}

if let Some(flags) = config.parse_name_value_directive(ln, COMPILE_FLAGS) {
self.compile_flags.extend(split_flags(&flags));
}
if config.parse_name_value_directive(ln, INCORRECT_COMPILER_FLAGS).is_some() {
panic!("`compiler-flags` directive should be spelled `compile-flags`");
Expand Down Expand Up @@ -488,6 +500,10 @@ impl TestProps {
COMPARE_OUTPUT_LINES_BY_SUBSET,
&mut self.compare_output_lines_by_subset,
);

if let Some(flags) = config.parse_name_value_directive(ln, LLVM_COV_FLAGS) {
self.llvm_cov_flags.extend(split_flags(&flags));
}
});
}

Expand Down
13 changes: 12 additions & 1 deletion src/tools/compiletest/src/header/cfg.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use crate::common::{CompareMode, Config, Debugger};
use crate::common::{CompareMode, Config, Debugger, Mode};
use crate::header::IgnoreDecision;
use std::collections::HashSet;

Expand Down Expand Up @@ -208,6 +208,17 @@ pub(super) fn parse_cfg_name_directive<'a>(
},
message: "when comparing with {name}",
}
// Coverage tests run the same test file in multiple modes.
// If a particular test should not be run in one of the modes, ignore it
// with "ignore-mode-coverage-map" or "ignore-mode-coverage-run".
condition! {
name: format!("mode-{}", config.mode.to_str()),
allowed_names: ContainsPrefixed {
prefix: "mode-",
inner: Mode::STR_VARIANTS,
},
message: "when the test mode is {name}",
}

if prefix == "ignore" && outcome == MatchOutcome::Invalid {
// Don't error out for ignore-tidy-* diretives, as those are not handled by compiletest.
Expand Down
2 changes: 1 addition & 1 deletion src/tools/compiletest/src/header/needs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,7 @@ impl CachedNeedsConditions {
sanitizer_memtag: sanitizers.contains(&Sanitizer::Memtag),
sanitizer_shadow_call_stack: sanitizers.contains(&Sanitizer::ShadowCallStack),
sanitizer_safestack: sanitizers.contains(&Sanitizer::Safestack),
profiler_support: std::env::var_os("RUSTC_PROFILER_SUPPORT").is_some(),
profiler_support: config.profiler_support,
xray: config.target_cfg().xray,

// For tests using the `needs-rust-lld` directive (e.g. for `-Clink-self-contained=+linker`),
Expand Down
44 changes: 42 additions & 2 deletions src/tools/compiletest/src/header/tests.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
use std::io::Read;
use std::path::Path;
use std::str::FromStr;

use crate::common::{Config, Debugger};
use crate::common::{Config, Debugger, Mode};
use crate::header::{parse_normalization_string, EarlyProps, HeadersCache};

fn make_test_description<R: Read>(
Expand Down Expand Up @@ -55,16 +56,23 @@ fn test_parse_normalization_string() {

#[derive(Default)]
struct ConfigBuilder {
mode: Option<String>,
channel: Option<String>,
host: Option<String>,
target: Option<String>,
stage_id: Option<String>,
llvm_version: Option<String>,
git_hash: bool,
system_llvm: bool,
profiler_support: bool,
}

impl ConfigBuilder {
fn mode(&mut self, s: &str) -> &mut Self {
self.mode = Some(s.to_owned());
self
}

fn channel(&mut self, s: &str) -> &mut Self {
self.channel = Some(s.to_owned());
self
Expand Down Expand Up @@ -100,10 +108,16 @@ impl ConfigBuilder {
self
}

fn profiler_support(&mut self, s: bool) -> &mut Self {
self.profiler_support = s;
self
}

fn build(&mut self) -> Config {
let args = &[
"compiletest",
"--mode=ui",
"--mode",
self.mode.as_deref().unwrap_or("ui"),
"--suite=ui",
"--compile-lib-path=",
"--run-lib-path=",
Expand Down Expand Up @@ -142,6 +156,9 @@ impl ConfigBuilder {
if self.system_llvm {
args.push("--system-llvm".to_owned());
}
if self.profiler_support {
args.push("--profiler-support".to_owned());
}

args.push("--rustc-path".to_string());
// This is a subtle/fragile thing. On rust-lang CI, there is no global
Expand Down Expand Up @@ -340,6 +357,15 @@ fn sanitizers() {
assert!(check_ignore(&config, "// needs-sanitizer-thread"));
}

#[test]
fn profiler_support() {
let config: Config = cfg().profiler_support(false).build();
assert!(check_ignore(&config, "// needs-profiler-support"));

let config: Config = cfg().profiler_support(true).build();
assert!(!check_ignore(&config, "// needs-profiler-support"));
}

#[test]
fn asm_support() {
let asms = [
Expand Down Expand Up @@ -530,3 +556,17 @@ fn families() {
assert!(!check_ignore(&config, &format!("// ignore-{other}")));
}
}

#[test]
fn ignore_mode() {
for &mode in Mode::STR_VARIANTS {
// Indicate profiler support so that "coverage-run" tests aren't skipped.
let config: Config = cfg().mode(mode).profiler_support(true).build();
let other = if mode == "coverage-run" { "coverage-map" } else { "coverage-run" };
assert_ne!(mode, other);
assert_eq!(config.mode, Mode::from_str(mode).unwrap());
assert_ne!(config.mode, Mode::from_str(other).unwrap());
assert!(check_ignore(&config, &format!("// ignore-mode-{mode}")));
assert!(!check_ignore(&config, &format!("// ignore-mode-{other}")));
}
}
3 changes: 3 additions & 0 deletions src/tools/compiletest/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,7 @@ pub fn parse_config(args: Vec<String>) -> Config {
.optflag("", "force-rerun", "rerun tests even if the inputs are unchanged")
.optflag("", "only-modified", "only run tests that result been modified")
.optflag("", "nocapture", "")
.optflag("", "profiler-support", "is the profiler runtime enabled for this target")
.optflag("h", "help", "show this message")
.reqopt("", "channel", "current Rust channel", "CHANNEL")
.optflag("", "git-hash", "run tests which rely on commit version being compiled into the binaries")
Expand Down Expand Up @@ -315,6 +316,8 @@ pub fn parse_config(args: Vec<String>) -> Config {

git_repository: matches.opt_str("git-repository").unwrap(),
nightly_branch: matches.opt_str("nightly-branch").unwrap(),

profiler_support: matches.opt_present("profiler-support"),
}
}

Expand Down
2 changes: 2 additions & 0 deletions src/tools/compiletest/src/runtest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -575,6 +575,8 @@ impl<'test> TestCx<'test> {
cmd.arg("--object");
cmd.arg(bin);
}

cmd.args(&self.props.llvm_cov_flags);
});
if !proc_res.status.success() {
self.fatal_proc_rec("llvm-cov show failed!", &proc_res);
Expand Down
13 changes: 13 additions & 0 deletions tests/coverage/color.coverage
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
LL| |// edition: 2021
LL| |// ignore-mode-coverage-map
LL| |// ignore-windows
LL| |// llvm-cov-flags: --use-color
LL| |
LL| |// Verify that telling `llvm-cov` to use colored output actually works.
LL| |// Ignored on Windows because we can't tell the tool to use ANSI escapes.
LL| |
LL| 1|fn main() {
LL| 1| for _i in 0..0 {}
^0 ^0
LL| 1|}

11 changes: 11 additions & 0 deletions tests/coverage/color.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
// edition: 2021
// ignore-mode-coverage-map
// ignore-windows
// llvm-cov-flags: --use-color

// Verify that telling `llvm-cov` to use colored output actually works.
// Ignored on Windows because we can't tell the tool to use ANSI escapes.

fn main() {
for _i in 0..0 {}
}
4 changes: 4 additions & 0 deletions tests/coverage/ignore_map.coverage
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
LL| |// ignore-mode-coverage-map
LL| |
LL| 1|fn main() {}

3 changes: 3 additions & 0 deletions tests/coverage/ignore_map.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
// ignore-mode-coverage-map

fn main() {}
8 changes: 8 additions & 0 deletions tests/coverage/ignore_run.cov-map
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
Function name: ignore_run::main
Raw bytes (9): 0x[01, 01, 00, 01, 01, 03, 01, 00, 0d]
Number of files: 1
- file 0 => global file 1
Number of expressions: 0
Number of file 0 mappings: 1
- Code(Counter(0)) at (prev + 3, 1) to (start + 0, 13)

3 changes: 3 additions & 0 deletions tests/coverage/ignore_run.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
// ignore-mode-coverage-run

fn main() {}

0 comments on commit d180e91

Please sign in to comment.