Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[6.1.0]Make bazel coverage work with minimal mode #17322

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 28 additions & 0 deletions src/main/java/com/google/devtools/build/lib/analysis/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -2527,6 +2527,34 @@ 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"],
deps = [
":config/build_options",
":config/core_option_converters",
":config/core_options",
":config/fragment",
":config/fragment_options",
"//src/main/java/com/google/devtools/build/lib/analysis/starlark/annotations",
"//src/main/java/com/google/devtools/build/lib/cmdline",
"//src/main/java/com/google/devtools/build/lib/concurrent",
"//src/main/java/com/google/devtools/build/lib/starlarkbuildapi/test",
"//src/main/java/com/google/devtools/common/options",
"//third_party:jsr305",
],
)

java_library(
name = "test/coverage_report_action_factory",
srcs = ["test/CoverageReportActionFactory.java"],
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
// 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<Path> files;

public CoverageReport(ImmutableList<Path> files) {
this.files = files;
}

public ImmutableList<Path> getFiles() {
return files;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,14 @@ 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",
"//src/main/java/com/google/devtools/build/lib/events",
"//src/main/java/com/google/devtools/build/lib/exec:spawn_strategy_resolver",
"//src/main/java/com/google/devtools/build/lib/util",
"//src/main/java/com/google/devtools/build/lib/vfs",
"//src/main/java/com/google/devtools/build/lib/vfs:pathfragment",
"//src/main/java/com/google/devtools/common/options",
"//third_party:auto_value",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@

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;
import com.google.common.collect.ImmutableMap;
Expand All @@ -30,6 +32,7 @@
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 @@ -45,6 +48,7 @@
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 @@ -57,6 +61,7 @@
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 @@ -146,6 +151,12 @@ 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
Original file line number Diff line number Diff line change
Expand Up @@ -1053,5 +1053,78 @@ public TestAttemptContinuation execute() throws InterruptedException, ExecExcept
testResultDataBuilder,
primarySpawnResults);
}

Verify.verify(
!(testAction.isCoverageMode() && testAction.getSplitCoveragePostProcessing())
|| actionExecutionContext
.getPathResolver()
.convertPath(testAction.getCoverageData().getPath())
.exists());
Verify.verifyNotNull(spawnResults);
Verify.verifyNotNull(testResultDataBuilder);

try {
if (!fileOutErr.hasRecordedOutput()) {
// Make sure that the test.log exists.Spaw
FileSystemUtils.touchFile(fileOutErr.getOutputPath());
}
// Append any error output to the test.log. This is very rare.
writeOutFile(fileOutErr.getErrorPath(), fileOutErr.getOutputPath());
fileOutErr.close();
if (streamed != null) {
streamed.close();
}
} catch (IOException e) {
throw new EnvironmentalExecException(e, Code.TEST_OUT_ERR_IO_EXCEPTION);
}

Path xmlOutputPath = resolvedPaths.getXmlOutputPath();

// If the test did not create a test.xml, and --experimental_split_xml_generation is enabled,
// then we run a separate action to create a test.xml from test.log. We do this as a spawn
// rather than doing it locally in-process, as the test.log file may only exist remotely (when
// remote execution is enabled), and we do not want to have to download it.
if (executionOptions.splitXmlGeneration
&& fileOutErr.getOutputPath().exists()
&& !xmlOutputPath.exists()) {
Spawn xmlGeneratingSpawn =
createXmlGeneratingSpawn(testAction, spawn.getEnvironment(), spawnResults.get(0));
SpawnStrategyResolver spawnStrategyResolver =
actionExecutionContext.getContext(SpawnStrategyResolver.class);
// We treat all failures to generate the test.xml here as catastrophic, and won't rerun
// the test if this fails. We redirect the output to a temporary file.
FileOutErr xmlSpawnOutErr = actionExecutionContext.getFileOutErr().childOutErr();
try {

ImmutableList<SpawnResult> xmlSpawnResults =
spawnStrategyResolver.exec(
xmlGeneratingSpawn, actionExecutionContext.withFileOutErr(xmlSpawnOutErr));
spawnResults =
ImmutableList.<SpawnResult>builder()
.addAll(spawnResults)
.addAll(xmlSpawnResults)
.build();
} catch (InterruptedException | ExecException e) {
closeSuppressed(e, xmlSpawnOutErr);
throw e;
}
}

TestCase details = parseTestResult(xmlOutputPath);
if (details != null) {
testResultDataBuilder.setTestCase(details);
}

