Filter ui revision tests

[bovinebuddha]

Updates UI test output filtering to also filter away test annotations for revisions:

Previously filtered: //~ ERROR [XXXX]
Now also filters: //[revision]~ ERROR [XXXX]

I reckon, if we have the one, we should have the other for consistency, its lack was probably an oversight (the existence of revision testing is not really well documented...)

[rust-highfive]

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @nikomatsakis (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

[petrochenkov]

This is just an oversight, yes, thanks for fixing it.

You may want to check whether fulldeps tests and NLL tests are updated properly.

./x.py test --bless src/test/ui --compare-mode nll
./x.py test --bless src/test/ui-fulldeps

and otherwise search for accidental //[revision]~s in .stderr and .stdout files that could remain due to tests not running on all platforms.

r=me after that

[bovinebuddha]

r? @petrochenkov

I applied some sed-magic to the test folder, hopefully this will fix all the tests I didn't/couldn't run.

[petrochenkov]

@bors r+ p=1

I suspect that these ones are just stray files and can be removed though

E0508.ast.stderr
E0508.mir.stderr
borrowck/borrowck-union-borrow.mir.stderr

[bors]

📌 Commit 9eedfdea35797da3cc03acf9882ca2b9df87f2f2 has been approved by petrochenkov

[bovinebuddha]

@petrochenkov Do you want me to remove those files? Also, what is the common practice, should I squash the last two commits (and possibly the last one) into one commit? Or maybe noone cares?

[petrochenkov]

@bovinebuddha
If you remove the tests, I'll re-r+.
In this case I don't think any squashing is required (people generally care to various degree, depending on reviewer).

[bovinebuddha]

I found 402 other stderr revision files which are no longer used.... I'd like to remove them all, but maybe I'll do that in another PR once this lands.

Would it be okay to add a bash script under src/test with a make command to remove these extraneous files? It was a 'simple' exercise of find, grep, sed. Or should something maybe be added to x.py instead?

[petrochenkov]

@bovinebuddha

I found 402 other stderr revision files which are no longer used

Are you sure those are not produced by NLL tests or something?

Would it be okay to add a bash script under src/test with a make command to remove these extraneous files?

We already have a check for unused files in tools/tidy/src/ui_tests.rs, it's run during x.py test tidy and can be expanded if necessary.

[bovinebuddha]

Ah, you were right of course. Those were all nll checks plus a few false positives by me. I didn't realize that revisions and the nll compare mode had the same mechanism for generating stderr files...

I pushed a commit which removed the files you mentioned, there were no other obsolete files I could find.

[bovinebuddha]

@petrochenkov Thanks for the help, and please r+ when you have the time :)

[petrochenkov]

@bors r+

[bors]

📌 Commit 704649d has been approved by petrochenkov

[bovinebuddha]

@petrochenkov

How do you actually know which tests are executed for each compare mode? I was thinking about updating 'tools/tidy/src/ui_tests.rs' to check if the test actually specified the relevant revision. I don't know where to check to resolve mode/revision ambiguity though.

[petrochenkov]

I'm not entirely sure, but from what I can infer from code in compiletest the .nll versions of output are created only if they are different from non-NLL output.
So, each test in test suites that are run with NLL (i.e. ui and run-pass) can potentially produce an .nll file.

Revisions that are run for each specific test are specified in its // revisions: clause.

[bovinebuddha]

Hmmm, I see. I also find this fascinating 'ignore-compare-mode-nll' annotations in tests which actually specify nll as a revision. I guess any check would have to hardcode all possible compare modes for their possible existence.

[bors]

⌛ Testing commit 704649d with merge 817d074...

[bors]

☀️ Test successful - checks-travis, status-appveyor
Approved by: petrochenkov
Pushing 817d074 to master...