Automated rollback of commit 7ad7324.

*** Reason for rollback *** Breaks Guitar due to platform option default values, b/127451107 *** Original change description *** Automated rollback of commit d04ba2f. *** Reason for rollback *** Rolling forward with fix for non-standard JDKs in tests. RELNOTES: None *** Original change description *** Automated rollback of commit dd1d148. *** Reason for rollback *** Some breakages caught by CI, even though global presubmit didn't fail :/ *** Original change description *** Get rid of custom invocation policy in tests. PiperOrigin-RevId: 236859498
bazelbuild · Mar 5, 2019 · 61c0b38 · 61c0b38
1 parent ee34392
commit 61c0b38
Show file tree

Hide file tree

Showing 9 changed files with 52 additions and 30 deletions.
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisMock.java b/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisMock.java
@@ -39,6 +39,7 @@
 import com.google.devtools.build.lib.vfs.Path;
 import com.google.devtools.build.skyframe.SkyFunction;
 import com.google.devtools.build.skyframe.SkyFunctionName;
+import com.google.devtools.common.options.InvocationPolicyEnforcer;
 import java.io.IOException;
 import java.lang.reflect.Field;
 import java.util.List;
@@ -73,6 +74,11 @@ public PackageFactoryBuilderWithSkyframeForTesting getPackageFactoryBuilderForTe
         .setExtraSkyFunctions(getSkyFunctions(directories));
   }
 
+  @Override
+  public InvocationPolicyEnforcer getInvocationPolicyEnforcer() {
+    return new InvocationPolicyEnforcer(TestConstants.TEST_INVOCATION_POLICY);
+  }
+
   /**
    * This is called from test setup to create the mock directory layout needed to create the
    * configuration.
@@ -178,6 +184,11 @@ public ConfiguredRuleClassProvider createRuleClassProvider() {
       return delegate.createRuleClassProvider();
     }
 
+    @Override
+    public InvocationPolicyEnforcer getInvocationPolicyEnforcer() {
+      return delegate.getInvocationPolicyEnforcer();
+    }
+
     @Override
     public boolean isThisBazel() {
       return delegate.isThisBazel();

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
@@ -82,6 +82,7 @@
 import com.google.devtools.build.lib.vfs.Root;
 import com.google.devtools.build.lib.vfs.RootedPath;
 import com.google.devtools.build.skyframe.SkyKey;
+import com.google.devtools.common.options.InvocationPolicyEnforcer;
 import com.google.devtools.common.options.Options;
 import com.google.devtools.common.options.OptionsParser;
 import java.util.Arrays;
@@ -281,12 +282,13 @@ public final void useConfiguration(String... args) throws Exception {
                     LoadingPhaseThreadsOption.class,
                     LoadingOptions.class),
                 ruleClassProvider.getConfigurationOptions()));
-    optionsParser.parse("--default_visibility=public", "--cpu=k8", "--host_cpu=k8");
-    optionsParser.parse(TestConstants.PRODUCT_SPECIFIC_FLAGS);
+    optionsParser.parse(new String[] {"--default_visibility=public", "--cpu=k8", "--host_cpu=k8"});
     optionsParser.parse(args);
     if (defaultFlags().contains(Flag.TRIMMED_CONFIGURATIONS)) {
       optionsParser.parse("--experimental_dynamic_configs=on");
     }
+    InvocationPolicyEnforcer optionsPolicyEnforcer = analysisMock.getInvocationPolicyEnforcer();
+    optionsPolicyEnforcer.enforce(optionsParser);
 
     buildOptions = ruleClassProvider.createBuildOptions(optionsParser);
   }

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
@@ -157,6 +157,7 @@
 import com.google.devtools.build.skyframe.ErrorInfo;
 import com.google.devtools.build.skyframe.MemoizingEvaluator;
 import com.google.devtools.build.skyframe.SkyFunction;
+import com.google.devtools.common.options.InvocationPolicyEnforcer;
 import com.google.devtools.common.options.Options;
 import com.google.devtools.common.options.OptionsParser;
 import com.google.devtools.common.options.OptionsParsingException;
@@ -364,6 +365,10 @@ protected final BuildConfigurationCollection createConfigurations(
     // TODO(juliexxia): when the starlark options parsing work goes in, add type verification here.
     optionsParser.setStarlarkOptions(skylarkOptions);
 
+    InvocationPolicyEnforcer optionsPolicyEnforcer =
+        getAnalysisMock().getInvocationPolicyEnforcer();
+    optionsPolicyEnforcer.enforce(optionsParser);
+
     BuildOptions buildOptions = ruleClassProvider.createBuildOptions(optionsParser);
     return skyframeExecutor.createConfigurations(
         reporter, buildOptions, ImmutableSet.<String>of(), false);
@@ -513,17 +518,14 @@ protected void invalidatePackages(boolean alsoConfigs) throws InterruptedExcepti
    */
   protected void useConfiguration(ImmutableMap<String, Object> skylarkOptions, String... args)
       throws Exception {
-    ImmutableList<String> actualArgs =
-        ImmutableList.<String>builder()
-            .addAll(TestConstants.PRODUCT_SPECIFIC_FLAGS)
-            .add(args)
-            .add("--experimental_dynamic_configs=" + Ascii.toLowerCase(configsMode.toString()))
-            .build();
-
-    masterConfig = createConfigurations(skylarkOptions, actualArgs.toArray(new String[0]));
+    String[] actualArgs;
+    actualArgs = Arrays.copyOf(args, args.length + 1);
+    actualArgs[args.length] =
+        "--experimental_dynamic_configs=" + Ascii.toLowerCase(configsMode.toString());
+    masterConfig = createConfigurations(skylarkOptions, actualArgs);
     targetConfig = getTargetConfiguration();
     targetConfigKey = BuildConfigurationValue.key(targetConfig);
-    configurationArgs = actualArgs;
+    configurationArgs = Arrays.asList(actualArgs);
     createBuildView();
   }
 

