Skip to content

Allow ui_test style headers in compiletest #114072

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
Closed
wants to merge 8 commits into from
Closed

Allow ui_test style headers in compiletest #114072

wants to merge 8 commits into from

Conversation

asquared31415
Copy link
Contributor

r? @oli-obk

This is a step towards the MCP to replace compiletest rust-lang/compiler-team#536

This PR allows compiletest to accept certain headers that ui_test can understand. Tests that don't use any compiletest-specific features can now be run under ui_test and compiletest.

The second commit changes all the tests to use the new directives.

@rustbot rustbot added 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) 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 Jul 25, 2023
@rustbot
Copy link
Collaborator

rustbot commented Jul 25, 2023

Changes to the code generated for builtin derived traits.

cc @nnethercote

These commits modify the Cargo.lock file. Unintentional changes to Cargo.lock can be introduced when switching branches and rebasing PRs.

If this was unintentional then you should revert the changes before this PR is merged.
Otherwise, you can ignore this comment.

Some changes occurred in compiler/rustc_codegen_cranelift

cc @bjorn3

@rust-log-analyzer

This comment has been minimized.

Sign in to view
@asquared31415 asquared31415 changed the title Compiletest allow uitest Allow ui_test style headers in compiletest Jul 25, 2023
@bors
Copy link
Collaborator

bors commented Jul 26, 2023

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

oli-obk
oli-obk requested changes Jul 26, 2023
View reviewed changes
Copy link
Contributor

@oli-obk oli-obk left a comment

Choose a reason for hiding this comment

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

I reviewed up to the point where github wanted me to expand the files.

I guess the only thing is that I'd have preferred the compiletest changes to be in a few individual commits, but I reviewed the entire diff on compiletest itself and it lgtm.

tests/ui/asm/aarch64/type-check-3.rs Outdated
@@ -1,5 +1,5 @@
// only-aarch64
Copy link
Contributor

Choose a reason for hiding this comment

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

These haven't gotten converted

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I hadn't yet implemented the only and ignore things, looking into that now.

tests/ui/asm/aarch64/srcloc.rs
@@ -1,7 +1,7 @@
// only-aarch64
// build-fail
Copy link
Contributor

Choose a reason for hiding this comment

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

this isn't supported yet in ui-test

tests/ui/asm/x86_64/issue-96797.rs
@@ -1,5 +1,5 @@
// build-pass
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe ui-test should have a way to register custom commands that can be converted to other commands. This command is just changing compiler flags, and only really something the rustc test suite cares about

tests/ui/asm/x86_64/issue-96797.rs Outdated
@@ -1,5 +1,5 @@
// build-pass
// compile-flags: -O
//@compile-flags: -O
// min-llvm-version: 14.0.5
Copy link
Contributor

Choose a reason for hiding this comment

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

fun. Not sure how to do this yet

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well you're in luck, most of these got removed :D

(though yes it's still a general issue for migration)

@asquared31415 asquared31415 force-pushed the compiletest_allow_uitest branch from f57331a to a57841e Compare July 27, 2023 02:11
@rust-cloud-vms rust-cloud-vms bot force-pushed the compiletest_allow_uitest branch from a57841e to c85c7d4 Compare July 27, 2023 03:08
oli-obk
oli-obk reviewed Jul 27, 2023
View reviewed changes
src/tools/compiletest/src/runtest.rs Outdated
@@ -234,7 +234,7 @@ enum Emit {

impl<'test> TestCx<'test> {
/// Code executed for each revision in turn (or, if there are no
/// revisions, exactly once, with revision == None).
///@revisions, exactly once, with revision == None).
Copy link
Contributor

Choose a reason for hiding this comment

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

accidental replacement

@bors
Copy link
Collaborator

bors commented Jul 27, 2023

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

@rust-cloud-vms rust-cloud-vms bot force-pushed the compiletest_allow_uitest branch from c85c7d4 to ae5e93d Compare July 29, 2023 23:28
@bors
Copy link
Collaborator

bors commented Jul 31, 2023

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

@rust-cloud-vms rust-cloud-vms bot force-pushed the compiletest_allow_uitest branch from ae5e93d to 0e33c33 Compare August 16, 2023 20:47
@apiraino
Copy link
Contributor

apiraino commented Aug 24, 2023 •
edited
Loading

ping @asquared31415 for a rebase, apart from that who else should have a look. This PR maybe also needs a sign-off from:

r? boostrap
r? libs

thanks

@rustbot
Copy link
Collaborator

rustbot commented Aug 24, 2023

Failed to set assignee to t-boostrap: invalid assignee

Note: Only org members, users with write permissions, or people who have commented on the PR may be assigned.

@rustbot
Copy link
Collaborator

rustbot commented Aug 24, 2023

Failed to set assignee to t-libs: invalid assignee

Note: Only org members, users with write permissions, or people who have commented on the PR may be assigned.

@rustbot
Copy link
Collaborator

rustbot commented Aug 24, 2023

Failed to set assignee to boostrap: invalid assignee

Note: Only org members, users with write permissions, or people who have commented on the PR may be assigned.

@rustbot rustbot assigned m-ou-se and unassigned oli-obk Aug 24, 2023
@asquared31415
Copy link
Contributor Author

@rustbot author

Sorry this was not ready for review, there's still lots to do.

@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 Aug 24, 2023
@dtolnay dtolnay removed T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Sep 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@oli-obk oli-obk oli-obk requested changes

Assignees

@oli-obk oli-obk

Labels
A-testsuite Area: The testsuite used to check the correctness of rustc S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@asquared31415 @rustbot @rust-log-analyzer @bors @apiraino @oli-obk @m-ou-se @dtolnay