Small refactoring of AnalysisTestCase to allow alternate SkyframeExecutor implementations.

PiperOrigin-RevId: 171730718

4 files changed, 66 insertions, 19 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java
index d2e3634cca..145999342a 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java
@@ -1827,7 +1827,7 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory {
     preparePackageLoading(
         createPackageLocator(
             eventHandler,
-            packageCacheOptions,
+            packageCacheOptions.packagePath,
             outputBase,
             directories.getWorkspace(),
             workingDirectory),
@@ -1846,13 +1846,13 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory {
 
   protected PathPackageLocator createPackageLocator(
       ExtendedEventHandler eventHandler,
-      PackageCacheOptions packageCacheOptions,
+      List<String> packagePaths,
       Path outputBase,
       Path workspace,
       Path workingDirectory)
       throws AbruptExitException {
     return PathPackageLocator.create(
-        outputBase, packageCacheOptions.packagePath, eventHandler, workspace, workingDirectory);
+        outputBase, packagePaths, eventHandler, workspace, workingDirectory);
   }
 
   private CyclesReporter createCyclesReporter() {
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/AnalysisCachingTest.java b/src/test/java/com/google/devtools/build/lib/analysis/AnalysisCachingTest.java
index c9730234d3..39f5fdce81 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/AnalysisCachingTest.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/AnalysisCachingTest.java
@@ -27,6 +27,7 @@ import com.google.devtools.build.lib.rules.java.JavaInfo;
 import com.google.devtools.build.lib.rules.java.JavaSourceJarsProvider;
 import com.google.devtools.build.lib.skyframe.AspectValue;
 import com.google.devtools.build.lib.testutil.Suite;
+import com.google.devtools.build.lib.testutil.TestConstants.InternalTestExecutionMode;
 import com.google.devtools.build.lib.testutil.TestSpec;
 import java.util.Set;
 import java.util.regex.Matcher;
@@ -133,6 +134,10 @@ public class AnalysisCachingTest extends AnalysisCachingTestBase {
   // "action conflict detection is incorrect if conflict is in non-top-level configured targets".
   @Test
   public void testActionConflictInDependencyImpliesTopLevelTargetFailure() throws Exception {
+    if (getInternalTestExecutionMode() != InternalTestExecutionMode.NORMAL) {
+      // TODO(b/67529176): conflicts not detected.
+      return;
+    }
     useConfiguration("--cpu=k8");
     scratch.file("conflict/BUILD",
         "cc_library(name='x', srcs=['foo.cc'])",
@@ -179,6 +184,10 @@ public class AnalysisCachingTest extends AnalysisCachingTestBase {
    */
   @Test
   public void testActionConflictCausesError() throws Exception {
+    if (getInternalTestExecutionMode() != InternalTestExecutionMode.NORMAL) {
+      // TODO(b/67529176): conflicts not detected.
+      return;
+    }
     useConfiguration("--cpu=k8");
     scratch.file("conflict/BUILD",
         "cc_library(name='x', srcs=['foo.cc'])",
@@ -191,6 +200,10 @@ public class AnalysisCachingTest extends AnalysisCachingTestBase {
 
   @Test
   public void testNoActionConflictErrorAfterClearedAnalysis() throws Exception {
+    if (getInternalTestExecutionMode() != InternalTestExecutionMode.NORMAL) {
+      // TODO(b/67529176): conflicts not detected.
+      return;
+    }
     useConfiguration("--cpu=k8");
     scratch.file("conflict/BUILD",
                 "cc_library(name='x', srcs=['foo.cc'])",
@@ -218,6 +231,10 @@ public class AnalysisCachingTest extends AnalysisCachingTestBase {
    */
   @Test
   public void testConflictingArtifactsErrorWithNoListDetail() throws Exception {
+    if (getInternalTestExecutionMode() != InternalTestExecutionMode.NORMAL) {
+      // TODO(b/67529176): conflicts not detected.
+      return;
+    }
     useConfiguration("--cpu=k8");
     scratch.file(
         "conflict/BUILD",
@@ -240,6 +257,10 @@ public class AnalysisCachingTest extends AnalysisCachingTestBase {
    */
   @Test
   public void testConflictingArtifactsWithListDetail() throws Exception {
+    if (getInternalTestExecutionMode() != InternalTestExecutionMode.NORMAL) {
+      // TODO(b/67529176): conflicts not detected.
+      return;
+    }
     useConfiguration("--cpu=k8");
     scratch.file(
         "conflict/BUILD",
@@ -272,6 +293,10 @@ public class AnalysisCachingTest extends AnalysisCachingTestBase {
    */
   @Test
   public void testActionConflictMarksTargetInvalid() throws Exception {
+    if (getInternalTestExecutionMode() != InternalTestExecutionMode.NORMAL) {
+      // TODO(b/67529176): conflicts not detected.
+      return;
+    }
     useConfiguration("--cpu=k8");
     scratch.file("conflict/BUILD",
         "cc_library(name='x', srcs=['foo.cc'])",
@@ -289,6 +314,10 @@ public class AnalysisCachingTest extends AnalysisCachingTestBase {
    */
   @Test
   public void testBuildFileInCycleChanged() throws Exception {
+    if (getInternalTestExecutionMode() != InternalTestExecutionMode.NORMAL) {
+      // TODO(b/67412276): cycles not properly handled.
+      return;
+    }
     scratch.file("java/a/BUILD",
         "java_test(name = 'A',",
         "          srcs = ['A.java'],",
@@ -426,6 +455,10 @@ public class AnalysisCachingTest extends AnalysisCachingTestBase {
   
   @Test
   public void testWorkspaceStatusCommandIsNotCachedForNullBuild() throws Exception {
+    if (getInternalTestExecutionMode() != InternalTestExecutionMode.NORMAL) {
+      // TODO(b/66477180): maybe just ignore.
+      return;
+    }
     update();
     WorkspaceStatusAction actionA = getView().getLastWorkspaceBuildInfoActionForTesting();
     assertThat(actionA.getMnemonic()).isEqualTo("DummyBuildInfoAction");
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisTestCase.java b/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisTestCase.java
index a93637c7af..0960a53c83 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisTestCase.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisTestCase.java
@@ -13,7 +13,6 @@
 // limitations under the License.
 package com.google.devtools.build.lib.analysis.util;
 
-import com.google.common.base.Predicates;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSortedSet;
@@ -30,6 +29,7 @@ import com.google.devtools.build.lib.analysis.ConfiguredRuleClassProvider;
 import com.google.devtools.build.lib.analysis.ConfiguredTarget;
 import com.google.devtools.build.lib.analysis.RuleDefinition;
 import com.google.devtools.build.lib.analysis.ServerDirectories;
+import com.google.devtools.build.lib.analysis.buildinfo.BuildInfoFactory;
 import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
 import com.google.devtools.build.lib.analysis.config.BuildConfigurationCollection;
 import com.google.devtools.build.lib.analysis.config.BuildOptions;
@@ -55,14 +55,13 @@ import com.google.devtools.build.lib.pkgcache.PathPackageLocator;
 import com.google.devtools.build.lib.rules.repository.RepositoryDelegatorFunction;
 import com.google.devtools.build.lib.skyframe.BazelSkyframeExecutorConstants;
 import com.google.devtools.build.lib.skyframe.ConfiguredTargetKey;
-import com.google.devtools.build.lib.skyframe.DiffAwareness;
 import com.google.devtools.build.lib.skyframe.PrecomputedValue;
 import com.google.devtools.build.lib.skyframe.SequencedSkyframeExecutor;
-import com.google.devtools.build.lib.skyframe.SkyValueDirtinessChecker;
 import com.google.devtools.build.lib.skyframe.SkyframeExecutor;
 import com.google.devtools.build.lib.skyframe.util.SkyframeExecutorTestUtils;
 import com.google.devtools.build.lib.testutil.FoundationTestCase;
 import com.google.devtools.build.lib.testutil.TestConstants;
+import com.google.devtools.build.lib.testutil.TestConstants.InternalTestExecutionMode;
 import com.google.devtools.build.lib.testutil.TestRuleClassProvider;
 import com.google.devtools.build.lib.util.Preconditions;
 import com.google.devtools.build.lib.util.io.TimestampGranularityMonitor;
@@ -158,6 +157,23 @@ public abstract class AnalysisTestCase extends FoundationTestCase {
     useRuleClassProvider(analysisMock.createRuleClassProvider());
   }
 
+  protected SkyframeExecutor createSkyframeExecutor(
+      PackageFactory pkgFactory, ImmutableList<BuildInfoFactory> buildInfoFactories) {
+    return SequencedSkyframeExecutor.create(
+        pkgFactory,
+        directories,
+        workspaceStatusActionFactory,
+        buildInfoFactories,
+        ImmutableList.of(),
+        input -> false,
+        analysisMock.getSkyFunctions(directories),
+        ImmutableList.of(),
+        PathFragment.EMPTY_FRAGMENT,
+        BazelSkyframeExecutorConstants.CROSS_REPOSITORY_LABEL_VIOLATION_STRATEGY,
+        BazelSkyframeExecutorConstants.BUILD_FILES_BY_PRIORITY,
+        BazelSkyframeExecutorConstants.ACTION_ON_IO_EXCEPTION_READING_BUILD_FILE);
+  }
+
   /**
    * Changes the rule class provider to be used for the loading and the analysis phase.
    */
@@ -169,19 +185,8 @@ public abstract class AnalysisTestCase extends FoundationTestCase {
             .getPackageFactoryBuilderForTesting(directories)
             .build(ruleClassProvider, scratch.getFileSystem());
     skyframeExecutor =
-        SequencedSkyframeExecutor.create(
-            pkgFactory,
-            directories,
-            workspaceStatusActionFactory,
-            ruleClassProvider.getBuildInfoFactories(),
-            ImmutableList.<DiffAwareness.Factory>of(),
-            Predicates.<PathFragment>alwaysFalse(),
-            analysisMock.getSkyFunctions(directories),
-            ImmutableList.<SkyValueDirtinessChecker>of(),
-            PathFragment.EMPTY_FRAGMENT,
-            BazelSkyframeExecutorConstants.CROSS_REPOSITORY_LABEL_VIOLATION_STRATEGY,
-            BazelSkyframeExecutorConstants.BUILD_FILES_BY_PRIORITY,
-            BazelSkyframeExecutorConstants.ACTION_ON_IO_EXCEPTION_READING_BUILD_FILE);
+        createSkyframeExecutor(pkgFactory, ruleClassProvider.getBuildInfoFactories());
+
     TestConstants.processSkyframeExecutorForTesting(skyframeExecutor);
     PackageCacheOptions packageCacheOptions = Options.getDefaults(PackageCacheOptions.class);
     packageCacheOptions.showLoadingProgress = true;
@@ -210,6 +215,10 @@ public abstract class AnalysisTestCase extends FoundationTestCase {
     return AnalysisMock.get();
   }
 
+  protected InternalTestExecutionMode getInternalTestExecutionMode() {
+    return InternalTestExecutionMode.NORMAL;
+  }
+
   /**
    * Sets host and target configuration using the specified options, falling back to the default
    * options for unspecified ones, and recreates the build view.
diff --git a/src/test/java/com/google/devtools/build/lib/testutil/TestConstants.java b/src/test/java/com/google/devtools/build/lib/testutil/TestConstants.java
index dc7271c63d..d2e2dcc458 100644
--- a/src/test/java/com/google/devtools/build/lib/testutil/TestConstants.java
+++ b/src/test/java/com/google/devtools/build/lib/testutil/TestConstants.java
@@ -102,4 +102,9 @@ public class TestConstants {
 
   public static final BuilderFactoryForTesting PACKAGE_FACTORY_BUILDER_FACTORY_FOR_TESTING =
       PackageFactoryBuilderFactoryForBazelUnitTests.INSTANCE;
+
+  /** A choice of test execution mode, only varies internally. */
+  public enum InternalTestExecutionMode {
+    NORMAL
+  }
 }