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.

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

Move tests from test/run-fail to UI #71185

Merged
merged 3 commits into from
May 9, 2020
Merged

Move tests from test/run-fail to UI #71185

merged 3 commits into from
May 9, 2020

Conversation

JohnTitor
Copy link
Member

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 16, 2020
@petrochenkov
Copy link
Contributor

If we are doing this, #65865 needs to be prioritized and implemented soon, IMO.

@nikomatsakis
Copy link
Contributor

I'm in favor of unifying these tests, but I think this qualifies as a "major change".

@JohnTitor could you file a 'major change proposal'? A short write-up would suffice of what the plans are -- just a paragraph or so should be fine.

Some things to include I think:

@nikomatsakis nikomatsakis added S-blocked Status: Marked as blocked ❌ on something else such as an RFC or other implementation work. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 16, 2020
@JohnTitor JohnTitor mentioned this pull request Apr 16, 2020
Closed
@JohnTitor
Copy link
Member Author

Sure thing, opened rust-lang/compiler-team#274.

@bors
Copy link
Contributor

bors commented Apr 29, 2020

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

@nikomatsakis
Copy link
Contributor

r? @petrochenkov -- are you good with reviewing this?

@petrochenkov petrochenkov added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-blocked Status: Marked as blocked ❌ on something else such as an RFC or other implementation work. labels May 2, 2020
@petrochenkov
Copy link
Contributor

Ok, the RFC says

To be announced at a triage meeting and go through final comment period (a 1 week wait to give people time).

so apparently I can just r+ this now?
It's not clear whether it was "announced at a triage meeting" or not, and what date the time is counting from.

@petrochenkov
Copy link
Contributor

r=me after rebase.

@petrochenkov petrochenkov 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 May 2, 2020
@JohnTitor
Copy link
Member Author

JohnTitor commented May 6, 2020 •
edited
Loading

Rebased. Also dropped two tests since they were removed by #70175.

@petrochenkov
Copy link
Contributor

@bors r+

@bors
Copy link
Contributor

bors commented May 6, 2020

📌 Commit 82cb88b has been approved by petrochenkov

@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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels May 6, 2020
@bors
Copy link
Contributor

bors commented May 7, 2020

⌛ Testing commit 82cb88b with merge 3bf191f86d1d01a564dc7e9088387ae3e9297eb3...

@bors
Copy link
Contributor

bors commented May 7, 2020

💔 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 May 7, 2020
@bors bors added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 7, 2020
@JohnTitor
Copy link
Member Author

Ouch, needs to skip some env, one sec..

@JohnTitor
Copy link
Member Author

Should be fixed.
@bors r=petrochenkov

@bors
Copy link
Contributor

bors commented May 7, 2020

📌 Commit 9a164ff has been approved by petrochenkov

@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 May 7, 2020
@bors
Copy link
Contributor

bors commented May 7, 2020

⌛ Testing commit 9a164ff with merge 9be22e32cecc92803b5d1378bc1a611694438c4d...

@bors
Copy link
Contributor

bors commented May 7, 2020

💔 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 May 7, 2020
@JohnTitor
Copy link
Member Author

This error occurs on some other PRs so should be spurious.
@bors retry

@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 May 7, 2020
@RalfJung RalfJung mentioned this pull request May 9, 2020
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request May 9, 2020
@bors
Auto merge of rust-lang#72041 - RalfJung:rollup-xivrvy2, r=RalfJung
bad3bf6 
Rollup of 5 pull requests

Successful merges:

 - rust-lang#69406 (upgrade chalk and use chalk-solve/chalk-ir/chalk-rust-ir)
 - rust-lang#71185 (Move tests from `test/run-fail` to UI)
 - rust-lang#71234 (rustllvm: Use .init_array rather than .ctors)
 - rust-lang#71508 (Simplify the `tcx.alloc_map` API)
 - rust-lang#71555 (Remove ast::{Ident, Name} reexports.)

Failed merges:

