Clean up test tag handling

PiperOrigin-RevId: 186427907
mmikitka · Feb 21, 2018 · 404a670 · 404a670
1 parent f4d2dad
commit 404a670
Show file tree

Hide file tree

Showing 2 changed files with 8 additions and 14 deletions.
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/test/TestTargetProperties.java b/src/main/java/com/google/devtools/build/lib/analysis/test/TestTargetProperties.java
@@ -81,30 +81,24 @@ private static ResourceSet getResourceSetFromSize(TestSize size) {
     size = TestSize.getTestSize(rule);
     timeout = TestTimeout.getTestTimeout(rule);
     tags = ruleContext.attributes().get("tags", Type.STRING_LIST);
-    boolean isTaggedLocal = TargetUtils.isLocalTestRule(rule)
-        || TargetUtils.isExclusiveTestRule(rule);
 
     // We need to use method on ruleConfiguredTarget to perform validation.
     isFlaky = ruleContext.attributes().get("flaky", Type.BOOLEAN);
     isExternal = TargetUtils.isExternalTestRule(rule);
 
     Map<String, String> executionInfo = Maps.newLinkedHashMap();
     executionInfo.putAll(TargetUtils.getExecutionInfo(rule));
-    if (isTaggedLocal) {
-      executionInfo.put("local", "");
+    if (TargetUtils.isLocalTestRule(rule) || TargetUtils.isExclusiveTestRule(rule)) {
+      executionInfo.put(ExecutionRequirements.LOCAL, "");
     }
 
-    boolean isRequestedLocalByProvider = false;
     if (executionRequirements != null) {
       // This will overwrite whatever TargetUtils put there, which might be confusing.
       executionInfo.putAll(executionRequirements.getExecutionInfo());
-
-      // We also need to mark it as local if the execution requirements specifies it.
-      isRequestedLocalByProvider = executionRequirements.getExecutionInfo().containsKey("local");
     }
     this.executionInfo = ImmutableMap.copyOf(executionInfo);
 
-    isLocal = isTaggedLocal || isRequestedLocalByProvider;
+    isLocal = executionInfo.containsKey(ExecutionRequirements.LOCAL);
 
     language = TargetUtils.getRuleLanguage(rule);
   }

diff --git a/src/main/java/com/google/devtools/build/lib/exec/StandaloneTestStrategy.java b/src/main/java/com/google/devtools/build/lib/exec/StandaloneTestStrategy.java
@@ -50,6 +50,7 @@
 import java.io.IOException;
 import java.util.List;
 import java.util.Map;
+import java.util.TreeMap;
 
 /** Runs TestRunnerAction actions. */
 // TODO(bazel-team): add tests for this strategy.
@@ -100,13 +101,12 @@ public List<SpawnResult> exec(
 
     ResolvedPaths resolvedPaths = action.resolve(execRoot);
 
-    ImmutableMap.Builder<String, String> executionInfo = ImmutableMap.builder();
+    Map<String, String> executionInfo =
+        new TreeMap<>(action.getTestProperties().getExecutionInfo());
     if (!action.shouldCacheResult()) {
       executionInfo.put(ExecutionRequirements.NO_CACHE, "");
     }
-    // This key is only understood by StandaloneSpawnStrategy.
-    executionInfo.put("timeout", "" + getTimeout(action).getSeconds());
-    executionInfo.putAll(action.getTestProperties().getExecutionInfo());
+    executionInfo.put(ExecutionRequirements.TIMEOUT, "" + getTimeout(action).getSeconds());
 
     ResourceSet localResourceUsage =
         action
@@ -119,7 +119,7 @@ public List<SpawnResult> exec(
             action,
             getArgs(action),
             ImmutableMap.copyOf(env),
-            executionInfo.build(),
+            ImmutableMap.copyOf(executionInfo),
             new RunfilesSupplierImpl(
                 runfilesDir.relativeTo(execRoot), action.getExecutionSettings().getRunfiles()),
             /*inputs=*/ ImmutableList.copyOf(action.getInputs()),