Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Support multiple revisions in compiletest #66524

Merged
merged 4 commits into from
Nov 22, 2019

Conversation

ecstatic-morse
Copy link
Contributor

The //[X]~ syntax filters errors for tests that are run across multiple cfgs with // revisions:. This commit extends that syntax to accept //[X,Y]~, which will match multiple cfgs to the same error annotation. This is functionally the same as writing two comments, //[X]~ and //[Y]~, but can fit on a single line.

While refactoring compiletest to support this, I also uncovered a small bug that was causing an incremental test to always pass, despite no errors being emitted.

r? @Centril

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Nov 18, 2019
match line[kind_start..]

// Get the part of the comment after the sigil (e.g. `~^^` or ~|).
let (_, mut msg) = line.split_at(captures.get(0).unwrap().end());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@BurntSushi Have you considered providing a method for the 0th-always-OK group to make things clearer and avoid the .unwrap() in client code? Would have been helpful during review.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Haven't considered it, but it's not a bad idea! Thanks.

@Centril
Copy link
Contributor

Centril commented Nov 19, 2019

@bors r+

@bors
Copy link
Contributor

bors commented Nov 19, 2019

📌 Commit fd49547d03a0277818495f4028475d425abe4fe0 has been approved by Centril

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 19, 2019
@bors
Copy link
Contributor

bors commented Nov 20, 2019

☔ The latest upstream changes (presumably #66578) made this pull request unmergeable. Please resolve the merge conflicts.

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Nov 20, 2019
The `//[X]~` syntax filters errors for tests that are run across
multiple cfgs with  `// revisions:`. This commit extends that syntax to
accept `//[X,Y]~`, which will match multiple cfgs to the same error
annotation. This is functionally the same as writing two comments,
`//[X]~` and `//[Y]~`, but can fit on a single line.
This test does not actually emit any warnings, since
`#![allow(warnings)]` was specified. `compiletest` was erroneously
ignoring `//~` tests and looking only for `//[X]~` ones. As a result of
the changes in the previous commit, we now look for `//~` comments in
incremental tests and expect them to appear in *all* revisions.
@ecstatic-morse ecstatic-morse force-pushed the compiletest-multiple-revisions branch from fd49547 to ae22938 Compare November 21, 2019 22:10
@ecstatic-morse
Copy link
Contributor Author

@bors r=Centril

@bors
Copy link
Contributor

bors commented Nov 21, 2019

📌 Commit c537f22 has been approved by Centril

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Nov 21, 2019
@bors
Copy link
Contributor

bors commented Nov 22, 2019

⌛ Testing commit c537f22 with merge bd816fd...

bors added a commit that referenced this pull request Nov 22, 2019
… r=Centril

Support multiple revisions in `compiletest`

The `//[X]~` syntax filters errors for tests that are run across multiple cfgs with  `// revisions:`. This commit extends that syntax to accept `//[X,Y]~`, which will match multiple cfgs to the same error annotation. This is functionally the same as writing two comments, `//[X]~` and `//[Y]~`, but can fit on a single line.

While refactoring `compiletest` to support this, I also uncovered a small bug that was causing an incremental test to always pass, despite no errors being emitted.

r? @Centril
@bors
Copy link
Contributor

bors commented Nov 22, 2019

☀️ Test successful - checks-azure
Approved by: Centril
Pushing bd816fd to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Nov 22, 2019
@bors bors merged commit c537f22 into rust-lang:master Nov 22, 2019
@rust-highfive
Copy link
Collaborator

📣 Toolstate changed by #66524!

Tested on commit bd816fd.
Direct link to PR: #66524

💔 rustc-guide on linux: test-pass → test-fail (cc @JohnTitor @amanjeev @spastorino @mark-i-m, @rust-lang/infra).

rust-highfive added a commit to rust-lang-nursery/rust-toolstate that referenced this pull request Nov 22, 2019
Tested on commit rust-lang/rust@bd816fd.
Direct link to PR: <rust-lang/rust#66524>

💔 rustc-guide on linux: test-pass → test-fail (cc @JohnTitor @amanjeev @spastorino @mark-i-m, @rust-lang/infra).
@ecstatic-morse ecstatic-morse deleted the compiletest-multiple-revisions branch October 6, 2020 01:42
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants