Skip to content

Remove tidy checks for tests/ui/issues/ #144173

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 1 commit into from
Jul 23, 2025
Merged

Conversation

Kivooeo
Copy link
Contributor

@Kivooeo Kivooeo commented Jul 19, 2025

r? @jieyouxu

As it is making cleanup efforts more difficult.

This change was discussed here #t-compiler > Discussion for ui test suite improvements @ 💬

@rustbot
Copy link
Collaborator

rustbot commented Jul 19, 2025

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

@rustbot rustbot added A-meta Area: Issues & PRs about the rust-lang/rust repository itself A-tidy Area: The tidy tool 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) labels Jul 19, 2025
@rustbot
Copy link
Collaborator

rustbot commented Jul 19, 2025

There are changes to the tidy tool.

cc @jieyouxu

triagebot.toml has been modified, there may have been changes to the review queue.

cc @davidtwco, @wesleywiser

@Kivooeo Kivooeo changed the title removed tidy check on issues files Removed tidy checks on issues.txt Jul 19, 2025
Copy link
Member

@jieyouxu jieyouxu left a comment

Choose a reason for hiding this comment

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

Don't leave commented out code, just delete it.

@jieyouxu
Copy link
Member

jieyouxu commented Jul 19, 2025

Once work on tests/ui/issues/ is complete, this change will be reverted accordingly.

Once that work is complete, we won't need this check (there were way too many tests under tests/ui/issues/, which is why this check exists in the first place).

@Kivooeo Kivooeo force-pushed the tidy_checks branch 2 times, most recently from da05401 to 3720916 Compare July 19, 2025 10:56
@jieyouxu
Copy link
Member

spellcheck is broken for other reasons, u can ignore.

@Kivooeo
Copy link
Contributor Author

Kivooeo commented Jul 19, 2025

Should be fine now, only reason I need tidy to ignore deleted issue-*.rs files from issues/ is to completely remove any interactions with issues.txt file while working on tests

I will remove all moved tests from issues.txt in separate PR after issues/ is clean

@bors
Copy link
Collaborator

bors commented Jul 20, 2025

