Only try to create groups of test actions in the ui.

benjaminp · copybara-github · commit fcb007749f7f · 2023-01-23T05:46:28.000-08:00
A target with test actions may have other actions registered that should not be reported in the UI as tests themselves. Fixes bazelbuild#16174. Closes bazelbuild#16176. PiperOrigin-RevId: 503961303 Change-Id: I7df82a6f7c01532cd8f2feac50078daf599fc56a
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/UiStateTracker.java b/src/main/java/com/google/devtools/build/lib/runtime/UiStateTracker.java
@@ -530,7 +530,7 @@ void actionStarted(ActionStartedEvent event) {
 
     getActionState(action, actionId, event.getNanoTimeStart());
 
-    if (action.getOwner() != null) {
+    if (action.getOwner() != null && action.getOwner().getMnemonic().equals("TestRunner")) {
       Label owner = action.getOwner().getLabel();
       if (owner != null) {
         Set<Artifact> testActionsForOwner = testActions.get(owner);
@@ -604,7 +604,7 @@ void actionCompletion(ActionCompletionEvent event) {
 
     checkNotNull(activeActions.remove(actionId), "%s not active after %s", actionId, event);
 
-    if (action.getOwner() != null) {
+    if (action.getOwner() != null && action.getOwner().getMnemonic().equals("TestRunner")) {
       Label owner = action.getOwner().getLabel();
       if (owner != null) {
         Set<Artifact> testActionsForOwner = testActions.get(owner);
@@ -750,7 +750,7 @@ private String describeActionProgress(ActionState action, int desiredWidth) {
   protected String describeAction(
       ActionState actionState, long nanoTime, int desiredWidth, Set<Artifact> toSkip) {
     ActionExecutionMetadata action = actionState.action;
-    if (action.getOwner() != null) {
+    if (action.getOwner() != null && action.getOwner().getMnemonic().equals("TestRunner")) {
       Label owner = action.getOwner().getLabel();
       if (owner != null) {
         Set<Artifact> allRelatedActions = testActions.get(owner);
diff --git a/src/test/java/com/google/devtools/build/lib/runtime/UiStateTrackerTest.java b/src/test/java/com/google/devtools/build/lib/runtime/UiStateTrackerTest.java
@@ -1174,6 +1174,7 @@ public void testAggregation() throws Exception {
     ManualClock clock = new ManualClock();
     clock.advanceMillis(TimeUnit.SECONDS.toMillis(1234));
     UiStateTracker stateTracker = getUiStateTracker(clock, /*targetWidth=*/ 80);
+    stateTracker.setProgressSampleSize(4);
     // Mimic being at the execution phase.
     simulateExecutionPhase(stateTracker);
 
@@ -1185,7 +1186,7 @@ public void testAggregation() throws Exception {
             labelFooTest,
             ImmutableList.of(),
             new Location("dummy-file", 0, 0),
-            "dummy-mnemonic",
+            "TestRunner",
             "dummy-target-kind",
             "abcdef",
             new BuildConfigurationEvent(
@@ -1198,15 +1199,12 @@ public void testAggregation() throws Exception {
     Label labelBarTest = Label.parseCanonical("//baz:bartest");
     ConfiguredTarget targetBarTest = mock(ConfiguredTarget.class);
     when(targetBarTest.getLabel()).thenReturn(labelBarTest);
-    TestFilteringCompleteEvent filteringComplete = mock(TestFilteringCompleteEvent.class);
-    when(filteringComplete.getTestTargets())
-        .thenReturn(ImmutableSet.of(targetFooTest, targetBarTest));
     ActionOwner barOwner =
         ActionOwner.create(
             labelBarTest,
             ImmutableList.of(),
             new Location("dummy-file", 0, 0),
-            "dummy-mnemonic",
+            "TestRunner",
             "dummy-target-kind",
             "fedcba",
             new BuildConfigurationEvent(
@@ -1216,6 +1214,27 @@ public void testAggregation() throws Exception {
             ImmutableMap.of(),
             null);
 
+    Label labelBazTest = Label.parseCanonical("//baz:baztest");
+    ConfiguredTarget targetBazTest = mock(ConfiguredTarget.class);
+    when(targetBazTest.getLabel()).thenReturn(labelBazTest);
+    ActionOwner bazOwner =
+        ActionOwner.create(
+            labelBazTest,
+            ImmutableList.of(),
+            new Location("dummy-file", 0, 0),
+            "NonTestAction",
+            "dummy-target-kind",
+            "fedcba",
+            new BuildConfigurationEvent(
+                BuildEventStreamProtos.BuildEventId.getDefaultInstance(),
+                BuildEventStreamProtos.BuildEvent.getDefaultInstance()),
+            null,
+            ImmutableMap.of(),
+            null);
+
+    TestFilteringCompleteEvent filteringComplete = mock(TestFilteringCompleteEvent.class);
+    when(filteringComplete.getTestTargets())
+        .thenReturn(ImmutableSet.of(targetFooTest, targetBarTest, targetBazTest));
     stateTracker.testFilteringComplete(filteringComplete);
 
     // First produce 10 actions for footest...
@@ -1232,10 +1251,17 @@ public void testAggregation() throws Exception {
       when(action.getOwner()).thenReturn(barOwner);
       stateTracker.actionStarted(new ActionStartedEvent(action, clock.nanoTime()));
     }
-    // ...and finally a completely unrelated action
+    // ...run a completely unrelated action..
     clock.advanceMillis(TimeUnit.SECONDS.toMillis(1));
     stateTracker.actionStarted(
         new ActionStartedEvent(mockAction("Other action", "other/action"), clock.nanoTime()));
+    // ...and finally, run actions that are associated with baztest but are not a test.
+    for (int i = 0; i < 10; i++) {
+      clock.advanceMillis(1_000);
+      Action action = mockAction("Doing something " + i, "someartifact_" + i);
+      when(action.getOwner()).thenReturn(bazOwner);
+      stateTracker.actionStarted(new ActionStartedEvent(action, clock.nanoTime()));
+    }
     clock.advanceMillis(TimeUnit.SECONDS.toMillis(1));
 
     LoggingTerminalWriter terminalWriter = new LoggingTerminalWriter(/*discardHighlight=*/ true);
@@ -1251,6 +1277,8 @@ public void testAggregation() throws Exception {
     assertWithMessage("Progress bar should contain 'Other action', but was:\n" + output)
         .that(output.contains("Other action"))
         .isTrue();
+    assertThat(output).doesNotContain("Testing //baz:baztest");
+    assertThat(output).contains("Doing something");
   }
 
   @Test
