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

reconsider magic -A unused for UI tests? #43896

Closed
zackmdavis opened this issue Aug 16, 2017 · 4 comments
Closed

reconsider magic -A unused for UI tests? #43896

zackmdavis opened this issue Aug 16, 2017 · 4 comments
Labels
A-testsuite Area: The testsuite used to check the correctness of rustc C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@zackmdavis
Copy link
Member

I agree with this reasoning for compile-fail tests (for which composing and revising test-expectation comments can be laborious), but for UI tests (for which the expectation files are automatically generated, anyway), test authors have a reasonable expectation that the test output will be the same as when manually invoking rustc on the test file; discovering that this is not the case may be something of a rude surprise.

cc @alexcrichton

@petrochenkov
Copy link
Contributor

but for UI tests (for which the expectation files are automatically generated, anyway), test authors have a reasonable expectation that the test output will be the same as when manually invoking rustc on the test file

On the other hand, unused warnings in the UI test expectation file are still noise obscuring the actually tested expectations.
(Note, that all compile-fail tests are going to be UI-tested in hopefully near future.)

discovering that this is not the case may be something of a rude surprise.

Documentation can solve this?

@alexcrichton alexcrichton added the A-testsuite Area: The testsuite used to check the correctness of rustc label Aug 16, 2017
@steveklabnik steveklabnik added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Aug 16, 2017
@Mark-Simulacrum Mark-Simulacrum added the C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC label Aug 20, 2017
@Mark-Simulacrum
Copy link
Member

I think there are significant downsides to this, and few gains; typically you check all UI test outputs when adding new tests or modifying old ones, so it seems rare that this comes up in practice.

I'm going to close this issue since it seems like we're unlikely to actually implement this; if we'd be interested then exploring avenues I listed above (compiler team meetings, or rfcbot poll) would be good.

@petrochenkov
Copy link
Contributor

We need to disable -A unused for // run-pass tests though, which are also a part of UI tests now, but should not have dead code in general.

@Mark-Simulacrum
Copy link
Member

I'll open a PR shortly doing that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-testsuite Area: The testsuite used to check the correctness of rustc C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

5 participants