Skip to content

Prevent should-fail to be used in rustdoc-ui testsuite - #162446

Merged
rust-bors[bot] merged 1 commit into
rust-lang:mainfrom
GuillaumeGomez:rustdoc-ui-should-fail
Sep 11, 2026
Merged

Prevent should-fail to be used in rustdoc-ui testsuite#162446
rust-bors[bot] merged 1 commit into
rust-lang:mainfrom
GuillaumeGomez:rustdoc-ui-should-fail

Conversation

@GuillaumeGomez

@GuillaumeGomez GuillaumeGomez commented Sep 7, 2026

Copy link
Copy Markdown
Member

Follow-up of #162030 and discussion with @fmease.

There is no reason to use should-fail for rustdoc-ui (or rustdoc-html) testsuite, so let's prevent it.

r? @fmease

@rustbot

rustbot commented Sep 7, 2026

Copy link
Copy Markdown
Collaborator

Some changes occurred in src/tools/compiletest

cc @jieyouxu

compiletest directives have been modified. Please add or update docs for the
new or modified directive in src/doc/rustc-dev-guide/.

@rustbot rustbot added A-compiletest Area: The compiletest test runner A-testsuite Area: The testsuite used to check the correctness of rustc S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. labels Sep 7, 2026
@Zalathar

Zalathar commented Sep 8, 2026

Copy link
Copy Markdown
Member

We really should rename should-fail to something scarier at some point; it’s such an attractive footgun that only exists for a narrow and specific purpose (compiletest self-testing).

@rust-bors

This comment has been minimized.

Sign in to view
@rustbot

rustbot commented Sep 11, 2026

Copy link
Copy Markdown
Collaborator

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.

@GuillaumeGomez

GuillaumeGomez commented Sep 11, 2026

Copy link
Copy Markdown
Member Author

r? @Kobzol

@rustbot rustbot assigned Kobzol and unassigned fmease Sep 11, 2026
@rustbot

rustbot commented Sep 11, 2026

Copy link
Copy Markdown
Collaborator

Kobzol is not on the review rotation at the moment.
They may take a while to respond.

@fmease

fmease commented Sep 11, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Should we rename it to self-test-should-fail or something like that instead? I figure we don't want to use it in any of the test suites except for self tests (are they in their own test suite? if so we could just say "reject unless suite is self")

@GuillaumeGomez

GuillaumeGomez commented Sep 11, 2026

Copy link
Copy Markdown
Member Author

Sounds like a discussion for another issue/PR. 😉

@Kobzol

Kobzol commented Sep 11, 2026

Copy link
Copy Markdown
Member

The goal looks reasonable, I'm not completely sure about the implementation though, as I'm not familiar with the directives much, so:

r? jieyouxu

@rustbot rustbot assigned jieyouxu and unassigned Kobzol Sep 11, 2026
@jieyouxu

jieyouxu commented Sep 11, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Should we rename it to self-test-should-fail or something like that instead? I figure we don't want to use it in none of the test suites except for self tests (are they in their own test suite? if so we could just say "reject unless suite is self")

Yeah, we can probably do that in a follow-up. IIRC should-fail goes way back.

jieyouxu
jieyouxu approved these changes Sep 11, 2026
View reviewed changes

@jieyouxu jieyouxu left a comment •
edited by rustbot
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks, this is fine. We can make renames etc. to should-fail in a follow-up.

@bors r+ rollup

View changes since this review

@rust-bors

rust-bors Bot commented Sep 11, 2026

Copy link
Copy Markdown
Contributor

📌 Commit c03ff79 has been approved by jieyouxu

It is now in the queue for this repository.

@rust-bors rust-bors Bot 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 Sep 11, 2026
JonathanBrouwer added a commit to JonathanBrouwer/rust that referenced this pull request Sep 11, 2026
@JonathanBrouwer
Rollup merge of rust-lang#162446 - GuillaumeGomez:rustdoc-ui-should-f…
a5e64e6 
…ail, r=jieyouxu

Prevent `should-fail` to be used in `rustdoc-ui` testsuite

Follow-up of rust-lang#162030 and discussion with @fmease.

There is no reason to use `should-fail` for `rustdoc-ui` (or `rustdoc-html`) testsuite, so let's prevent it.

r? @fmease
This was referenced Sep 11, 2026
Closed
Merged
rust-bors Bot pushed a commit that referenced this pull request Sep 11, 2026
@bors
Auto merge of #162664 - JonathanBrouwer:rollup-s1jX04L, r=JonathanBro…
0fc1413 
…uwer

Rollup of 7 pull requests

Successful merges:

 - #161413 (mir-transform: Remove `is_optimization_stage`)
 - #161430 (Re-export `core::fmt::NumBuffer` in `alloc` (and `std`))
 - #162446 (Prevent `should-fail` to be used in `rustdoc-ui` testsuite)
 - #162621 (Clarify examples related to `Sync` and `SyncView`)
 - #162650 (Initialize mingw for all mingw targets)
 - #162651 (Remove needless test running instructions)
 - #162658 (Remove noras target docs mention)
@rust-bors
rust-bors Bot merged commit 53b596e into rust-lang:main Sep 11, 2026
13 checks passed
@rustbot rustbot added this to the 1.100.0 milestone Sep 11, 2026
rust-bors Bot pushed a commit that referenced this pull request Sep 11, 2026
@bors
Unrolled build for #162446 in rollup 162664
30fa8b1 
Rollup merge of #162446 - GuillaumeGomez:rustdoc-ui-should-fail, r=jieyouxu

Prevent `should-fail` to be used in `rustdoc-ui` testsuite

Follow-up of #162030 and discussion with @fmease.

There is no reason to use `should-fail` for `rustdoc-ui` (or `rustdoc-html`) testsuite, so let's prevent it.

r? @fmease
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jieyouxu jieyouxu jieyouxu approved these changes

Assignees

@jieyouxu jieyouxu

Labels

A-compiletest Area: The compiletest test runner A-testsuite Area: The testsuite used to check the correctness of rustc S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.

Projects

None yet

Milestone

1.100.0

Development

Successfully merging this pull request may close these issues.

6 participants

@GuillaumeGomez @rustbot @Zalathar @fmease @Kobzol @jieyouxu