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.

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
merged 6 commits into from
Jul 13, 2025
Merged

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

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
33668c1
Remove outdated debugger version warning
jieyouxu Jul 5, 2025
832b627
Move `string_enum` to `util` module
jieyouxu Jul 12, 2025
b4dfd51
Do not allow defaults for `Mode` and `Config`
jieyouxu Jul 12, 2025
4aad9ab
Rename `Mode` to `TestMode`
jieyouxu Jul 12, 2025
b9ae117
Do not glob-reexport `TestMode` variants
jieyouxu Jul 12, 2025
6f8a7f0
Make rustdoc-gui-test dummy compiletest config purpose explicit
jieyouxu Jul 12, 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
5 changes: 2 additions & 3 deletions src/tools/compiletest/src/common.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ use camino::{Utf8Path, Utf8PathBuf};
use semver::Version;
use serde::de::{Deserialize, Deserializer, Error as _};

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.

Expand Down Expand Up @@ -39,7 +38,7 @@ impl TestMode {
// Pretty-printing tests could run concurrently, and if they do,
// they need to keep their output segregated.
match self {
Pretty => ".pretty",
TestMode::Pretty => ".pretty",
_ => "",
}
}
Expand All @@ -48,7 +47,7 @@ impl TestMode {
// Coverage tests use the same test files for multiple test modes,
// so each mode should have a separate output directory.
match self {
CoverageMap | CoverageRun => self.to_str(),
TestMode::CoverageMap | TestMode::CoverageRun => self.to_str(),
_ => "",
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/tools/compiletest/src/directives.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1443,7 +1443,7 @@ pub(crate) fn make_test_description<R: Read>(
// since we run the pretty printer across all tests by default.
// If desired, we could add a `should-fail-pretty` annotation.
let should_panic = match config.mode {
crate::common::Pretty => ShouldPanic::No,
TestMode::Pretty => ShouldPanic::No,
_ if should_fail => ShouldPanic::Yes,
_ => ShouldPanic::No,
};
Expand Down
111 changes: 62 additions & 49 deletions src/tools/compiletest/src/runtest.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,10 @@ use regex::{Captures, Regex};
use tracing::*;

use crate::common::{
Assembly, Codegen, CodegenUnits, CompareMode, Config, CoverageMap, CoverageRun, Crashes,
DebugInfo, Debugger, FailMode, Incremental, MirOpt, PassMode, Pretty, RunMake, Rustdoc,
RustdocJs, RustdocJson, TestPaths, UI_EXTENSIONS, UI_FIXED, UI_RUN_STDERR, UI_RUN_STDOUT,
UI_STDERR, UI_STDOUT, UI_SVG, UI_WINDOWS_SVG, Ui, expected_output_path, incremental_dir,
output_base_dir, output_base_name, output_testname_unique,
Comment on lines -19 to -23
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: also, I just find this quite nasty.

CompareMode, Config, Debugger, FailMode, PassMode, TestMode, TestPaths, UI_EXTENSIONS,
UI_FIXED, UI_RUN_STDERR, UI_RUN_STDOUT, UI_STDERR, UI_STDOUT, UI_SVG, UI_WINDOWS_SVG,
expected_output_path, incremental_dir, output_base_dir, output_base_name,
output_testname_unique,
};
use crate::compute_diff::{DiffLine, make_diff, write_diff, write_filtered_diff};
use crate::directives::TestProps;
Expand Down Expand Up @@ -154,7 +153,7 @@ pub fn run(config: Arc<Config>, testpaths: &TestPaths, revision: Option<&str>) {
cx.init_incremental_test();
}

if config.mode == Incremental {
if config.mode == TestMode::Incremental {
// Incremental tests are special because they cannot be run in
// parallel.
assert!(!props.revisions.is_empty(), "Incremental tests require revisions.");
Expand Down Expand Up @@ -203,7 +202,7 @@ pub fn compute_stamp_hash(config: &Config) -> String {
None => {}
}

if let Ui = config.mode {
if config.mode == TestMode::Ui {
config.force_pass_mode.hash(&mut hash);
Comment on lines +205 to 206
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: as a concrete example, tests/ui/ is both TestMode::Ui and TestSuite::Ui, but these are very different things and must not be mixed up, because a test mode can correspond to multiple test suites.

(TestSuite is something I want to introduce over stringly-typed test suite names in a follow-up.)

Copy link
Member Author

Choose a reason for hiding this comment

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

#143870

}

Expand Down Expand Up @@ -251,25 +250,28 @@ impl<'test> TestCx<'test> {
/// Code executed for each revision in turn (or, if there are no
/// revisions, exactly once, with revision == None).
fn run_revision(&self) {
if self.props.should_ice && self.config.mode != Incremental && self.config.mode != Crashes {
if self.props.should_ice
&& self.config.mode != TestMode::Incremental
&& self.config.mode != TestMode::Crashes
{
self.fatal("cannot use should-ice in a test that is not cfail");
}
match self.config.mode {
Pretty => self.run_pretty_test(),
DebugInfo => self.run_debuginfo_test(),
Codegen => self.run_codegen_test(),
Rustdoc => self.run_rustdoc_test(),
RustdocJson => self.run_rustdoc_json_test(),
CodegenUnits => self.run_codegen_units_test(),
Incremental => self.run_incremental_test(),
RunMake => self.run_rmake_test(),
Ui => self.run_ui_test(),
MirOpt => self.run_mir_opt_test(),
Assembly => self.run_assembly_test(),
RustdocJs => self.run_rustdoc_js_test(),
CoverageMap => self.run_coverage_map_test(), // see self::coverage
CoverageRun => self.run_coverage_run_test(), // see self::coverage
Crashes => self.run_crash_test(),
TestMode::Pretty => self.run_pretty_test(),
TestMode::DebugInfo => self.run_debuginfo_test(),
TestMode::Codegen => self.run_codegen_test(),
TestMode::Rustdoc => self.run_rustdoc_test(),
TestMode::RustdocJson => self.run_rustdoc_json_test(),
TestMode::CodegenUnits => self.run_codegen_units_test(),
TestMode::Incremental => self.run_incremental_test(),
TestMode::RunMake => self.run_rmake_test(),
TestMode::Ui => self.run_ui_test(),
TestMode::MirOpt => self.run_mir_opt_test(),
TestMode::Assembly => self.run_assembly_test(),
TestMode::RustdocJs => self.run_rustdoc_js_test(),
TestMode::CoverageMap => self.run_coverage_map_test(), // see self::coverage
TestMode::CoverageRun => self.run_coverage_run_test(), // see self::coverage
TestMode::Crashes => self.run_crash_test(),
}
}

Expand All @@ -279,9 +281,13 @@ impl<'test> TestCx<'test> {

fn should_run(&self, pm: Option<PassMode>) -> WillExecute {
let test_should_run = match self.config.mode {
Ui if pm == Some(PassMode::Run) || self.props.fail_mode == Some(FailMode::Run) => true,
MirOpt if pm == Some(PassMode::Run) => true,
Ui | MirOpt => false,
TestMode::Ui
if pm == Some(PassMode::Run) || self.props.fail_mode == Some(FailMode::Run) =>
{
true
}
TestMode::MirOpt if pm == Some(PassMode::Run) => true,
TestMode::Ui | TestMode::MirOpt => false,
mode => panic!("unimplemented for mode {:?}", mode),
};
if test_should_run { self.run_if_enabled() } else { WillExecute::No }
Expand All @@ -293,17 +299,17 @@ impl<'test> TestCx<'test> {

fn should_run_successfully(&self, pm: Option<PassMode>) -> bool {
match self.config.mode {
Ui | MirOpt => pm == Some(PassMode::Run),
TestMode::Ui | TestMode::MirOpt => pm == Some(PassMode::Run),
mode => panic!("unimplemented for mode {:?}", mode),
}
}

fn should_compile_successfully(&self, pm: Option<PassMode>) -> bool {
match self.config.mode {
RustdocJs => true,
Ui => pm.is_some() || self.props.fail_mode > Some(FailMode::Build),
Crashes => false,
Incremental => {
TestMode::RustdocJs => true,
TestMode::Ui => pm.is_some() || self.props.fail_mode > Some(FailMode::Build),
TestMode::Crashes => false,
TestMode::Incremental => {
let revision =
self.revision.expect("incremental tests require a list of revisions");
if revision.starts_with("cpass")
Expand Down Expand Up @@ -892,7 +898,9 @@ impl<'test> TestCx<'test> {

fn should_emit_metadata(&self, pm: Option<PassMode>) -> Emit {
match (pm, self.props.fail_mode, self.config.mode) {
(Some(PassMode::Check), ..) | (_, Some(FailMode::Check), Ui) => Emit::Metadata,
(Some(PassMode::Check), ..) | (_, Some(FailMode::Check), TestMode::Ui) => {
Emit::Metadata
}
_ => Emit::None,
}
}
Expand Down Expand Up @@ -926,7 +934,7 @@ impl<'test> TestCx<'test> {
};

let allow_unused = match self.config.mode {
Ui => {
TestMode::Ui => {
// UI tests tend to have tons of unused code as
// it's just testing various pieces of the compile, but we don't
// want to actually assert warnings about all this code. Instead
Expand Down Expand Up @@ -1021,7 +1029,7 @@ impl<'test> TestCx<'test> {
.args(&self.props.compile_flags)
.args(&self.props.doc_flags);

if self.config.mode == RustdocJson {
if self.config.mode == TestMode::RustdocJson {
rustdoc.arg("--output-format").arg("json").arg("-Zunstable-options");
}

Expand Down Expand Up @@ -1372,7 +1380,7 @@ impl<'test> TestCx<'test> {
|| self.is_vxworks_pure_static()
|| self.config.target.contains("bpf")
|| !self.config.target_cfg().dynamic_linking
|| matches!(self.config.mode, CoverageMap | CoverageRun)
|| matches!(self.config.mode, TestMode::CoverageMap | TestMode::CoverageRun)
{
// We primarily compile all auxiliary libraries as dynamic libraries
// to avoid code size bloat and large binaries as much as possible
Expand Down Expand Up @@ -1562,14 +1570,14 @@ impl<'test> TestCx<'test> {
rustc.args(&["-Z", "incremental-verify-ich"]);
}

if self.config.mode == CodegenUnits {
if self.config.mode == TestMode::CodegenUnits {
rustc.args(&["-Z", "human_readable_cgu_names"]);
}
}

if self.config.optimize_tests && !is_rustdoc {
match self.config.mode {
Ui => {
TestMode::Ui => {
// If optimize-tests is true we still only want to optimize tests that actually get
// executed and that don't specify their own optimization levels.
// Note: aux libs don't have a pass-mode, so they won't get optimized
Expand All @@ -1585,8 +1593,8 @@ impl<'test> TestCx<'test> {
rustc.arg("-O");
}
}
DebugInfo => { /* debuginfo tests must be unoptimized */ }
CoverageMap | CoverageRun => {
TestMode::DebugInfo => { /* debuginfo tests must be unoptimized */ }
TestMode::CoverageMap | TestMode::CoverageRun => {
// Coverage mappings and coverage reports are affected by
// optimization level, so they ignore the optimize-tests
// setting and set an optimization level in their mode's
Expand All @@ -1607,7 +1615,7 @@ impl<'test> TestCx<'test> {
};

match self.config.mode {
Incremental => {
TestMode::Incremental => {
// If we are extracting and matching errors in the new
// fashion, then you want JSON mode. Old-skool error
// patterns still match the raw compiler output.
Expand All @@ -1620,7 +1628,7 @@ impl<'test> TestCx<'test> {
rustc.arg("-Zui-testing");
rustc.arg("-Zdeduplicate-diagnostics=no");
}
Ui => {
TestMode::Ui => {
if !self.props.compile_flags.iter().any(|s| s.starts_with("--error-format")) {
rustc.args(&["--error-format", "json"]);
rustc.args(&["--json", "future-incompat"]);
Expand All @@ -1633,7 +1641,7 @@ impl<'test> TestCx<'test> {
// FIXME: use this for other modes too, for perf?
rustc.arg("-Cstrip=debuginfo");
}
MirOpt => {
TestMode::MirOpt => {
// We check passes under test to minimize the mir-opt test dump
// if files_for_miropt_test parses the passes, we dump only those passes
// otherwise we conservatively pass -Zdump-mir=all
Expand Down Expand Up @@ -1663,7 +1671,7 @@ impl<'test> TestCx<'test> {

set_mir_dump_dir(&mut rustc);
}
CoverageMap => {
TestMode::CoverageMap => {
rustc.arg("-Cinstrument-coverage");
// These tests only compile to LLVM IR, so they don't need the
// profiler runtime to be present.
Expand All @@ -1673,23 +1681,28 @@ impl<'test> TestCx<'test> {
// by `compile-flags`.
rustc.arg("-Copt-level=2");
}
CoverageRun => {
TestMode::CoverageRun => {
rustc.arg("-Cinstrument-coverage");
// Coverage reports are sometimes sensitive to optimizations,
// and the current snapshots assume `opt-level=2` unless
// overridden by `compile-flags`.
rustc.arg("-Copt-level=2");
}
Assembly | Codegen => {
TestMode::Assembly | TestMode::Codegen => {
rustc.arg("-Cdebug-assertions=no");
}
Crashes => {
TestMode::Crashes => {
set_mir_dump_dir(&mut rustc);
}
CodegenUnits => {
TestMode::CodegenUnits => {
rustc.arg("-Zprint-mono-items");
}
Pretty | DebugInfo | Rustdoc | RustdocJson | RunMake | RustdocJs => {
TestMode::Pretty
| TestMode::DebugInfo
| TestMode::Rustdoc
| TestMode::RustdocJson
| TestMode::RunMake
| TestMode::RustdocJs => {
// do not use JSON output
}
}
Expand Down Expand Up @@ -1962,7 +1975,7 @@ impl<'test> TestCx<'test> {
/// The revision, ignored for incremental compilation since it wants all revisions in
/// the same directory.
fn safe_revision(&self) -> Option<&str> {
if self.config.mode == Incremental { None } else { self.revision }
if self.config.mode == TestMode::Incremental { None } else { self.revision }
}

/// Gets the absolute path to the directory where all output for the given
Expand Down
Loading