Add package validation tests for PackageFactory

Some basic tests to ensure validation is happening at the expected point in
PackageFactory - after loading has completed.

RELNOTES: None
PiperOrigin-RevId: 297371908

src/test/java/com/google/devtools/build/lib/packages/PackageFactoryTest.java

diff --git a/src/test/java/com/google/devtools/build/lib/packages/PackageFactoryTest.java b/src/test/java/com/google/devtools/build/lib/packages/PackageFactoryTest.java
index 507990d..7c743c1 100644
--- a/src/test/java/com/google/devtools/build/lib/packages/PackageFactoryTest.java
+++ b/src/test/java/com/google/devtools/build/lib/packages/PackageFactoryTest.java
@@ -25,10 +25,12 @@
 import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.cmdline.PackageIdentifier;
 import com.google.devtools.build.lib.events.Reporter;
-import com.google.devtools.build.lib.packages.util.PackageFactoryApparatus;
+import com.google.devtools.build.lib.packages.PackageFactory.EnvironmentExtension;
+import com.google.devtools.build.lib.packages.PackageValidator.InvalidPackageException;
 import com.google.devtools.build.lib.packages.util.PackageFactoryTestBase;
 import com.google.devtools.build.lib.syntax.ParserInput;
 import com.google.devtools.build.lib.syntax.StarlarkFile;
+import com.google.devtools.build.lib.syntax.StarlarkSemantics;
 import com.google.devtools.build.lib.testutil.TestUtils;
 import com.google.devtools.build.lib.vfs.Path;
 import com.google.devtools.build.lib.vfs.PathFragment;
