Hundreds of ignore'd tests are bitrotting in the test suite

[bstrie]

git grep -l xfail -- src/test/ | wc -l shows 235 xfail'd tests. Some of these are using ancient language features; e.g. https://github.com/mozilla/rust/blob/master/src/test/run-fail/too-much-recursion-unwinding.rs is still making use of classes. Surely some of these tests can be updated and made to work with the current compiler.

[catamorphism]

I'll see how far I can get with this.

[catamorphism]

Down to 217! This can be an ongoing "brain is too dead to do anything else" project.

[catamorphism]

Down to 140, not counting ones that are marked xfail-fast.

Nominating for milestone 4, "well-covered". This could be an easy starter project for many people: pick a few tests that are xfailed, and either fix them so they work, or file a bug saying that the test is still broken.

[ghost]

I have been suggested this bug by cmr on IRC as a very good first bug as it shall enable me to see lot of code and I wish to work on this

[bstrie]

@rajul-iitkgp be warned, this may not be so simple. You must be very careful to ensure that every test is still exercising the same code paths today as it was originally. In addition, many concepts from ancient Rust have no direct equivalent in modern Rust. Some tests may simply not be salvageable and may need to be deleted. When in doubt, ask a developer.

[emberian]

Up to 374 ;(

[graydon]

accepted for well-covered milestone

[astrieanna]

What is the right thing to do with xfailed tests that are for issues that have been closed as won't fix?
For example, #912 and its test run-pass/issue-912.rs.
The test now fails for a different reason (the [{id: 20}] syntax), but it seems like it should just be removed.

Additionally, there are (well, at least one) tests that are xfailed because they aren't tests. For example, run-pass/mod_file_aux.rs, which says xfail-test Not a test. Used by other tests. Is there a better way to indicate "not a test" than xfail-test? (It's reasonable for there not to be, since it's probably a very small category; just annoying for the uninitiated looking for tests to fix.)

[alexcrichton]

The test files like mod_file_aux as xfail-test are fine, they're just used by other tests when exercising parser features.

As for tests like issue-912.rs, I think that the idea here is to update as many as possible to the current "today syntax" while jettisoning those which are broken beyond repair or are simply never going to get fixed. For the one in the case of 912, it sounds like it just needs to get removed.

[pnkfelix]

Assigning P-low.

[steveklabnik]

$ git grep -l ignore -- src/test | wc -l
327

[tamird]

$ git grep -l '//\s*ignore' -- src/test | wc -l
321

[klutzy]

cc #13745

[steveklabnik]

Looking at my previous attempts to tackle this issue, I now realize that my regular expression may have been a bit too lose: we have ignore-test, ignore-pretty, and such. It seems to me that this ticket is mostly about ignore-test.

In that case,

$ git grep -l '//\s*ignore-test' -- src/test head | wc -l
57

Still a ways to go, but much shorter :)

[steveklabnik]

@bstrie now that #26253 has landed, does that address your original ticket? we're never going to get to zero of these issues, but it seems to me the more specific ignores aren't nearly as bad as the straight-up ignored ones.

[tamird]

@steveklabnik I think this issue is asking for a systematic way to keep these tests from rotting. In the rspec terminology these tests are currently skipped, while they should be pending.

[tshepang]

git grep -l '//\s*ignore-test' -- src/test | wc -l

now outputs

18

[bstrie]

@tamird I'm actually fine closing this now. Not only have we excised the rotten cases from the test suite, but Rust is no longer breaking and so tests have no good reason to be ignored in the future. As @tshepang notes we still have a handful left, but this number is entirely manageable.