Repository navigation
Conversation
When several lines of a source given to `CompilationTestHelper` failed their expectations, the test failure named only the first of them. `DiagnosticTestHelper.assertHasDiagnosticOnAllMatchingLines` threw at the first mismatched line, so each further mismatch showed up only after the previous one was fixed and the test was run again. For a source with markers on lines 4 and 8 and no diagnostics, the failure was: ```text Did not see an error on line 4 matching DeadException. There were no errors. ``` Fixes google#6116. The helper now checks every line of every source and fails once, with one message for each line that fails, followed by the list of all diagnostics: ```text Saw unexpected error on line 3. Saw unexpected error on line 6. All errors: /Test.java:3: error: [ReturnTreeChecker] Method may return normally. return true; ^ (see https://errorprone.info/bugpattern/ReturnTreeChecker) /Test.java:6: error: [ReturnTreeChecker] Method may return normally. return false; ^ (see https://errorprone.info/bugpattern/ReturnTreeChecker) ``` A line's message is the one the helper printed for it before, naming the first expectation the line fails: a marker that names a key missing from `expectErrorMessage`, a missing diagnostic, a diagnostic without the check name, or an unexpected diagnostic. Where the test compiles more than one source, each message starts with the source's name, because a line number no longer says which file it belongs to. `assertHasDiagnosticOnAllMatchingLines` keeps its signature and reports all mismatches in its source the same way. `CompilationTestHelperTest` has five new tests, which fail on the current master: two marked lines without diagnostics, two unexpected diagnostics, an unexpected diagnostic before a missing one, an unknown key together with another mismatch, and mismatches in two sources. `fileWithSyntaxErrorFails` now also asserts the message for a diagnostic without the check name. Assisted-by: Claude Code (claude-opus-5)
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
…ssing When a line marked with `// BUG: Diagnostic contains:` or `// BUG: Diagnostic matches:` gets no matching diagnostic, `CompilationTestHelper` reports only the line number and the marker's text, for example `Did not see an error on line 4 matching dereferenced expression 'x' is @nullable.` The `All errors:` list that follows prints each diagnostic with its source line, but no diagnostic exists for this line, so nothing in the report shows the code the marker is on. A regression test usually fails this way: the fix is reverted and the expected diagnostic disappears. Where several markers carry the same text, the reader of a CI log has to open the test source and count lines to find the failing case. `DiagnosticTestHelper` now follows each such message with the source line it names, trimmed and indented by four spaces, the way javac prints a diagnostic's line: ```text Did not see an error on line 4 matching dereferenced expression 'x' is @nullable. x.hashCode(); ``` Caller sees: - `Did not see an error on line N matching <text>.` followed by line N of the source, trimmed, on a line of its own indented by four spaces. - `Did not see an error on line N containing [<CheckName>].` followed by line N the same way. Not changed: `Saw unexpected error on line N.` stays one line, since `All errors:` already prints the diagnostic on that line with its source. Depends on google#6117, which reports every failing line; its commit is the first on this branch. Assisted-by: Claude Code (claude-opus-5-5)
vlsi
force-pushed
the
show-missed-diagnostic-line
branch
from
September 25, 2026 16:16
efb6635 to
6cde4a4
Compare
vlsi
marked this pull request as draft
September 25, 2026 16:40
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
When a line marked with
// BUG: Diagnostic contains:or// BUG: Diagnostic matches:gets no matching diagnostic,CompilationTestHelperreports only the line number and the marker's text, for exampleDid not see an error on line 4 matching dereferenced expression 'x' is @Nullable.TheAll errors:list that follows prints each diagnostic with its source line, but no diagnostic exists for this line, so nothing in the report shows the code the marker is on. A regression test usually fails this way: the fix is reverted and the expected diagnostic disappears. Where several markers carry the same text, the reader of a CI log has to open the test source and count lines to find the failing case.DiagnosticTestHelpernow follows each such message with the source line it names, trimmed and indented by four spaces, the way javac prints a diagnostic's line:Caller sees:
Did not see an error on line N matching <text>.followed by line N of the source, trimmed, on a line of its own indented by four spaces.Did not see an error on line N containing [<CheckName>].followed by line N the same way.Not changed:
Saw unexpected error on line N.stays one line, sinceAll errors:already prints the diagnostic on that line with its source.The four
CompilationTestHelperTesttests that compare the whole failure message now expect the source line; each fails on the helper from #6117 and passes with this change.Depends on #6117, which reports every failing line; its commit is the first on this branch, so review only the second commit.
🤖 Generated with Claude Code