BuildEventStreamProtos.TestResult.ExecutionInfo executionInfo =
extractExecutionInfo(spawnResults.get(0), testResultDataBuilder);
return StandaloneTestResult.builder()
.setSpawnResults(spawnResults)
// We return the TestResultData.Builder rather than the finished TestResultData
// instance, as we may have to rename the output files in case the test needs to be
// rerun (if it failed here _and_ is marked flaky _and_ the number of flaky attempts
// is larger than 1).
.setTestResultDataBuilder(testResultDataBuilder)
.setExecutionInfo(executionInfo)
.build();
}
}
1 change: 1 addition & 0 deletions src/main/java/com/google/devtools/build/lib/remote/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,7 @@ java_library(
"//src/main/java/com/google/devtools/build/lib/analysis:analysis_cluster",
"//src/main/java/com/google/devtools/build/lib/analysis:configured_target",
"//src/main/java/com/google/devtools/build/lib/analysis:configured_target_value",
"//src/main/java/com/google/devtools/build/lib/analysis:test/coverage_report",
"//src/main/java/com/google/devtools/build/lib/packages",
"//src/main/java/com/google/devtools/build/lib/remote/util",
"//src/main/java/com/google/devtools/build/lib/skyframe:action_execution_value",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
import com.google.devtools.build.lib.analysis.TargetCompleteEvent;
import com.google.devtools.build.lib.analysis.TopLevelArtifactHelper.ArtifactsInOutputGroup;
import com.google.devtools.build.lib.analysis.configuredtargets.RuleConfiguredTarget;
import com.google.devtools.build.lib.analysis.test.CoverageReport;
import com.google.devtools.build.lib.analysis.test.TestAttempt;
import com.google.devtools.build.lib.remote.AbstractActionInputPrefetcher.Priority;
import com.google.devtools.build.lib.remote.util.StaticMetadataProvider;
Expand All @@ -56,7 +57,8 @@ private enum CommandMode {
UNKNOWN,
BUILD,
TEST,
RUN;
RUN,
COVERAGE;
}

private static final GoogleLogger logger = GoogleLogger.forEnclosingClass();
Expand All @@ -83,6 +85,9 @@ public ToplevelArtifactsDownloader(
case "run":
this.commandMode = CommandMode.RUN;
break;
case "coverage":
this.commandMode = CommandMode.COVERAGE;
break;
default:
this.commandMode = CommandMode.UNKNOWN;
}
Expand All @@ -104,32 +109,48 @@ public interface PathToMetadataConverter {
FileArtifactValue getMetadata(Path path);
}

private void downloadTestOutput(Path path) {
// Since the event is fired within action execution, the skyframe doesn't know the outputs of
// test actions yet, so we can't get their metadata through skyframe. However, the fileSystem
// of the path is an ActionFileSystem, we use it to get the metadata for this file.
//
// If the test hit action cache, the filesystem is local filesystem because the actual test
// action didn't get the chance to execute. In this case the metadata is null which is fine
// because test outputs are already downloaded (otherwise it cannot hit the action cache).
FileArtifactValue metadata = pathToMetadataConverter.getMetadata(path);
if (metadata != null) {
ListenableFuture<Void> future =
actionInputPrefetcher.downloadFileAsync(path.asFragment(), metadata, Priority.LOW);
addCallback(
future,
new FutureCallback<Void>() {
@Override
public void onSuccess(Void unused) {}

@Override
public void onFailure(Throwable throwable) {
logger.atWarning().withCause(throwable).log(
"Failed to download test output %s.", path);
}
},
directExecutor());
}
}

@Subscribe
@AllowConcurrentEvents
public void onTestAttempt(TestAttempt event) {
for (Pair<String, Path> pair : event.getFiles()) {
Path path = checkNotNull(pair.getSecond());
// Since the event is fired within action execution, the skyframe doesn't know the outputs of
// test actions yet, so we can't get their metadata through skyframe. However, the fileSystem
// of the path is an ActionFileSystem, we use it to get the metadata for this file.
FileArtifactValue metadata = pathToMetadataConverter.getMetadata(path);
if (metadata != null) {
ListenableFuture<Void> future =
actionInputPrefetcher.downloadFileAsync(path.asFragment(), metadata, Priority.LOW);
addCallback(
future,
new FutureCallback<Void>() {
@Override
public void onSuccess(Void unused) {}
downloadTestOutput(path);
}
}

@Override
public void onFailure(Throwable throwable) {
logger.atWarning().withCause(throwable).log(
"Failed to download test output %s.", path);
}
},
directExecutor());
}
@Subscribe
@AllowConcurrentEvents
public void onCoverageReport(CoverageReport event) {
for (var file : event.getFiles()) {
downloadTestOutput(file);
}
}

Expand Down Expand Up @@ -168,8 +189,9 @@ private boolean shouldDownloadToplevelOutputs(ConfiguredTargetKey configuredTarg
case RUN:
// Always download outputs of toplevel targets in RUN mode
return true;
case COVERAGE:
case TEST:
// Do not download test binary in test mode.
// Do not download test binary in test/coverage mode.
try {
var configuredTargetValue =
(ConfiguredTargetValue) memoizingEvaluator.getExistingValue(configuredTargetKey);
Expand Down
Loading