☔ The latest upstream changes (presumably #144181) made this pull request unmergeable. Please resolve the merge conflicts.

@jieyouxu
Copy link
Member

Should be fine now, only reason I need tidy to ignore deleted issue-*.rs files from issues/ is to completely remove any interactions with issues.txt file while working on tests

I will remove all moved tests from issues.txt in separate PR after issues/ is clean

Can you rebless issues.txt (perhaps manually) to remove all ui/issues/** entries? I feel like leaving that in is quite surprising (especially given that said tests might not even exist after getting moved).

@rustbot author

@rustbot rustbot 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 21, 2025
@rustbot
Copy link
Collaborator

rustbot commented Jul 21, 2025

Reminder, once the PR becomes ready for a review, use @rustbot ready.

@Kivooeo
Copy link
Contributor Author

Kivooeo commented Jul 21, 2025

Can you rebless issues.txt (perhaps manually) to remove all ui/issues/** entries

With keeping removing this checks?

@jieyouxu
Copy link
Member

My understanding of the intention is that:

  • We want to skip the issues-* check for tests/ui/issues/, because there are now actually contributors (like you) trying to clean it up, so that gets in the way as issue-* tests get removed from tests/ui/issues/. The issues.txt is a listing of all issues-* tests. We should update this listing to remove ui/issues/ ones, as we are no longer tracking issue-* tests under tests/ui/issues/.

@Kivooeo
Copy link
Contributor Author

Kivooeo commented Jul 21, 2025

@rustbot ready

deleted all lines that starts with ui/issues/

reverted this filter on issues/ tests from here

let mut remaining_issue_names: BTreeSet<&str> = allowed_issue_names.clone();

but kept this, because otherwise it's threw errors about existed tests

if !remaining_issue_names.remove(stripped_path.as_str())
&& !stripped_path.starts_with("ui/issues/")

@rustbot rustbot removed the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Jul 21, 2025
@bors bors added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Jul 22, 2025
@jieyouxu jieyouxu changed the title Removed tidy checks on issues.txt Removed tidy checks for tests/ui/issues/ Jul 22, 2025
@Kivooeo Kivooeo changed the title Removed tidy checks for tests/ui/issues/ Remove tidy checks for tests/ui/issues/ Jul 22, 2025
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jul 22, 2025
Remove tidy checks for `tests/ui/issues/`

r? `@jieyouxu`

As it is making cleanup efforts more difficult.

This change was discussed here [#t-compiler > Discussion for ui test suite improvements @ 💬](https://rust-lang.zulipchat.com/#narrow/channel/131828-t-compiler/topic/Discussion.20for.20ui.20test.20suite.20improvements/near/529566433)
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jul 22, 2025
Remove tidy checks for `tests/ui/issues/`

r? ``@jieyouxu``

As it is making cleanup efforts more difficult.

This change was discussed here [#t-compiler > Discussion for ui test suite improvements @ 💬](https://rust-lang.zulipchat.com/#narrow/channel/131828-t-compiler/topic/Discussion.20for.20ui.20test.20suite.20improvements/near/529566433)
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jul 22, 2025
Remove tidy checks for `tests/ui/issues/`

r? ```@jieyouxu```

As it is making cleanup efforts more difficult.

This change was discussed here [#t-compiler > Discussion for ui test suite improvements @ 💬](https://rust-lang.zulipchat.com/#narrow/channel/131828-t-compiler/topic/Discussion.20for.20ui.20test.20suite.20improvements/near/529566433)
bors added a commit that referenced this pull request Jul 22, 2025
Rollup of 8 pull requests

Successful merges:

 - #144094 (Ensure we codegen the main fn)
 - #144173 (Remove tidy checks for `tests/ui/issues/`)
 - #144218 (Use serde for target spec json deserialize)
 - #144221 (generate elf symbol version in raw-dylib)
 - #144234 (Fix broken TLS destructors on 32-bit win7)
 - #144256 (Don't ICE on non-TypeId metadata within TypeId)
 - #144272 (resolve: Make disambiguators for underscore bindings module-local (take 2))
 - #144276 (Use less HIR in check_private_in_public.)

r? `@ghost`
`@rustbot` modify labels: rollup
jhpratt added a commit to jhpratt/rust that referenced this pull request Jul 23, 2025
Remove tidy checks for `tests/ui/issues/`

r? ````@jieyouxu````

As it is making cleanup efforts more difficult.

This change was discussed here [#t-compiler > Discussion for ui test suite improvements @ 💬](https://rust-lang.zulipchat.com/#narrow/channel/131828-t-compiler/topic/Discussion.20for.20ui.20test.20suite.20improvements/near/529566433)
jhpratt added a commit to jhpratt/rust that referenced this pull request Jul 23, 2025
Remove tidy checks for `tests/ui/issues/`

r? `````@jieyouxu`````

As it is making cleanup efforts more difficult.

This change was discussed here [#t-compiler > Discussion for ui test suite improvements @ 💬](https://rust-lang.zulipchat.com/#narrow/channel/131828-t-compiler/topic/Discussion.20for.20ui.20test.20suite.20improvements/near/529566433)
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jul 23, 2025
Remove tidy checks for `tests/ui/issues/`

r? ``````@jieyouxu``````

As it is making cleanup efforts more difficult.

This change was discussed here [#t-compiler > Discussion for ui test suite improvements @ 💬](https://rust-lang.zulipchat.com/#narrow/channel/131828-t-compiler/topic/Discussion.20for.20ui.20test.20suite.20improvements/near/529566433)
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jul 23, 2025
Remove tidy checks for `tests/ui/issues/`

r? ```````@jieyouxu```````

As it is making cleanup efforts more difficult.

This change was discussed here [#t-compiler > Discussion for ui test suite improvements @ 💬](https://rust-lang.zulipchat.com/#narrow/channel/131828-t-compiler/topic/Discussion.20for.20ui.20test.20suite.20improvements/near/529566433)
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jul 23, 2025
Remove tidy checks for `tests/ui/issues/`

r? ````````@jieyouxu````````

As it is making cleanup efforts more difficult.

This change was discussed here [#t-compiler > Discussion for ui test suite improvements @ 💬](https://rust-lang.zulipchat.com/#narrow/channel/131828-t-compiler/topic/Discussion.20for.20ui.20test.20suite.20improvements/near/529566433)
bors added a commit that referenced this pull request Jul 23, 2025
Rollup of 10 pull requests

Successful merges:

 - #144173 (Remove tidy checks for `tests/ui/issues/`)
 - #144234 (Fix broken TLS destructors on 32-bit win7)
 - #144239 (Clean `rustc/parse/src/lexer` to improve maintainability)
 - #144247 (coretests/num: use ldexp instead of hard-coding a power of 2)
 - #144256 (Don't ICE on non-TypeId metadata within TypeId)
 - #144290 (update tests/ui/SUMMARY.md)
 - #144292 (mbe: Use concrete type for `get_unused_rule`)
 - #144298 (coverage: Enlarge empty spans during MIR instrumentation, not codegen)
 - #144311 (Add powerpc64le-unknown-linux-musl to CI rustc targets)
 - #144315 (bootstrap: add package.json and package-lock.json to dist tarball)

r? `@ghost`
`@rustbot` modify labels: rollup
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Jul 23, 2025
Remove tidy checks for `tests/ui/issues/`

r? `````````@jieyouxu`````````

As it is making cleanup efforts more difficult.

This change was discussed here [#t-compiler > Discussion for ui test suite improvements @ 💬](https://rust-lang.zulipchat.com/#narrow/channel/131828-t-compiler/topic/Discussion.20for.20ui.20test.20suite.20improvements/near/529566433)
bors added a commit that referenced this pull request Jul 23, 2025
Rollup of 10 pull requests

Successful merges:

 - #144173 (Remove tidy checks for `tests/ui/issues/`)
 - #144234 (Fix broken TLS destructors on 32-bit win7)
 - #144239 (Clean `rustc/parse/src/lexer` to improve maintainability)
 - #144247 (coretests/num: use ldexp instead of hard-coding a power of 2)
 - #144256 (Don't ICE on non-TypeId metadata within TypeId)
 - #144290 (update tests/ui/SUMMARY.md)
 - #144292 (mbe: Use concrete type for `get_unused_rule`)
 - #144298 (coverage: Enlarge empty spans during MIR instrumentation, not codegen)
 - #144311 (Add powerpc64le-unknown-linux-musl to CI rustc targets)
 - #144315 (bootstrap: add package.json and package-lock.json to dist tarball)

r? `@ghost`
`@rustbot` modify labels: rollup

try-job: x86_64-gnu-aux
bors added a commit that referenced this pull request Jul 23, 2025
Rollup of 9 pull requests

Successful merges:

 - #144173 (Remove tidy checks for `tests/ui/issues/`)
 - #144234 (Fix broken TLS destructors on 32-bit win7)
 - #144239 (Clean `rustc/parse/src/lexer` to improve maintainability)
 - #144256 (Don't ICE on non-TypeId metadata within TypeId)
 - #144290 (update tests/ui/SUMMARY.md)
 - #144292 (mbe: Use concrete type for `get_unused_rule`)
 - #144298 (coverage: Enlarge empty spans during MIR instrumentation, not codegen)
 - #144311 (Add powerpc64le-unknown-linux-musl to CI rustc targets)
 - #144315 (bootstrap: add package.json and package-lock.json to dist tarball)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 4177f19 into rust-lang:master Jul 23, 2025
11 checks passed
@rustbot rustbot added this to the 1.90.0 milestone Jul 23, 2025
rust-timer added a commit that referenced this pull request Jul 23, 2025
Rollup merge of #144173 - Kivooeo:tidy_checks, r=jieyouxu

Remove tidy checks for `tests/ui/issues/`

r? ``````````@jieyouxu``````````

As it is making cleanup efforts more difficult.

This change was discussed here [#t-compiler > Discussion for ui test suite improvements @ 💬](https://rust-lang.zulipchat.com/#narrow/channel/131828-t-compiler/topic/Discussion.20for.20ui.20test.20suite.20improvements/near/529566433)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-meta Area: Issues & PRs about the rust-lang/rust repository itself A-tidy Area: The tidy tool 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)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants