Skip to content

Add new ignore-backends and needs-backends tests annotations #144125

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

Jump to bottom
Merged
merged 2 commits into from
Jul 20, 2025
Merged

Add new ignore-backends and needs-backends tests annotations #144125

merged 2 commits into from
Jul 20, 2025
+106 −2

Conversation

GuillaumeGomez
Copy link
Member

Part of rust-lang/compiler-team#891.

Next step will be to add these annotations in the files where either the output is different based on the codegen (like asm tests) or that are known to fail in the GCC backend.

cc @oli-obk @antoyo
r? @Kobzol

@rustbot rustbot added A-compiletest Area: The compiletest test runner A-rustc-dev-guide Area: rustc-dev-guide 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) labels Jul 18, 2025
@rustbot
Copy link
Collaborator

rustbot commented Jul 18, 2025

Some changes occurred in src/tools/compiletest

cc @jieyouxu

The rustc-dev-guide subtree was changed. If this PR only touches the dev guide consider submitting a PR directly to rust-lang/rustc-dev-guide otherwise thank you for updating the dev guide with your changes.

cc @BoxyUwU, @jieyouxu, @Kobzol, @tshepang

@bjorn3
Copy link
Member

bjorn3 commented Jul 18, 2025

For asm tests I think adding //@ needs-backends: llvm to every test is a bad idea. Rather I think gating the entire asm test suite behind the LLVM backend inside compiletest itself or moving the test assertions to backend specific files would be better options. For ui tests these test annotations do make sense to me however.

@GuillaumeGomez
Copy link
Member Author

I didn't look how many tests were there but you're right, would make more sense to just ignore these tests completely if it's not llvm backend. I think I'll rename the folder to better express it (asm -> asm-llvm). It could also allow to add a more generic check for this folder by directly checking if the backend name matches the current one, allowing to add asm tests for cranelift and gcc backends.

Kobzol
Kobzol reviewed Jul 18, 2025
View reviewed changes
Copy link
Member

@Kobzol Kobzol left a comment

Choose a reason for hiding this comment

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

CC @jieyouxu

