Thread configuration map through TestFilteringCompleteEvent. This isn't the most elegant solution, but I don't have a better idea.

PiperOrigin-RevId: 190656869

7 files changed, 75 insertions, 9 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/buildtool/BuildTool.java b/src/main/java/com/google/devtools/build/lib/buildtool/BuildTool.java
index 00138b3125..ed9024a4d0 100644
--- a/src/main/java/com/google/devtools/build/lib/buildtool/BuildTool.java
+++ b/src/main/java/com/google/devtools/build/lib/buildtool/BuildTool.java
@@ -18,6 +18,7 @@ import com.google.common.base.Preconditions;
 import com.google.common.base.Stopwatch;
 import com.google.common.base.Throwables;
 import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableSet;
 import com.google.devtools.build.lib.actions.BuildFailedException;
 import com.google.devtools.build.lib.actions.TestExecException;
 import com.google.devtools.build.lib.analysis.AnalysisPhaseCompleteEvent;
@@ -66,15 +67,19 @@ import com.google.devtools.build.lib.profiler.ProfilePhase;
 import com.google.devtools.build.lib.profiler.Profiler;
 import com.google.devtools.build.lib.runtime.BlazeRuntime;
 import com.google.devtools.build.lib.runtime.CommandEnvironment;
+import com.google.devtools.build.lib.skyframe.BuildConfigurationValue;
 import com.google.devtools.build.lib.util.AbruptExitException;
 import com.google.devtools.build.lib.util.ExitCode;
 import com.google.devtools.build.lib.util.RegexFilter;
 import com.google.devtools.common.options.OptionsParsingException;
 import java.util.Collection;
+import java.util.Map;
+import java.util.Objects;
 import java.util.Set;
 import java.util.concurrent.TimeUnit;
 import java.util.logging.Logger;
 import java.util.regex.Pattern;
