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

jieyouxu · 2025-07-12T08:31:36Z

This is part of a patch series to untangle compiletest to hopefully nudge it towards being more maintainable.

This PR should contain no functional changes modulo the removed debugger version warning.

Commit 1: Removes a very outdated debugger version warning.
Commit 2: Moves string_enum out of common into util module.
Commit 3: Remove #[derive(Default) for Mode and Config. It is very important for correctness that we don't #[derive(Default)], because there are no sensible defaults, so stop pretending there is.
Commit 4: Rename Mode -> TestMode, because I would like to introduce a TestSuite enum to stop using stringly-typed test suite names where test mode names can be the same as test suite names, and we also have a bunch of other "modes" in compiletest. Make this as unambiguous as possible. A corollary is that now it's more natural to reference via intra-doc links as [`TestMode`].
Commit 5: Ditto on TestSuite, stop glob-reexporting TestMode::* variants, and always use EnumName::VariantName form.
Commit 6: Apparently, src/tools/rustdoc-gui-test/ depends on compiletest for //@ {compile,run}-paths directive parsing and extraction, which involves creating a dummy compiletest config (hence the existence of the default impls removed in Commit 3). Make this a specific associated function with a FIXME pointing to src/tools/rustdoc-gui-test uses compiletest directives + TestProp handling internals in a questionable way #143827 as I think this setup is quite questionable.

Commits {4, 5} are also intended to help improve the self-consistency in nomenclature used within compiletest.

Best reviewed commit-by-commit.

They do not have sensible defaults, and it is crucial that we get them right.

It is *critical* that we maintain clear nomenclature in `compiletest`. We have many types of "modes" in `compiletest` -- pass modes, coverage modes, compare modes, you name it. `Mode` is also a *super* general term. Rename it to `TestMode` to leave no room for such ambiguity. As a follow-up, I also intend to introduce an enum for `TestSuite`, then rid of all usage of glob re-exported `TestMode::*` enum variants -- many test suites share the same name as the test mode.

I would like to introduce `TestSuite` over stringly-typed test suite names, and some test suite names are the same as test modes, which can make this very confusing.

rustbot · 2025-07-12T08:31:40Z

r? @compiler-errors

rustbot has assigned @compiler-errors.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

jieyouxu · 2025-07-12T08:34:11Z

src/tools/compiletest/src/common.rs

-// Make the macro visible outside of this module, for tests.
-#[cfg(test)]
-pub(crate) use string_enum;
+use crate::util::{Utf8PathBufExt, add_dylib_path, string_enum};


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.

jieyouxu · 2025-07-12T08:34:55Z

src/tools/compiletest/src/common.rs