diff --git a/src/test/java/com/google/devtools/build/lib/analysis/util/ConfigurationTestCase.java b/src/test/java/com/google/devtools/build/lib/analysis/util/ConfigurationTestCase.java
@@ -52,6 +52,7 @@
 import com.google.devtools.build.lib.vfs.Root;
 import com.google.devtools.build.lib.vfs.RootedPath;
 import com.google.devtools.common.options.Converters;
+import com.google.devtools.common.options.InvocationPolicyEnforcer;
 import com.google.devtools.common.options.Option;
 import com.google.devtools.common.options.OptionDocumentationCategory;
 import com.google.devtools.common.options.OptionEffectTag;
@@ -204,7 +205,9 @@ protected BuildConfigurationCollection createCollection(String... args) throws E
         .add(TestOptions.class)
         .build());
     parser.parse(args);
-    parser.parse(TestConstants.PRODUCT_SPECIFIC_FLAGS);
+
+    InvocationPolicyEnforcer optionsPolicyEnforcer = analysisMock.getInvocationPolicyEnforcer();
+    optionsPolicyEnforcer.enforce(parser);
 
     ImmutableSortedSet<String> multiCpu = ImmutableSortedSet.copyOf(
         parser.getOptions(TestOptions.class).multiCpus);

diff --git a/src/test/java/com/google/devtools/build/lib/packages/util/LoadingMock.java b/src/test/java/com/google/devtools/build/lib/packages/util/LoadingMock.java
@@ -18,6 +18,7 @@
 import com.google.devtools.build.lib.skyframe.packages.PackageFactoryBuilderWithSkyframeForTesting;
 import com.google.devtools.build.lib.testutil.TestConstants;
 import com.google.devtools.build.lib.testutil.TestRuleClassProvider;
+import com.google.devtools.common.options.InvocationPolicyEnforcer;
 
 /** Create a mock client for the loading phase, as well as a configuration factory. */
 public class LoadingMock {
@@ -38,4 +39,8 @@ public PackageFactoryBuilderWithSkyframeForTesting getPackageFactoryBuilderForTe
   public ConfiguredRuleClassProvider createRuleClassProvider() {
     return TestRuleClassProvider.getRuleClassProvider();
   }
+
+  public InvocationPolicyEnforcer getInvocationPolicyEnforcer() {
+    return new InvocationPolicyEnforcer(TestConstants.TEST_INVOCATION_POLICY);
+  }
 }
