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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

UI Test Cleanup: Split up checked_unwrap tests #4276

Merged
merged 1 commit into from Jul 16, 2019

Conversation

@phansch
Copy link
Member

commented Jul 15, 2019

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

This comment has been minimized.

Copy link
Member Author

commented Jul 15, 2019

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

This comment has been minimized.

Copy link
Member

commented Jul 16, 2019

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)

UI Test Cleanup: Split up checked_unwrap tests
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 force-pushed the phansch:uitestcleanup branch from e4cd23c to 48bff49 Jul 16, 2019

@phansch

This comment has been minimized.

Copy link
Member Author

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

This comment has been minimized.

Copy link
Contributor

commented Jul 16, 2019

馃搶 Commit 48bff49 has been approved by flip1995

@bors

This comment has been minimized.

Copy link
Contributor

commented Jul 16, 2019

鈱涳笍 Testing commit 48bff49 with merge f53e55f...

bors added a commit that referenced this pull request Jul 16, 2019
Auto merge of #4276 - phansch:uitestcleanup, r=flip1995
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
@bors

This comment has been minimized.

Copy link
Contributor

commented Jul 16, 2019

馃挃 Test failed - checks-travis

@phansch

This comment has been minimized.

Copy link
Member Author

commented Jul 16, 2019

@bors retry (forgot the changelog)

@bors

This comment has been minimized.

Copy link
Contributor

commented Jul 16, 2019

鈱涳笍 Testing commit 48bff49 with merge 7498a5f...

bors added a commit that referenced this pull request Jul 16, 2019
Auto merge of #4276 - phansch:uitestcleanup, r=flip1995
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

This comment has been minimized.

Copy link
Contributor

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

2 checks passed

Travis CI - Pull Request Build Passed
Details
homu Test successful
Details

@phansch phansch deleted the phansch:uitestcleanup branch Jul 17, 2019

@flip1995

This comment has been minimized.

Copy link
Member

commented Jul 17, 2019

(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
Projects
None yet
4 participants
You can鈥檛 perform that action at this time.