From fcb007749f7f24b36c2b7c4284378bba20fc8b69 Mon Sep 17 00:00:00 2001 From: Benjamin Peterson Date: Mon, 23 Jan 2023 05:44:57 -0800 Subject: [PATCH] Only try to create groups of test actions in the ui. A target with test actions may have other actions registered that should not be reported in the UI as tests themselves. Fixes https://github.com/bazelbuild/bazel/issues/16174. Closes #16176. PiperOrigin-RevId: 503961303 Change-Id: I7df82a6f7c01532cd8f2feac50078daf599fc56a --- .../build/lib/runtime/UiStateTracker.java | 6 +-- .../build/lib/runtime/UiStateTrackerTest.java | 40 ++++++++++++++++--- 2 files changed, 37 insertions(+), 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 index 050eac447ebc8a..f82d7c9b12623d 100644 --- 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 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 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 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 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 index eb3a5b58408044..65f68c88188f42 100644 --- 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