diff options
10 files changed, 46 insertions, 31 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java b/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java index 8fea672387..75115112ef 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java @@ -105,7 +105,7 @@ import javax.annotation.Nullable; category = SkylarkModuleCategory.BUILTIN, doc = "Data required for the analysis of a target that comes from targets that " + "depend on it and not targets that it depends on.") -public final class BuildConfiguration implements BuildEvent { +public class BuildConfiguration implements BuildEvent { /** * An interface for language-specific configurations. * @@ -1959,7 +1959,7 @@ public final class BuildConfiguration implements BuildEvent { } /** Returns the cache key of the build options used to create this configuration. */ - public final String checksum() { + public String checksum() { return checksum; } 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 5628035543..11b6e22377 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 @@ -26,6 +26,7 @@ import com.google.common.eventbus.EventBus; import com.google.common.eventbus.Subscribe; import com.google.devtools.build.lib.actions.ActionOwner; import com.google.devtools.build.lib.actions.Artifact; +import com.google.devtools.build.lib.analysis.AliasProvider; import com.google.devtools.build.lib.analysis.AnalysisFailureEvent; import com.google.devtools.build.lib.analysis.ConfiguredTarget; import com.google.devtools.build.lib.analysis.LabelAndConfiguration; @@ -261,6 +262,7 @@ public class AggregatingTestListener { } private LabelAndConfiguration asKey(ConfiguredTarget target) { - return LabelAndConfiguration.of(target); + return LabelAndConfiguration.of( + AliasProvider.getDependencyLabel(target), target.getConfiguration()); } } diff --git a/src/main/java/com/google/devtools/build/lib/runtime/ExperimentalEventHandler.java b/src/main/java/com/google/devtools/build/lib/runtime/ExperimentalEventHandler.java index 7b243546b4..1e32df901e 100644 --- a/src/main/java/com/google/devtools/build/lib/runtime/ExperimentalEventHandler.java +++ b/src/main/java/com/google/devtools/build/lib/runtime/ExperimentalEventHandler.java @@ -591,7 +591,7 @@ public class ExperimentalEventHandler implements EventHandler { setEventKindColor(EventKind.ERROR); terminal.writeString("" + summary.getStatus() + ": "); terminal.resetTerminal(); - terminal.writeString(summary.getTarget().getLabel().toString()); + terminal.writeString(summary.getLabel().toString()); terminal.writeString(" (Summary)"); crlf(); for (Path logPath : summary.getFailedLogs()) { diff --git a/src/main/java/com/google/devtools/build/lib/runtime/ExperimentalStateTracker.java b/src/main/java/com/google/devtools/build/lib/runtime/ExperimentalStateTracker.java index fb63f7ee2b..d03805dbaa 100644 --- a/src/main/java/com/google/devtools/build/lib/runtime/ExperimentalStateTracker.java +++ b/src/main/java/com/google/devtools/build/lib/runtime/ExperimentalStateTracker.java @@ -549,8 +549,8 @@ class ExperimentalStateTracker { } else { terminalWriter.failStatus(); } - terminalWriter.append( - shortenedLabelString(mostRecentTest.getTarget().getLabel(), width - prefix.length())); + terminalWriter.append(shortenedLabelString( + mostRecentTest.getLabel(), width - prefix.length())); terminalWriter.normal(); } return true; diff --git a/src/main/java/com/google/devtools/build/lib/runtime/TerminalTestResultNotifier.java b/src/main/java/com/google/devtools/build/lib/runtime/TerminalTestResultNotifier.java index af05b032cc..4cf3567fc5 100644 --- a/src/main/java/com/google/devtools/build/lib/runtime/TerminalTestResultNotifier.java +++ b/src/main/java/com/google/devtools/build/lib/runtime/TerminalTestResultNotifier.java @@ -96,10 +96,10 @@ public class TerminalTestResultNotifier implements TestResultNotifier { private boolean duplicateLabels(Set<TestSummary> summaries) { Set<Label> labelsSeen = new HashSet<>(); for (TestSummary summary : summaries) { - if (labelsSeen.contains(summary.getTarget().getLabel())) { + if (labelsSeen.contains(summary.getLabel())) { return true; } - labelsSeen.add(summary.getTarget().getLabel()); + labelsSeen.add(summary.getLabel()); } return false; } diff --git a/src/main/java/com/google/devtools/build/lib/runtime/TestResultAnalyzer.java b/src/main/java/com/google/devtools/build/lib/runtime/TestResultAnalyzer.java index 1daec5e5cc..dd7e79d197 100644 --- a/src/main/java/com/google/devtools/build/lib/runtime/TestResultAnalyzer.java +++ b/src/main/java/com/google/devtools/build/lib/runtime/TestResultAnalyzer.java @@ -17,6 +17,7 @@ import com.google.common.collect.Sets; import com.google.common.eventbus.EventBus; import com.google.common.eventbus.Subscribe; import com.google.devtools.build.lib.actions.Artifact; +import com.google.devtools.build.lib.analysis.AliasProvider; import com.google.devtools.build.lib.analysis.ConfiguredTarget; import com.google.devtools.build.lib.analysis.TransitiveInfoCollection; import com.google.devtools.build.lib.analysis.test.TestProvider; @@ -143,7 +144,7 @@ public class TestResultAnalyzer { // If already reported by the listener, no work remains for this target. TestSummary.Builder summary = listener.getCurrentSummary(testTarget); - Label testLabel = testTarget.getLabel(); + Label testLabel = AliasProvider.getDependencyLabel(testTarget); Preconditions.checkNotNull(summary, "%s did not complete test filtering, but has a test result", testLabel); if (listener.targetReported(testTarget)) { @@ -200,10 +201,6 @@ public class TestResultAnalyzer { Preconditions.checkNotNull(summaryBuilder); TestSummary existingSummary = Preconditions.checkNotNull(summaryBuilder.peek()); - TransitiveInfoCollection target = existingSummary.getTarget(); - Preconditions.checkNotNull( - target, "The existing TestSummary must be associated with a target"); - BlazeTestStatus status = existingSummary.getStatus(); int numCached = existingSummary.numCached(); int numLocalActionCached = existingSummary.numLocalActionCached(); @@ -224,6 +221,9 @@ public class TestResultAnalyzer { summaryBuilder.addCoverageFiles(Collections.singletonList(coverageData)); } + TransitiveInfoCollection target = existingSummary.getTarget(); + Preconditions.checkNotNull(target, "The existing TestSummary must be associated with a target"); + if (!executionOptions.runsPerTestDetectsFlakes) { status = aggregateStatus(status, result.getData().getStatus()); } else { @@ -342,7 +342,7 @@ public class TestResultAnalyzer { StringBuilder builder = new StringBuilder(String.format( "%s: Test execution time (%.1fs excluding execution overhead) outside of " + "range for %s tests. Consider setting timeout=\"%s\"", - target.getLabel(), + AliasProvider.getDependencyLabel(target), maxTimeOfShard / 1000.0, specifiedTimeout.prettyPrint(), expectedTimeout)); 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 6f6900c961..da8c23ff71 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 @@ -15,18 +15,19 @@ package com.google.devtools.build.lib.runtime; import com.google.common.annotations.VisibleForTesting; import com.google.common.collect.ArrayListMultimap; +import com.google.common.collect.ComparisonChain; import com.google.common.collect.ImmutableList; import com.google.common.collect.Multimap; import com.google.common.collect.MultimapBuilder; -import com.google.devtools.build.lib.actions.Artifact; +import com.google.devtools.build.lib.analysis.AliasProvider; import com.google.devtools.build.lib.analysis.ConfiguredTarget; -import com.google.devtools.build.lib.analysis.FilesToRunProvider; import com.google.devtools.build.lib.buildeventstream.BuildEvent; import com.google.devtools.build.lib.buildeventstream.BuildEventConverters; import com.google.devtools.build.lib.buildeventstream.BuildEventId; import com.google.devtools.build.lib.buildeventstream.BuildEventStreamProtos; import com.google.devtools.build.lib.buildeventstream.GenericBuildEvent; import com.google.devtools.build.lib.buildeventstream.PathConverter; +import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.util.Preconditions; import com.google.devtools.build.lib.util.io.AnsiTerminalPrinter.Mode; import com.google.devtools.build.lib.vfs.Path; @@ -334,6 +335,10 @@ public class TestSummary implements Comparable<TestSummary>, BuildEvent { return builder; } + public Label getLabel() { + return AliasProvider.getDependencyLabel(target); + } + public ConfiguredTarget getTarget() { return target; } @@ -421,17 +426,15 @@ public class TestSummary implements Comparable<TestSummary>, BuildEvent { @Override public int compareTo(TestSummary that) { - if (this.isCached() != that.isCached()) { - return this.isCached() ? -1 : 1; - } else if ((this.isCached() && that.isCached()) && (this.numUncached() != that.numUncached())) { - return this.numUncached() - that.numUncached(); - } else if (this.status != that.status) { - return getSortKey(this.status) - getSortKey(that.status); - } else { - Artifact thisExecutable = this.target.getProvider(FilesToRunProvider.class).getExecutable(); - Artifact thatExecutable = that.target.getProvider(FilesToRunProvider.class).getExecutable(); - return thisExecutable.getPath().compareTo(thatExecutable.getPath()); - } + return ComparisonChain.start() + .compareTrueFirst(this.isCached(), that.isCached()) + .compare(this.numUncached(), that.numUncached()) + .compare(getSortKey(this.status), getSortKey(that.status)) + .compare(this.getLabel(), that.getLabel()) + .compare( + this.getTarget().getConfiguration().checksum(), + that.getTarget().getConfiguration().checksum()) + .result(); } public List<Long> getTestTimes() { @@ -456,7 +459,7 @@ public class TestSummary implements Comparable<TestSummary>, BuildEvent { @Override public BuildEventId getEventId() { return BuildEventId.testSummary( - target.getTarget().getLabel(), target.getConfiguration().getEventId()); + AliasProvider.getDependencyLabel(target), target.getConfiguration().getEventId()); } @Override 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 694eda641f..486b1505d9 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 @@ -42,7 +42,7 @@ public class TestSummaryPrinter { TestOutputFormat testOutput, AnsiTerminalPrinter printer) { - String testName = summary.getTarget().getLabel().toString(); + String testName = summary.getLabel().toString(); List<String> allLogs = new ArrayList<>(); for (Path path : summary.getFailedLogs()) { allLogs.add(path.getPathString()); @@ -111,7 +111,7 @@ public class TestSummaryPrinter { return; } String message = getCacheMessage(summary) + statusString(summary.getStatus()); - String targetName = summary.getTarget().getLabel().toString(); + String targetName = summary.getLabel().toString(); if (withConfigurationName) { targetName += " (" + summary.getTarget().getConfiguration().getMnemonic() + ")"; } @@ -240,7 +240,9 @@ public class TestSummaryPrinter { } else { // We previously used com.google.math for this, which added about 1 MB of deps to the total // size. If we re-introduce a dependency on that package, we could revert this change. - long min = summary.getTestTimes().get(0).longValue(), max = min, sum = 0; + long min = summary.getTestTimes().get(0).longValue(); + long max = min; + long sum = 0; double sumOfSquares = 0.0; for (Long l : summary.getTestTimes()) { long value = l.longValue(); diff --git a/src/test/java/com/google/devtools/build/lib/runtime/ExperimentalStateTrackerTest.java b/src/test/java/com/google/devtools/build/lib/runtime/ExperimentalStateTrackerTest.java index 09b5dda3fb..d827f64b4c 100644 --- a/src/test/java/com/google/devtools/build/lib/runtime/ExperimentalStateTrackerTest.java +++ b/src/test/java/com/google/devtools/build/lib/runtime/ExperimentalStateTrackerTest.java @@ -341,6 +341,7 @@ public class ExperimentalStateTrackerTest extends FoundationTestCase { when(filteringComplete.getTestTargets()).thenReturn(ImmutableSet.of(targetA, targetB)); TestSummary testSummary = Mockito.mock(TestSummary.class); when(testSummary.getTarget()).thenReturn(targetA); + when(testSummary.getLabel()).thenReturn(labelA); stateTracker.testFilteringComplete(filteringComplete); stateTracker.testSummary(testSummary); @@ -374,6 +375,7 @@ public class ExperimentalStateTrackerTest extends FoundationTestCase { TestSummary testSummary = Mockito.mock(TestSummary.class); when(testSummary.getStatus()).thenReturn(BlazeTestStatus.PASSED); when(testSummary.getTarget()).thenReturn(targetA); + when(testSummary.getLabel()).thenReturn(labelA); stateTracker.testFilteringComplete(filteringComplete); stateTracker.testSummary(testSummary); @@ -404,6 +406,7 @@ public class ExperimentalStateTrackerTest extends FoundationTestCase { TestSummary testSummary = Mockito.mock(TestSummary.class); when(testSummary.getStatus()).thenReturn(BlazeTestStatus.FAILED); when(testSummary.getTarget()).thenReturn(targetA); + when(testSummary.getLabel()).thenReturn(labelA); stateTracker.testFilteringComplete(filteringComplete); stateTracker.testSummary(testSummary); @@ -507,6 +510,7 @@ public class ExperimentalStateTrackerTest extends FoundationTestCase { TestSummary testSummary = Mockito.mock(TestSummary.class); when(testSummary.getStatus()).thenReturn(BlazeTestStatus.PASSED); when(testSummary.getTarget()).thenReturn(bartestTarget); + when(testSummary.getLabel()).thenReturn(bartestLabel); if (actions >= 1) { stateTracker.actionStarted(new ActionStartedEvent(foobuildAction, 123456789)); 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 0ea18afac7..e3ff952481 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 @@ -25,6 +25,7 @@ import static org.mockito.Mockito.when; 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.clock.BlazeClock; import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.util.io.AnsiTerminalPrinter; @@ -452,8 +453,11 @@ public class TestSummaryTest { } private ConfiguredTarget target(String path, String targetName) throws Exception { + BuildConfiguration configuration = Mockito.mock(BuildConfiguration.class); + when(configuration.checksum()).thenReturn("abcdef"); ConfiguredTarget target = Mockito.mock(ConfiguredTarget.class); when(target.getLabel()).thenReturn(Label.create(path, targetName)); + when(target.getConfiguration()).thenReturn(configuration); return target; } |