+import java.util.stream.Stream;
 
 /**
  * Provides the bulk of the implementation of the 'blaze build' command.
@@ -489,8 +494,31 @@ public class BuildTool {
     env.getEventBus().post(new AnalysisPhaseCompleteEvent(analysisResult.getTargetsToBuild(),
         view.getTargetsVisited(), timer.stop().elapsed(TimeUnit.MILLISECONDS),
         view.getAndClearPkgManagerStatistics()));
-    env.getEventBus().post(new TestFilteringCompleteEvent(analysisResult.getTargetsToBuild(),
-        analysisResult.getTargetsToTest()));
+    ImmutableSet<BuildConfigurationValue.Key> configurationKeys =
+        Stream.concat(
+                analysisResult
+                    .getTargetsToBuild()
+                    .stream()
+                    .map(ConfiguredTarget::getConfigurationKey)
+                    .distinct(),
+                analysisResult.getTargetsToTest() == null
+                    ? Stream.empty()
+                    : analysisResult
+                        .getTargetsToTest()
+                        .stream()
+                        .map(ConfiguredTarget::getConfigurationKey)
+                        .distinct())
+            .filter(Objects::nonNull)
+            .distinct()
+            .collect(ImmutableSet.toImmutableSet());
+    Map<BuildConfigurationValue.Key, BuildConfiguration> configurationMap =
+        env.getSkyframeExecutor().getConfigurations(env.getReporter(), configurationKeys);
+    env.getEventBus()
+        .post(
+            new TestFilteringCompleteEvent(
+                analysisResult.getTargetsToBuild(),
+                analysisResult.getTargetsToTest(),
+                configurationMap));
 
     // Check licenses.
     // We check licenses if the first target configuration has license checking enabled. Right now,
diff --git a/src/main/java/com/google/devtools/build/lib/buildtool/buildevent/TestFilteringCompleteEvent.java b/src/main/java/com/google/devtools/build/lib/buildtool/buildevent/TestFilteringCompleteEvent.java
index b205763cf7..d8e8876493 100644
--- a/src/main/java/com/google/devtools/build/lib/buildtool/buildevent/TestFilteringCompleteEvent.java
+++ b/src/main/java/com/google/devtools/build/lib/buildtool/buildevent/TestFilteringCompleteEvent.java
@@ -17,8 +17,11 @@ package com.google.devtools.build.lib.buildtool.buildevent;
 import com.google.common.base.Preconditions;
 import com.google.common.collect.ImmutableList;
 import com.google.devtools.build.lib.analysis.ConfiguredTarget;
+import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
 import com.google.devtools.build.lib.analysis.test.TestProvider;
+import com.google.devtools.build.lib.skyframe.BuildConfigurationValue;
 import java.util.Collection;
+import java.util.Map;
 import javax.annotation.concurrent.Immutable;
 
 /**
@@ -31,17 +34,22 @@ import javax.annotation.concurrent.Immutable;
 public class TestFilteringCompleteEvent {
   private final Collection<ConfiguredTarget> targets;
   private final Collection<ConfiguredTarget> testTargets;
+  private final Map<BuildConfigurationValue.Key, BuildConfiguration> configurationMap;
 
   /**
    * Construct the event.
+   *
    * @param targets The set of active targets that remain.
    * @param testTargets The collection of tests to be run. May be null.
+   * @param configurationMap A map from configuration keys of all targets to the configurations.
    */
   public TestFilteringCompleteEvent(
       Collection<? extends ConfiguredTarget> targets,
-      Collection<? extends ConfiguredTarget> testTargets) {
+      Collection<? extends ConfiguredTarget> testTargets,
+      Map<BuildConfigurationValue.Key, BuildConfiguration> configurationMap) {
     this.targets = ImmutableList.copyOf(targets);
     this.testTargets = testTargets == null ? null : ImmutableList.copyOf(testTargets);
+    this.configurationMap = configurationMap;
     if (testTargets == null) {
       return;
     }
@@ -65,4 +73,8 @@ public class TestFilteringCompleteEvent {
   public Collection<ConfiguredTarget> getTestTargets() {
     return testTargets;
   }
+
+  public BuildConfiguration getConfigurationForTarget(ConfiguredTarget target) {
+    return Preconditions.checkNotNull(configurationMap.get(target.getConfigurationKey()));
+  }
 }
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/AggregatingTestListener.java b/src/main/java/com/google/devtools/build/lib/runtime/AggregatingTestListener.java
index 28cc7d6130..8b021c522e 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/AggregatingTestListener.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/AggregatingTestListener.java
@@ -100,9 +100,11 @@ public class AggregatingTestListener {
 
         // And create an empty summary suitable for incremental analysis.
         // Also has the nice side effect of mapping labels to RuleConfiguredTargets.
-        TestSummary.Builder summary = TestSummary.newBuilder()
-            .setTarget(target)
-            .setStatus(BlazeTestStatus.NO_STATUS);
+        TestSummary.Builder summary =
+            TestSummary.newBuilder()
+                .setTarget(target)
+                .setConfiguration(event.getConfigurationForTarget(target))
+                .setStatus(BlazeTestStatus.NO_STATUS);
         TestSummary.Builder oldSummary = summaries.put(asKey(target), summary);
         Preconditions.checkState(
             oldSummary == null, "target: %s, summaries: %s %s", target, oldSummary, summary);
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/TestSummary.java b/src/main/java/com/google/devtools/build/lib/runtime/TestSummary.java
index cb87802faa..903916eeba 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/TestSummary.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/TestSummary.java
@@ -22,6 +22,7 @@ import com.google.common.collect.Multimap;
 import com.google.common.collect.MultimapBuilder;
 import com.google.devtools.build.lib.analysis.AliasProvider;
 import com.google.devtools.build.lib.analysis.ConfiguredTarget;
+import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
 import com.google.devtools.build.lib.buildeventstream.BuildEventConverters;
 import com.google.devtools.build.lib.buildeventstream.BuildEventId;
 import com.google.devtools.build.lib.buildeventstream.BuildEventStreamProtos;
@@ -67,6 +68,7 @@ public class TestSummary implements Comparable<TestSummary>, BuildEventWithOrder
       summary.shardRunStatuses =
           MultimapBuilder.hashKeys().arrayListValues().build(existingSummary.shardRunStatuses);
       setTarget(existingSummary.target);
+      setConfiguration(existingSummary.configuration);
       setStatus(existingSummary.status);
       addCoverageFiles(existingSummary.coverageFiles);
       addPassedLogs(existingSummary.passedLogs);
@@ -110,6 +112,12 @@ public class TestSummary implements Comparable<TestSummary>, BuildEventWithOrder
       return this;
     }
 
+    public Builder setConfiguration(BuildConfiguration configuration) {
+      checkMutation(configuration);
+      summary.configuration = Preconditions.checkNotNull(configuration, summary);
+      return this;
+    }
+
     public Builder setStatus(BlazeTestStatus status) {
       checkMutation(status);
       summary.status = status;
@@ -299,6 +307,7 @@ public class TestSummary implements Comparable<TestSummary>, BuildEventWithOrder
   }
 
   private ConfiguredTarget target;
+  private BuildConfiguration configuration;
   private BlazeTestStatus status;
   // Currently only populated if --runs_per_test_detects_flakes is enabled.
   private Multimap<Integer, BlazeTestStatus> shardRunStatuses = ArrayListMultimap.create();
@@ -343,6 +352,10 @@ public class TestSummary implements Comparable<TestSummary>, BuildEventWithOrder
     return target;
   }
 
