Skip to content
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.

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

Allow coverage tests to ignore test modes, and to enable color in coverage reports #119034

Merged
merged 4 commits into from
Jan 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
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");
}
davidtwco marked this conversation as resolved.
Show resolved Hide resolved

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() {}
Loading