Rollup merge of #149755 - Zalathar:test-mode, r=jieyouxu

bootstrap: Use a `CompiletestMode` enum instead of bare strings

This gives better static checking when dealing with compiletest modes in bootstrap, especially for those modes that overlap with the name of a test suite directory.

r? jieyouxu (or bootstrap)
This commit is contained in:
Matthias Krüger 2025-12-09 06:17:25 +01:00 committed by GitHub
commit b13a25377d
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 153 additions and 58 deletions

View file

@ -3,6 +3,9 @@
//! `./x.py test` (aka [`Kind::Test`]) is currently allowed to reach build steps in other modules.
//! However, this contains ~all test parts we expect people to be able to build and run locally.
// (This file should be split up, but having tidy block all changes is not helpful.)
// ignore-tidy-filelength
use std::collections::HashSet;
use std::env::split_paths;
use std::ffi::{OsStr, OsString};
@ -17,6 +20,7 @@ use crate::core::build_steps::gcc::{Gcc, add_cg_gcc_cargo_flags};
use crate::core::build_steps::llvm::get_llvm_version;
use crate::core::build_steps::run::{get_completion_paths, get_help_path};
use crate::core::build_steps::synthetic_targets::MirOptPanicAbortSyntheticTarget;
use crate::core::build_steps::test::compiletest::CompiletestMode;
use crate::core::build_steps::tool::{
self, RustcPrivateCompilers, SourceType, TEST_FLOAT_PARSE_ALLOW_FEATURES, Tool,
ToolTargetBuildMode, get_tool_target_compiler,
@ -39,6 +43,8 @@ use crate::utils::helpers::{
use crate::utils::render_tests::{add_flags_and_try_run_tests, try_run_tests};
use crate::{CLang, CodegenBackendKind, DocTests, GitRepo, Mode, PathSet, envify};
mod compiletest;
/// Runs `cargo test` on various internal tools used by bootstrap.
#[derive(Debug, Clone, PartialEq, Eq, Hash)]
pub struct CrateBootstrap {
@ -1085,7 +1091,7 @@ impl Step for RustdocJSNotStd {
builder.ensure(Compiletest {
test_compiler: self.compiler,
target: self.target,
mode: "rustdoc-js",
mode: CompiletestMode::RustdocJs,
suite: "rustdoc-js",
path: "tests/rustdoc-js",
compare_mode: None,
@ -1478,7 +1484,7 @@ macro_rules! test {
builder.ensure(Compiletest {
test_compiler: self.test_compiler,
target: self.target,
mode: $mode,
mode: const { $mode },
suite: $suite,
path: $path,
compare_mode: (const {
@ -1493,34 +1499,39 @@ macro_rules! test {
};
}
test!(Ui { path: "tests/ui", mode: "ui", suite: "ui", default: true });
test!(Ui { path: "tests/ui", mode: CompiletestMode::Ui, suite: "ui", default: true });
test!(Crashes { path: "tests/crashes", mode: "crashes", suite: "crashes", default: true });
test!(Crashes {
path: "tests/crashes",
mode: CompiletestMode::Crashes,
suite: "crashes",
default: true,
});
test!(CodegenLlvm {
path: "tests/codegen-llvm",
mode: "codegen",
mode: CompiletestMode::Codegen,
suite: "codegen-llvm",
default: true
});
test!(CodegenUnits {
path: "tests/codegen-units",
mode: "codegen-units",
mode: CompiletestMode::CodegenUnits,
suite: "codegen-units",
default: true,
});
test!(Incremental {
path: "tests/incremental",
mode: "incremental",
mode: CompiletestMode::Incremental,
suite: "incremental",
default: true,
});
test!(Debuginfo {
path: "tests/debuginfo",
mode: "debuginfo",
mode: CompiletestMode::Debuginfo,
suite: "debuginfo",
default: true,
compare_mode: Some("split-dwarf"),
@ -1528,7 +1539,7 @@ test!(Debuginfo {
test!(UiFullDeps {
path: "tests/ui-fulldeps",
mode: "ui",
mode: CompiletestMode::Ui,
suite: "ui-fulldeps",
default: true,
IS_HOST: true,
@ -1536,14 +1547,14 @@ test!(UiFullDeps {
test!(Rustdoc {
path: "tests/rustdoc",
mode: "rustdoc",
mode: CompiletestMode::Rustdoc,
suite: "rustdoc",
default: true,
IS_HOST: true,
});
test!(RustdocUi {
path: "tests/rustdoc-ui",
mode: "ui",
mode: CompiletestMode::Ui,
suite: "rustdoc-ui",
default: true,
IS_HOST: true,
@ -1551,7 +1562,7 @@ test!(RustdocUi {
test!(RustdocJson {
path: "tests/rustdoc-json",
mode: "rustdoc-json",
mode: CompiletestMode::RustdocJson,
suite: "rustdoc-json",
default: true,
IS_HOST: true,
@ -1559,40 +1570,46 @@ test!(RustdocJson {
test!(Pretty {
path: "tests/pretty",
mode: "pretty",
mode: CompiletestMode::Pretty,
suite: "pretty",
default: true,
IS_HOST: true,
});
test!(RunMake { path: "tests/run-make", mode: "run-make", suite: "run-make", default: true });
test!(RunMake {
path: "tests/run-make",
mode: CompiletestMode::RunMake,
suite: "run-make",
default: true,
});
test!(RunMakeCargo {
path: "tests/run-make-cargo",
mode: "run-make",
mode: CompiletestMode::RunMake,
suite: "run-make-cargo",
default: true
});
test!(AssemblyLlvm {
path: "tests/assembly-llvm",
mode: "assembly",
mode: CompiletestMode::Assembly,
suite: "assembly-llvm",
default: true
});
/// Runs the coverage test suite at `tests/coverage` in some or all of the
/// coverage test modes.
#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)]
#[derive(Debug, Clone, PartialEq, Eq, Hash)]
pub struct Coverage {
pub compiler: Compiler,
pub target: TargetSelection,
pub mode: &'static str,
pub(crate) mode: CompiletestMode,
}
impl Coverage {
const PATH: &'static str = "tests/coverage";
const SUITE: &'static str = "coverage";
const ALL_MODES: &[&str] = &["coverage-map", "coverage-run"];
const ALL_MODES: &[CompiletestMode] =
&[CompiletestMode::CoverageMap, CompiletestMode::CoverageRun];
}
impl Step for Coverage {
@ -1608,7 +1625,7 @@ impl Step for Coverage {
// - `./x test coverage-run -- tests/coverage/trivial.rs`
run = run.suite_path(Self::PATH);
for mode in Self::ALL_MODES {
run = run.alias(mode);
run = run.alias(mode.as_str());
}
run
}
@ -1631,15 +1648,15 @@ impl Step for Coverage {
for path in &run.paths {
match path {
PathSet::Set(_) => {
for mode in Self::ALL_MODES {
if path.assert_single_path().path == Path::new(mode) {
for &mode in Self::ALL_MODES {
if path.assert_single_path().path == Path::new(mode.as_str()) {
modes.push(mode);
break;
}
}
}
PathSet::Suite(_) => {
modes.extend(Self::ALL_MODES);
modes.extend_from_slice(Self::ALL_MODES);
break;
}
}
@ -1647,7 +1664,9 @@ impl Step for Coverage {
// Skip any modes that were explicitly skipped/excluded on the command-line.
// FIXME(Zalathar): Integrate this into central skip handling somehow?
modes.retain(|mode| !run.builder.config.skip.iter().any(|skip| skip == Path::new(mode)));
modes.retain(|mode| {
!run.builder.config.skip.iter().any(|skip| skip == Path::new(mode.as_str()))
});
// FIXME(Zalathar): Make these commands skip all coverage tests, as expected:
// - `./x test --skip=tests`
@ -1678,7 +1697,7 @@ impl Step for Coverage {
test!(CoverageRunRustdoc {
path: "tests/coverage-run-rustdoc",
mode: "coverage-run",
mode: CompiletestMode::CoverageRun,
suite: "coverage-run-rustdoc",
default: true,
IS_HOST: true,
@ -1712,7 +1731,7 @@ impl Step for MirOpt {
builder.ensure(Compiletest {
test_compiler: self.compiler,
target,
mode: "mir-opt",
mode: CompiletestMode::MirOpt,
suite: "mir-opt",
path: "tests/mir-opt",
compare_mode: None,
@ -1755,7 +1774,7 @@ struct Compiletest {
/// The compiler that we're testing.
test_compiler: Compiler,
target: TargetSelection,
mode: &'static str,
mode: CompiletestMode,
suite: &'static str,
path: &'static str,
compare_mode: Option<&'static str>,
@ -1791,7 +1810,7 @@ NOTE: if you're sure you want to do this, please open an issue as to why. In the
let suite_path = self.path;
// Skip codegen tests if they aren't enabled in configuration.
if !builder.config.codegen_tests && mode == "codegen" {
if !builder.config.codegen_tests && mode == CompiletestMode::Codegen {
return;
}
@ -1829,7 +1848,7 @@ NOTE: if you're sure you want to do this, please open an issue as to why. In the
target,
});
}
if mode == "run-make" {
if mode == CompiletestMode::RunMake {
builder.tool_exe(Tool::RunMakeSupport);
}
@ -1886,7 +1905,7 @@ NOTE: if you're sure you want to do this, please open an issue as to why. In the
// suites, `run-make` and `run-make-cargo`. That way, contributors who do not need to run
// the `run-make` tests that need in-tree cargo do not need to spend time building in-tree
// cargo.
if mode == "run-make" {
if mode == CompiletestMode::RunMake {
// We need to pass the compiler that was used to compile run-make-support,
// because we have to use the same compiler to compile rmake.rs recipes.
let stage0_rustc_path = builder.compiler(0, test_compiler.host);
@ -1910,17 +1929,18 @@ NOTE: if you're sure you want to do this, please open an issue as to why. In the
}
// Avoid depending on rustdoc when we don't need it.
if mode == "rustdoc"
|| mode == "run-make"
|| (mode == "ui" && is_rustdoc)
|| mode == "rustdoc-js"
|| mode == "rustdoc-json"
|| suite == "coverage-run-rustdoc"
if matches!(
mode,
CompiletestMode::RunMake
| CompiletestMode::Rustdoc
| CompiletestMode::RustdocJs
| CompiletestMode::RustdocJson
) || matches!(suite, "rustdoc-ui" | "coverage-run-rustdoc")
{
cmd.arg("--rustdoc-path").arg(builder.rustdoc_for_compiler(test_compiler));
}
if mode == "rustdoc-json" {
if mode == CompiletestMode::RustdocJson {
// Use the stage0 compiler for jsondocck
let json_compiler = builder.compiler(0, builder.host_target);
cmd.arg("--jsondocck-path")
@ -1930,7 +1950,7 @@ NOTE: if you're sure you want to do this, please open an issue as to why. In the
);
}
if matches!(mode, "coverage-map" | "coverage-run") {
if matches!(mode, CompiletestMode::CoverageMap | CompiletestMode::CoverageRun) {
let coverage_dump = builder.tool_exe(Tool::CoverageDump);
cmd.arg("--coverage-dump-path").arg(coverage_dump);
}
@ -1957,7 +1977,7 @@ NOTE: if you're sure you want to do this, please open an issue as to why. In the
cmd.arg("--sysroot-base").arg(sysroot);
cmd.arg("--suite").arg(suite);
cmd.arg("--mode").arg(mode);
cmd.arg("--mode").arg(mode.as_str());
cmd.arg("--target").arg(target.rustc_target_arg());
cmd.arg("--host").arg(&*test_compiler.host.triple);
cmd.arg("--llvm-filecheck").arg(builder.llvm_filecheck(builder.config.host_target));
@ -2036,7 +2056,7 @@ Please disable assertions with `rust.debug-assertions = false`.
if let Some(ref nodejs) = builder.config.nodejs {
cmd.arg("--nodejs").arg(nodejs);
} else if mode == "rustdoc-js" {
} else if mode == CompiletestMode::RustdocJs {
panic!("need nodejs to run rustdoc-js suite");
}
if builder.config.rust_optimize_tests {
@ -2055,7 +2075,7 @@ Please disable assertions with `rust.debug-assertions = false`.
let mut flags = if is_rustdoc { Vec::new() } else { vec!["-Crpath".to_string()] };
flags.push(format!(
"-Cdebuginfo={}",
if mode == "codegen" {
if mode == CompiletestMode::Codegen {
// codegen tests typically check LLVM IR and are sensitive to additional debuginfo.
// So do not apply `rust.debuginfo-level-tests` for codegen tests.
if builder.config.rust_debuginfo_level_tests
@ -2122,7 +2142,7 @@ Please disable assertions with `rust.debug-assertions = false`.
cmd.arg("--android-cross-path").arg(android_cross_path);
}
if mode == "debuginfo" {
if mode == CompiletestMode::Debuginfo {
if let Some(debuggers::Gdb { gdb }) = debuggers::discover_gdb(builder, android.as_ref())
{
cmd.arg("--gdb").arg(gdb.as_ref());
@ -2155,7 +2175,7 @@ Please disable assertions with `rust.debug-assertions = false`.
// in rustdoc-js mode, allow filters to be rs files or js files.
// use a late-initialized Vec to avoid cloning for other modes.
let mut paths_v;
if mode == "rustdoc-js" {
if mode == CompiletestMode::RustdocJs {
paths_v = paths.to_vec();
for p in &mut paths_v {
if let Some(ext) = p.extension()
@ -2237,7 +2257,9 @@ Please disable assertions with `rust.debug-assertions = false`.
cmd.arg("--host-rustcflags").arg(link_llvm);
}
if !builder.config.dry_run() && matches!(mode, "run-make" | "coverage-run") {
if !builder.config.dry_run()
&& matches!(mode, CompiletestMode::RunMake | CompiletestMode::CoverageRun)
{
// The llvm/bin directory contains many useful cross-platform
// tools. Pass the path to run-make tests so they can use them.
// (The coverage-run tests also need these tools to process
@ -2249,7 +2271,7 @@ Please disable assertions with `rust.debug-assertions = false`.
cmd.arg("--llvm-bin-dir").arg(llvm_bin_path);
}
if !builder.config.dry_run() && mode == "run-make" {
if !builder.config.dry_run() && mode == CompiletestMode::RunMake {
// If LLD is available, add it to the PATH
if builder.config.lld_enabled {
let lld_install_root =
@ -2269,7 +2291,7 @@ Please disable assertions with `rust.debug-assertions = false`.
// 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() && mode == "run-make" {
if !builder.config.dry_run() && mode == CompiletestMode::RunMake {
let mut cflags = builder.cc_handled_clags(target, CLang::C);
cflags.extend(builder.cc_unhandled_cflags(target, GitRepo::Rustc, CLang::C));
let mut cxxflags = builder.cc_handled_clags(target, CLang::Cxx);
@ -2385,7 +2407,7 @@ Please disable assertions with `rust.debug-assertions = false`.
builder.metrics.begin_test_suite(
build_helper::metrics::TestSuiteMetadata::Compiletest {
suite: suite.into(),
mode: mode.into(),
mode: mode.to_string(),
compare_mode: None,
target: self.target.triple.to_string(),
host: self.test_compiler.host.triple.to_string(),
@ -2408,7 +2430,7 @@ Please disable assertions with `rust.debug-assertions = false`.
builder.metrics.begin_test_suite(
build_helper::metrics::TestSuiteMetadata::Compiletest {
suite: suite.into(),
mode: mode.into(),
mode: mode.to_string(),
compare_mode: Some(compare_mode.into()),
target: self.target.triple.to_string(),
host: self.test_compiler.host.triple.to_string(),

View file

@ -0,0 +1,71 @@
use std::fmt;
/// Enum of all the "test modes" understood by compiletest.
///
/// Some of these mode names happen to overlap with the names of test suite
/// directories, but the relationship between modes and suites is not 1:1.
/// For example:
/// - Mode `ui` is used by suites `tests/ui` and `tests/rustdoc-ui`
/// - Suite `tests/coverage` uses modes `coverage-map` and `coverage-run`
#[derive(Clone, Copy, PartialEq, Eq, Hash)]
pub(crate) enum CompiletestMode {
// tidy-alphabetical-start
Assembly,
Codegen,
CodegenUnits,
CoverageMap,
CoverageRun,
Crashes,
Debuginfo,
Incremental,
MirOpt,
Pretty,
RunMake,
Rustdoc,
RustdocJs,
RustdocJson,
Ui,
// tidy-alphabetical-end
}
impl CompiletestMode {
/// Returns a string representing this mode, which can be passed to
/// compiletest via a command-line argument.
///
/// These mode names must be kept in sync with the ones understood by
/// compiletest's `TestMode`, but they change so rarely that doing so
/// manually should not be burdensome.
pub(crate) const fn as_str(self) -> &'static str {
match self {
// tidy-alphabetical-start
Self::Assembly => "assembly",
Self::Codegen => "codegen",
Self::CodegenUnits => "codegen-units",
Self::CoverageMap => "coverage-map",
Self::CoverageRun => "coverage-run",
Self::Crashes => "crashes",
Self::Debuginfo => "debuginfo",
Self::Incremental => "incremental",
Self::MirOpt => "mir-opt",
Self::Pretty => "pretty",
Self::RunMake => "run-make",
Self::Rustdoc => "rustdoc",
Self::RustdocJs => "rustdoc-js",
Self::RustdocJson => "rustdoc-json",
Self::Ui => "ui",
// tidy-alphabetical-end
}
}
}
impl fmt::Display for CompiletestMode {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
f.write_str(self.as_str())
}
}
impl fmt::Debug for CompiletestMode {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
<Self as fmt::Display>::fmt(self, f)
}
}

View file

@ -346,7 +346,7 @@ fn test_test_coverage() {
let mut cache = run_build(&config.paths.clone(), config);
let modes =
cache.all::<test::Coverage>().iter().map(|(step, ())| step.mode).collect::<Vec<_>>();
cache.inspect_all_steps_of_type::<test::Coverage, _>(|step, ()| step.mode.as_str());
assert_eq!(modes, expected);
}
}

View file

@ -270,16 +270,18 @@ impl Cache {
#[cfg(test)]
impl Cache {
pub fn all<S: Ord + Step>(&mut self) -> Vec<(S, S::Output)> {
let cache = self.cache.get_mut();
let type_id = TypeId::of::<S>();
let mut v = cache
.remove(&type_id)
.map(|b| b.downcast::<HashMap<S, S::Output>>().expect("correct type"))
.map(|m| m.into_iter().collect::<Vec<_>>())
pub(crate) fn inspect_all_steps_of_type<S: Step, T: Ord>(
&self,
map_fn: impl Fn(&S, &S::Output) -> T,
) -> Vec<T> {
let cache = self.cache.borrow();
let mut values = cache
.get(&TypeId::of::<S>())
.map(|any| any.downcast_ref::<HashMap<S, S::Output>>().expect("correct type"))
.map(|m| m.iter().map(|(step, output)| map_fn(step, output)).collect::<Vec<_>>())
.unwrap_or_default();
v.sort_by_key(|(s, _)| s.clone());
v
values.sort();
values
}
pub fn contains<S: Step>(&self) -> bool {