+  public BuildConfiguration getConfiguration() {
+    return configuration;
+  }
+
   public BlazeTestStatus getStatus() {
     return status;
   }
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/TestSummaryPrinter.java b/src/main/java/com/google/devtools/build/lib/runtime/TestSummaryPrinter.java
index 486b1505d9..b1371fe0b1 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/TestSummaryPrinter.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/TestSummaryPrinter.java
@@ -113,7 +113,7 @@ public class TestSummaryPrinter {
     String message = getCacheMessage(summary) + statusString(summary.getStatus());
     String targetName = summary.getLabel().toString();
     if (withConfigurationName) {
-      targetName += " (" + summary.getTarget().getConfiguration().getMnemonic() + ")";
+      targetName += " (" + summary.getConfiguration().getMnemonic() + ")";
     }
     terminalPrinter.print(
         Strings.padEnd(targetName, 78 - message.length(), ' ')
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 3e9c1f1c2f..cd5b827773 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
@@ -177,6 +177,7 @@ import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.concurrent.atomic.AtomicInteger;
 import java.util.concurrent.atomic.AtomicReference;
 import java.util.logging.Logger;
+import java.util.stream.Collectors;
 import javax.annotation.Nullable;
 
 /**
@@ -1494,6 +1495,15 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory {
         .getConfiguration();
   }
 
+  public Map<BuildConfigurationValue.Key, BuildConfiguration> getConfigurations(
+      ExtendedEventHandler eventHandler, ImmutableSet<BuildConfigurationValue.Key> keys) {
+    EvaluationResult<SkyValue> evaluationResult = evaluateSkyKeys(eventHandler, keys);
+    return keys.stream()
+        .collect(
+            Collectors.toMap(
+                java.util.function.Function.identity(),
+                (key) -> ((BuildConfigurationValue) evaluationResult.get(key)).getConfiguration()));
+  }
   /**
    * Returns the configurations corresponding to the given sets of build options. Output order is
    * the same as input order.
@@ -1665,7 +1675,7 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory {
    * first evaluation error.
    */
   private EvaluationResult<SkyValue> evaluateSkyKeys(
-      final ExtendedEventHandler eventHandler, final Iterable<SkyKey> skyKeys) {
+      final ExtendedEventHandler eventHandler, final Iterable<? extends SkyKey> skyKeys) {
     return evaluateSkyKeys(eventHandler, skyKeys, false);
   }
 
@@ -1675,7 +1685,7 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory {
    */
   EvaluationResult<SkyValue> evaluateSkyKeys(
       final ExtendedEventHandler eventHandler,
-      final Iterable<SkyKey> skyKeys,
+      final Iterable<? extends SkyKey> skyKeys,
       final boolean keepGoing) {
     EvaluationResult<SkyValue> result;
     try {
diff --git a/src/test/java/com/google/devtools/build/lib/runtime/TestSummaryTest.java b/src/test/java/com/google/devtools/build/lib/runtime/TestSummaryTest.java
index e3ff952481..05d16a34b5 100644
--- a/src/test/java/com/google/devtools/build/lib/runtime/TestSummaryTest.java
+++ b/src/test/java/com/google/devtools/build/lib/runtime/TestSummaryTest.java
@@ -71,6 +71,7 @@ public class TestSummaryTest {
   private TestSummary.Builder getTemplateBuilder() {
     return TestSummary.newBuilder()
         .setTarget(stubTarget)
+        .setConfiguration(stubTarget.getConfiguration())
         .setStatus(BlazeTestStatus.PASSED)
         .setNumCached(NOT_CACHED)
         .setActionRan(true)