From 115014eb920e361e43880c8ad607f06c7e048ff8 Mon Sep 17 00:00:00 2001 From: Compile-Testing Team Date: Tue, 16 Aug 2022 15:05:04 -0700 Subject: [PATCH] Improve error messages rendered by `JavaSourcesSubject.parsesAs()` and `JavaFileObjectSubject.containsElementsIn()` to more easily distinguish between errors incurred in *actual* vs *expected* source. 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 --- .../compile/JavaFileObjectSubject.java | 4 +-- .../testing/compile/JavaSourcesSubject.java | 4 +-- .../com/google/testing/compile/MoreTrees.java | 4 +-- .../com/google/testing/compile/Parser.java | 11 ++++-- .../compile/JavaFileObjectSubjectTest.java | 36 +++++++++++++++++++ .../JavaSourcesSubjectFactoryTest.java | 4 +-- 6 files changed, 52 insertions(+), 11 deletions(-) diff --git a/src/main/java/com/google/testing/compile/JavaFileObjectSubject.java b/src/main/java/com/google/testing/compile/JavaFileObjectSubject.java index b0b26971..eb27c3a3 100644 --- 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 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 index f55276fe..394eb002 100644 --- 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> 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 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 index 6be48865..5a10e044 100644 --- 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 source) { Iterable 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 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 index e5ad76ac..e75d9076 100644 --- 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 * does not 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 sources) { + static ParseResult parse(Iterable sources, String sourcesDescription) { JavaCompiler compiler = ToolProvider.getSystemJavaCompiler(); DiagnosticCollector diagnosticCollector = new DiagnosticCollector<>(); InMemoryJavaFileManager fileManager = @@ -72,8 +75,8 @@ static ParseResult parse(Iterable sources) { Iterable parsedCompilationUnits = task.parse(); List> 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 index 485c84f5..b3aaf218 100644 --- 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 index f140fd72..267634f9 100644 --- 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"); } }