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

compiletest: warn/error on redundant check-fail directives #130742

Open
jieyouxu opened this issue Sep 23, 2024 · 6 comments
Open

compiletest: warn/error on redundant check-fail directives #130742

jieyouxu opened this issue Sep 23, 2024 · 6 comments
Labels
A-compiletest Area: The compiletest test runner C-enhancement Category: An issue proposing an enhancement or a PR with one. E-medium Call for participation: Medium difficulty. Experience needed to fix: Intermediate. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)

Comments

@jieyouxu
Copy link
Member

jieyouxu commented Sep 23, 2024

check-fail here is redundant

Originally posted by @compiler-errors in #130718 (comment)

Some test suites have a default test behavior, like //@ check-fail, in which case specifying that explicitly in the test is redundant and useless noise. When compiletest directive handling is worked, we should warn or error on redundant directives like these and also explain why it's redundant, e.g. "ui test mode is check-fail by default".

Remark: this check should not be added before reworking how compiletest directives are handled as it's not just one test suite or directive.

@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Sep 23, 2024
@jieyouxu jieyouxu added C-enhancement Category: An issue proposing an enhancement or a PR with one. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. A-compiletest Area: The compiletest test runner and removed needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels Sep 23, 2024
@jieyouxu
Copy link
Member Author

This is E-easy but just randomly adding checks here an there isn't a maintainable solution.

@jieyouxu jieyouxu added T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) and removed T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Oct 17, 2024
@Shunpoco
Copy link

@jieyouxu
If it is not blocked by other issues, can I take this ticket?

@jieyouxu
Copy link
Member Author

@Shunpoco Hi, I would prefer to not spot-fix this, and instead rework the logic such that it falls naturally out of test suite presets vs incompatible flag checking.

@jieyouxu jieyouxu added the E-medium Call for participation: Medium difficulty. Experience needed to fix: Intermediate. label Nov 22, 2024
@Shunpoco
Copy link

rework the logic such that it falls naturally out of test suite presets vs incompatible flag checking.

I thought that I can modify TestProps's function (perhaps update_fail_mode) to warn/error on such a redundant directive, but do you mean we will reconstruct functions to achieve your thought?

@jieyouxu
Copy link
Member Author

jieyouxu commented Nov 22, 2024

I moreso meant that it's not just //@ check-fail, that compiletest should have a centralized mechanism for checking for incompatibilities between directive <-> test suite presets <-> compile-flags. If such infra is implemented, then this particular instance should just be a matter of registering as a conflict.

@Shunpoco
Copy link

Thanks. I didn't find #130565, but this issue belongs to the phase3 in that tracking issue, right?
I will check the draft PR and zulip channel to grasp more context of issues 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-compiletest Area: The compiletest test runner C-enhancement Category: An issue proposing an enhancement or a PR with one. E-medium Call for participation: Medium difficulty. Experience needed to fix: Intermediate. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)
Projects
Development

No branches or pull requests

3 participants