Fix some tests to invalidate after changing core files.

In some cases where errors are tested the invalidations will throw exceptions after platform based flags are enabled, so wrap them in try/catch blocks now to be forward-compatible. Work towards platform-based flags: #19409. PiperOrigin-RevId: 617858233 Change-Id: Ie0ee49e63cfce19cfd9f498f6922e28709e50c39
bazelbuild · Mar 21, 2024 · 1a35d68 · 1a35d68
1 parent 5594166
commit 1a35d68
Show file tree

Hide file tree

Showing 2 changed files with 55 additions and 6 deletions.
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/PackageFunctionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/PackageFunctionTest.java
@@ -21,7 +21,7 @@
 import static org.junit.Assert.fail;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.Mockito.doAnswer;
-import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.reset;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
@@ -246,19 +246,18 @@ public void testPackageOverheadPassedToValidationLogic() throws Exception {
         .thenReturn(OptionalLong.of(42));
     ArgumentCaptor<Package> packageCaptor = ArgumentCaptor.forClass(Package.class);
 
-    invalidatePackages();
+    invalidatePackages(true);
+    reset(mockPackageValidator);
 
     SkyframeExecutorTestUtils.evaluate(
         getSkyframeExecutor(),
         PackageIdentifier.createInMainRepo("pkg"),
         /* keepGoing= */ false,
         reporter);
 
-    verify(mockPackageValidator, times(2))
-        .validate(packageCaptor.capture(), any(ExtendedEventHandler.class));
+    verify(mockPackageValidator).validate(packageCaptor.capture(), any(ExtendedEventHandler.class));
     List<Package> packages = packageCaptor.getAllValues();
-    assertThat(packages.get(0).getPackageOverhead()).isEmpty(); // Workspace pkg
-    assertThat(packages.get(1).getPackageOverhead()).isEqualTo(OptionalLong.of(42));
+    assertThat(packages.get(0).getPackageOverhead()).isEqualTo(OptionalLong.of(42));
   }
 
   @Test
@@ -1564,6 +1563,8 @@ public void testPreludeDefinedSymbolIsUsable() throws Exception {
           "pkg/BUILD", //
           "print(foo)");
 
+      invalidatePackages();
+
       getConfiguredTarget("//pkg:BUILD");
       assertContainsEvent("FOO");
     }
@@ -1582,6 +1583,8 @@ public void testPreludeAutomaticallyReexportsLoadedSymbols() throws Exception {
           "pkg/BUILD", //
           "print(foo)");
 
+      invalidatePackages();
+
       getConfiguredTarget("//pkg:BUILD");
       assertContainsEvent("FOO");
     }
@@ -1598,6 +1601,8 @@ public void testPreludeCanExportUnderscoreSymbols() throws Exception {
           "pkg/BUILD", //
           "print(_foo)");
 
+      invalidatePackages();
+
       getConfiguredTarget("//pkg:BUILD");
       assertContainsEvent("FOO");
     }
@@ -1612,6 +1617,8 @@ public void testPreludeCanShadowUniversal() throws Exception {
           "pkg/BUILD", //
           "print(len)");
 
+      invalidatePackages();
+
       getConfiguredTarget("//pkg:BUILD");
       assertContainsEvent("FOO");
     }
@@ -1626,6 +1633,8 @@ public void testPreludeCanShadowPredeclareds() throws Exception {
           "pkg/BUILD", //
           "print(cc_library)");
 
+      invalidatePackages();
+
       getConfiguredTarget("//pkg:BUILD");
       assertContainsEvent("FOO");
     }
@@ -1646,6 +1655,14 @@ public void testPreludeCanShadowInjectedPredeclareds() throws Exception {
           "pkg/BUILD", //
           "print(cc_library)");
 
+      try {
+        invalidatePackages();
+      } catch (
+          @SuppressWarnings("InterruptedExceptionSwallowed")
+          Exception e) {
+        // Ignore any errors.
+      }
+
       getConfiguredTarget("//pkg:BUILD");
       assertContainsEvent("FOO");
     }
@@ -1661,6 +1678,8 @@ public void testPreludeSymbolCannotBeMutated() throws Exception {
           "foo.append('BAR')");
 
       reporter.removeHandler(failFastHandler);
