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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tracking Issue: Split up our UI-tests into smaller parts #2038

Closed
oli-obk opened this issue Sep 10, 2017 · 8 comments · Fixed by #5130
Closed

Tracking Issue: Split up our UI-tests into smaller parts #2038

oli-obk opened this issue Sep 10, 2017 · 8 comments · Fixed by #5130
Labels
C-an-interesting-project Category: Interesting projects, that usually are more involved design/code wise. C-tracking-issue Category: Tracking Issue E-help-wanted Call for participation: Help is requested to fix this issue. good-first-issue These issues are a good way to get started with Clippy

Comments

@oli-obk
Copy link
Contributor

oli-obk commented Sep 10, 2017

Changing files like tests/ui/methods.rs causes a lot of diff noise. I think the ui output of every file should be kept below 100 lines preferrably and below 200 as a hard limit.

The files would best be split up to only check one or two lints

Current status:

$ wc -l tests/ui/*.stderr | sort -nr | head -n 15
   272 tests/ui/cognitive_complexity.stderr
   260 tests/ui/collapsible_if.stderr
   259 tests/ui/non_copy_const.stderr
   258 tests/ui/missing-doc.stderr
   247 tests/ui/if_same_then_else.stderr
   246 tests/ui/transmute.stderr
   245 tests/ui/match_same_arms.stderr
   224 tests/ui/needless_range_loop.stderr
   220 tests/ui/drop_forget_ref.stderr
   214 tests/ui/use_self.stderr
   214 tests/ui/eq_op.stderr
   213 tests/ui/indexing_slicing.stderr
   210 tests/ui/option_map_unit_fn.stderr
   203 tests/ui/booleans.stderr
@oli-obk oli-obk added C-an-interesting-project Category: Interesting projects, that usually are more involved design/code wise. good-first-issue These issues are a good way to get started with Clippy labels Sep 10, 2017
@llogiq
Copy link
Contributor

llogiq commented Sep 10, 2017

Perhaps we should also add a check to our test suite so we get a warning if a test gets too big?

@oli-obk
Copy link
Contributor Author

oli-obk commented Sep 10, 2017

We'd need to patch compiletest to do that nicely. Which reminds me... I wanted to make compiletest work on stable ^^

@mcarton
Copy link
Member

mcarton commented Sep 11, 2017

👍 UI tests can be really hard to check for updates when they are too long

@llogiq
Copy link
Contributor

llogiq commented Sep 11, 2017

Looking at wc output, I think we should set the limit at 300 lines of stderr output:

   464 tests/ui/cast.stderr
   582 tests/ui/for_loop.stderr
   742 tests/ui/methods.stderr

phansch added a commit to phansch/rust-clippy that referenced this issue Jan 5, 2018
phansch added a commit to phansch/rust-clippy that referenced this issue Jan 5, 2018
phansch added a commit to phansch/rust-clippy that referenced this issue Jan 6, 2018
@phansch

This comment has been minimized.

phansch added a commit to phansch/rust-clippy that referenced this issue Apr 5, 2018
@phansch
Copy link
Member

phansch commented Apr 7, 2018

We'd need to patch compiletest to do that nicely. Which reminds me... I wanted to make compiletest work on stable ^^

Looks like stable compiletest-rs is being worked on 🎉 Manishearth/compiletest-rs#107

bors bot added a commit that referenced this issue Oct 28, 2018
3372: UI test cleanup: Extract explicit_counter_loop tests r=matthiaskrgr a=phansch

cc #2038 

Co-authored-by: Philipp Hansch <dev@phansch.net>
bors bot added a commit that referenced this issue Oct 28, 2018
3373: UI test cleanup: Extract unnecessary_operation tests r=matthiaskrgr a=phansch

cc #2038 

Co-authored-by: Philipp Hansch <dev@phansch.net>
bors bot added a commit that referenced this issue Oct 31, 2018
3392: UI test cleanup: Extract for_loop_over_x tests r=matthiaskrgr a=phansch

cc #2038

Co-authored-by: Philipp Hansch <dev@phansch.net>
bors bot added a commit that referenced this issue Nov 2, 2018
3397: UI test cleanup: Extract expect_fun_call tests r=matthiaskrgr a=phansch

Note that the new stderr file does not include a `shadow-unrelated`
error, because the new UI test file does not use `#![warn(clippy::all)]`

cc #2038 

3398: UI test cleanup: Extract match_overlapping_arm tests r=matthiaskrgr a=phansch

cc #2038

Co-authored-by: Philipp Hansch <dev@phansch.net>
phansch added a commit to phansch/rust-clippy that referenced this issue Dec 4, 2018
There's only one test currently.
I also updated the lint doc with a 'good' example and changed the lint
help text a bit.

cc rust-lang#2038
bors added a commit that referenced this issue Oct 28, 2019
UI test cleanup: Extract derive_hash_xor_eq tests

changelog: none

cc #2038
bors added a commit that referenced this issue Jan 7, 2020
Split up `collapsible_if` ui test

Part of #2038

changelog: none
bors added a commit that referenced this issue Jan 9, 2020
Split up `missing-doc` ui test

Part of #2038

changelog: none
bors added a commit that referenced this issue Jan 13, 2020
Split up `use_self` ui test

Part of #2038

changelog: none
bors added a commit that referenced this issue Jan 19, 2020
Split up `if_same_then_else` ui test

Part of #2038

changelog: none
bors added a commit that referenced this issue Jan 21, 2020
Split up `transmute` ui test

Part of #2038

changelog: none
bors added a commit that referenced this issue Jan 24, 2020
Split up `needless_range_loop` ui test

Part of #2038

changelog: none
bors added a commit that referenced this issue Jan 25, 2020
Split up `match_same_arms` ui test

Part of #2038

changelog: none
bors added a commit that referenced this issue Jan 27, 2020
Split up `non_copy_const` ui test

Part of #2038
Maybe there is a better way to avoid duplications of constants.

changelog: none
bors added a commit that referenced this issue Jan 30, 2020
Split up `match` ui test

Part of #2038

Also, this decreases the line length limit to 220.

changelog: none
bors added a commit that referenced this issue Feb 2, 2020
Split up `drop_forget_ref` ui test

Part of #2038

changelog: none
bors added a commit that referenced this issue Feb 2, 2020
Split up `drop_forget_ref` ui test

Part of #2038

changelog: none
@bors bors closed this as completed in 536c255 Feb 3, 2020
@bors bors closed this as completed in #5130 Feb 3, 2020
@basil-cow
Copy link
Contributor

This can now be unpinned

@phansch phansch unpinned this issue Feb 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-an-interesting-project Category: Interesting projects, that usually are more involved design/code wise. C-tracking-issue Category: Tracking Issue E-help-wanted Call for participation: Help is requested to fix this issue. good-first-issue These issues are a good way to get started with Clippy
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants