Skip to content
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

tidy: Fix paths to coretests and alloctests #139123

Merged
merged 1 commit into from
Apr 6, 2025

Conversation

thaliaarchi
Copy link
Contributor

@thaliaarchi thaliaarchi commented Mar 30, 2025

Following #135937 and #136642, tests for core and alloc are in coretests and alloctests. Fix tidy to lint for the new paths. Also, update comments referring to the old locations.

Some context for changes which don't match that pattern:

  • library/std/src/thread/local/dynamic_tests.rs and library/std/src/sync/mpsc/sync_tests.rs were moved under library/std/tests/ in 332fb7e (Move std::thread_local unit tests to integration tests, 2025-01-17) and b8ae372 (Move std::sync unit tests to integration tests, 2025-01-17), respectively, so are no longer special cases.
  • There never was a library/core/tests/fmt.rs file. That comment previously referred to src/test/ui/ifmt.rs, which was folded into library/alloc/tests/fmt.rs in 949c966 (move format! interface tests, 2020-09-08).

Now, the only matches for (alloc|core)/tests are in compiler/rustc_codegen_{cranelift,gcc}/patches. I don't know why CI hasn't broken because those patches can't apply. Or maybe they somehow still can apply?

r? @bjorn3

@rustbot rustbot added 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) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Mar 30, 2025
@rustbot
Copy link
Collaborator

rustbot commented Mar 30, 2025

Some changes occurred in compiler/rustc_codegen_gcc

cc @antoyo, @GuillaumeGomez

@rust-log-analyzer

This comment has been minimized.

@thaliaarchi
Copy link
Contributor Author

Looks like my changes to tidy work! Mostly.

Since those particular tests which CI detected exercise the internals of collections, they currently have to be part of alloc. Perhaps they should be added an allowlist?

@thaliaarchi thaliaarchi force-pushed the core-alloc-test-paths branch from 2db2a3f to d5760fe Compare March 30, 2025 00:44
@bjorn3
Copy link
Member

bjorn3 commented Mar 31, 2025

Now, the only matches for (alloc|core)/tests are in compiler/rustc_codegen_{cranelift,gcc}/patches. I don't know why CI hasn't broken because those patches can't apply. Or maybe they somehow still can apply?

For cg_clif at least this is because the core tests don't run in CI here. In any case yesterday's sync PR fixed all references in cg_clif.

@thaliaarchi thaliaarchi force-pushed the core-alloc-test-paths branch from d5760fe to d3581b2 Compare April 5, 2025 04:40
@thaliaarchi
Copy link
Contributor Author

Thanks for the review. I've rebased to incorporate the Cranelift changes and addressed your feedback.

@thaliaarchi thaliaarchi force-pushed the core-alloc-test-paths branch from d3581b2 to 38b933a Compare April 5, 2025 04:44
Copy link
Member

@bjorn3 bjorn3 left a comment

Choose a reason for hiding this comment

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

Tidy changes LGTM.

Following `rust-lang#135937` and `rust-lang#136642`, tests for core and alloc are in
coretests and alloctests. Fix tidy to lint for the new paths. Also,
update comments referring to the old locations.

Some context for changes which don't match that pattern:
* library/std/src/thread/local/dynamic_tests.rs and
  library/std/src/sync/mpsc/sync_tests.rs were moved under
  library/std/tests/ in 332fb7e (Move std::thread_local unit tests
  to integration tests, 2025-01-17) and b8ae372 (Move std::sync unit
  tests to integration tests, 2025-01-17), respectively, so are no
  longer special cases.
* There never was a library/core/tests/fmt.rs file. That comment
  previously referred to src/test/ui/ifmt.rs, which was folded into
  library/alloc/tests/fmt.rs in 949c966 (move format! interface
  tests, 2020-09-08).
@thaliaarchi thaliaarchi force-pushed the core-alloc-test-paths branch from 38b933a to 3af666e Compare April 5, 2025 19:16
@thaliaarchi thaliaarchi changed the title Fix paths to coretests and alloctests tidy: Fix paths to coretests and alloctests Apr 5, 2025
@thaliaarchi
Copy link
Contributor Author

Great. We should be good to go now.

@bjorn3
Copy link
Member

bjorn3 commented Apr 5, 2025

@bors r+

@bors
Copy link
Collaborator

bors commented Apr 5, 2025

📌 Commit 3af666e has been approved by bjorn3

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 Apr 5, 2025
bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 6, 2025
…=bjorn3

tidy: Fix paths to `coretests` and `alloctests`

Following `rust-lang#135937` and `rust-lang#136642`, tests for core and alloc are in coretests and alloctests. Fix tidy to lint for the new paths. Also, update comments referring to the old locations.

Some context for changes which don't match that pattern:
- `library/std/src/thread/local/dynamic_tests.rs` and `library/std/src/sync/mpsc/sync_tests.rs` were moved under `library/std/tests/` in 332fb7e (Move std::thread_local unit tests to integration tests, 2025-01-17) and b8ae372 (Move std::sync unit tests to integration tests, 2025-01-17), respectively, so are no longer special cases.
- There never was a `library/core/tests/fmt.rs` file. That comment previously referred to `src/test/ui/ifmt.rs`, which was folded into `library/alloc/tests/fmt.rs` in 949c966 (move format! interface tests, 2020-09-08).

Now, the only matches for `(alloc|core)/tests` are in `compiler/rustc_codegen_{cranelift,gcc}/patches`. I don't know why CI hasn't broken because those patches can't apply. Or maybe they somehow still can apply?

r? `@bjorn3`
@bors
Copy link
Collaborator

bors commented Apr 6, 2025

⌛ Testing commit 3af666e with merge b754f4c...

@rust-log-analyzer
Copy link
Collaborator

The job aarch64-apple failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)
failures:

---- [codegen] tests/codegen/issues/issue-122600-ptr-discriminant-update.rs stdout ----

error: verification with 'FileCheck' failed
status: exit status: 1
command: "/Users/runner/work/rust/rust/build/aarch64-apple-darwin/ci-llvm/bin/FileCheck" "--input-file" "/Users/runner/work/rust/rust/build/aarch64-apple-darwin/test/codegen/issues/issue-122600-ptr-discriminant-update/issue-122600-ptr-discriminant-update.ll" "/Users/runner/work/rust/rust/tests/codegen/issues/issue-122600-ptr-discriminant-update.rs" "--check-prefix=CHECK" "--allow-unused-prefixes" "--dump-input-context" "100"
stdout: none
--- stderr -------------------------------
/Users/runner/work/rust/rust/tests/codegen/issues/issue-122600-ptr-discriminant-update.rs:38:16: error: CHECK-NOT: excluded string found in input
 // CHECK-NOT: 75{{3|4}}
               ^
/Users/runner/work/rust/rust/build/aarch64-apple-darwin/test/codegen/issues/issue-122600-ptr-discriminant-update/issue-122600-ptr-discriminant-update.ll:27:41: note: found here
!1 = !{!"rustc version 1.88.0-nightly (b754f4c31 2025-04-06)"}
                                        ^~~

Input file: /Users/runner/work/rust/rust/build/aarch64-apple-darwin/test/codegen/issues/issue-122600-ptr-discriminant-update/issue-122600-ptr-discriminant-update.ll
Check file: /Users/runner/work/rust/rust/tests/codegen/issues/issue-122600-ptr-discriminant-update.rs

-dump-input=help explains the following input dump.

Input was:
<<<<<<
        1: ; ModuleID = 'issue_122600_ptr_discriminant_update.b05f82f6ceffeaed-cgu.0' 
        2: source_filename = "issue_122600_ptr_discriminant_update.b05f82f6ceffeaed-cgu.0" 
        3: target datalayout = "e-m:o-p270:32:32-p271:32:32-p272:64:64-i64:64-i128:128-n32:64-S128-Fn32" 
        4: target triple = "arm64-apple-macosx11.0.0" 
        5:  
        6: ; Function Attrs: mustprogress nofree norecurse nosync nounwind willreturn memory(argmem: readwrite, inaccessiblemem: write) uwtable 
        7: define void @update(ptr nocapture noundef %s) unnamed_addr #0 { 
        8: start: 
        9:  %_3.sroa.0.0.copyload = load i8, ptr %s, align 1 
       10:  %0 = trunc nuw i8 %_3.sroa.0.0.copyload to i1 
       11:  %1 = xor i1 %0, true 
       12:  tail call void @llvm.assume(i1 %1) 
       13:  store i8 1, ptr %s, align 1 
       14:  ret void 
       15: } 
       16:  
       17: ; Function Attrs: nocallback nofree nosync nounwind willreturn memory(inaccessiblemem: write) 
       18: declare void @llvm.assume(i1 noundef) #1 
       19:  
       20: attributes #0 = { mustprogress nofree norecurse nosync nounwind willreturn memory(argmem: readwrite, inaccessiblemem: write) uwtable "frame-pointer"="non-leaf" "probe-stack"="inline-asm" "target-cpu"="apple-m1" } 
       21: attributes #1 = { nocallback nofree nosync nounwind willreturn memory(inaccessiblemem: write) } 
       22:  
       23: !llvm.module.flags = !{!0} 
       24: !llvm.ident = !{!1} 
       25:  
       26: !0 = !{i32 8, !"PIC Level", i32 2} 
       27: !1 = !{!"rustc version 1.88.0-nightly (b754f4c31 2025-04-06)"} 
