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: Support matching on diagnostics without a span #138865

Merged
merged 1 commit into from
Mar 25, 2025

Conversation

petrochenkov
Copy link
Contributor

Using //~? ERROR my message on any line of the test.

The new checks are exhaustive, like all other //~ checks, and unlike the error-pattern directive that is sometimes used now to check for span-less diagnostics.

This will allow to eliminate most on error-pattern directives in compile-fail tests (except those that are intentionally imprecise due to platform-specific diagnostics).
I didn't migrate any of error-patterns in this PR though, except those where the migration was necessary for the tests to pass.

@rustbot
Copy link
Collaborator

rustbot commented Mar 23, 2025

r? @wesleywiser

rustbot has assigned @wesleywiser.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added A-compiletest Area: The compiletest test runner A-testsuite Area: The testsuite used to check the correctness of rustc PG-exploit-mitigations Project group: Exploit mitigations S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. labels Mar 23, 2025
@rustbot
Copy link
Collaborator

rustbot commented Mar 23, 2025

Some changes occurred in src/tools/compiletest

cc @jieyouxu

Some changes occurred in tests/ui/sanitizer

cc @rust-lang/project-exploit-mitigations, @rcvalle

Some changes occurred in tests/ui/stack-protector

cc @rust-lang/project-exploit-mitigations, @rcvalle

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@jieyouxu
Copy link
Member

r? jieyouxu

@rustbot rustbot assigned jieyouxu and unassigned wesleywiser Mar 23, 2025
@bors

This comment was marked as resolved.

Copy link
Member

@jieyouxu jieyouxu left a comment

Choose a reason for hiding this comment

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

Nice! The impl looks good to me, though some tests may need further adjusting (with some back and forth) due to platform specificness and other ignore conditions.

@jieyouxu
Copy link
Member

Please ping me when you rebase/rebless so I can try to review ASAP, otherwise this will likely conflict very quickly.
@bors p=10 (very conflict-prone)

@jieyouxu
Copy link
Member

jieyouxu commented Mar 24, 2025

Also, now that rustc-dev-guide is also a subtree, can you please adjust the UI annotations page in rustc-dev-guide at https://rustc-dev-guide.rust-lang.org/tests/ui.html#error-annotations in this same PR? So that people will know about what //~? actually means. It would nice if we can discuss also in the docs when to use //@ error-pattern vs //~?, i.e. sth like "prefer to use //~? in UI tests unless the diagnostics is intentionally platform-specific or the test intention needs the vagueness of //@ error-pattern intentionally".

@jieyouxu jieyouxu 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-review Status: Awaiting review from the assignee but also interested parties. labels Mar 24, 2025
@rustbot rustbot added the A-rustc-dev-guide Area: rustc-dev-guide label Mar 25, 2025
@rustbot
Copy link
Collaborator

rustbot commented Mar 25, 2025

The rustc-dev-guide subtree was changed. If this PR only touches the dev guide consider submitting a PR directly to rust-lang/rustc-dev-guide otherwise thank you for updating the dev guide with your changes.

cc @BoxyUwU, @jieyouxu, @Kobzol

Copy link
Member

@jieyouxu jieyouxu left a comment

Choose a reason for hiding this comment

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

Thanks, both the impl and the test diffs look good to me!

@jieyouxu
Copy link
Member

r=me if the PR CI comes back green.
@bors rollup=never (merge-conflict prone and a lot of tests)

@petrochenkov
Copy link
Contributor Author

@bors r=jieyouxu

@bors
Copy link
Contributor

bors commented Mar 25, 2025

📌 Commit 2b32c57 has been approved by jieyouxu

It is now in the queue for this repository.

@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 Mar 25, 2025
@bors
Copy link
Contributor

bors commented Mar 25, 2025

⌛ Testing commit 2b32c57 with merge 688a632...

bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 25, 2025
compiletest: Support matching on diagnostics without a span

Using `//~? ERROR my message` on any line of the test.

The new checks are exhaustive, like all other `//~` checks, and unlike the `error-pattern` directive that is sometimes used now to check for span-less diagnostics.

This will allow to eliminate most on `error-pattern` directives in compile-fail tests (except those that are intentionally imprecise due to platform-specific diagnostics).
I didn't migrate any of `error-pattern`s in this PR though, except those where the migration was necessary for the tests to pass.
@rust-log-analyzer

This comment has been minimized.

@bors

This comment was marked as resolved.

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

@bors r=jieyouxu

@bors
Copy link
Contributor

bors commented Mar 25, 2025

📌 Commit 8d5109a has been approved by jieyouxu

It is now in the queue for this repository.

@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 Mar 25, 2025
@bors
Copy link
Contributor

bors commented Mar 25, 2025

⌛ Testing commit 8d5109a with merge 40507bd...

@bors
Copy link
Contributor

bors commented Mar 25, 2025

☀️ Test successful - checks-actions
Approved by: jieyouxu
Pushing 40507bd to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Mar 25, 2025
@bors bors merged commit 40507bd into rust-lang:master Mar 25, 2025
7 checks passed
@rustbot rustbot added this to the 1.87.0 milestone Mar 25, 2025
Copy link

What is this? This is an experimental post-merge analysis report that shows differences in test outcomes between the merged PR and its parent PR.

Comparing 48994b1 (parent) -> 40507bd (this PR)

Test differences

No test diffs found

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (40507bd): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

Results (primary -2.1%, secondary 0.0%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
2.0% [2.0%, 2.0%] 1
Improvements ✅
(primary)
-2.1% [-2.1%, -2.1%] 1
Improvements ✅
(secondary)
-2.0% [-2.0%, -2.0%] 1
All ❌✅ (primary) -2.1% [-2.1%, -2.1%] 1

Cycles

This benchmark run did not return any relevant results for this metric.

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 777.999s -> 778.257s (0.03%)
Artifact size: 365.81 MiB -> 365.84 MiB (0.01%)

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 A-rustc-dev-guide Area: rustc-dev-guide A-testsuite Area: The testsuite used to check the correctness of rustc merged-by-bors This PR was explicitly merged by bors. PG-exploit-mitigations Project group: Exploit mitigations S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants