Skip to content

UI Test Cleanup: Split up checked_unwrap tests #4276

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
Merged
merged 1 commit into from
Jul 16, 2019
Merged

UI Test Cleanup: Split up checked_unwrap tests #4276

merged 1 commit into from
Jul 16, 2019

Conversation

phansch
Copy link
Member

@phansch phansch commented Jul 15, 2019 •
edited
Loading

Let's slowly bring that ticket closer to the finish line 🐌 🏁

This splits up tests/ui/checked_unwrap.rs into:

  • tests/ui/checked_unwrap/complex.rs
  • tests/ui/checked_unwrap/simple.rs

Based on the naming of the methods in the tests.

changelog: none

cc #2038

@phansch
Copy link
Member Author

phansch commented Jul 15, 2019 •
edited
Loading

A small bike-shedding opportunity: I'm totally open to other naming schemes for these files:

  • simple and complex seem a bit arbitrary, but I can't think of anything better.
  • Maybe not associating any meaning at all makes it easier to decide where to put new tests for this lint? So maybe 1.rs, 2.rs, 3.rs, etc?

🚲 🏚️

@flip1995
Copy link
Member

Maybe single_conditional vs multiple_conditionals? I don't think allowing names like 1.rs, ... would be a good idea, because you would need to look through all those files to find where to put a potential new test case.

r=me with telling names (simple and complex is also good enough IMO)

@matthiaskrgr matthiaskrgr added the S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status) label Jul 16, 2019
@phansch
UI Test Cleanup: Split up checked_unwrap tests
48bff49 
This splits up `tests/ui/checked_unwrap.rs` into:

 * `tests/ui/checked_unwrap/complex.rs`
 * `tests/ui/checked_unwrap/simple.rs`

Based on the naming of the methods in the tests.

cc rust-lang#2038
@phansch
Copy link
Member Author

phansch commented Jul 16, 2019

@bors r=flip1995 I changed the filenames to include conditionals, that makes them a bit more clear. Thanks for the suggestion!

@bors
Copy link
Contributor

bors commented Jul 16, 2019

📌 Commit 48bff49 has been approved by flip1995

@bors
Copy link
Contributor

bors commented Jul 16, 2019

⌛ Testing commit 48bff49 with merge f53e55f...

bors added a commit that referenced this pull request Jul 16, 2019
@bors
Auto merge of #4276 - phansch:uitestcleanup, r=flip1995
f53e55f 
UI Test Cleanup: Split up checked_unwrap tests

Let's slowly bring that ticket closer to the finish line 🐌 🏁

This splits up `tests/ui/checked_unwrap.rs` into:

 * `tests/ui/checked_unwrap/complex.rs`
 * `tests/ui/checked_unwrap/simple.rs`

Based on the naming of the methods in the tests.

cc #2038
@phansch phansch removed the S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status) label Jul 16, 2019
@bors
Copy link
Contributor

bors commented Jul 16, 2019

💔 Test failed - checks-travis

@phansch
Copy link
Member Author

phansch commented Jul 16, 2019

@bors retry (forgot the changelog)

@bors
Copy link
Contributor

bors commented Jul 16, 2019

⌛ Testing commit 48bff49 with merge 7498a5f...

bors added a commit that referenced this pull request Jul 16, 2019
@bors
Auto merge of #4276 - phansch:uitestcleanup, r=flip1995
7498a5f 
UI Test Cleanup: Split up checked_unwrap tests

Let's slowly bring that ticket closer to the finish line 🐌 🏁

This splits up `tests/ui/checked_unwrap.rs` into:

 * `tests/ui/checked_unwrap/complex.rs`
 * `tests/ui/checked_unwrap/simple.rs`

Based on the naming of the methods in the tests.

changelog: none

cc #2038
@bors
Copy link
Contributor

bors commented Jul 16, 2019

☀️ Test successful - checks-travis, status-appveyor
Approved by: flip1995
Pushing 7498a5f to master...

@bors bors merged commit 48bff49 into rust-lang:master Jul 16, 2019
@phansch phansch deleted the uitestcleanup branch July 17, 2019 05:03
@flip1995
Copy link
Member

flip1995 commented Jul 17, 2019 •
edited
Loading

(forgot the changelog)

Yay, so that works 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@phansch @flip1995 @bors @matthiaskrgr