not:38                                             !~~                     error: no match expected
>>>>>>
------------------------------------------



@bors
Copy link
Collaborator

bors commented Apr 6, 2025

💔 Test failed - checks-actions

@bors bors removed the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Apr 6, 2025
@bors bors added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 6, 2025
@Zalathar
Copy link
Contributor

Zalathar commented Apr 6, 2025

It looks like the // CHECK-NOT: 75{{3|4}} FileCheck directives added by #137500 are detecting those digits when they appear in the (git?) hash in the compiler version string, causing false failures in CI.

@thaliaarchi
Copy link
Contributor Author

Since the failure last time was from the merge commit hash (b754f4c) matching the pattern 75{{3|4}}, it's exceedingly unlikely that we'd fail in the same way with a new merge commit. Can we try again in the queue?

@Zalathar
Copy link
Contributor

Zalathar commented Apr 6, 2025

@bors r=bjorn3

@bors
Copy link
Collaborator

bors commented Apr 6, 2025

💡 This pull request was already approved, no need to approve it again.

  • This pull request previously failed. You should add more commits to fix the bug, or use retry to trigger a build again.
  • There's another pull request that is currently being tested, blocking this pull request: Fix typo in RawList's documentation #139414

@bors
Copy link
Collaborator

bors commented Apr 6, 2025

📌 Commit 3af666e has been approved by bjorn3

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 Apr 6, 2025
bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 6, 2025
Rollup of 3 pull requests

Successful merges:

 - rust-lang#139123 (tidy: Fix paths to `coretests` and `alloctests`)
 - rust-lang#139347 (Only build `rust_test_helpers` for `{incremental,ui}` test suites)
 - rust-lang#139438 (Prevent a test from seeing forbidden numbers in the rustc version)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit fededb9 into rust-lang:master Apr 6, 2025
6 of 7 checks passed
@rustbot rustbot added this to the 1.88.0 milestone Apr 6, 2025
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Apr 6, 2025

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
Rollup merge of rust-lang#139123 - thaliaarchi:core-alloc-test-paths, r=bjorn3

tidy: Fix paths to `coretests` and `alloctests`

Following `rust-lang#135937` and `rust-lang#136642`, tests for core and alloc are in coretests and alloctests. Fix tidy to lint for the new paths. Also, update comments referring to the old locations.

Some context for changes which don't match that pattern:
- `library/std/src/thread/local/dynamic_tests.rs` and `library/std/src/sync/mpsc/sync_tests.rs` were moved under `library/std/tests/` in 332fb7e (Move std::thread_local unit tests to integration tests, 2025-01-17) and b8ae372 (Move std::sync unit tests to integration tests, 2025-01-17), respectively, so are no longer special cases.
- There never was a `library/core/tests/fmt.rs` file. That comment previously referred to `src/test/ui/ifmt.rs`, which was folded into `library/alloc/tests/fmt.rs` in 949c966 (move format! interface tests, 2020-09-08).

Now, the only matches for `(alloc|core)/tests` are in `compiler/rustc_codegen_{cranelift,gcc}/patches`. I don't know why CI hasn't broken because those patches can't apply. Or maybe they somehow still can apply?

r? `@bjorn3`
@thaliaarchi thaliaarchi deleted the core-alloc-test-paths branch April 6, 2025 20:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants