bootstrap: Use a CompiletestMode enum instead of bare strings
#149755
Conversation
rustbot
added
A-testsuite
This comment has been minimized.
This comment has been minimized.
| // (This file should be split up, but having tidy block all changes is not helpful.) | ||
| // ignore-tidy-filelength | ||
|
|
There was a problem hiding this comment.
I don't know why this file is suddenly failing tidy, when it has been well over 3000 lines for a long time.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
| if matches!( | ||
| mode, | ||
| CompiletestMode::RunMake | ||
| | CompiletestMode::Rustdoc | ||
| | CompiletestMode::RustdocJs | ||
| | CompiletestMode::RustdocJson | ||
| ) || matches!(suite, "rustdoc-ui" | "coverage-run-rustdoc") |
There was a problem hiding this comment.
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.
This comment has been minimized.
This comment has been minimized.
|
Ugh, build failures in |
|
So one thing I was wondering is if we should be pulling {TestMode, TestSuite} from compiletest -> build_helpers then share that in bootstrap. The downside is that where do you put the stringify and stuff? Putting those in build_helpers seem fine. Though there's some prerequisite cleanups in compiletest to get rid of e.g. test build dir calculations that's added on {TestMode} or sth. |
|
That being said, I'm also perfectly fine with keeping a separate copy in bootstrap, it's still way better than string matching |
|
Yeah, I think having a duplicated enum is a clear win over bare strings, whereas trying to deduplicate the enum adds more complexity and hassle that isn't obviously worthwhile. |
|
@rustbot author |
rustbot
added
S-waiting-on-author
This derive was an artifact of test-only method `Cache::all` wanting to automatically sort its output to hide HashMap iteration order. We can achieve an equivalent result by requiring the caller to provide a projection function that returns results that _are_ sortable.
|
This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
|
Rebased to fix a trivial conflict. @bors r=jieyouxu |
bors
added
S-waiting-on-bors
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)
Rollup of 12 pull requests Successful merges: - #147572 (std: Use more `unix.rs` code on WASI targets) - #148491 ( Correctly provide suggestions when encountering `async fn` with a `dyn Trait` return type) - #149215 (Emit `check-cfg` lints during attribute parsing rather than evaluation) - #149652 (Add release notes for 1.92.0) - #149720 (rustdoc book: mention inner doc attribute) - #149730 (lint: emit proper diagnostic for unsafe binders in improper_ctypes instead of ICE) - #149754 (Retire `opt_str2` from compiletest cli parsing) - #149755 (bootstrap: Use a `CompiletestMode` enum instead of bare strings) - #149763 (Add inline attribute to generated delegation function if needed) - #149772 (test: Add a test for 146133) - #149779 (Fix typo "an" → "and") - #149782 (Remove `[no-mentions]` handler in the triagebot config) r? `@ghost` `@rustbot` modify labels: rollup
Rollup of 11 pull requests Successful merges: - #147585 (Suppress the error for private fields with non_exhaustive attribute) - #149215 (Emit `check-cfg` lints during attribute parsing rather than evaluation) - #149652 (Add release notes for 1.92.0) - #149720 (rustdoc book: mention inner doc attribute) - #149730 (lint: emit proper diagnostic for unsafe binders in improper_ctypes instead of ICE) - #149754 (Retire `opt_str2` from compiletest cli parsing) - #149755 (bootstrap: Use a `CompiletestMode` enum instead of bare strings) - #149763 (Add inline attribute to generated delegation function if needed) - #149772 (test: Add a test for 146133) - #149779 (Fix typo "an" → "and") - #149782 (Remove `[no-mentions]` handler in the triagebot config) Failed merges: - #148491 ( Correctly provide suggestions when encountering `async fn` with a `dyn Trait` return type) r? `@ghost` `@rustbot` modify labels: rollup
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)
5 participants
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)