From 3b44cec1635b31ac1ecfd1ba7ed41e90efc30338 Mon Sep 17 00:00:00 2001 From: Fabian Meumertzheim Date: Fri, 10 May 2024 17:07:49 -0700 Subject: [PATCH] Announce combined coverage report on the BES This allows BES consumers to access this file without guessing its path and existence, as well as when the actual coverage generation action runs locally. Also removes an obsolete `CoverageReport` event. RELNOTES: The combined coverage report produced via `--combined_report=lcov` is now announced on the BES via the new `CoverageReport` event. Closes #22171. PiperOrigin-RevId: 632641499 Change-Id: I0c323a371ec2fdd085bea23a772a85b72c52093f --- .../google/devtools/build/lib/analysis/BUILD | 10 ----- .../lib/analysis/test/CoverageReport.java | 31 -------------- .../test/CoverageReportActionFactory.java | 5 +++ .../devtools/build/lib/bazel/coverage/BUILD | 1 - .../coverage/BazelCoverageReportModule.java | 40 +++++++++++++------ .../coverage/CoverageReportActionBuilder.java | 10 ----- .../remote/build_without_the_bytes_test.sh | 5 +++ 7 files changed, 38 insertions(+), 64 deletions(-) delete mode 100644 src/main/java/com/google/devtools/build/lib/analysis/test/CoverageReport.java diff --git a/src/main/java/com/google/devtools/build/lib/analysis/BUILD b/src/main/java/com/google/devtools/build/lib/analysis/BUILD index 90930bfff2b53e..767469bab938a3 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/BUILD +++ b/src/main/java/com/google/devtools/build/lib/analysis/BUILD @@ -2656,16 +2656,6 @@ java_library( ], ) -java_library( - name = "test/coverage_report", - srcs = ["test/CoverageReport.java"], - deps = [ - "//src/main/java/com/google/devtools/build/lib/events", - "//src/main/java/com/google/devtools/build/lib/vfs", - "//third_party:guava", - ], -) - java_library( name = "test/coverage_configuration", srcs = ["test/CoverageConfiguration.java"], diff --git a/src/main/java/com/google/devtools/build/lib/analysis/test/CoverageReport.java b/src/main/java/com/google/devtools/build/lib/analysis/test/CoverageReport.java deleted file mode 100644 index c90fb880f6b067..00000000000000 --- a/src/main/java/com/google/devtools/build/lib/analysis/test/CoverageReport.java +++ /dev/null @@ -1,31 +0,0 @@ -// Copyright 2023 The Bazel Authors. All rights reserved. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. -package com.google.devtools.build.lib.analysis.test; - -import com.google.common.collect.ImmutableList; -import com.google.devtools.build.lib.events.ExtendedEventHandler; -import com.google.devtools.build.lib.vfs.Path; - -/** This event is used to notify about a successfully generated coverage report. */ -public final class CoverageReport implements ExtendedEventHandler.Postable { - private final ImmutableList files; - - public CoverageReport(ImmutableList files) { - this.files = files; - } - - public ImmutableList getFiles() { - return files; - } -} diff --git a/src/main/java/com/google/devtools/build/lib/analysis/test/CoverageReportActionFactory.java b/src/main/java/com/google/devtools/build/lib/analysis/test/CoverageReportActionFactory.java index eba4903bbef5e8..4625c78bcb1f8b 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/test/CoverageReportActionFactory.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/test/CoverageReportActionFactory.java @@ -15,6 +15,7 @@ package com.google.devtools.build.lib.analysis.test; import com.google.common.collect.ImmutableList; +import com.google.common.collect.Iterables; import com.google.common.eventbus.EventBus; import com.google.devtools.build.lib.actions.ActionAnalysisMetadata; import com.google.devtools.build.lib.actions.ActionKeyContext; @@ -72,6 +73,10 @@ public ImmutableList getActions() { public Collection getCoverageOutputs() { return coverageReportAction.getOutputs(); } + + public Artifact getCoverageReportArtifact() { + return Iterables.getOnlyElement(coverageReportAction.getOutputs()); + } } /** diff --git a/src/main/java/com/google/devtools/build/lib/bazel/coverage/BUILD b/src/main/java/com/google/devtools/build/lib/bazel/coverage/BUILD index 67e095e132b533..6d80ba2534d50c 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/coverage/BUILD +++ b/src/main/java/com/google/devtools/build/lib/bazel/coverage/BUILD @@ -28,7 +28,6 @@ java_library( "//src/main/java/com/google/devtools/build/lib/analysis:blaze_directories", "//src/main/java/com/google/devtools/build/lib/analysis:configured_target", "//src/main/java/com/google/devtools/build/lib/analysis:incompatible_platform_provider", - "//src/main/java/com/google/devtools/build/lib/analysis:test/coverage_report", "//src/main/java/com/google/devtools/build/lib/analysis:test/coverage_report_action_factory", "//src/main/java/com/google/devtools/build/lib/collect/nestedset", "//src/main/java/com/google/devtools/build/lib/concurrent", diff --git a/src/main/java/com/google/devtools/build/lib/bazel/coverage/BazelCoverageReportModule.java b/src/main/java/com/google/devtools/build/lib/bazel/coverage/BazelCoverageReportModule.java index 5ae67604883d5b..d5c541337c7dae 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/coverage/BazelCoverageReportModule.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/coverage/BazelCoverageReportModule.java @@ -17,6 +17,7 @@ import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableList; import com.google.common.eventbus.EventBus; +import com.google.common.eventbus.Subscribe; import com.google.devtools.build.lib.actions.ActionKeyContext; import com.google.devtools.build.lib.actions.ActionLookupKey; import com.google.devtools.build.lib.actions.Artifact; @@ -24,6 +25,8 @@ import com.google.devtools.build.lib.analysis.BlazeDirectories; import com.google.devtools.build.lib.analysis.ConfiguredTarget; import com.google.devtools.build.lib.analysis.test.CoverageReportActionFactory; +import com.google.devtools.build.lib.analysis.test.CoverageReportActionFactory.CoverageReportActionsWrapper; +import com.google.devtools.build.lib.buildtool.buildevent.BuildCompleteEvent; import com.google.devtools.build.lib.collect.nestedset.NestedSet; import com.google.devtools.build.lib.events.EventHandler; import com.google.devtools.build.lib.runtime.BlazeModule; @@ -97,18 +100,21 @@ public CoverageReportActionsWrapper createCoverageReportActionsWrapper( } Preconditions.checkArgument(options.combinedReport == ReportType.LCOV); CoverageReportActionBuilder builder = new CoverageReportActionBuilder(); - return builder.createCoverageActionsWrapper( - eventHandler, - directories, - targetsToTest, - baselineCoverageArtifacts, - artifactFactory, - actionKeyContext, - actionLookupKey, - workspaceName, - this::getArgs, - this::getLocationMessage, - /*htmlReport=*/ false); + CoverageReportActionsWrapper wrapper = + builder.createCoverageActionsWrapper( + eventHandler, + directories, + targetsToTest, + baselineCoverageArtifacts, + artifactFactory, + actionKeyContext, + actionLookupKey, + workspaceName, + this::getArgs, + this::getLocationMessage, + /* htmlReport= */ false); + eventBus.register(new CoverageReportCollector(wrapper)); + return wrapper; } private ImmutableList getArgs(CoverageArgs args) { @@ -130,4 +136,14 @@ private String getLocationMessage(CoverageArgs args) { } }; } + + private record CoverageReportCollector(CoverageReportActionsWrapper wrapper) { + @Subscribe + public void buildComplete(BuildCompleteEvent event) { + event + .getResult() + .getBuildToolLogCollection() + .addLocalFile("coverage_report.lcov", wrapper.getCoverageReportArtifact().getPath()); + } + } } diff --git a/src/main/java/com/google/devtools/build/lib/bazel/coverage/CoverageReportActionBuilder.java b/src/main/java/com/google/devtools/build/lib/bazel/coverage/CoverageReportActionBuilder.java index 04836bc16b49fc..d062797131abcb 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/coverage/CoverageReportActionBuilder.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/coverage/CoverageReportActionBuilder.java @@ -14,7 +14,6 @@ package com.google.devtools.build.lib.bazel.coverage; -import static com.google.common.collect.ImmutableList.toImmutableList; import com.google.common.base.Joiner; import com.google.common.collect.ImmutableList; @@ -31,7 +30,6 @@ import com.google.devtools.build.lib.actions.Artifact.ArtifactExpander; import com.google.devtools.build.lib.actions.ArtifactFactory; import com.google.devtools.build.lib.actions.ArtifactOwner; -import com.google.devtools.build.lib.actions.ArtifactPathResolver; import com.google.devtools.build.lib.actions.ArtifactRoot; import com.google.devtools.build.lib.actions.BaseSpawn; import com.google.devtools.build.lib.actions.ExecException; @@ -48,7 +46,6 @@ import com.google.devtools.build.lib.analysis.RunfilesSupport; import com.google.devtools.build.lib.analysis.actions.Compression; import com.google.devtools.build.lib.analysis.actions.FileWriteAction; -import com.google.devtools.build.lib.analysis.test.CoverageReport; import com.google.devtools.build.lib.analysis.test.CoverageReportActionFactory.CoverageReportActionsWrapper; import com.google.devtools.build.lib.analysis.test.TestProvider; import com.google.devtools.build.lib.analysis.test.TestProvider.TestParams; @@ -60,7 +57,6 @@ import com.google.devtools.build.lib.events.EventHandler; import com.google.devtools.build.lib.exec.SpawnStrategyResolver; import com.google.devtools.build.lib.util.Fingerprint; -import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.build.lib.vfs.PathFragment; import java.util.ArrayList; import java.util.Collection; @@ -139,12 +135,6 @@ public ActionResult execute(ActionExecutionContext actionExecutionContext) .getContext(SpawnStrategyResolver.class) .exec(spawn, actionExecutionContext); actionExecutionContext.getEventHandler().handle(Event.info(locationMessage)); - ArtifactPathResolver pathResolver = actionExecutionContext.getPathResolver(); - ImmutableList files = - getOutputs().stream() - .map(artifact -> pathResolver.convertPath(artifact.getPath())) - .collect(toImmutableList()); - actionExecutionContext.getEventHandler().post(new CoverageReport(files)); return ActionResult.create(spawnResults); } catch (ExecException e) { throw ActionExecutionException.fromExecException(e, this); diff --git a/src/test/shell/bazel/remote/build_without_the_bytes_test.sh b/src/test/shell/bazel/remote/build_without_the_bytes_test.sh index 71cf0742e9b2a8..45dd0007c60151 100755 --- a/src/test/shell/bazel/remote/build_without_the_bytes_test.sh +++ b/src/test/shell/bazel/remote/build_without_the_bytes_test.sh @@ -1933,6 +1933,7 @@ EOF --spawn_strategy=remote \ --remote_executor=grpc://localhost:${worker_port} \ --instrumentation_filter=//java/factorial \ + --build_event_text_file=bes.txt \ //java/factorial:fact-test &> $TEST_log || fail "Shouldn't fail" # Test binary shouldn't be downloaded @@ -1958,6 +1959,10 @@ end_of_record" expect_log "$expected_result" cat bazel-out/_coverage/_coverage_report.dat > $TEST_log expect_log "$expected_result" + + cat bes.txt | tr '\n' ' ' > $TEST_log + report_sha=$(sha256sum bazel-out/_coverage/_coverage_report.dat | cut -d ' ' -f 1) + expect_log "log { name: \"coverage_report.lcov\" uri: \"bytestream://[^\"]*/${report_sha}/[^\"]*\"" } function test_remote_cache_eviction_retries() {