@@ -55,6 +57,11 @@
 @RunWith(JUnit4.class)
 public class PackageFactoryTest extends PackageFactoryTestBase {
 
+  @Override
+  public List<EnvironmentExtension> getEnvironmentExtensions() {
+    return ImmutableList.of();
+  }
+
   @Test
   public void testCreatePackage() throws Exception {
     Path buildFile = scratch.file("/pkgname/BUILD", "# empty build file ");
@@ -527,6 +534,31 @@
   }
 
   @Test
+  public void testPackageValidationFailureRegisteredAfterLoading() throws Exception {
+    Path path = scratch.file("/x/BUILD", "sh_library(name='y')");
+
+    dummyPackageValidator.setImpl(
+        pkg -> {
+          if (pkg.getName().equals("x")) {
+            throw new InvalidPackageException(pkg.getPackageIdentifier(), "nope");
+          }
+        });
+
+    Package pkg = packages.createPackage("x", RootedPath.toRootedPath(root, path));
+    assertThat(pkg.containsErrors()).isFalse();
+
+    InvalidPackageException expected =
+        assertThrows(
+            InvalidPackageException.class,
+            () ->
+                packages
+                    .factory()
+                    .afterDoneLoadingPackage(
+                        pkg, StarlarkSemantics.DEFAULT_SEMANTICS, /*loadTimeNanos=*/ 0));
+    assertThat(expected).hasMessageThat().contains("no such package 'x': nope");
+  }
+
+  @Test
   public void testGlobDirectoryExclusion() throws Exception {
     emptyFile("/fruit/data/apple");
     emptyFile("/fruit/data/pear");
@@ -1181,11 +1213,6 @@
   }
 
   @Override
-  protected PackageFactoryApparatus createPackageFactoryApparatus() {
-    return new PackageFactoryApparatus(events.reporter());
-  }
-
-  @Override
   protected String getPathPrefix() {
     return "";
   }

src/test/java/com/google/devtools/build/lib/packages/util/PackageFactoryApparatus.java

diff --git a/src/test/java/com/google/devtools/build/lib/packages/util/PackageFactoryApparatus.java b/src/test/java/com/google/devtools/build/lib/packages/util/PackageFactoryApparatus.java
index eda3d33..8d61e5f 100644
--- a/src/test/java/com/google/devtools/build/lib/packages/util/PackageFactoryApparatus.java
+++ b/src/test/java/com/google/devtools/build/lib/packages/util/PackageFactoryApparatus.java
@@ -28,6 +28,7 @@
 import com.google.devtools.build.lib.packages.NoSuchPackageException;
 import com.google.devtools.build.lib.packages.Package;
 import com.google.devtools.build.lib.packages.PackageFactory;
+import com.google.devtools.build.lib.packages.PackageFactory.EnvironmentExtension;
 import com.google.devtools.build.lib.packages.PackageValidator;
 import com.google.devtools.build.lib.packages.RuleClassProvider;
 import com.google.devtools.build.lib.packages.StarlarkSemanticsOptions;
@@ -43,6 +44,7 @@
 import com.google.devtools.build.lib.vfs.RootedPath;
 import com.google.devtools.common.options.OptionsParser;
 import java.io.IOException;
+import java.util.List;
 
 /**
  * An apparatus that creates / maintains a {@link PackageFactory}.
@@ -52,9 +54,17 @@
   private final ExtendedEventHandler eventHandler;
   private final PackageFactory factory;
 
+  public PackageFactoryApparatus(ExtendedEventHandler eventHandler) {
+    this(
+        eventHandler,
+        /*environmentExtensions=*/ ImmutableList.of(),
+        PackageValidator.NOOP_VALIDATOR);
+  }
+
   public PackageFactoryApparatus(
       ExtendedEventHandler eventHandler,
-      PackageFactory.EnvironmentExtension... environmentExtensions) {
+      List<EnvironmentExtension> environmentExtensions,
+      PackageValidator packageValidator) {
     this.eventHandler = eventHandler;
     RuleClassProvider ruleClassProvider = TestRuleClassProvider.getRuleClassProvider();
     factory =
@@ -63,7 +73,7 @@
             ImmutableList.copyOf(environmentExtensions),
             "test",
             Package.Builder.DefaultHelper.INSTANCE,
-            PackageValidator.NOOP_VALIDATOR);
+            packageValidator);
   }
 
   /**

src/test/java/com/google/devtools/build/lib/packages/util/PackageFactoryTestBase.java

diff --git a/src/test/java/com/google/devtools/build/lib/packages/util/PackageFactoryTestBase.java b/src/test/java/com/google/devtools/build/lib/packages/util/PackageFactoryTestBase.java
index c0c1ffd..449648a 100644
--- a/src/test/java/com/google/devtools/build/lib/packages/util/PackageFactoryTestBase.java
+++ b/src/test/java/com/google/devtools/build/lib/packages/util/PackageFactoryTestBase.java
@@ -31,6 +31,8 @@
 import com.google.devtools.build.lib.packages.NoSuchPackageException;
 import com.google.devtools.build.lib.packages.OutputFile;
 import com.google.devtools.build.lib.packages.Package;
+import com.google.devtools.build.lib.packages.PackageFactory.EnvironmentExtension;
+import com.google.devtools.build.lib.packages.PackageValidator;
 import com.google.devtools.build.lib.packages.RawAttributeMapper;
 import com.google.devtools.build.lib.packages.Rule;
 import com.google.devtools.build.lib.syntax.Starlark;
@@ -61,7 +63,10 @@
 
   protected Scratch scratch;
   protected EventCollectionApparatus events = new EventCollectionApparatus();
-  protected PackageFactoryApparatus packages = createPackageFactoryApparatus();
+  protected DummyPackageValidator dummyPackageValidator = new DummyPackageValidator();
+  protected PackageFactoryApparatus packages =
+      new PackageFactoryApparatus(
+          events.reporter(), getEnvironmentExtensions(), dummyPackageValidator);
   protected Root root;
 
   protected com.google.devtools.build.lib.packages.Package expectEvalSuccess(String... content)
@@ -82,7 +87,7 @@
     events.assertContainsError(expectedError);
   }
 
-  protected abstract PackageFactoryApparatus createPackageFactoryApparatus();
+  protected abstract List<EnvironmentExtension> getEnvironmentExtensions();
 
   protected Path throwOnReaddir = null;
 
@@ -365,4 +370,19 @@
       }
     }
   }
+
+  /** {@PackageValidator} whose functionality can be swapped out on demand via {@link #setImpl}. */
+  protected static class DummyPackageValidator implements PackageValidator {
+    private PackageValidator underlying = PackageValidator.NOOP_VALIDATOR;
+
+    /** Sets {@link PackageValidator} implementation to use. */
+    public void setImpl(PackageValidator impl) {
+      this.underlying = impl;
+    }
+
+    @Override
+    public void validate(Package pkg) throws InvalidPackageException {
+      underlying.validate(pkg);
+    }
+  }
 }