diff --git a/src/test/java/com/google/devtools/build/lib/pkgcache/BuildFileModificationTest.java b/src/test/java/com/google/devtools/build/lib/pkgcache/BuildFileModificationTest.java
@@ -111,6 +111,7 @@ public final void initializeSkyframeExecutor() throws Exception {
     TestConstants.processSkyframeExecutorForTesting(skyframeExecutor);
     OptionsParser parser =
         OptionsParser.newOptionsParser(PackageCacheOptions.class, StarlarkSemanticsOptions.class);
+    analysisMock.getInvocationPolicyEnforcer().enforce(parser);
     setUpSkyframe(
         parser.getOptions(PackageCacheOptions.class),
         parser.getOptions(StarlarkSemanticsOptions.class));

diff --git a/src/test/java/com/google/devtools/build/lib/pkgcache/PackageCacheTest.java b/src/test/java/com/google/devtools/build/lib/pkgcache/PackageCacheTest.java
@@ -53,6 +53,7 @@
 import com.google.devtools.build.lib.vfs.Path;
 import com.google.devtools.build.lib.vfs.Root;
 import com.google.devtools.build.lib.vfs.RootedPath;
+import com.google.devtools.common.options.InvocationPolicyEnforcer;
 import com.google.devtools.common.options.OptionsParser;
 import com.google.devtools.common.options.OptionsParsingException;
 import java.io.IOException;
@@ -157,6 +158,13 @@ private OptionsParser parse(String... options) throws Exception {
     parser.parse("--default_visibility=public");
     parser.parse(options);
 
+    InvocationPolicyEnforcer optionsPolicyEnforcer = analysisMock.getInvocationPolicyEnforcer();
+    try {
+      optionsPolicyEnforcer.enforce(parser);
+    } catch (OptionsParsingException e) {
+      throw new IllegalStateException(e);
+    }
+
     return parser;
   }
 

diff --git a/src/test/java/com/google/devtools/build/lib/testutil/BlazeTestUtils.java b/src/test/java/com/google/devtools/build/lib/testutil/BlazeTestUtils.java
@@ -14,16 +14,18 @@
 
 package com.google.devtools.build.lib.testutil;
 
+import static com.google.common.collect.ImmutableList.toImmutableList;
+
 import com.google.common.base.Joiner;
 import com.google.common.base.Preconditions;
+import com.google.common.collect.Streams;
 import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.cmdline.LabelSyntaxException;
 import com.google.devtools.build.lib.vfs.FileSystemUtils;
 import com.google.devtools.build.lib.vfs.Path;
 import java.io.File;
 import java.io.IOException;
-import java.util.ArrayList;
-import java.util.Collections;
+import java.util.List;
 
 /**
  * Some static utility functions for testing Blaze code. In contrast to {@link TestUtils}, these
@@ -95,15 +97,7 @@ public static Label convertLabel(Label label) {
     }
   }
 
-  /**
-   * Creates a list of arguments to pass to Bazel, with flags necessary for Bazel to work properly
-   * appended to the original {@code args} array.
-   */
-  public static ArrayList<String> makeArgs(String... args) {
-    ArrayList<String> result =
-        new ArrayList<>(args.length + TestConstants.PRODUCT_SPECIFIC_FLAGS.size());
-    Collections.addAll(result, args);
-    result.addAll(TestConstants.PRODUCT_SPECIFIC_FLAGS);
-    return result;
+  public static List<Label> convertLabels(Iterable<Label> labels) {
+    return Streams.stream(labels).map(BlazeTestUtils::convertLabel).collect(toImmutableList());
   }
 }
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
@@ -113,12 +113,8 @@ public static void processSkyframeExecutorForTesting(SkyframeExecutor skyframeEx
   public static final ImmutableList<String> OSX_CROSSTOOL_FLAGS =
       ImmutableList.of();
 
-  /**
-   * Flags that must be set for Bazel to work properly, if the default values are unusable for
-   * some reason.
-   */
-  public static final ImmutableList<String> PRODUCT_SPECIFIC_FLAGS =
-      ImmutableList.of();
+  public static final InvocationPolicy TEST_INVOCATION_POLICY =
+      InvocationPolicy.getDefaultInstance();
 
   public static final BuilderFactoryForTesting PACKAGE_FACTORY_BUILDER_FACTORY_FOR_TESTING =
       PackageFactoryBuilderFactoryForBazelUnitTests.INSTANCE;