CompilationTestHelper reports only the first line that fails its expectation
When several lines of a source given to CompilationTestHelper.addSourceLines fail their expectations, the test failure names only the first of them. The next one is reported only after the first is fixed and the test runs again.
Reproducer
Error Prone 2.50.0 (error_prone_core and error_prone_test_helpers), JUnit 4.13.2, OpenJDK 21.0.9, Maven 3.9.16.
package repro;
import com.google.errorprone.CompilationTestHelper;
import com.google.errorprone.bugpatterns.DeadException;
import org.junit.Test;
public class FirstMismatchTest {
private final CompilationTestHelper helper =
CompilationTestHelper.newInstance(DeadException.class, getClass());
@Test
public void twoMarkersWithoutDiagnostics() {
helper
.addSourceLines(
"Test.java",
"""
class Test {
void first() {
// BUG: Diagnostic contains: DeadException
int a = 1;
}
void second() {
// BUG: Diagnostic contains: DeadException
int b = 2;
}
}
""")
.doTest();
}
@Test
public void twoDiagnosticsWithoutMarkers() {
helper
.addSourceLines(
"Test.java",
"""
class Test {
void first() {
new RuntimeException();
}
void second() {
new IllegalStateException();
}
}
""")
.doTest();
}
}mvn test prints, for the first test:
com.google.common.truth.AssertionErrorWithFacts:
Did not see an error on line 4 matching DeadException. There were no errors.
expected to be true
at repro.FirstMismatchTest.twoMarkersWithoutDiagnostics(FirstMismatchTest.java:28)Line 8 carries a marker and has no diagnostic either, and the failure does not mention it.
For the second test:
java.lang.AssertionError:
Saw unexpected error on line 3. All errors:
/Test.java:3: error: [DeadException] Exception created but not thrown
new RuntimeException();
^
(see https://errorprone.info/bugpattern/DeadException)
Did you mean 'throw new RuntimeException();'?
/Test.java:6: error: [DeadException] Exception created but not thrown
new IllegalStateException();
^
(see https://errorprone.info/bugpattern/DeadException)
Did you mean 'throw new IllegalStateException();'?
at repro.FirstMismatchTest.twoDiagnosticsWithoutMarkers(FirstMismatchTest.java:46)The diagnostic on line 6 is in the list of all errors, but the failure reports only line 3 as a mismatch. To find the other mismatches, the reader compares every listed diagnostic with the markers in the source, line by line.
Where it stops
DiagnosticTestHelper.assertHasDiagnosticOnAllMatchingLines reads the source one line at a time and throws at the first line that does not match: through assertWithMessage(...).isTrue() for a missing diagnostic (L264-268) or a missing check name (L277-281), and through fail for an unexpected one (L289). The file is the same on master today.
Who runs into it
Any test whose source holds more than one expectation, when a change to the check breaks two of them at once. A rough regex count over core/src/test/java finds 201 of 5177 addSourceLines calls with two or more // BUG: Diagnostic markers. That count leaves out sources with one marker and several lines expected to stay clean, which can fail on two lines as well.
It came up in the review of uber/NullAway#1834. A reviewer asked for a case that NullAway reports and its unreported counterpart to share one source instead of two copies of it, so a reader sees the one line in which they differ. With the helper as it is, the price of that layout is a report that hides the second failure whenever one change breaks both lines.
Expected
One failure that lists every line that failed its expectation, each with the message the helper prints for it today. For the first test that means both line 4 and line 8; for the second, both line 3 and line 6.
One possible shape, for the first test:
2 lines did not match their expectations:
Did not see an error on line 4 matching DeadException.
Did not see an error on line 8 matching DeadException.
There were no errors.The wording and layout are yours to choose. What I am asking for is that no mismatched line is left out of the failure. This is a request for a more useful report rather than a defect against a documented contract: the Javadoc of addSourceLines says what is checked on every line, not how failures are reported. Go's analysistest, for comparison, reports each unexpected and each missing diagnostic through its own t.Errorf (analysistest.go).
I searched the tracker for CompilationTestHelper, DiagnosticTestHelper, and the text of both failure messages, and found no earlier report.
I can send the pull request: collect the messages inside the loop and fail once after it, with each message unchanged.
Source: google/error-prone