TMP: Test that all Starlark options are loaded

bazelbuild · Jul 20, 2023 · e55b1ba · e55b1ba
1 parent 6609d6d
commit e55b1ba
Show file tree

Hide file tree

Showing 4 changed files with 12 additions and 10 deletions.
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/BlazeOptionHandler.java b/src/main/java/com/google/devtools/build/lib/runtime/BlazeOptionHandler.java
@@ -17,6 +17,7 @@
 
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Joiner;
+import com.google.common.base.Preconditions;
 import com.google.common.collect.ArrayListMultimap;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableSet;
@@ -418,9 +419,10 @@ DetailedExitCode parseStarlarkOptions(CommandEnvironment env) {
       return DetailedExitCode.success();
     }
     try {
-      StarlarkOptionsParser.newStarlarkOptionsParser(
-              new SkyframeExecutorTargetLoader(env), optionsParser)
-          .parse();
+      Preconditions.checkState(
+          StarlarkOptionsParser.newStarlarkOptionsParser(
+                  new SkyframeExecutorTargetLoader(env), optionsParser)
+              .parse());
     } catch (OptionsParsingException e) {
       String logMessage = "Error parsing Starlark options";
       logger.atInfo().withCause(e).log("%s", logMessage);

diff --git a/src/main/java/com/google/devtools/build/lib/runtime/StarlarkOptionsParser.java b/src/main/java/com/google/devtools/build/lib/runtime/StarlarkOptionsParser.java
@@ -34,7 +34,6 @@
 import com.google.devtools.common.options.Converter;
 import com.google.devtools.common.options.OptionsParser;
 import com.google.devtools.common.options.OptionsParsingException;
-import com.google.errorprone.annotations.CanIgnoreReturnValue;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.HashMap;
@@ -103,7 +102,6 @@ public static StarlarkOptionsParser newStarlarkOptionsParser(
   // OptionsParserImpl.identifyOptionAndPossibleArgument. Consider combining. This would probably
   // require multiple rounds of parsing to fit starlark-defined options into native option format.
   @VisibleForTesting
-  @CanIgnoreReturnValue
   public boolean parse() throws OptionsParsingException {
     return parseGivenArgs(nativeOptionsParser.getSkippedArgs());
   }
@@ -115,7 +113,6 @@ public boolean parse() throws OptionsParsingException {
    *     work to retrieve build setting targets (after which it'll call this method again)
    */
   @VisibleForTesting
-  @CanIgnoreReturnValue
   public boolean parseGivenArgs(List<String> args) throws OptionsParsingException {
     // Map of <option name (label), <unparsed option value, loaded option>>.
     Multimap<String, Pair<String, Target>> unparsedOptions = LinkedListMultimap.create();

diff --git a/src/main/java/com/google/devtools/build/lib/runtime/commands/CanonicalizeCommand.java b/src/main/java/com/google/devtools/build/lib/runtime/commands/CanonicalizeCommand.java
@@ -16,6 +16,7 @@
 import static com.google.devtools.common.options.Converters.BLAZE_ALIASING_FLAG;
 
 import com.google.common.base.Joiner;
+import com.google.common.base.Preconditions;
 import com.google.common.collect.ImmutableList;
 import com.google.devtools.build.lib.cmdline.RepositoryMapping;
 import com.google.devtools.build.lib.events.Event;
@@ -200,7 +201,7 @@ public BlazeCommandResult exec(CommandEnvironment env, OptionsParsingResult opti
         StarlarkOptionsParser.newStarlarkOptionsParser(
             new SkyframeExecutorTargetLoader(env), parser);
     try {
-      starlarkOptionsParser.parse();
+      Preconditions.checkState(starlarkOptionsParser.parse());
     } catch (OptionsParsingException e) {
       return reportAndCreateCommandFailure(
           env, e.getMessage(), FailureDetails.Command.Code.STARLARK_OPTIONS_PARSE_FAILURE);

diff --git a/src/test/java/com/google/devtools/build/lib/starlark/util/StarlarkOptionsTestCase.java b/src/test/java/com/google/devtools/build/lib/starlark/util/StarlarkOptionsTestCase.java
@@ -14,6 +14,8 @@
 
 package com.google.devtools.build.lib.starlark.util;
 
+import static com.google.common.truth.Truth.assertThat;
+
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.Iterables;
 import com.google.devtools.build.lib.analysis.util.BuildViewTestCase;
@@ -76,7 +78,7 @@ protected OptionsParsingResult parseStarlarkOptions(String options, boolean only
     if (!onlyStarlarkParser) {
       optionsParser.parse(asList);
     }
-    starlarkOptionsParser.parseGivenArgs(asList);
+    assertThat(starlarkOptionsParser.parseGivenArgs(asList)).isTrue();
     return starlarkOptionsParser.getNativeOptionsParserFortesting();
   }
 
@@ -86,11 +88,11 @@ protected OptionsParsingResult parseStarlarkOptions(
     List<String> bazelrcOptionsList = Arrays.asList(bazelrcOptions.split(" "));
     optionsParser.parse(PriorityCategory.COMMAND_LINE, /* source= */ null, commandLineOptionsList);
     optionsParser.parse(PriorityCategory.RC_FILE, "fake.bazelrc", bazelrcOptionsList);
-    starlarkOptionsParser.parseGivenArgs(
+    assertThat(starlarkOptionsParser.parseGivenArgs(
         ImmutableList.<String>builder()
             .addAll(commandLineOptionsList)
             .addAll(bazelrcOptionsList)
-            .build());
+            .build())).isTrue();
     return starlarkOptionsParser.getNativeOptionsParserFortesting();
   }