Improve error messages rendered by JavaSourcesSubject.parsesAs() and JavaFileObjectSubject.containsElementsIn() to more easily distinguish between errors incurred in *actual* vs *expected* source.

java-team-github-bot · Compile-Testing Team · commit 115014eb920e · 2022-08-16T15:05:48.000-07:00
RELNOTES=Improve error messages rendered by `JavaSourcesSubject.parsesAs()` and `JavaFileObjectSubject.containsElementsIn()` to more easily distinguish between errors incurred in *actual* vs *expected* source.
PiperOrigin-RevId: 468034719
diff --git a/src/main/java/com/google/testing/compile/JavaFileObjectSubject.java b/src/main/java/com/google/testing/compile/JavaFileObjectSubject.java
@@ -170,10 +170,10 @@ private void performTreeDifference(
       String failureVerb,
       String expectedTitle,
       BiFunction<ParseResult, ParseResult, TreeDifference> differencingFunction) {
-    ParseResult actualResult = Parser.parse(ImmutableList.of(actual));
+    ParseResult actualResult = Parser.parse(ImmutableList.of(actual), "*actual* source");
     CompilationUnitTree actualTree = getOnlyElement(actualResult.compilationUnits());
 
-    ParseResult expectedResult = Parser.parse(ImmutableList.of(expected));
+    ParseResult expectedResult = Parser.parse(ImmutableList.of(expected), "*expected* source");
     CompilationUnitTree expectedTree = getOnlyElement(expectedResult.compilationUnits());
 
     TreeDifference treeDifference = differencingFunction.apply(expectedResult, actualResult);
diff --git a/src/main/java/com/google/testing/compile/JavaSourcesSubject.java b/src/main/java/com/google/testing/compile/JavaSourcesSubject.java
@@ -158,7 +158,7 @@ public void parsesAs(JavaFileObject first, JavaFileObject... rest) {
                 "Compilation generated no additional source files, though some were expected."));
         return;
       }
