Skip to content

Commit

Permalink
[7.2.0] Announce combined coverage report on the BES (#22327)
Browse files Browse the repository at this point in the history
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

Commit
bb1fb53

Co-authored-by: Fabian Meumertzheim <fabian@meumertzhe.im>
  • Loading branch information
bazel-io and fmeum committed May 11, 2024
1 parent b090599 commit 3506ca1
Show file tree
Hide file tree
Showing 7 changed files with 38 additions and 64 deletions.
10 changes: 0 additions & 10 deletions src/main/java/com/google/devtools/build/lib/analysis/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -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"],
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -72,6 +73,10 @@ public ImmutableList<ActionAnalysisMetadata> getActions() {
public Collection<Artifact> getCoverageOutputs() {
return coverageReportAction.getOutputs();
}

public Artifact getCoverageReportArtifact() {
return Iterables.getOnlyElement(coverageReportAction.getOutputs());
}
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,16 @@
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;
import com.google.devtools.build.lib.actions.ArtifactFactory;
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;
Expand Down Expand Up @@ -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<String> getArgs(CoverageArgs args) {
Expand All @@ -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());
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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<Path> 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);
Expand Down
5 changes: 5 additions & 0 deletions src/test/shell/bazel/remote/build_without_the_bytes_test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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() {
Expand Down

0 comments on commit 3506ca1

Please sign in to comment.