Skip to content

Commit

Permalink
StandaloneTestStrategy: add lcov_merger to tools of TestRunner spawns
Browse files Browse the repository at this point in the history
The default lcov_merger (@bazel_tools//tools/test:lcov_merger) is a
java_binary that requires runfiles to work.
In StandaloneTestStrategy, it was added to the inputs of TestRunner
spawns, but not to tools.
This prevents the runfiles from being available to the action,
preventing coverage to be collected during remote execution.

Fixes bazelbuild#4033
  • Loading branch information
malt3 committed Nov 20, 2024
1 parent 222d47f commit e7ea302
Show file tree
Hide file tree
Showing 3 changed files with 6 additions and 15 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -248,7 +248,6 @@ private TestParams createTestAction()
int shardCount = getShardCount(ruleContext);

NestedSetBuilder<Artifact> lcovMergerFilesToRun = NestedSetBuilder.compileOrder();
Artifact lcovMergerRunfilesTree = null;

TestTargetExecutionSettings executionSettings;
if (collectCodeCoverage) {
Expand Down Expand Up @@ -307,9 +306,6 @@ private TestParams createTestAction()
extraTestEnv.put(LCOV_MERGER, lcovFilesToRun.getExecutable().getExecPathString());
inputsBuilder.addTransitive(lcovFilesToRun.getFilesToRun());
lcovMergerFilesToRun.addTransitive(lcovFilesToRun.getFilesToRun());
if (lcovFilesToRun.getRunfilesSupport() != null) {
lcovMergerRunfilesTree = lcovFilesToRun.getRunfilesSupport().getRunfilesTreeArtifact();
}
} else {
NestedSet<Artifact> filesToBuild =
lcovMerger.getProvider(FileProvider.class).getFilesToBuild();
Expand Down Expand Up @@ -439,7 +435,6 @@ private TestParams createTestAction()
cancelConcurrentTests,
splitCoveragePostProcessing,
lcovMergerFilesToRun,
lcovMergerRunfilesTree,
// Network allowlist only makes sense in workspaces which explicitly add it, use an
// empty one as a fallback.
MoreObjects.firstNonNull(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,6 @@ public class TestRunnerAction extends AbstractAction

private final boolean splitCoveragePostProcessing;
private final NestedSetBuilder<Artifact> lcovMergerFilesToRun;
@Nullable private final Artifact lcovMergerRunfilesTree;

// TODO(b/192694287): Remove once we migrate all tests from the allowlist.
private final PackageSpecificationProvider networkAllowlist;
Expand Down Expand Up @@ -213,7 +212,6 @@ private static ImmutableSet<Artifact> nonNullAsSet(Artifact... artifacts) {
boolean cancelConcurrentTestsOnSuccess,
boolean splitCoveragePostProcessing,
NestedSetBuilder<Artifact> lcovMergerFilesToRun,
@Nullable Artifact lcovMergerRunfilesTree,
PackageSpecificationProvider networkAllowlist) {
super(
owner,
Expand Down Expand Up @@ -271,7 +269,6 @@ private static ImmutableSet<Artifact> nonNullAsSet(Artifact... artifacts) {
this.cancelConcurrentTestsOnSuccess = cancelConcurrentTestsOnSuccess;
this.splitCoveragePostProcessing = splitCoveragePostProcessing;
this.lcovMergerFilesToRun = lcovMergerFilesToRun;
this.lcovMergerRunfilesTree = lcovMergerRunfilesTree;
this.networkAllowlist = networkAllowlist;

// Mark all possible test outputs for deletion before test execution.
Expand Down Expand Up @@ -330,11 +327,6 @@ public final ActionEnvironment getEnvironment() {
return configuration.getActionEnvironment();
}

@Nullable
public Artifact getLcovMergerRunfilesTree() {
return lcovMergerRunfilesTree;
}

public BuildConfigurationValue getConfiguration() {
return configuration;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,9 @@ public TestRunnerSpawn createTestRunnerSpawn(
ImmutableMap.copyOf(executionInfo),
ImmutableMap.of(),
/* inputs= */ action.getInputs(),
NestedSetBuilder.emptySet(Order.STABLE_ORDER),
/* tools= */ NestedSetBuilder.<ActionInput>compileOrder()
.addTransitive(action.getLcovMergerFilesToRun().build())
.build(),
ImmutableSet.copyOf(action.getSpawnOutputs()),
/* mandatoryOutputs= */ ImmutableSet.of(),
localResourcesSupplier);
Expand Down Expand Up @@ -509,7 +511,9 @@ private static Spawn createCoveragePostProcessingSpawn(
.add(action.getCoverageManifest())
.addTransitive(action.getLcovMergerFilesToRun().build())
.build(),
/* tools= */ NestedSetBuilder.emptySet(Order.STABLE_ORDER),
/* tools= */ NestedSetBuilder.<ActionInput>compileOrder()
.addTransitive(action.getLcovMergerFilesToRun().build())
.build(),
/* outputs= */ ImmutableSet.of(
ActionInputHelper.fromPath(action.getCoverageData().getExecPath())),
/* mandatoryOutputs= */ null,
Expand Down

0 comments on commit e7ea302

Please sign in to comment.