Report every line that fails its expectation in CompilationTestHelper by vlsi · Pull Request #6117 · google/error-prone

vlsi · 2026-09-15T08:37:49Z

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:

Did not see an error on line 4 matching DeadException. There were no errors.

Fixes #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:

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.

🤖 Generated with Claude Code

google-cla · 2026-09-15T08:38:08Z

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.

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)

cpovirk

Thanks, I have wanted something like this for years!

cpovirk · 2026-09-16T16:02:09Z

(as apparently have some other users, I see from the emoji reactions :))

…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)

…arness, and name cases per kind of mismatch The reviewer of uber/NullAway#1834 asked for the file's existing shape: several markers of one rule in one compiled source. Error Prone's CompilationTestHelper stops at the first mismatched marker, so §7 split every second marker that expects a report into a test of its own, and §9 ranked that rule above the neighbors' shape. The skill thus argued a maintainer out of the form they asked for, although they may accept a second run to see the next mismatch, and google/error-prone#6117 makes the helper report every mismatch. Two more defects sat in the same rules. §7's batch-validator example said that a check which stops at the first mismatch "costs them a second run and hides nothing about another rule", which lets two cases of one rule share a test, while the next sentences allowed at most one input that expects a report per test. And "names each case" was one answer per harness, although Error Prone prints the source line of an unexpected diagnostic and only the line number of a missing one, and a case and its control often sit on identical lines. - §0 and §7: the stack line may say that the tests are written as if the harness reported every mismatch, or named each case by content, before it does. Every rule that asks either question then takes the line's answer, and the change proposes nothing about the harness. A writer's preference for fewer tests and the neighbors' shape do not stand in for the line; §9 says so too. - §7: the example keeps the split, and both cases may share a test only where the check carries every mismatch. - §7, §10, §11: a report names the cases of a test only where it does so for every kind of mismatch the test can produce, and printed text names a case only where it differs between the cases. - README: an Error Prone stack line that accepts the form, and the lint(...) line naming both kinds of mismatch. - New case nullaway-1834-rewrite-harness-ahead: the rewrite with that stack line in NullAway's instructions. run-nullaway-case.sh appends a case's instructions.md to NullAway's CLAUDE.md in the system prompt. Results on skill tree 4f33396, one run per cell, graded against every check of the case READMEs by reading; about $10 for the six runs. The build passes on the fix in every result. - rewrite, Opus 5.5: 12 tests (3 with no marker); no check failed; partial 1 (one input moved from a method's type parameter to the class's) and 7 (the note says 21 tests). - rewrite, Sonnet 5: 16 tests (7 with no marker); failed 2 (four controls stay tests of their own); partial 7. - from scratch, Opus 5.5: 6 tests; no check failed; partial 2 (no captured-type case, no @nullable use). - from scratch, Sonnet 5: 4 tests; failed 2 (two of seven behaviors missing, two partial). - harness ahead, Opus 5.5: 4 tests, one per rule, with up to four markers each; no check failed; partial 1 (one input changed from a return to a call argument). The note cites the stack line and proposes nothing about Error Prone. - harness ahead, Sonnet 5: 11 tests (4 with no marker); failed 2 (two controls stand alone) and 6 (proposes the stack line that exists); partial 3 (markers of one rule split on other grounds) and 5. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

