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

Move almost all run-make-fulldeps tests to run-make #109770

Merged
merged 6 commits into from
Apr 3, 2023

Conversation

jyn514
Copy link
Member

@jyn514 jyn514 commented Mar 30, 2023

They pass fine, and this avoids having to build the compiler twice.

There are few enough tests left that I think it should be possible to get rid of this test suite altogether, but I expect this PR to fail at least a few times in bors and want to get it merged before tackling further changes. cc #83775

Fixes #66085. Fixes #83773.

@jyn514 jyn514 changed the title Move almost all run-make-fulldeps to run-make Move almost all run-make-fulldeps tests to run-make Mar 30, 2023
@rustbot
Copy link
Collaborator

rustbot commented Mar 30, 2023

r? @Mark-Simulacrum

(rustbot has picked a reviewer for you, use r? to override)

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 30, 2023
@petrochenkov
Copy link
Contributor

You may need to add # ignore-cross-compile to some of these if errors about something like missing ARM linker appear.
For run-make-fulldeps tests cross-compilation is not performed.

apparently I missed some tests in the last commit. Rather than having
dozens of tests use the long version, use the short version in
`run-make` and the long version in `run-make-fulldeps` (which is now
only three tests)
`run-make-fulldeps` is never cross-compiled, so a lot of these tests
never accounted for --target. Ignore them when cross-compiling for
now.
@Mark-Simulacrum
Copy link
Member

@bors r+ rollup=iffy

Presuming this passes CI, I think it makes sense - I'm a little surprised we put so many tests in the wrong category, but not that much :)

@bors
Copy link
Contributor

bors commented Apr 2, 2023

📌 Commit 4851d56 has been approved by Mark-Simulacrum

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 2, 2023
@bors
Copy link
Contributor

bors commented Apr 2, 2023

⌛ Testing commit 4851d56 with merge af86a1d8720c1fcb8184a05eeba5eb8f2f4aea2f...

@bors
Copy link
Contributor

bors commented Apr 2, 2023

💔 Test failed - checks-actions

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Apr 2, 2023
@rust-log-analyzer

This comment has been minimized.

@rustbot rustbot added A-testsuite Area: The testsuite used to check the correctness of rustc T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue. labels Apr 2, 2023
@jyn514
Copy link
Member Author

jyn514 commented Apr 2, 2023

4e5064f is the only change since the last approval.

@bors r=Mark-Simulacrum rollup=iffy

@bors
Copy link
Contributor

bors commented Apr 2, 2023

📌 Commit c45037b has been approved by Mark-Simulacrum

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 2, 2023
@bors
Copy link
Contributor

bors commented Apr 3, 2023

⌛ Testing commit c45037b with merge 3328913...

@bors
Copy link
Contributor

bors commented Apr 3, 2023

☀️ Test successful - checks-actions
Approved by: Mark-Simulacrum
Pushing 3328913 to master...

1 similar comment
@bors
Copy link
Contributor

bors commented Apr 3, 2023

☀️ Test successful - checks-actions
Approved by: Mark-Simulacrum
Pushing 3328913 to master...

@bors bors added merged-by-bors This PR was explicitly merged by bors. labels Apr 3, 2023
@bors bors merged commit 3328913 into rust-lang:master Apr 3, 2023
@rustbot rustbot added this to the 1.70.0 milestone Apr 3, 2023
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (3328913): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
3.6% [3.6%, 3.6%] 1
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) - - 0

Cycles

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
4.4% [3.8%, 4.8%] 4
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) - - 0

@jyn514 jyn514 deleted the run-make-fulldeps branch April 3, 2023 07:24
bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 14, 2023
Move most ui-fulldeps tests to ui/

Same rationale as rust-lang#109770, they don't actually need a stage 2 build.

This increases the limit for the UI directory because otherwise it was annoying to be constantly moving files into subdirectories when I fixed a test; rust-lang#109873 makes up for it.

cc rust-lang#109770, rust-lang#109874
RalfJung pushed a commit to RalfJung/miri that referenced this pull request Apr 14, 2023
Move most ui-fulldeps tests to ui/

Same rationale as rust-lang/rust#109770, they don't actually need a stage 2 build.

This increases the limit for the UI directory because otherwise it was annoying to be constantly moving files into subdirectories when I fixed a test; rust-lang/rust#109873 makes up for it.

cc rust-lang/rust#109770, rust-lang/rust#109874
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jun 9, 2024
…-ozkan

Remove empty test suite `tests/run-make-fulldeps`

After rust-lang#109770, there were only a handful of tests left in the run-make-fulldeps suite.

As of rust-lang#126111, there are no longer *any* run-make-fulldeps tests, so now we can:

- Remove the directory
- Remove related bootstrap/compiletest code
- Remove various other references in CI scripts and documentation.

By removing this suite, we also no longer need to worry about discrepancies between it and ui-fulldeps, and we don't have to worry about porting tests from Makefile to [rmake](rust-lang#121876) (or whether rmake even works with fulldeps).
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Jun 9, 2024
Rollup merge of rust-lang#126155 - Zalathar:run-make-fulldeps, r=onur-ozkan

Remove empty test suite `tests/run-make-fulldeps`

After rust-lang#109770, there were only a handful of tests left in the run-make-fulldeps suite.

As of rust-lang#126111, there are no longer *any* run-make-fulldeps tests, so now we can:

- Remove the directory
- Remove related bootstrap/compiletest code
- Remove various other references in CI scripts and documentation.

By removing this suite, we also no longer need to worry about discrepancies between it and ui-fulldeps, and we don't have to worry about porting tests from Makefile to [rmake](rust-lang#121876) (or whether rmake even works with fulldeps).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-testsuite Area: The testsuite used to check the correctness of rustc merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue.
Projects
None yet
7 participants