Skip to content

Clean UI tests 2 of n #138639

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 1 commit into from
Mar 22, 2025
Merged

Conversation

spencer3035
Copy link
Contributor

Modified 4 tests in tests/ui. Cleaned 3 and deleted one.

I have a final commit changing the values in src/tools/tidy/src/ui_tests.rs.
I wasn't sure if it was best practice to change this value as you go along or
once at the end. I can rebase to something that incrementally changes the value
in the "cleaned" commits if that is preferred.

Related Issues:
#73494
#133895

r? jieyouxu

@rustbot
Copy link
Collaborator

rustbot commented Mar 18, 2025

Could not assign reviewer from: jieyouxu.
User(s) jieyouxu are either the PR author, already assigned, or on vacation. Please use r? to specify someone else to assign.

@rustbot
Copy link
Collaborator

rustbot commented Mar 18, 2025

r? @compiler-errors

rustbot has assigned @compiler-errors.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@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. labels Mar 18, 2025
@spencer3035
Copy link
Contributor Author

Note on modification of issues/issue-9382.rs:

This test diverged a lot from it's original intention as updates to Rust were
made. Originally, the intention was to check a compiler error for different
interactions of the ~ operator and the & operator, but ~ has since been
replaced with Box, which has it's own suite of tests. I made the decision to
keep it as a test of coercion from a &'a Vec::new() to &'a [] because I
couldn't find a similar test. I would almost remove the link to the issue in
the source because it isn't really relevant except for historical reasons.

Quoted issue: #9382
Original Commit: 920ca61

Excerpt of what the struct definitions looked like originally.

struct Thing1<'self> {
    baz: &'self [~int],
    bar: ~u64,
}

struct Thing2<'self> {
    baz: &'self [~int],
    bar: u64,
}

@jieyouxu jieyouxu assigned jieyouxu and unassigned compiler-errors Mar 18, 2025
@jieyouxu
Copy link
Member

I wasn't sure if it was best practice to change this value as you go along or
once at the end. I can rebase to something that incrementally changes the value
in the "cleaned" commits if that is preferred.

At the end is best, otherwise there's a bunch of unnecessary number changes that is super prone to merge conflicts.

Copy link
Member

@jieyouxu jieyouxu left a comment

Choose a reason for hiding this comment

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

Thanks for the PR! I left some feedback.

@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 Mar 18, 2025
@compiler-errors
Copy link
Member

Can you make sure this is squashed into one commit at the end? There's no reason that this needs 8 commits to change 8 test files.

@spencer3035
Copy link
Contributor Author

Addressed the feedback

@rustbot ready

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Mar 21, 2025
Copy link
Member

@jieyouxu jieyouxu left a comment

Choose a reason for hiding this comment

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

One comment nit, then please squash the commits into one

@jieyouxu jieyouxu 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 Mar 21, 2025
@spencer3035 spencer3035 force-pushed the clean-ui-tests-2-of-n branch from 3f5a8aa to 5e6b459 Compare March 22, 2025 04:58
@spencer3035
Copy link
Contributor Author

Squashed the commits and applied minor change to wording of coercion/struct-literal-field-type-coercion-to-expected-type.rs.

@rustbot ready

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Mar 22, 2025
Copy link
Member

@jieyouxu jieyouxu left a comment

Choose a reason for hiding this comment

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

Thanks!

@jieyouxu
Copy link
Member

@bors r+ rollup

@bors
Copy link
Collaborator

bors commented Mar 22, 2025

📌 Commit 5e6b459 has been approved by jieyouxu

It is now in the queue for this repository.

@bors bors removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 22, 2025
@bors bors added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Mar 22, 2025
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 22, 2025
…iaskrgr

Rollup of 7 pull requests

Successful merges:

 - rust-lang#138609 (Add stack overflow handler for cygwin)
 - rust-lang#138639 (Clean UI tests 2 of n)
 - rust-lang#138773 (catch_unwind intrinsic: document return value)
 - rust-lang#138782 (test(ui): add tuple-struct-where-clause-suggestion ui test for rust-lang#91520)
 - rust-lang#138794 (expand: Do not report `cfg_attr` traces on macros as unused attributes)
 - rust-lang#138801 (triagebot: add autolabel rules for D-* and L-*)
 - rust-lang#138804 (Allow inlining for `Atomic*::from_ptr`)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit f01d096 into rust-lang:master Mar 22, 2025
6 checks passed
@rustbot rustbot added this to the 1.87.0 milestone Mar 22, 2025
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Mar 22, 2025
Rollup merge of rust-lang#138639 - spencer3035:clean-ui-tests-2-of-n, r=jieyouxu

Clean UI tests 2 of n

Modified 4 tests in tests/ui. Cleaned 3 and deleted one.

I have a final commit changing the values in `src/tools/tidy/src/ui_tests.rs`.
I wasn't sure if it was best practice to change this value as you go along or
once at the end. I can rebase to something that incrementally changes the value
in the "cleaned" commits if that is preferred.

Related Issues:
rust-lang#73494
rust-lang#133895

r? jieyouxu
github-actions bot pushed a commit to model-checking/verify-rust-std that referenced this pull request Apr 2, 2025
…iaskrgr

Rollup of 7 pull requests

Successful merges:

 - rust-lang#138609 (Add stack overflow handler for cygwin)
 - rust-lang#138639 (Clean UI tests 2 of n)
 - rust-lang#138773 (catch_unwind intrinsic: document return value)
 - rust-lang#138782 (test(ui): add tuple-struct-where-clause-suggestion ui test for rust-lang#91520)
 - rust-lang#138794 (expand: Do not report `cfg_attr` traces on macros as unused attributes)
 - rust-lang#138801 (triagebot: add autolabel rules for D-* and L-*)
 - rust-lang#138804 (Allow inlining for `Atomic*::from_ptr`)

r? `@ghost`
`@rustbot` modify labels: rollup
# for free to join this conversation on GitHub. Already have an account? # 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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants