Fix regex event matching This is the prerequisite to #13371. Closes #13375. PiperOrigin-RevId: 369286187
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java b/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java index 9728117..c9098a0 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java +++ b/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java
@@ -195,6 +195,7 @@ import java.util.TreeMap; import java.util.UUID; import java.util.function.Predicate; +import java.util.regex.Pattern; import javax.annotation.Nullable; import net.starlark.java.eval.StarlarkSemantics; import org.junit.Before; @@ -1212,6 +1213,31 @@ } /** + * Check that configuration of the target named 'ruleName' in the specified BUILD file fails with + * an error message matching 'expectedErrorPattern'. + * + * @param packageName the package name of the generated BUILD file + * @param ruleName the rule name for the rule in the generated BUILD file + * @param expectedErrorPattern a regex that matches the expected error. + * @param lines the text of the rule. + * @return the found error. + */ + protected Event checkError( + String packageName, String ruleName, Pattern expectedErrorPattern, String... lines) + throws Exception { + eventCollector.clear(); + reporter.removeHandler(failFastHandler); // expect errors + ConfiguredTarget target = scratchConfiguredTarget(packageName, ruleName, lines); + if (target != null) { + assertWithMessage( + "Rule '" + "//" + packageName + ":" + ruleName + "' did not contain an error") + .that(view.hasErrors(target)) + .isTrue(); + } + return assertContainsEvent(expectedErrorPattern); + } + + /** * Check that configuration of the target named 'label' fails with an error message containing * 'expectedErrorMessage'. *
diff --git a/src/test/java/com/google/devtools/build/lib/buildtool/CustomRealFilesystemBuildIntegrationTest.java b/src/test/java/com/google/devtools/build/lib/buildtool/CustomRealFilesystemBuildIntegrationTest.java index 84d31cb..f13093b 100644 --- a/src/test/java/com/google/devtools/build/lib/buildtool/CustomRealFilesystemBuildIntegrationTest.java +++ b/src/test/java/com/google/devtools/build/lib/buildtool/CustomRealFilesystemBuildIntegrationTest.java
@@ -50,6 +50,7 @@ import java.util.HashSet; import java.util.Map; import java.util.Set; +import java.util.regex.Pattern; import org.junit.Test; import org.junit.runner.RunWith; @@ -353,11 +354,12 @@ buildTarget("//hello:hello"); BuildFailedException e = assertThrows(BuildFailedException.class, () -> buildTarget("//hello:hello")); - MoreAsserts.assertContainsEventRegex( + MoreAsserts.assertContainsEvent( events.collector(), - "^ERROR.*Compiling hello/hello.cc failed: Unable to resolve hello/subdir/undeclared.h as an" - + " artifact: no such package 'hello/subdir': IO errors while looking for BUILD file" - + " reading .*hello/subdir/BUILD: nope"); + Pattern.compile( + "^ERROR.*Compiling hello/hello.cc failed: Unable to resolve hello/subdir/undeclared.h" + + " as an artifact: no such package 'hello/subdir': IO errors while looking for" + + " BUILD file reading .*hello/subdir/BUILD: nope")); assertThat(e.getDetailedExitCode().getFailureDetail()) .comparingExpectedFieldsOnly() .isEqualTo( @@ -382,12 +384,14 @@ customFileSystem.errorInsideStat(buildFile, 0); BuildFailedException e = assertThrows(BuildFailedException.class, () -> buildTarget("//hello:hello")); - MoreAsserts.assertContainsEventRegex( + MoreAsserts.assertContainsEvent( events.collector(), - ".*Compiling hello/hello.cc failed: Unable to resolve hello/subdir/undeclared.h as an" - + " artifact: Inconsistent filesystem operations. 'stat' said .*/hello/subdir/BUILD is" - + " a file but then we later encountered error 'nope for .*/hello/subdir/BUILD' which" - + " indicates that .*/hello/subdir/BUILD is no longer a file.*"); + Pattern.compile( + ".*Compiling hello/hello.cc failed: Unable to resolve hello/subdir/undeclared.h as an" + + " artifact: Inconsistent filesystem operations. 'stat' said" + + " .*/hello/subdir/BUILD is a file but then we later encountered error 'nope for" + + " .*/hello/subdir/BUILD' which indicates that .*/hello/subdir/BUILD is no longer" + + " a file.*")); events.assertContainsError("hello/subdir/BUILD "); assertThat(e.getDetailedExitCode().getFailureDetail()) .comparingExpectedFieldsOnly() @@ -529,14 +533,16 @@ @Test public void ioExceptionInTopLevelSource_noKeepGoing() throws Exception { runIoExceptionInTopLevelSource(); - MoreAsserts.assertContainsEventRegex( + MoreAsserts.assertContainsEvent( events.collector(), - ".*foo/BUILD:2:11: //foo:foo: (error reading file '//foo:error.in': nope|missing input file" - + " '//foo:missing.in')"); - MoreAsserts.assertContainsEventRegex( + Pattern.compile( + ".*foo/BUILD:2:11: //foo:foo: (error reading file '//foo:error.in': nope|missing input" + + " file '//foo:missing.in')")); + MoreAsserts.assertContainsEvent( events.collector(), - ".*(1 input file\\(s\\) (are in error|do not exist)|2 input file\\(s\\) are in error or do" - + " not exist)"); + Pattern.compile( + ".*(1 input file\\(s\\) (are in error|do not exist)|2 input file\\(s\\) are in error" + + " or do not exist)")); } private void runMissingFileAndIoException() throws Exception { @@ -565,14 +571,16 @@ @Test public void missingFileAndIoException_noKeepGoing() throws Exception { runMissingFileAndIoException(); - MoreAsserts.assertContainsEventRegex( + MoreAsserts.assertContainsEvent( events.collector(), - ".*foo/BUILD:1:8: Executing genrule //foo:foo failed: (error reading file '//foo:error.in':" - + " nope|missing input file '//foo:missing.in')"); - MoreAsserts.assertContainsEventRegex( + Pattern.compile( + ".*foo/BUILD:1:8: Executing genrule //foo:foo failed: (error reading file" + + " '//foo:error.in': nope|missing input file '//foo:missing.in')")); + MoreAsserts.assertContainsEvent( events.collector(), - ".*(1 input file\\(s\\) (are in error|do not exist)|2 input file\\(s\\) are in error or do" - + " not exist)"); + Pattern.compile( + ".*(1 input file\\(s\\) (are in error|do not exist)|2 input file\\(s\\) are in error" + + " or do not exist)")); } private static class CustomRealFilesystem extends UnixFileSystem {
diff --git a/src/test/java/com/google/devtools/build/lib/events/util/EventCollectionApparatus.java b/src/test/java/com/google/devtools/build/lib/events/util/EventCollectionApparatus.java index 6e036e6..8881537 100644 --- a/src/test/java/com/google/devtools/build/lib/events/util/EventCollectionApparatus.java +++ b/src/test/java/com/google/devtools/build/lib/events/util/EventCollectionApparatus.java
@@ -25,6 +25,7 @@ import java.util.ArrayList; import java.util.List; import java.util.Set; +import java.util.regex.Pattern; /** * An apparatus for reporting / collecting events. @@ -175,8 +176,16 @@ } /** - * Utility method: Assert that the {@link #collector()} has received a - * warning with the {@code expectedMessage}. + * Utility method: Assert that the {@link #collector()} has received an error that matches {@code + * expectedPattern}. + */ + public Event assertContainsError(Pattern expectedPattern) { + return MoreAsserts.assertContainsEvent(eventCollector, expectedPattern, EventKind.ERROR); + } + + /** + * Utility method: Assert that the {@link #collector()} has received a warning with the {@code + * expectedMessage}. */ public Event assertContainsWarning(String expectedMessage) { return MoreAsserts.assertContainsEvent(eventCollector, expectedMessage, EventKind.WARNING);
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutorTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutorTest.java index c8b91d6..e3a8fed 100644 --- a/src/test/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutorTest.java +++ b/src/test/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutorTest.java
@@ -18,9 +18,7 @@ import static com.google.common.truth.Truth.assertWithMessage; import static com.google.devtools.build.lib.actions.util.ActionCacheTestHelper.AMNESIAC_CACHE; import static com.google.devtools.build.lib.actions.util.ActionsTestUtil.NULL_ACTION_OWNER; -import static com.google.devtools.build.lib.testutil.MoreAsserts.assertContainsEventRegex; import static com.google.devtools.build.lib.testutil.MoreAsserts.assertEventCount; -import static com.google.devtools.build.lib.testutil.MoreAsserts.assertNotContainsEventRegex; import static java.nio.charset.StandardCharsets.UTF_8; import static org.junit.Assert.assertThrows; import static org.junit.Assert.fail; @@ -160,6 +158,7 @@ import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicReference; +import java.util.regex.Pattern; import javax.annotation.Nullable; import net.starlark.java.eval.EvalException; import net.starlark.java.eval.Printer; @@ -2274,9 +2273,10 @@ null, null, /* trustRemoteArtifacts= */ false); - assertContainsEventRegex(eventCollector, ".*during scanning.*\n.*Scanning.*\n.*Test dir/top.*"); - assertNotContainsEventRegex( - eventCollector, ".*after scanning.*\n.*Scanning.*\n.*Test dir/top.*"); + MoreAsserts.assertContainsEvent( + eventCollector, Pattern.compile(".*during scanning.*\n.*Scanning.*\n.*Test dir/top.*")); + MoreAsserts.assertNotContainsEvent( + eventCollector, Pattern.compile(".*after scanning.*\n.*Scanning.*\n.*Test dir/top.*")); } private static AnalysisProtos.Artifact getArtifact(
diff --git a/src/test/java/com/google/devtools/build/lib/testutil/FoundationTestCase.java b/src/test/java/com/google/devtools/build/lib/testutil/FoundationTestCase.java index 114e923..e6baee9 100644 --- a/src/test/java/com/google/devtools/build/lib/testutil/FoundationTestCase.java +++ b/src/test/java/com/google/devtools/build/lib/testutil/FoundationTestCase.java
@@ -28,6 +28,7 @@ import com.google.devtools.build.lib.vfs.Root; import com.google.devtools.build.lib.vfs.inmemoryfs.InMemoryFileSystem; import java.util.Set; +import java.util.regex.Pattern; import org.junit.After; import org.junit.Before; @@ -113,6 +114,10 @@ expectedMessage); } + protected Event assertContainsEvent(Pattern expectedMessagePattern) { + return MoreAsserts.assertContainsEvent(eventCollector, expectedMessagePattern); + } + protected Event assertContainsEvent(String expectedMessage, Set<EventKind> kinds) { return MoreAsserts.assertContainsEvent(eventCollector, expectedMessage,
diff --git a/src/test/java/com/google/devtools/build/lib/testutil/MoreAsserts.java b/src/test/java/com/google/devtools/build/lib/testutil/MoreAsserts.java index 7e11610..fcbb4a7 100644 --- a/src/test/java/com/google/devtools/build/lib/testutil/MoreAsserts.java +++ b/src/test/java/com/google/devtools/build/lib/testutil/MoreAsserts.java
@@ -35,9 +35,9 @@ import java.util.Arrays; import java.util.Collections; import java.util.Comparator; +import java.util.IdentityHashMap; import java.util.Iterator; import java.util.List; -import java.util.Map; import java.util.Queue; import java.util.Set; import java.util.regex.Pattern; @@ -84,7 +84,7 @@ static final Predicate<Field> ALL_STRONG_REFS = Predicates.equalTo(NON_STRONG_REF); private static boolean isRetained(Predicate<Object> predicate, Object start) { - Map<Object, Object> visited = Maps.newIdentityHashMap(); + IdentityHashMap<Object, Object> visited = Maps.newIdentityHashMap(); visited.put(start, start); Queue<Object> toScan = new ArrayDeque<>(); toScan.add(start); @@ -301,28 +301,45 @@ } /** - * If {@code eventCollector} does not contain an event which matches {@code expectedEventRegex}, + * If {@code eventCollector} does not contain an event which matches {@code expectedEventPattern}, * fails with an informative assertion. */ - public static void assertContainsEventRegex( - Iterable<Event> eventCollector, String expectedEventRegex) { + public static Event assertContainsEvent( + Iterable<Event> eventCollector, Pattern expectedEventPattern, EventKind... kinds) { + return assertContainsEvent(eventCollector, expectedEventPattern, ImmutableSet.copyOf(kinds)); + } + + /** + * If {@code eventCollector} does not contain an event which matches {@code expectedEventPattern}, + * fails with an informative assertion. + */ + public static Event assertContainsEvent( + Iterable<Event> eventCollector, Pattern expectedEventPattern, Set<EventKind> kinds) { for (Event event : eventCollector) { - if (event.toString().matches(expectedEventRegex)) { - return; + // Does the event message match the expected regex? + if (!expectedEventPattern.matcher(event.toString()).find()) { + continue; } + // Was an expected kind given, and does the event match? + if (!kinds.isEmpty() && !kinds.contains(event.getKind())) { + continue; + } + // Return the event, assertion successful + return event; } String eventsString = eventsToString(eventCollector); - String failureMessage = "Event matching '" + expectedEventRegex + "' not found"; + String failureMessage = "Event matching '" + expectedEventPattern + "' not found"; if (!eventsString.isEmpty()) { failureMessage += "; found these though: " + eventsString; } fail(failureMessage); + return null; // unreachable } - public static void assertNotContainsEventRegex( - Iterable<Event> eventCollector, String unexpectedEventRegex) { + public static void assertNotContainsEvent( + Iterable<Event> eventCollector, Pattern unexpectedEventPattern) { for (Event event : eventCollector) { - assertThat(event.toString()).doesNotMatch(unexpectedEventRegex); + assertThat(event.toString()).doesNotMatch(unexpectedEventPattern); } } @@ -384,12 +401,12 @@ } /** - * If "expectedSublist" is not a sublist of "arguments", an informative - * assertion is failed in the context of the specified TestCase. + * If "expectedSublist" is not a sublist of "arguments", an informative assertion is failed in the + * context of the specified TestCase. * * <p>Argument order mnemonic: assert(X)ContainsSublist(Y). */ - @SuppressWarnings({"unchecked", "varargs"}) + @SuppressWarnings("varargs") public static <T> void assertContainsSublist(List<T> arguments, T... expectedSublist) { List<T> sublist = Arrays.asList(expectedSublist); try { @@ -400,12 +417,12 @@ } /** - * If "expectedSublist" is a sublist of "arguments", an informative - * assertion is failed in the context of the specified TestCase. + * If "expectedSublist" is a sublist of "arguments", an informative assertion is failed in the + * context of the specified TestCase. * * <p>Argument order mnemonic: assert(X)DoesNotContainSublist(Y). */ - @SuppressWarnings({"unchecked", "varargs"}) + @SuppressWarnings("varargs") public static <T> void assertDoesNotContainSublist(List<T> arguments, T... expectedSublist) { List<T> sublist = Arrays.asList(expectedSublist); try { @@ -442,7 +459,6 @@ * if the elements of the pair are equal by its lights. * @return first element not in arguments in order, or null if success. */ - @SuppressWarnings({"unchecked"}) protected static <S, T> T containsSublistWithGapsAndEqualityChecker(List<S> arguments, Function<Pair<S, T>, Boolean> equalityChecker, T... expectedSublist) { Iterator<S> iter = arguments.iterator();