r? @ghost
@bors bors merged commit 1704dca into rust-lang:master May 9, 2020
@JohnTitor JohnTitor deleted the run-fail branch May 9, 2020 22:13
@ehuss ehuss mentioned this pull request Jun 12, 2020
Merged
RalfJung added a commit to RalfJung/rust that referenced this pull request Jun 15, 2020
@RalfJung
Rollup merge of rust-lang#73296 - ehuss:remove-msvc-aux, r=Mark-Simul…
baedb48 
…acrum

Remove vestigial CI job msvc-aux.

This CI job isn't really doing anything, so it seems prudent to remove it.

For some history:
* This was introduced in rust-lang#48809 when the msvc job was split in two to keep it under 2 hours (oh the good old days). At the time, this check-aux job did a bunch of things:
    * tidy
    * src/test/pretty
    * src/test/run-pass/pretty
    * src/test/run-fail/pretty
    * src/test/run-pass-valgrind/pretty
    * src/test/run-pass-fulldeps/pretty
    * src/test/run-fail-fulldeps/pretty
* Tidy was removed in rust-lang#60777.
* run-pass and run-pass-fulldeps moved to UI in rust-lang#63029
* src/test/pretty removed in rust-lang#58140
* src/test/run-fail moved to UI in rust-lang#71185
* run-fail-fulldeps removed in rust-lang#51285

Over time through attrition, the job was left with one lonely thing: `src/test/run-pass-valgrind/pretty`. And of course, this wasn't actually running the "pretty" tests. The normal `run-pass-valgrind` tests ran, and then when it tried to run in "pretty" mode, all the tests were ignored because compiletest thought nothing had changed (apparently compiletest isn't fingerprinting the mode?  Needs more investigation…). `run-pass-valgrind` is already being run as part of `x86_64-msvc-1`, so there's no need to run it here.

I've taken the liberty of removing `src/test/run-pass-valgrind/pretty` as a distinct test. I'm guessing from the other PR's that the pretty tests should now live in `src/test/pretty`, and that the team has moved away from doing pretty tests on other parts of the `src/test` tree.
RalfJung added a commit to RalfJung/rust that referenced this pull request Jun 15, 2020
@RalfJung
Rollup merge of rust-lang#73296 - ehuss:remove-msvc-aux, r=Mark-Simul…
fb75d4a 
…acrum

Remove vestigial CI job msvc-aux.

This CI job isn't really doing anything, so it seems prudent to remove it.

For some history:
* This was introduced in rust-lang#48809 when the msvc job was split in two to keep it under 2 hours (oh the good old days). At the time, this check-aux job did a bunch of things:
    * tidy
    * src/test/pretty
    * src/test/run-pass/pretty
    * src/test/run-fail/pretty
    * src/test/run-pass-valgrind/pretty
    * src/test/run-pass-fulldeps/pretty
    * src/test/run-fail-fulldeps/pretty
* Tidy was removed in rust-lang#60777.
* run-pass and run-pass-fulldeps moved to UI in rust-lang#63029
* src/test/pretty removed in rust-lang#58140
* src/test/run-fail moved to UI in rust-lang#71185
* run-fail-fulldeps removed in rust-lang#51285

Over time through attrition, the job was left with one lonely thing: `src/test/run-pass-valgrind/pretty`. And of course, this wasn't actually running the "pretty" tests. The normal `run-pass-valgrind` tests ran, and then when it tried to run in "pretty" mode, all the tests were ignored because compiletest thought nothing had changed (apparently compiletest isn't fingerprinting the mode?  Needs more investigation…). `run-pass-valgrind` is already being run as part of `x86_64-msvc-1`, so there's no need to run it here.

I've taken the liberty of removing `src/test/run-pass-valgrind/pretty` as a distinct test. I'm guessing from the other PR's that the pretty tests should now live in `src/test/pretty`, and that the team has moved away from doing pretty tests on other parts of the `src/test` tree.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@petrochenkov petrochenkov

Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Cleanup: Move src/test/run-fail to src/test/ui
5 participants
@JohnTitor @petrochenkov @nikomatsakis @bors @rust-highfive