Skip to content

bootstrap: Use a CompiletestMode enum instead of bare strings #149755

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.

Sign up for GitHub

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

Merged
bors merged 2 commits into from
Dec 9, 2025
Merged

bootstrap: Use a CompiletestMode enum instead of bare strings #149755

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8c582e1
Don't require `test::Coverage` to implement Ord
Zalathar Dec 7, 2025
dc8cdb1
Use a `CompiletestMode` enum in bootstrap
Zalathar Dec 7, 2025
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
118 changes: 70 additions & 48 deletions src/bootstrap/src/core/build_steps/test.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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

Comment on lines +6 to +8
Copy link
Member Author

@Zalathar Zalathar Dec 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know why this file is suddenly failing tidy, when it has been well over 3000 lines for a long time.

Copy link
Member Author

@Zalathar Zalathar Dec 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess it's counting non-blank non-comment lines, and this PR does push it slightly over 3000 of those, mostly due to formatting changes.

Copy link
Member

@jieyouxu jieyouxu Dec 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's been just about 3000 lines yeah. I planned to break the test.rs source into a smaller modules, but haven't gotten around to figuring out a nicer structure.

use std::collections::HashSet;
use std::env::split_paths;
use std::ffi::{OsStr, OsString};
Expand All @@ -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,
Expand All @@ -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 {
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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 {
Expand All @@ -1493,106 +1499,117 @@ 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"),
});

test!(UiFullDeps {
path: "tests/ui-fulldeps",
mode: "ui",
mode: CompiletestMode::Ui,
suite: "ui-fulldeps",
default: true,
IS_HOST: true,
});

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,
});

test!(RustdocJson {
path: "tests/rustdoc-json",
mode: "rustdoc-json",
mode: CompiletestMode::RustdocJson,
suite: "rustdoc-json",
default: true,
IS_HOST: true,
});

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 {
Expand All @@ -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
}
Expand All @@ -1631,23 +1648,25 @@ 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;
}
}
}

// 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`
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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>,
Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -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);
}

Expand Down Expand Up @@ -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);
Expand All @@ -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")
Comment on lines +1932 to +1938
Copy link
Member Author

@Zalathar Zalathar Dec 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here I've replaced (mode == "ui" && is_rustdoc) with matches!(suite, "rustdoc-ui"), which has the same outcome but is more direct about what it's doing.

{
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")
Expand All @@ -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);
}
Expand All @@ -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));
Expand Down Expand Up @@ -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 {
Expand All @@ -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
Expand Down Expand Up @@ -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());
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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
Expand All @@ -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 =
Expand All @@ -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);
Expand Down Expand Up @@ -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(),
Expand All @@ -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(),
Expand Down
Loading
Loading