+      invalidatePackages();
+
       getConfiguredTarget("//pkg:BUILD");
       assertContainsEvent("trying to mutate a frozen list value");
     }
@@ -1677,6 +1696,8 @@ public void testPreludeCanAccessBzlDialectFeatures() throws Exception {
           "pkg/BUILD", //
           "print(foo())");
 
+      invalidatePackages();
+
       getConfiguredTarget("//pkg:BUILD");
       // Prelude can access native.glob (though only a BUILD thread can call it).
       assertContainsEvent("<built-in method glob of native value>");
@@ -1731,6 +1752,15 @@ public void testPreludeFailsWhenErrorInPreludeFile() throws Exception {
           "print(foo)");
 
       reporter.removeHandler(failFastHandler);
+
+      try {
+        invalidatePackages();
+      } catch (
+          @SuppressWarnings("InterruptedExceptionSwallowed")
+          Exception e) {
+        // Ignore any errors.
+      }
+
       getConfiguredTarget("//pkg:BUILD");
       assertContainsEvent(
           "File \"/workspace/tools/build_rules/test_prelude\", line 1, column 2, in <toplevel>");
@@ -1749,6 +1779,8 @@ public void testPreludeWorksEvenWhenPreludePackageInError() throws Exception {
           "pkg/BUILD", //
           "print(foo)");
 
+      invalidatePackages();
+
       // Succeeds because prelude loading is only dependent on the prelude package's existence, not
       // its evaluation.
       getConfiguredTarget("//pkg:BUILD");

diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/RepoFileFunctionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/RepoFileFunctionTest.java
@@ -33,6 +33,7 @@ public class RepoFileFunctionTest extends BuildViewTestCase {
   public void defaultVisibility() throws Exception {
     scratch.overwriteFile("REPO.bazel", "repo(default_visibility=['//some:thing'])");
     scratch.overwriteFile("p/BUILD", "sh_library(name = 't')");
+    invalidatePackages();
     Target t = getTarget("//p:t");
     assertThat(t.getVisibility().getDeclaredLabels())
         .containsExactly(Label.parseCanonical("//some:thing"));
@@ -42,6 +43,7 @@ public void defaultVisibility() throws Exception {
   public void repoFileInTheMainRepo() throws Exception {
     scratch.overwriteFile("REPO.bazel", "repo(default_deprecation='EVERYTHING IS DEPRECATED')");
     scratch.overwriteFile("abc/def/BUILD", "filegroup(name='what')");
+    invalidatePackages();
     assertThat(
             getRuleContext(getConfiguredTarget("//abc/def:what"))
                 .attributes()
@@ -83,6 +85,13 @@ public void cantCallRepoTwice() throws Exception {
         "repo(features=['abc'])");
     scratch.overwriteFile("abc/def/BUILD", "filegroup(name='what')");
     reporter.removeHandler(failFastHandler);
+    try {
+      invalidatePackages();
+    } catch (
+        @SuppressWarnings("InterruptedExceptionSwallowed")
+        Exception e) {
+      // Ignore any errors.
+    }
     assertTargetError("//abc/def:what", "'repo' can only be called once");
   }
 
@@ -93,6 +102,7 @@ public void featureMerger() throws Exception {
         "abc/def/BUILD",
         "package(features=['-a','-b','d'])",
         "filegroup(name='what', features=['b'])");
+    invalidatePackages();
     RuleContext ruleContext = getRuleContext(getConfiguredTarget("//abc/def:what"));
     assertThat(ruleContext.getFeatures()).containsExactly("b", "c", "d");
     assertThat(ruleContext.getDisabledFeatures()).containsExactly("a");
@@ -104,6 +114,13 @@ public void restrictedSyntax() throws Exception {
         "REPO.bazel", "if 3+5>7: repo(default_deprecation='EVERYTHING IS DEPRECATED')");
     scratch.overwriteFile("abc/def/BUILD", "filegroup(name='what')");
     reporter.removeHandler(failFastHandler);
+    try {
+      invalidatePackages();
+    } catch (
+        @SuppressWarnings("InterruptedExceptionSwallowed")
+        Exception e) {
+      // Ignore any errors.
+    }
     assertTargetError(
         "//abc/def:what",
         "`if` statements are not allowed in REPO.bazel files. You may use an `if` expression for"