Only try to create groups of test actions in the ui. (#17343)

A target with test actions may have other actions registered that should not be reported in the UI as tests themselves. Fixes #16174. Closes #16176. PiperOrigin-RevId: 503961303 Change-Id: I7df82a6f7c01532cd8f2feac50078daf599fc56a Co-authored-by: Benjamin Peterson <[email protected]>
bazelbuild · Jan 27, 2023 · 625128c · 625128c
1 parent baf97c0
commit 625128c
Show file tree

Hide file tree

Showing 2 changed files with 37 additions and 9 deletions.
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
@@ -525,7 +525,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);
@@ -599,7 +599,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);
@@ -745,7 +745,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
@@ -1159,6 +1159,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);
 
@@ -1170,7 +1171,7 @@ public void testAggregation() throws Exception {
             labelFooTest,
             ImmutableList.of(),
             new Location("dummy-file", 0, 0),
-            "dummy-mnemonic",
+            "TestRunner",
             "dummy-target-kind",
             "abcdef",
             new BuildConfigurationEvent(
@@ -1183,15 +1184,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(
@@ -1201,6 +1199,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...
@@ -1217,10 +1236,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);
@@ -1236,6 +1262,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