Skip to content

Commit

Permalink
Only try to create groups of test actions in the ui.
Browse files Browse the repository at this point in the history
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
  • Loading branch information
benjaminp authored and copybara-github committed Jan 23, 2023
1 parent 761b280 commit fcb0077
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 9 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand All @@ -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(
Expand All @@ -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(
Expand All @@ -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...
Expand All @@ -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);
Expand All @@ -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
Expand Down

0 comments on commit fcb0077

Please sign in to comment.