src/tools/compiletest/src/directives.rs Outdated
{
return IgnoreDecision::Ignore {
reason: format!(
"{} backend is not part of backends",
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
"{} backend is not part of backends",
"{} backend is not part of required backends",

@jieyouxu
Copy link
Member

For asm tests I think adding //@ needs-backends: llvm to every test is a bad idea. Rather I think gating the entire asm test suite behind the LLVM backend inside compiletest itself or moving the test assertions to backend specific files would be better options. For ui tests these test annotations do make sense to me however.

Yeah, I think this is for the best.

@GuillaumeGomez
Copy link
Member Author

Applied suggestion.

@jieyouxu jieyouxu assigned jieyouxu and unassigned jieyouxu Jul 18, 2025
@bors
Copy link
Collaborator

bors commented Jul 20, 2025

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

@GuillaumeGomez
Copy link
Member Author

Fixed merge conflict. Was there anything else to be done here?

@Kobzol
Copy link
Member

Kobzol commented Jul 20, 2025

Fine by me.

@bors r+

@bors
Copy link
Collaborator

bors commented Jul 20, 2025

📌 Commit 4a35a9f has been approved by Kobzol

It is now in the queue for this repository.

@bors 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 20, 2025
@GuillaumeGomez GuillaumeGomez mentioned this pull request Jul 20, 2025
Merged
bors added a commit that referenced this pull request Jul 20, 2025
@bors
Auto merge of #144219 - GuillaumeGomez:rollup-ha29sql, r=GuillaumeGomez
0864097 
Rollup of 9 pull requests

Successful merges:

 - #143282 (Add `uX::strict_sub_signed`)
 - #143423 (address clippy formatting nits)
 - #143720 (Allow `Rvalue::Repeat` to return true in `rvalue_creates_operand` too)
 - #144011 (bootstrap: Don't trigger an unnecessary LLVM build from check builds)
 - #144112 (bootstrap: Ignore `rust.debuginfo-level-tests` for codegen tests)
 - #144125 (Add new `ignore-backends` and `needs-backends` tests annotations)
 - #144143 (Fix `-Ctarget-feature`s getting ignored after `crt-static`)
 - #144150 (tests: assembly: cstring-merging: Disable GlobalMerge pass)
 - #144190 (Give a message with a span on MIR validation error)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 2abca9c into rust-lang:master Jul 20, 2025
11 checks passed
@rustbot rustbot added this to the 1.90.0 milestone Jul 20, 2025
rust-timer added a commit that referenced this pull request Jul 20, 2025
@rust-timer
Unrolled build for #144125
b2459e8 
Rollup merge of #144125 - GuillaumeGomez:new-annotations, r=Kobzol

Add new `ignore-backends` and `needs-backends` tests annotations

Part of rust-lang/compiler-team#891.

Next step will be to add these annotations in the files where either the output is different based on the codegen (like `asm` tests) or that are known to fail in the GCC backend.

cc `@oli-obk` `@antoyo`
r? `@Kobzol`
github-actions bot pushed a commit to rust-lang/rustc-dev-guide that referenced this pull request Jul 21, 2025
@bors
Auto merge of #144219 - GuillaumeGomez:rollup-ha29sql, r=GuillaumeGomez
9dda25f 
Rollup of 9 pull requests

Successful merges:

 - rust-lang/rust#143282 (Add `uX::strict_sub_signed`)
 - rust-lang/rust#143423 (address clippy formatting nits)
 - rust-lang/rust#143720 (Allow `Rvalue::Repeat` to return true in `rvalue_creates_operand` too)
 - rust-lang/rust#144011 (bootstrap: Don't trigger an unnecessary LLVM build from check builds)
 - rust-lang/rust#144112 (bootstrap: Ignore `rust.debuginfo-level-tests` for codegen tests)
 - rust-lang/rust#144125 (Add new `ignore-backends` and `needs-backends` tests annotations)
 - rust-lang/rust#144143 (Fix `-Ctarget-feature`s getting ignored after `crt-static`)
 - rust-lang/rust#144150 (tests: assembly: cstring-merging: Disable GlobalMerge pass)
 - rust-lang/rust#144190 (Give a message with a span on MIR validation error)

r? `@ghost`
`@rustbot` modify labels: rollup
github-actions bot pushed a commit to rust-lang/miri that referenced this pull request Jul 21, 2025
@bors
Auto merge of #144219 - GuillaumeGomez:rollup-ha29sql, r=GuillaumeGomez
f288c40 
Rollup of 9 pull requests

Successful merges:

 - rust-lang/rust#143282 (Add `uX::strict_sub_signed`)
 - rust-lang/rust#143423 (address clippy formatting nits)
 - rust-lang/rust#143720 (Allow `Rvalue::Repeat` to return true in `rvalue_creates_operand` too)
 - rust-lang/rust#144011 (bootstrap: Don't trigger an unnecessary LLVM build from check builds)
 - rust-lang/rust#144112 (bootstrap: Ignore `rust.debuginfo-level-tests` for codegen tests)
 - rust-lang/rust#144125 (Add new `ignore-backends` and `needs-backends` tests annotations)
 - rust-lang/rust#144143 (Fix `-Ctarget-feature`s getting ignored after `crt-static`)
 - rust-lang/rust#144150 (tests: assembly: cstring-merging: Disable GlobalMerge pass)
 - rust-lang/rust#144190 (Give a message with a span on MIR validation error)

r? `@ghost`
`@rustbot` modify labels: rollup
@GuillaumeGomez GuillaumeGomez deleted the new-annotations branch July 21, 2025 08:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Kobzol Kobzol Kobzol left review comments

Assignees

@Kobzol Kobzol

Labels
A-compiletest Area: The compiletest test runner A-rustc-dev-guide Area: rustc-dev-guide 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)
Projects
None yet
Milestone
1.90.0
Development

Successfully merging this pull request may close these issues.
6 participants
@GuillaumeGomez @rustbot @bjorn3 @jieyouxu @bors @Kobzol