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.

Sign up for GitHub

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

Jump to bottom

compiletest should check for possible typos in annotated test directives #83551

Closed
klensy opened this issue Mar 27, 2021 · 4 comments · Fixed by #121561
Closed

compiletest should check for possible typos in annotated test directives #83551

klensy opened this issue Mar 27, 2021 · 4 comments · Fixed by #121561
Labels
A-testsuite Area: The testsuite used to check the correctness of rustc C-enhancement Category: An issue proposing an enhancement or a PR with one. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)

Comments

@klensy
Copy link
Contributor

klensy commented Mar 27, 2021

This should help with things like #83348 (comment)
Levenshtein distance with list of known directives will be enough?
@klensy
Copy link
Contributor Author

klensy commented Mar 27, 2021

@rustbot label: A-testsuite

@rustbot rustbot added the A-testsuite Area: The testsuite used to check the correctness of rustc label Mar 27, 2021
@osa1
Copy link
Contributor

osa1 commented Mar 27, 2021

Levenshtein distance with list of known directives will be enough?

Is this to distinguish an ordinary comment from a directive?

@klensy
Copy link
Contributor Author

klensy commented Mar 27, 2021

@osa1
Yes, this is the first idea that came into my mind.

@wesleywiser wesleywiser mentioned this issue Nov 5, 2021
Closed
@Enselic Enselic added C-enhancement Category: An issue proposing an enhancement or a PR with one. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) labels Dec 17, 2023
@jieyouxu
Copy link
Member

This is easier if/when we port over all current // directives in all test suites and modes over to use ui_test-style //@ directives, so that it is easier to detect between regular comments and directives.

Other bootstrap tools like tidy should also probably use something else to distinguish them from regular comments. Otherwise, trying to check between a valid directive vs comment is really cursed, and comments like

// testing testing one two three
// ignore-test is a very useful
// directive to have in compiletest

gets treated as // ignore-test, making it quite surprising.

@jieyouxu jieyouxu mentioned this issue Feb 24, 2024
Merged
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Mar 9, 2024
@matthiaskrgr
Rollup merge of rust-lang#121561 - jieyouxu:compiletest-directive-typ…
0953296 
…o-check, r=onur-ozkan

Detect typos for compiletest test directives

Checks directives against a known list of compiletest directives collected during migration from legacy-style compiletest directives. A suggestion for the best matching known directive will be made if an invalid directive is found.

This PR does not attempt to implement checks for Makefile directives because they still have the problem of regular comments and directives sharing the same comment prefix `#`.

Closes rust-lang#83551.
workingjubilee added a commit to workingjubilee/rustc that referenced this issue Mar 10, 2024
@workingjubilee
Rollup merge of rust-lang#121561 - jieyouxu:compiletest-directive-typ…
b928e12 
…o-check, r=onur-ozkan

Detect typos for compiletest test directives

Checks directives against a known list of compiletest directives collected during migration from legacy-style compiletest directives. A suggestion for the best matching known directive will be made if an invalid directive is found.

This PR does not attempt to implement checks for Makefile directives because they still have the problem of regular comments and directives sharing the same comment prefix `#`.

Closes rust-lang#83551.
@bors bors closed this as completed in af69f4c Mar 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
A-testsuite Area: The testsuite used to check the correctness of rustc C-enhancement Category: An issue proposing an enhancement or a PR with one. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Detect typos for compiletest test directives jieyouxu/rust
5 participants
@Enselic @osa1 @klensy @jieyouxu @rustbot