-      ParseResult actualResult = Parser.parse(actual);
+      ParseResult actualResult = Parser.parse(actual, "*actual* source");
       ImmutableList<Diagnostic<? extends JavaFileObject>> errors =
           actualResult.diagnosticsByKind().get(Kind.ERROR);
       if (!errors.isEmpty()) {
@@ -170,7 +170,7 @@ public void parsesAs(JavaFileObject first, JavaFileObject... rest) {
         failWithoutActual(simpleFact(message.toString()));
         return;
       }
-      ParseResult expectedResult = Parser.parse(Lists.asList(first, rest));
+      ParseResult expectedResult = Parser.parse(Lists.asList(first, rest), "*expected* source");
       ImmutableList<TypedCompilationUnit> actualTrees =
           actualResult.compilationUnits().stream()
               .map(TypedCompilationUnit::create)
diff --git a/src/main/java/com/google/testing/compile/MoreTrees.java b/src/main/java/com/google/testing/compile/MoreTrees.java
@@ -52,7 +52,7 @@ static CompilationUnitTree parseLinesToTree(String... source) {
   /** Parses the source given into a {@link CompilationUnitTree}. */
   static CompilationUnitTree parseLinesToTree(Iterable<String> source) {
     Iterable<? extends CompilationUnitTree> parseResults =
-        Parser.parse(ImmutableList.of(JavaFileObjects.forSourceLines("", source)))
+        Parser.parse(ImmutableList.of(JavaFileObjects.forSourceLines("", source)), "source")
             .compilationUnits();
     return Iterables.getOnlyElement(parseResults);
   }
@@ -64,7 +64,7 @@ static ParseResult parseLines(String... source) {
 
   /** Parses the source given and produces a {@link ParseResult}. */
   static ParseResult parseLines(Iterable<String> source) {
-    return Parser.parse(ImmutableList.of(JavaFileObjects.forSourceLines("", source)));
+    return Parser.parse(ImmutableList.of(JavaFileObjects.forSourceLines("", source)), "source");
   }
 
   /**
diff --git a/src/main/java/com/google/testing/compile/Parser.java b/src/main/java/com/google/testing/compile/Parser.java
@@ -50,8 +50,11 @@ public final class Parser {
   /**
    * Parses {@code sources} into {@linkplain CompilationUnitTree compilation units}. This method
    * <b>does not</b> compile the sources.
+   *
+   * @param sourcesDescription describes the sources. Parsing exceptions will contain this string.
+   * @throws IllegalStateException if any parsing errors occur.
    */
-  static ParseResult parse(Iterable<? extends JavaFileObject> sources) {
+  static ParseResult parse(Iterable<? extends JavaFileObject> sources, String sourcesDescription) {
     JavaCompiler compiler = ToolProvider.getSystemJavaCompiler();
     DiagnosticCollector<JavaFileObject> diagnosticCollector = new DiagnosticCollector<>();
     InMemoryJavaFileManager fileManager =
@@ -72,8 +75,8 @@ static ParseResult parse(Iterable<? extends JavaFileObject> sources) {
       Iterable<? extends CompilationUnitTree> parsedCompilationUnits = task.parse();
       List<Diagnostic<? extends JavaFileObject>> diagnostics = diagnosticCollector.getDiagnostics();
       if (foundParseErrors(parsedCompilationUnits, diagnostics)) {
-        throw new IllegalStateException(
-            "error while parsing:\n" + Joiner.on('\n').join(diagnostics));
+        String msgPrefix = String.format("Error while parsing %s:\n", sourcesDescription);
+        throw new IllegalStateException(msgPrefix + Joiner.on('\n').join(diagnostics));
       }
       return new ParseResult(
           sortDiagnosticsByKind(diagnostics), parsedCompilationUnits, Trees.instance(task));
@@ -195,4 +198,6 @@ private DummyJavaCompilerSubclass() {
       super(null);
     }
   }
+
+  private Parser() {}
 }
diff --git a/src/test/java/com/google/testing/compile/JavaFileObjectSubjectTest.java b/src/test/java/com/google/testing/compile/JavaFileObjectSubjectTest.java
@@ -21,6 +21,7 @@
 import static com.google.testing.compile.JavaFileObjectSubject.assertThat;
 import static com.google.testing.compile.JavaFileObjectSubject.javaFileObjects;
 import static java.nio.charset.StandardCharsets.UTF_8;
+import static org.junit.Assert.assertThrows;
 
 import com.google.common.truth.ExpectFailure;
 import java.io.IOException;
@@ -196,4 +197,39 @@ public void hasSourceEquivalentTo_failOnExtraInActual() throws IOException {
   public void containsElementsIn_completeMatch() {
     assertThat(SAMPLE_ACTUAL_FILE_FOR_MATCHING).containsElementsIn(SAMPLE_ACTUAL_FILE_FOR_MATCHING);
   }
+
+  private static final JavaFileObject SIMPLE_INVALID_FILE =
+      JavaFileObjects.forSourceLines(
+          "test.SomeClass", //
+          "package test;",
+          "",
+          "public syntax error class SomeClass {",
+          "}");
+  private static final JavaFileObject SIMPLE_VALID_FILE =
+      JavaFileObjects.forSourceLines(
+          "test.SomeClass", //
+          "package test;",
+          "",
+          "public class SomeClass {",
+          "}");
+
+  @Test
+  public void containsElementsIn_badActual() {
+    IllegalStateException ex =
+        assertThrows(
+            IllegalStateException.class,
+            () -> assertThat(SIMPLE_INVALID_FILE).containsElementsIn(SIMPLE_VALID_FILE));
+
+    assertThat(ex).hasMessageThat().startsWith("Error while parsing *actual* source:\n");
+  }
+
+  @Test
+  public void containsElementsIn_badExpected() {
+    IllegalStateException ex =
+        assertThrows(
+            IllegalStateException.class,
+            () -> assertThat(SIMPLE_VALID_FILE).containsElementsIn(SIMPLE_INVALID_FILE));
+
+    assertThat(ex).hasMessageThat().startsWith("Error while parsing *expected* source:\n");
+  }
 }
diff --git a/src/test/java/com/google/testing/compile/JavaSourcesSubjectFactoryTest.java b/src/test/java/com/google/testing/compile/JavaSourcesSubjectFactoryTest.java
@@ -398,7 +398,7 @@ public void parsesAs_expectedFileFailsToParse() {
           .parsesAs(JavaFileObjects.forResource("test/HelloWorld-broken.java"));
       fail();
     } catch (IllegalStateException expected) {
-      assertThat(expected.getMessage()).startsWith("error while parsing:");
+      assertThat(expected.getMessage()).startsWith("Error while parsing *expected* source:\n");
     }
   }
 
@@ -410,7 +410,7 @@ public void parsesAs_actualFileFailsToParse() {
           .parsesAs(HELLO_WORLD_RESOURCE);
       fail();
     } catch (IllegalStateException expected) {
-      assertThat(expected.getMessage()).startsWith("error while parsing:");
+      assertThat(expected.getMessage()).startsWith("Error while parsing *actual* source:\n");
     }
   }
 

Original file line number	Diff line number	Diff line change
`@@ -398,7 +398,7 @@ public void parsesAs_expectedFileFailsToParse() {`
`398`	`398`	`.parsesAs(JavaFileObjects.forResource("test/HelloWorld-broken.java"));`
`399`	`399`	`fail();`
`400`	`400`	`} catch (IllegalStateException expected) {`
`401`		`- assertThat(expected.getMessage()).startsWith("error while parsing:");`
	`401`	`+ assertThat(expected.getMessage()).startsWith("Error while parsing expected source:\n");`
`402`	`402`	`}`
`403`	`403`	`}`
`404`	`404`
`@@ -410,7 +410,7 @@ public void parsesAs_actualFileFailsToParse() {`
`410`	`410`	`.parsesAs(HELLO_WORLD_RESOURCE);`
`411`	`411`	`fail();`
`412`	`412`	`} catch (IllegalStateException expected) {`
`413`		`- assertThat(expected.getMessage()).startsWith("error while parsing:");`
	`413`	`+ assertThat(expected.getMessage()).startsWith("Error while parsing actual source:\n");`
`414`	`414`	`}`
`415`	`415`	`}`
`416`	`416`