Skip to content

[COMPILETEST-UNTANGLE 5/N] Test mode adjustments and other assorted cleanups #143823

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

Merged
merged 6 commits into from
Jul 13, 2025
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
24 changes: 12 additions & 12 deletions src/tools/compiletest/src/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,14 @@ use camino::{Utf8Path, Utf8PathBuf};
use semver::Version;
use serde::de::{Deserialize, Deserializer, Error as _};

pub use self::Mode::*;
pub use self::TestMode::*;
use crate::executor::{ColorConfig, OutputFormat};
use crate::fatal;
use crate::util::{Utf8PathBufExt, add_dylib_path, string_enum};
Copy link
Member Author

Choose a reason for hiding this comment

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

Note: I'm also going to double-check if we need string_enum at all (or if it even makes sense in this formulation) in a follow-up.


string_enum! {
#[derive(Clone, Copy, PartialEq, Debug)]
pub enum Mode {
pub enum TestMode {
Copy link
Member

Choose a reason for hiding this comment

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

How does TestMode differ from a "test suite"? One mode maps to 1-N test suites? And a test suites is essentially just a directory within tests?

Copy link
Member Author

Choose a reason for hiding this comment

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

It's roughly intended to be a 1-to-N mapping, i.e. each test mode can correspond to multiple test suites.

compiletest-test-mode

Copy link
Member Author

Choose a reason for hiding this comment

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

However, note that tests/coverage/ is a special case: it is TestSuite::Coverage, but the same test files can be run under two test modes, TestMode::{CoverageMap,CoverageRun}.

Pretty => "pretty",
DebugInfo => "debuginfo",
Codegen => "codegen",
Expand All @@ -34,7 +34,7 @@ string_enum! {
}
}

impl Mode {
impl TestMode {
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.
Expand Down Expand Up @@ -147,7 +147,7 @@ pub enum Sanitizer {
/// (for changed test detection).
#[derive(Debug, Clone)]
Copy link
Member Author

Choose a reason for hiding this comment

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

Remark: I was wondering what a default Config even means, turns out it's simply unused. Ditto on test mode.

pub struct Config {
/// Some test [`Mode`]s support [snapshot testing], where a *reference snapshot* of outputs (of
/// Some [`TestMode`]s support [snapshot testing], where a *reference snapshot* of outputs (of
/// `stdout`, `stderr`, or other form of artifacts) can be compared to the *actual output*.
///
/// This option can be set to `true` to update the *reference snapshots* in-place, otherwise
Expand Down Expand Up @@ -269,20 +269,20 @@ pub struct Config {
/// FIXME: reconsider this string; this is hashed for test build stamp.
pub stage_id: String,

/// The test [`Mode`]. E.g. [`Mode::Ui`]. Each test mode can correspond to one or more test
/// The [`TestMode`]. E.g. [`TestMode::Ui`]. Each test mode can correspond to one or more test
/// suites.
///
/// FIXME: stop using stringly-typed test suites!
pub mode: Mode,
pub mode: TestMode,

/// The test suite.
///
/// Example: `tests/ui/` is the "UI" test *suite*, which happens to also be of the [`Mode::Ui`]
/// test *mode*.
/// Example: `tests/ui/` is the "UI" test *suite*, which happens to also be of the
/// [`TestMode::Ui`] test *mode*.
///
/// Note that the same test directory (e.g. `tests/coverage/`) may correspond to multiple test
/// modes, e.g. `tests/coverage/` can be run under both [`Mode::CoverageRun`] and
/// [`Mode::CoverageMap`].
/// modes, e.g. `tests/coverage/` can be run under both [`TestMode::CoverageRun`] and
/// [`TestMode::CoverageMap`].
///
/// FIXME: stop using stringly-typed test suites!
pub suite: String,
Expand Down Expand Up @@ -538,8 +538,8 @@ pub struct Config {
// Configuration for various run-make tests frobbing things like C compilers or querying about
// various LLVM component information.
//
// FIXME: this really should be better packaged together.
// FIXME: these need better docs, e.g. for *host*, or for *target*?
// FIXME: this really should be better packaged together. FIXME: these need better docs, e.g.
// for *host*, or for *target*?
pub cc: String,
pub cxx: String,
pub cflags: String,
Expand Down
38 changes: 19 additions & 19 deletions src/tools/compiletest/src/directives.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use camino::{Utf8Path, Utf8PathBuf};
use semver::Version;
use tracing::*;

use crate::common::{Config, Debugger, FailMode, Mode, PassMode};
use crate::common::{Config, Debugger, FailMode, PassMode, TestMode};
use crate::debuggers::{extract_cdb_version, extract_gdb_version};
use crate::directives::auxiliary::{AuxProps, parse_and_update_aux};
use crate::directives::needs::CachedNeedsConditions;
Expand Down Expand Up @@ -328,7 +328,7 @@ impl TestProps {
props.exec_env.push(("RUSTC".to_string(), config.rustc_path.to_string()));

match (props.pass_mode, props.fail_mode) {
(None, None) if config.mode == Mode::Ui => props.fail_mode = Some(FailMode::Check),
(None, None) if config.mode == TestMode::Ui => props.fail_mode = Some(FailMode::Check),
(Some(_), Some(_)) => panic!("cannot use a *-fail and *-pass mode together"),
_ => {}
}
Expand Down Expand Up @@ -609,11 +609,11 @@ impl TestProps {
self.failure_status = Some(101);
}

if config.mode == Mode::Incremental {
if config.mode == TestMode::Incremental {
self.incremental = true;
}

if config.mode == Mode::Crashes {
if config.mode == TestMode::Crashes {
// we don't want to pollute anything with backtrace-files
// also turn off backtraces in order to save some execution
// time on the tests; we only need to know IF it crashes
Expand Down Expand Up @@ -641,11 +641,11 @@ impl TestProps {
fn update_fail_mode(&mut self, ln: &str, config: &Config) {
let check_ui = |mode: &str| {
// Mode::Crashes may need build-fail in order to trigger llvm errors or stack overflows
if config.mode != Mode::Ui && config.mode != Mode::Crashes {
if config.mode != TestMode::Ui && config.mode != TestMode::Crashes {
panic!("`{}-fail` directive is only supported in UI tests", mode);
}
};
if config.mode == Mode::Ui && config.parse_name_directive(ln, "compile-fail") {
if config.mode == TestMode::Ui && config.parse_name_directive(ln, "compile-fail") {
panic!("`compile-fail` directive is useless in UI tests");
}
let fail_mode = if config.parse_name_directive(ln, "check-fail") {
Expand All @@ -669,10 +669,10 @@ impl TestProps {

fn update_pass_mode(&mut self, ln: &str, revision: Option<&str>, config: &Config) {
let check_no_run = |s| match (config.mode, s) {
(Mode::Ui, _) => (),
(Mode::Crashes, _) => (),
(Mode::Codegen, "build-pass") => (),
(Mode::Incremental, _) => {
(TestMode::Ui, _) => (),
(TestMode::Crashes, _) => (),
(TestMode::Codegen, "build-pass") => (),
(TestMode::Incremental, _) => {
if revision.is_some() && !self.revisions.iter().all(|r| r.starts_with("cfail")) {
panic!("`{s}` directive is only supported in `cfail` incremental tests")
}
Expand Down Expand Up @@ -715,7 +715,7 @@ impl TestProps {
pub fn update_add_core_stubs(&mut self, ln: &str, config: &Config) {
let add_core_stubs = config.parse_name_directive(ln, directives::ADD_CORE_STUBS);
if add_core_stubs {
if !matches!(config.mode, Mode::Ui | Mode::Codegen | Mode::Assembly) {
if !matches!(config.mode, TestMode::Ui | TestMode::Codegen | TestMode::Assembly) {
panic!(
"`add-core-stubs` is currently only supported for ui, codegen and assembly test modes"
);
Expand Down Expand Up @@ -833,7 +833,7 @@ pub(crate) struct CheckDirectiveResult<'ln> {

pub(crate) fn check_directive<'a>(
directive_ln: &'a str,
mode: Mode,
mode: TestMode,
original_line: &str,
) -> CheckDirectiveResult<'a> {
let (directive_name, post) = directive_ln.split_once([':', ' ']).unwrap_or((directive_ln, ""));
Expand All @@ -842,11 +842,11 @@ pub(crate) fn check_directive<'a>(
let is_known = |s: &str| {
KNOWN_DIRECTIVE_NAMES.contains(&s)
|| match mode {
Mode::Rustdoc | Mode::RustdocJson => {
TestMode::Rustdoc | TestMode::RustdocJson => {
original_line.starts_with("//@")
&& match mode {
Mode::Rustdoc => KNOWN_HTMLDOCCK_DIRECTIVE_NAMES,
Mode::RustdocJson => KNOWN_JSONDOCCK_DIRECTIVE_NAMES,
TestMode::Rustdoc => KNOWN_HTMLDOCCK_DIRECTIVE_NAMES,
TestMode::RustdocJson => KNOWN_JSONDOCCK_DIRECTIVE_NAMES,
_ => unreachable!(),
}
.contains(&s)
Expand All @@ -868,7 +868,7 @@ pub(crate) fn check_directive<'a>(
const COMPILETEST_DIRECTIVE_PREFIX: &str = "//@";

fn iter_directives(
mode: Mode,
mode: TestMode,
_suite: &str,
poisoned: &mut bool,
testfile: &Utf8Path,
Expand All @@ -883,7 +883,7 @@ fn iter_directives(
// specify them manually in every test file.
//
// FIXME(jieyouxu): I feel like there's a better way to do this, leaving for later.
if mode == Mode::CoverageRun {
if mode == TestMode::CoverageRun {
let extra_directives: &[&str] = &[
"needs-profiler-runtime",
// FIXME(pietroalbini): this test currently does not work on cross-compiled targets
Expand Down Expand Up @@ -964,7 +964,7 @@ impl Config {
["CHECK", "COM", "NEXT", "SAME", "EMPTY", "NOT", "COUNT", "DAG", "LABEL"];

if let Some(raw) = self.parse_name_value_directive(line, "revisions") {
if self.mode == Mode::RunMake {
if self.mode == TestMode::RunMake {
panic!("`run-make` tests do not support revisions: {}", testfile);
}

Expand All @@ -981,7 +981,7 @@ impl Config {
);
}

if matches!(self.mode, Mode::Assembly | Mode::Codegen | Mode::MirOpt)
if matches!(self.mode, TestMode::Assembly | TestMode::Codegen | TestMode::MirOpt)
&& FILECHECK_FORBIDDEN_REVISION_NAMES.contains(&revision)
{
panic!(
Expand Down
4 changes: 2 additions & 2 deletions src/tools/compiletest/src/directives/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use super::{
DirectivesCache, EarlyProps, extract_llvm_version, extract_version_range, iter_directives,
parse_normalize_rule,
};
use crate::common::{Config, Debugger, Mode};
use crate::common::{Config, Debugger, TestMode};
use crate::executor::{CollectedTestDesc, ShouldPanic};

fn make_test_description<R: Read>(
Expand Down Expand Up @@ -785,7 +785,7 @@ fn threads_support() {

fn run_path(poisoned: &mut bool, path: &Utf8Path, buf: &[u8]) {
let rdr = std::io::Cursor::new(&buf);
iter_directives(Mode::Ui, "ui", poisoned, path, rdr, &mut |_| {});
iter_directives(TestMode::Ui, "ui", poisoned, path, rdr, &mut |_| {});
}

#[test]
Expand Down
20 changes: 10 additions & 10 deletions src/tools/compiletest/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,8 @@ use walkdir::WalkDir;

use self::directives::{EarlyProps, make_test_description};
use crate::common::{
CompareMode, Config, Debugger, Mode, PassMode, TestPaths, UI_EXTENSIONS, expected_output_path,
output_base_dir, output_relative_path,
CompareMode, Config, Debugger, PassMode, TestMode, TestPaths, UI_EXTENSIONS,
expected_output_path, output_base_dir, output_relative_path,
};
use crate::directives::DirectivesCache;
use crate::executor::{CollectedTest, ColorConfig, OutputFormat};
Expand Down Expand Up @@ -268,7 +268,7 @@ pub fn parse_config(args: Vec<String>) -> Config {
let with_rustc_debug_assertions = matches.opt_present("with-rustc-debug-assertions");
let with_std_debug_assertions = matches.opt_present("with-std-debug-assertions");
let mode = matches.opt_str("mode").unwrap().parse().expect("invalid mode");
let has_html_tidy = if mode == Mode::Rustdoc {
let has_html_tidy = if mode == TestMode::Rustdoc {
Command::new("tidy")
.arg("--version")
.stdout(Stdio::null())
Expand All @@ -279,7 +279,7 @@ pub fn parse_config(args: Vec<String>) -> Config {
false
};
let has_enzyme = matches.opt_present("has-enzyme");
let filters = if mode == Mode::RunMake {
let filters = if mode == TestMode::RunMake {
matches
.free
.iter()
Expand Down Expand Up @@ -545,7 +545,7 @@ pub fn run_tests(config: Arc<Config>) {
unsafe { env::set_var("TARGET", &config.target) };

let mut configs = Vec::new();
if let Mode::DebugInfo = config.mode {
if let TestMode::DebugInfo = config.mode {
// Debugging emscripten code doesn't make sense today
if !config.target.contains("emscripten") {
match config.debugger {
Expand Down Expand Up @@ -783,7 +783,7 @@ fn collect_tests_from_dir(
}

// For run-make tests, a "test file" is actually a directory that contains an `rmake.rs`.
if cx.config.mode == Mode::RunMake {
if cx.config.mode == TestMode::RunMake {
let mut collector = TestCollector::new();
if dir.join("rmake.rs").exists() {
let paths = TestPaths {
Expand Down Expand Up @@ -869,7 +869,7 @@ fn make_test(cx: &TestCollectorCx, collector: &mut TestCollector, testpaths: &Te
// For run-make tests, each "test file" is actually a _directory_ containing an `rmake.rs`. But
// for the purposes of directive parsing, we want to look at that recipe file, not the directory
// itself.
let test_path = if cx.config.mode == Mode::RunMake {
let test_path = if cx.config.mode == TestMode::RunMake {
testpaths.file.join("rmake.rs")
} else {
testpaths.file.clone()
Expand All @@ -884,7 +884,7 @@ fn make_test(cx: &TestCollectorCx, collector: &mut TestCollector, testpaths: &Te
// - Incremental tests inherently can't run their revisions in parallel, so
// we treat them like non-revisioned tests here. Incremental revisions are
// handled internally by `runtest::run` instead.
let revisions = if early_props.revisions.is_empty() || cx.config.mode == Mode::Incremental {
let revisions = if early_props.revisions.is_empty() || cx.config.mode == TestMode::Incremental {
vec![None]
} else {
early_props.revisions.iter().map(|r| Some(r.as_str())).collect()
Expand Down Expand Up @@ -1116,11 +1116,11 @@ fn check_for_overlapping_test_paths(found_path_stems: &HashSet<Utf8PathBuf>) {
}

pub fn early_config_check(config: &Config) {
if !config.has_html_tidy && config.mode == Mode::Rustdoc {
if !config.has_html_tidy && config.mode == TestMode::Rustdoc {
warning!("`tidy` (html-tidy.org) is not installed; diffs will not be generated");
}

if !config.profiler_runtime && config.mode == Mode::CoverageRun {
if !config.profiler_runtime && config.mode == TestMode::CoverageRun {
let actioned = if config.bless { "blessed" } else { "checked" };
warning!("profiler runtime is not available, so `.coverage` files won't be {actioned}");
help!("try setting `profiler = true` in the `[build]` section of `bootstrap.toml`");
Expand Down
2 changes: 1 addition & 1 deletion src/tools/compiletest/src/runtest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -349,7 +349,7 @@ impl<'test> TestCx<'test> {
if proc_res.status.success() {
{
self.error(&format!("{} test did not emit an error", self.config.mode));
if self.config.mode == crate::common::Mode::Ui {
if self.config.mode == crate::common::TestMode::Ui {
println!("note: by default, ui tests are expected not to compile");
}
proc_res.fatal(None, || ());
Expand Down
2 changes: 1 addition & 1 deletion src/tools/rustdoc-gui-test/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ If you want to install the `browser-ui-test` dependency, run `npm install browse
if let Some(librs) = find_librs(entry.path()) {
let compiletest_c = compiletest::common::Config {
edition: None,
mode: compiletest::common::Mode::Rustdoc,
mode: compiletest::common::TestMode::Rustdoc,
..Default::default()
};

Expand Down