@@ -151,7 +145,7 @@ pub enum Sanitizer {
 ///
 /// FIXME: audit these options to make sure we are not hashing less than necessary for build stamp
 /// (for changed test detection).
-#[derive(Debug, Default, Clone)]
+#[derive(Debug, Clone)]


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

jieyouxu · 2025-07-12T08:36:43Z

src/tools/compiletest/src/runtest.rs

+    if config.mode == TestMode::Ui {
        config.force_pass_mode.hash(&mut hash);


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.)

jieyouxu · 2025-07-12T08:37:42Z

src/tools/compiletest/src/runtest.rs

-    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,


Remark: also, I just find this quite nasty.

Kobzol · 2025-07-12T08:46:54Z

r? kobzol

Kobzol

Nice little cleanup! Left one question, but it's not blocking. Feel free to r=me after CI is green.

Kobzol · 2025-07-12T09:17:38Z

src/tools/compiletest/src/common.rs

 use crate::executor::{ColorConfig, OutputFormat};
 use crate::fatal;
 use crate::util::{Utf8PathBufExt, add_dylib_path, string_enum};

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


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?

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

jieyouxu · 2025-07-12T09:35:26Z

Oh I see where the default is used...

jieyouxu · 2025-07-12T11:05:53Z

I pushed a new commit which (for this PR) preserves the rustdoc-gui-test dummy Config setup, but as a dedicated associated function with a FIXME pointing to #143827.

It may seem rather verbose, but I really do not want to #[derive(Default)] on Config or TestMode, as it makes it easy to shoot yourself in the foot within compiletest -- directive handling assumes that the Config is fully populated.

@rustbot review

Kobzol · 2025-07-12T11:17:00Z

Fine as a temporary solution, better than making a weird Default impl available for everyone. r=me

jieyouxu · 2025-07-12T13:30:20Z

PR CI is 🟢
@bors r=Kobzol rollup

bors · 2025-07-12T13:30:23Z

📌 Commit 6f8a7f0 has been approved by Kobzol

It is now in the queue for this repository.

…, r=Kobzol [COMPILETEST-UNTANGLE 5/N] Test mode adjustments and other assorted cleanups This is part of a patch series to untangle `compiletest` to hopefully nudge it towards being more maintainable. This PR should contain no functional changes modulo the removed debugger version warning. - Commit 1: Removes a very outdated debugger version warning. - Commit 2: Moves `string_enum` out of `common` into `util` module. - Commit 3: Remove `#[derive(Default)` for `Mode` and `Config`. It is very important for correctness that we *don't* `#[derive(Default)]`, because there are no sensible defaults, so stop pretending there is. - Commit 4: Rename `Mode` -> `TestMode`, because I would like to introduce a `TestSuite` enum to stop using stringly-typed test suite names where test mode names can be the same as test suite names, and we also have a bunch of other "modes" in compiletest. Make this as unambiguous as possible. A corollary is that now it's more natural to reference via intra-doc links as ``[`TestMode`]``. - Commit 5: Ditto on `TestSuite`, stop glob-reexporting `TestMode::*` variants, and always use `EnumName::VariantName` form. - Commit 6: Apparently, `src/tools/rustdoc-gui-test/` depends on `compiletest` for `//@ {compile,run}-paths` directive parsing and extraction, which involves creating a dummy `compiletest` config (hence the existence of the default impls removed in Commit 3). Make this a specific associated function with a FIXME pointing to rust-lang#143827 as I think this setup is quite questionable. Commits {4, 5} are also intended to help improve the self-consistency in nomenclature used within compiletest. Best reviewed commit-by-commit.

Rollup of 15 pull requests Successful merges: - #143554 (slice: Mark `rotate_left`, `rotate_right` unstably const) - #143634 (interpret/allocation: expose init + write_wildcards on a range) - #143710 (Updates to random number generation APIs) - #143774 (constify `From` and `Into`) - #143776 (std: move NuttX to use arc4random for random number generation) - #143778 (Some const_trait_impl test cleanups) - #143782 (Disambiguate between rustc vs std having debug assertions in `run-make-support` and `run-make` tests) - #143791 (Update sysinfo version to `0.36.0`) - #143796 (Fix ICE for parsed attributes with longer path not handled by CheckAttribute) - #143798 (Remove format short command trait) - #143803 (New tracking issues for const_ops and const_cmp) - #143814 (htmldocck: better error messages for some negative directives) - #143817 (Access `wasi_sdk_path` instead of reading environment variable in bootstrap) - #143822 (./x test miri: fix cleaning the miri_ui directory) - #143823 ([COMPILETEST-UNTANGLE 5/N] Test mode adjustments and other assorted cleanups) r? `@ghost` `@rustbot` modify labels: rollup

…, r=Kobzol [COMPILETEST-UNTANGLE 5/N] Test mode adjustments and other assorted cleanups This is part of a patch series to untangle `compiletest` to hopefully nudge it towards being more maintainable. This PR should contain no functional changes modulo the removed debugger version warning. - Commit 1: Removes a very outdated debugger version warning. - Commit 2: Moves `string_enum` out of `common` into `util` module. - Commit 3: Remove `#[derive(Default)` for `Mode` and `Config`. It is very important for correctness that we *don't* `#[derive(Default)]`, because there are no sensible defaults, so stop pretending there is. - Commit 4: Rename `Mode` -> `TestMode`, because I would like to introduce a `TestSuite` enum to stop using stringly-typed test suite names where test mode names can be the same as test suite names, and we also have a bunch of other "modes" in compiletest. Make this as unambiguous as possible. A corollary is that now it's more natural to reference via intra-doc links as ``[`TestMode`]``. - Commit 5: Ditto on `TestSuite`, stop glob-reexporting `TestMode::*` variants, and always use `EnumName::VariantName` form. - Commit 6: Apparently, `src/tools/rustdoc-gui-test/` depends on `compiletest` for `//@ {compile,run}-paths` directive parsing and extraction, which involves creating a dummy `compiletest` config (hence the existence of the default impls removed in Commit 3). Make this a specific associated function with a FIXME pointing to rust-lang#143827 as I think this setup is quite questionable. Commits {4, 5} are also intended to help improve the self-consistency in nomenclature used within compiletest. Best reviewed commit-by-commit.

jieyouxu added 5 commits July 12, 2025 15:46

Remove outdated debugger version warning

33668c1

Move string_enum to util module

832b627

Do not allow defaults for Mode and Config

b4dfd51

They do not have sensible defaults, and it is crucial that we get them right.

Do not glob-reexport TestMode variants

b9ae117

I would like to introduce `TestSuite` over stringly-typed test suite names, and some test suite names are the same as test modes, which can make this very confusing.

rustbot assigned compiler-errors Jul 12, 2025

jieyouxu changed the title ~~[COMPILETEST-UNTANGLE 5/N] Move some some early config checks to the lib and move the compiletest binary~~ [COMPILETEST-UNTANGLE 5/N] Test mode adjustments and other assorted cleanups Jul 12, 2025

jieyouxu commented Jul 12, 2025

View reviewed changes

jieyouxu mentioned this pull request Jul 12, 2025

[COMPILETEST-UNTANGLE 4/N] Improve compiletest config documentation #143447

Merged

rustbot assigned Kobzol and unassigned compiler-errors Jul 12, 2025

Kobzol reviewed Jul 12, 2025

View reviewed changes

This comment has been minimized.

Sign in to view

jieyouxu added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 12, 2025

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jul 12, 2025

Make rustdoc-gui-test dummy compiletest config purpose explicit

6f8a7f0

jieyouxu force-pushed the compiletest-maintenance-5 branch from 35081bb to 6f8a7f0 Compare July 12, 2025 11:07

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 12, 2025

jhpratt mentioned this pull request Jul 12, 2025

Rollup of 15 pull requests #143853

Closed

fmease mentioned this pull request Jul 13, 2025

Rollup of 12 pull requests #143867

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

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

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

jieyouxu commented Jul 12, 2025 •

edited

Loading

Uh oh!

rustbot commented Jul 12, 2025

Uh oh!

jieyouxu Jul 12, 2025

Uh oh!

jieyouxu Jul 12, 2025

Uh oh!

jieyouxu Jul 12, 2025

Uh oh!

jieyouxu Jul 12, 2025

Uh oh!

Kobzol commented Jul 12, 2025

Uh oh!

Kobzol left a comment

Uh oh!

Kobzol Jul 12, 2025

Uh oh!

jieyouxu Jul 12, 2025

Uh oh!

This comment has been minimized.

jieyouxu commented Jul 12, 2025 •

edited

Loading

Uh oh!

jieyouxu commented Jul 12, 2025 •

edited

Loading

Uh oh!

Kobzol commented Jul 12, 2025

Uh oh!

jieyouxu commented Jul 12, 2025

Uh oh!

bors commented Jul 12, 2025

Uh oh!

Uh oh!

		if config.mode == TestMode::Ui {
		config.force_pass_mode.hash(&mut hash);

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

Are you sure you want to change the base?

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

Conversation

jieyouxu commented Jul 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rustbot commented Jul 12, 2025

Uh oh!

jieyouxu Jul 12, 2025

Choose a reason for hiding this comment

Uh oh!

jieyouxu Jul 12, 2025

Choose a reason for hiding this comment

Uh oh!

jieyouxu Jul 12, 2025

Choose a reason for hiding this comment

Uh oh!

jieyouxu Jul 12, 2025

Choose a reason for hiding this comment

Uh oh!

Kobzol commented Jul 12, 2025

Uh oh!

Kobzol left a comment

Choose a reason for hiding this comment

Uh oh!

Kobzol Jul 12, 2025

Choose a reason for hiding this comment

Uh oh!

jieyouxu Jul 12, 2025

Choose a reason for hiding this comment

Uh oh!

This comment has been minimized.

jieyouxu commented Jul 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jieyouxu commented Jul 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Kobzol commented Jul 12, 2025

Uh oh!

jieyouxu commented Jul 12, 2025

Uh oh!

bors commented Jul 12, 2025

Uh oh!

Uh oh!

jieyouxu commented Jul 12, 2025 •

edited

Loading

jieyouxu commented Jul 12, 2025 •

edited

Loading

jieyouxu commented Jul 12, 2025 •

edited

Loading