Enable the configuration sanity check for a subset of tests.

Also remove a try-catch block, which was converting all configuration
creation errors to IllegalStateException.

This uncovered a bug in CppOptions, which wasn't adding host_grte_top to the
set of explicit labels.

--
MOS_MIGRATED_REVID=118654358

3 files changed, 36 insertions, 42 deletions

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 6430c55455..dd799993e6 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
@@ -80,7 +80,6 @@ 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;
 import com.google.devtools.build.lib.analysis.config.ConfigurationFactory;
-import com.google.devtools.build.lib.analysis.config.InvalidConfigurationException;
 import com.google.devtools.build.lib.buildtool.BuildRequest;
 import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.cmdline.LabelSyntaxException;
@@ -140,7 +139,6 @@ import com.google.devtools.build.lib.vfs.PathFragment;
 import com.google.devtools.build.skyframe.SkyFunction;
 import com.google.devtools.common.options.Options;
 import com.google.devtools.common.options.OptionsParser;
-import com.google.devtools.common.options.OptionsParsingException;
 
 import org.junit.Before;
 
@@ -266,33 +264,34 @@ public abstract class BuildViewTestCase extends FoundationTestCase {
 
   protected final BuildConfigurationCollection createConfigurations(String... args)
       throws Exception {
+    return createConfigurations(false, args);
+  }
+
+  protected final BuildConfigurationCollection createConfigurations(
+      boolean sanityCheck, String... args) throws Exception {
     optionsParser = OptionsParser.newOptionsParser(Iterables.concat(Arrays.asList(
           ExecutionOptions.class,
           BuildRequest.BuildRequestOptions.class),
           ruleClassProvider.getConfigurationOptions()));
-    try {
-      List<String> configurationArgs = new ArrayList<>();
-      // TODO(dmarting): Add --stamp option only to test that requires it.
-      configurationArgs.add("--stamp");  // Stamp is now defaulted to false.
-      configurationArgs.add("--experimental_extended_sanity_checks");
-      configurationArgs.addAll(getAnalysisMock().getOptionOverrides());
-
-      optionsParser.parse(configurationArgs);
-      optionsParser.parse(args);
-
-      InvocationPolicyEnforcer optionsPolicyEnforcer =
-            new InvocationPolicyEnforcer(TestConstants.TEST_INVOCATION_POLICY);
-      optionsPolicyEnforcer.enforce(optionsParser, "");
-
-      configurationFactory.forbidSanityCheck();
-      BuildOptions buildOptions = ruleClassProvider.createBuildOptions(optionsParser);
-      ensureTargetsVisited(buildOptions.getAllLabels().values());
-      skyframeExecutor.invalidateConfigurationCollection();
-      return skyframeExecutor.createConfigurations(reporter, configurationFactory, buildOptions,
-          directories, ImmutableSet.<String>of(), false);
-    } catch (InvalidConfigurationException | OptionsParsingException e) {
-      throw new IllegalArgumentException(e);
-    }
+    List<String> configurationArgs = new ArrayList<>();
+    // TODO(dmarting): Add --stamp option only to test that requires it.
+    configurationArgs.add("--stamp");  // Stamp is now defaulted to false.
+    configurationArgs.add("--experimental_extended_sanity_checks");
+    configurationArgs.addAll(getAnalysisMock().getOptionOverrides());
+
+    optionsParser.parse(configurationArgs);
+    optionsParser.parse(args);
+
+    InvocationPolicyEnforcer optionsPolicyEnforcer =
+          new InvocationPolicyEnforcer(TestConstants.TEST_INVOCATION_POLICY);
+    optionsPolicyEnforcer.enforce(optionsParser, "");
+
+    configurationFactory.setSanityCheck(sanityCheck);
+    BuildOptions buildOptions = ruleClassProvider.createBuildOptions(optionsParser);
+    ensureTargetsVisited(buildOptions.getAllLabels().values());
+    skyframeExecutor.invalidateConfigurationCollection();
+    return skyframeExecutor.createConfigurations(reporter, configurationFactory, buildOptions,
+        directories, ImmutableSet.<String>of(), false);
   }
 
   protected Target getTarget(String label)
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
index 5a2d48c86d..f7c83ff810 100644
--- 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
@@ -162,7 +162,6 @@ public abstract class ConfigurationTestCase extends FoundationTestCase {
     ImmutableSortedSet<String> multiCpu = ImmutableSortedSet.copyOf(
         parser.getOptions(TestOptions.class).multiCpus);
 
-    configurationFactory.forbidSanityCheck();
     BuildOptions buildOptions = BuildOptions.of(buildOptionClasses, parser);
     BuildConfigurationCollection collection = skyframeExecutor.createConfigurations(
         reporter, configurationFactory, buildOptions,
diff --git a/src/test/java/com/google/devtools/build/lib/rules/cpp/CcCommonConfiguredTargetTest.java b/src/test/java/com/google/devtools/build/lib/rules/cpp/CcCommonConfiguredTargetTest.java
index 65e3044aa1..8d9795669b 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/cpp/CcCommonConfiguredTargetTest.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/cpp/CcCommonConfiguredTargetTest.java
@@ -25,16 +25,15 @@ import static org.junit.Assert.fail;
 import com.google.common.base.Predicate;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableSet;
-import com.google.common.eventbus.EventBus;
 import com.google.devtools.build.lib.actions.Action;
 import com.google.devtools.build.lib.actions.Artifact;
 import com.google.devtools.build.lib.actions.util.ActionsTestUtil;
 import com.google.devtools.build.lib.analysis.AnalysisUtils;
 import com.google.devtools.build.lib.analysis.ConfiguredTarget;
 import com.google.devtools.build.lib.analysis.OutputGroupProvider;
+import com.google.devtools.build.lib.analysis.config.InvalidConfigurationException;
 import com.google.devtools.build.lib.analysis.util.BuildViewTestCase;
 import com.google.devtools.build.lib.cmdline.PackageIdentifier;
-import com.google.devtools.build.lib.pkgcache.LoadingFailedException;
 import com.google.devtools.build.lib.testutil.MoreAsserts;
 import com.google.devtools.build.lib.testutil.TestConstants;
 import com.google.devtools.build.lib.util.FileType;
@@ -756,34 +755,31 @@ public class CcCommonConfiguredTargetTest extends BuildViewTestCase {
 
   @Test
   public void testExplicitBadStl() throws Exception {
-    scratch.file("x/BUILD",
-        "cc_binary(name = 'x', srcs = ['x.cc'])");
+    scratch.file("x/BUILD");
 
     reporter.removeHandler(failFastHandler);
     try {
-      useConfiguration("--experimental_stl=//x:blah");
-      update(Arrays.asList("//x:x"), true, 10, false, new EventBus());
+      createConfigurations(true, "--experimental_stl=//x:blah");
       fail("found non-existing target");
-    } catch (LoadingFailedException expected) {
+    } catch (InvalidConfigurationException expected) {
       assertThat(expected.getMessage()).contains("Failed to load required STL target: '//x:blah'");
     }
 
     try {
-      useConfiguration("--experimental_stl=//blah");
-      update(Arrays.asList("//x:x"), true, 10, false, new EventBus());
-      fail("found non-existsing target");
-    } catch (LoadingFailedException expected) {
+      createConfigurations(true, "--experimental_stl=//blah");
+      fail("found non-existing target");
+    } catch (InvalidConfigurationException expected) {
       assertThat(expected.getMessage())
           .contains("Failed to load required STL target: '//blah:blah'");
     }
 
     // Without -k.
     try {
-      useConfiguration("--experimental_stl=//blah");
-      update(Arrays.asList("//x:x"), false, 10, false, new EventBus());
-      fail("found non-existsing target");
-    } catch (LoadingFailedException expected) {
-      assertThat(expected.getMessage()).contains("Loading failed; build aborted");
+      createConfigurations(true, "--experimental_stl=//blah");
+      fail("found non-existing target");
+    } catch (InvalidConfigurationException expected) {
+      assertThat(expected.getMessage())
+          .contains("Failed to load required STL target: '//blah:blah'");
     }
   }
 }