…ome, and let the stack line accept a form ahead of the harness Adversarial reviews of the related-cases rules, and the review threads on the pull request, found places where the skill gave two answers or none: - The limit of one partition that expects the outcome per test depended on how the rule was worded: under "a timeout is accepted only when it is a nonnegative integer", -1 and "abc" became controls of 0. - A batch result had to use a grouped assertion that cargo test, node:assert, Jest, and plain pytest do not have. - libtest.md let a new case join a loop over a table that stops at the first failing row, and go/testing.md sent a new error case into the gotests table with a `wantErr bool` column, which only establishes that some error came back and compares `got` where the contract may leave it undefined. - The skill split every test whose name held `and`, so a rule whose condition is a conjunction could not be named. - §7 split every second marker of a rule under Error Prone's CompilationTestHelper, which stops at the first mismatch, and §9 ranked that above the neighbors' shape. The skill thus argued a maintainer out of the form the reviewer of uber/NullAway#1834 asked for, although they may accept a second run to see the next mismatch, and google/error-prone#6117 makes the helper report every mismatch. - "Names each case" was one answer per harness, although Error Prone prints the source line of an unexpected diagnostic and only the line number of a missing one. What changes: - §7 defines the case as the partition the test is named for, the one whose outcome the change sets or moves, and a control as a nearest input with the opposite outcome. Under a check that stops at the first mismatch, a test holds at most one input that expects a report and at most one input whose outcome the change moves. - Where the assertion library has no grouped form, one assertion compares the whole result, reduced to the fields the behavior defines and without the message text. The pytest output for a list and a set is measured in research/test-authoring/framework_output.md. - A loop in one test over a table of literal cases is a check that stops at the first mismatch, and an idiom a reference names keeps its shape only while it reports each case apart and asserts what §5 to §7 ask. A new error case is a test of its own; a row whose input is accepted joins the table. - An `and` or a `but` splits a test only where it joins the outcomes of two inputs or scenarios, in the §7 name row, the §7 sub-bullet, §11, the case READMEs, and the checker. - §0 and §7: the stack line may say that the tests are written as if the harness reported every mismatch, or named each case by content, before it does. Every rule that asks either question takes the line's answer, and the change proposes nothing about the harness. A writer's preference for fewer tests and the neighbors' shape do not stand in for the line. - §7, §10, §11: a report names the cases of a test only where it does so for every kind of mismatch the test can produce, and printed text names a case only where it differs between the cases. - New case nullaway-1834-rewrite-harness-ahead: the rewrite with that stack line in NullAway's instructions. run-nullaway-case.sh appends a case's instructions.md to the system prompt, deletes the test reports before each build, and records Gradle's exit status, so a build that fails before the tests no longer reports a stale run as its own. Results on skill tree 4f33396, one run per cell, graded against every check of the case READMEs by reading; about $10 for the six runs. The build passes on the fix in every result. - rewrite, Opus: 12 tests (3 with no marker); partial 1 (one input moved from a method's type parameter to the class's) and 7. - rewrite, Sonnet: 16 tests (7 with no marker); failed 2 (four controls stay tests of their own); partial 7. - from scratch, Opus: 6 tests; partial 2 (no captured-type case, no @nullable use). - from scratch, Sonnet: 4 tests; failed 2. - harness ahead, Opus: 4 tests, one per rule, with up to four markers each; partial 1. The note cites the stack line and proposes nothing about Error Prone. - harness ahead, Sonnet: 11 tests (4 with no marker); failed 2 and 6 (proposes the stack line that exists); partial 3 and 5. Assisted-by: Claude Code (claude-opus-5-5)

msridhar · 2026-10-05T15:36:36Z

@cpovirk any update on this one? Just ran into a case where this would have been handy 🙂

cpovirk · 2026-10-05T17:22:26Z

Sorry about that. It got caught on some review comments that the robots doing the import aren't configured to relay to anyone. I just pushed it through, though somehow GitHub hasn't marked this PR as closed yet in reponse.

…ome, and let the stack line accept a form ahead of the harness Adversarial reviews of the related-cases rules, and the review threads on the pull request, found places where the skill gave two answers or none: - The limit of one partition that expects the outcome per test depended on how the rule was worded: under "a timeout is accepted only when it is a nonnegative integer", -1 and "abc" became controls of 0. - A batch result had to use a grouped assertion that cargo test, node:assert, Jest, and plain pytest do not have. - libtest.md let a new case join a loop over a table that stops at the first failing row, and go/testing.md sent a new error case into the gotests table with a `wantErr bool` column, which only establishes that some error came back and compares `got` where the contract may leave it undefined. - The skill split every test whose name held `and`, so a rule whose condition is a conjunction could not be named. - §7 split every second marker of a rule under Error Prone's CompilationTestHelper, which stops at the first mismatch, and §9 ranked that above the neighbors' shape. The skill thus argued a maintainer out of the form the reviewer of uber/NullAway#1834 asked for, although they may accept a second run to see the next mismatch, and google/error-prone#6117 makes the helper report every mismatch. - "Names each case" was one answer per harness, although Error Prone prints the source line of an unexpected diagnostic and only the line number of a missing one. What changes: - §7 defines the case as the partition the test is named for, the one whose outcome the change sets or moves, and a control as a nearest input with the opposite outcome. Under a check that stops at the first mismatch, a test holds at most one input that expects a report and at most one input whose outcome the change moves. - Where the assertion library has no grouped form, one assertion compares the whole result, reduced to the fields the behavior defines and without the message text. The pytest output for a list and a set is measured in research/test-authoring/framework_output.md. - A loop in one test over a table of literal cases is a check that stops at the first mismatch, and an idiom a reference names keeps its shape only while it reports each case apart and asserts what §5 to §7 ask. A new error case is a test of its own; a row whose input is accepted joins the table. - An `and` or a `but` splits a test only where it joins the outcomes of two inputs or scenarios, in the §7 name row, the §7 sub-bullet, §11, the case READMEs, and the checker. - §0 and §7: the stack line may say that the tests are written as if the harness reported every mismatch, or named each case by content, before it does. Every rule that asks either question takes the line's answer, and the change proposes nothing about the harness. A writer's preference for fewer tests and the neighbors' shape do not stand in for the line. - §7, §10, §11: a report names the cases of a test only where it does so for every kind of mismatch the test can produce, and printed text names a case only where it differs between the cases. - New case nullaway-1834-rewrite-harness-ahead: the rewrite with that stack line in NullAway's instructions. run-nullaway-case.sh appends a case's instructions.md to the system prompt, deletes the test reports before each build, and records Gradle's exit status, so a build that fails before the tests no longer reports a stale run as its own. Results on skill tree 4f33396, one run per cell, graded against every check of the case READMEs by reading; about $10 for the six runs. The build passes on the fix in every result. - rewrite, Opus: 12 tests (3 with no marker); partial 1 (one input moved from a method's type parameter to the class's) and 7. - rewrite, Sonnet: 16 tests (7 with no marker); failed 2 (four controls stay tests of their own); partial 7. - from scratch, Opus: 6 tests; partial 2 (no captured-type case, no @nullable use). - from scratch, Sonnet: 4 tests; failed 2. - harness ahead, Opus: 4 tests, one per rule, with up to four markers each; partial 1. The note cites the stack line and proposes nothing about Error Prone. - harness ahead, Sonnet: 11 tests (4 with no marker); failed 2 and 6 (proposes the stack line that exists); partial 3 and 5. Assisted-by: Claude Code (claude-opus-5-5)

vlsi force-pushed the report-all-mismatched-lines branch from 965c692 to 6a00e00 Compare September 15, 2026 08:45

cpovirk approved these changes Sep 16, 2026

View reviewed changes

This was referenced Sep 25, 2026

Show the source line of a marked line whose expected diagnostic is missing #6143

Draft

CompilationTestHelper: a failure cannot say which of several identical cases lost its diagnostic #6144

Open

vlsi mentioned this pull request Sep 25, 2026

CompilationTestHelper: what a failing line expected and what it got are printed in different parts of the message #6145

Open

vlsi mentioned this pull request Sep 25, 2026

feat(test-authoring): share the setup of related cases, keep a case with its controls in one check, and add NullAway cases Netcracker/qubership-ai-packages#112

Merged

copybara-service Bot mentioned this pull request Oct 5, 2026

Report every line that fails its expectation in CompilationTestHelper. #6175

Merged

copybara-service Bot closed this in bfb9d88 Oct 5, 2026

Navigation Menu

Repository navigation

Report every line that fails its expectation in CompilationTestHelper - #6117

Report every line that fails its expectation in CompilationTestHelper#6117
vlsi wants to merge 1 commit into
google:masterfrom
vlsi:report-all-mismatched-lines

vlsi commented Sep 15, 2026

Uh oh!

google-cla Bot commented Sep 15, 2026

Uh oh!

cpovirk left a comment

Uh oh!

cpovirk commented Sep 16, 2026

Uh oh!

msridhar commented Oct 5, 2026

Uh oh!

cpovirk commented Oct 5, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

vlsi commented Sep 15, 2026

Uh oh!

google-cla Bot commented Sep 15, 2026

Uh oh!

cpovirk left a comment

Choose a reason for hiding this comment

Uh oh!

cpovirk commented Sep 16, 2026

Uh oh!

msridhar commented Oct 5, 2026

Uh oh!

cpovirk commented Oct 5, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants