diff --git a/.bazelrc b/.bazelrc index 3885564cfd3aa8..2a8cd803f3a0f8 100644 --- a/.bazelrc +++ b/.bazelrc @@ -35,4 +35,4 @@ build --java_language_version=11 build --tool_java_language_version=11 # User-specific .bazelrc -try-import user.bazelrc +try-import %workspace%/user.bazelrc diff --git a/src/main/java/com/google/devtools/build/lib/analysis/test/TestAttempt.java b/src/main/java/com/google/devtools/build/lib/analysis/test/TestAttempt.java index 3f971af86ff497..ff76502bd159f2 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/test/TestAttempt.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/test/TestAttempt.java @@ -17,6 +17,7 @@ import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.buildeventstream.BuildEvent.LocalFile.LocalFileType; import com.google.devtools.build.lib.buildeventstream.BuildEventContext; @@ -31,6 +32,7 @@ import com.google.devtools.build.lib.util.Pair; import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.build.lib.view.test.TestStatus.BlazeTestStatus; +import com.google.devtools.build.lib.view.test.TestStatus.PathMetadata; import com.google.devtools.build.lib.view.test.TestStatus.TestResultData; import com.google.protobuf.util.Durations; import com.google.protobuf.util.Timestamps; @@ -48,6 +50,7 @@ public class TestAttempt implements BuildEventWithOrderConstraint { private final boolean lastAttempt; private final Collection> files; private final List testWarnings; + private final ImmutableMap fileNameToMetadata; private final long durationMillis; private final long startTimeMillis; private final BuildEventStreamProtos.TestResult.ExecutionInfo executionInfo; @@ -69,6 +72,7 @@ private TestAttempt( long startTimeMillis, long durationMillis, Collection> files, + ImmutableMap fileNameToMetadata, List testWarnings, boolean lastAttempt) { this.testAction = testAction; @@ -80,6 +84,7 @@ private TestAttempt( this.startTimeMillis = startTimeMillis; this.durationMillis = durationMillis; this.files = Preconditions.checkNotNull(files); + this.fileNameToMetadata = Preconditions.checkNotNull(fileNameToMetadata); this.testWarnings = Preconditions.checkNotNull(testWarnings); this.lastAttempt = lastAttempt; } @@ -105,6 +110,7 @@ public static TestAttempt forExecutedTestResult( attemptData.getStartTimeMillisEpoch(), attemptData.getRunDurationMillis(), files, + attemptData.getAttemptsList().size() > 0 ? ImmutableMap.copyOf(attemptData.getAttempts(0).getPathMetadataMap()) : ImmutableMap.of(), attemptData.getWarningList(), lastAttempt); } @@ -126,6 +132,7 @@ public static TestAttempt fromCachedTestResult( attemptData.getStartTimeMillisEpoch(), attemptData.getRunDurationMillis(), files, + attemptData.getAttemptsList().size() > 0 ? ImmutableMap.copyOf(attemptData.getAttempts(0).getPathMetadataMap()) : ImmutableMap.of(), attemptData.getWarningList(), lastAttempt); } @@ -230,10 +237,19 @@ public BuildEventStreamProtos.TestResult asTestResult(BuildEventContext converte builder.setTestAttemptDurationMillis(durationMillis); builder.addAllWarning(testWarnings); for (Pair file : files) { - String uri = pathConverter.apply(file.getSecond()); - if (uri != null) { - builder.addTestActionOutput( + if (!fileNameToMetadata.containsKey(file.getFirst())) { + String uri = pathConverter.apply(file.getSecond()); + if (uri != null) { + builder.addTestActionOutput( + BuildEventStreamProtos.File.newBuilder().setName(file.getFirst()).setUri(uri).build()); + } + } else { + PathMetadata metadata = fileNameToMetadata.get(file.getFirst()); + if (metadata != null) { + String uri = pathConverter.applyForDigest(metadata.getHash(), metadata.getSizeBytes()); + builder.addTestActionOutput( BuildEventStreamProtos.File.newBuilder().setName(file.getFirst()).setUri(uri).build()); + } } } return builder.build(); diff --git a/src/main/java/com/google/devtools/build/lib/analysis/test/TestResult.java b/src/main/java/com/google/devtools/build/lib/analysis/test/TestResult.java index 250391f6687f0d..297f8b662b1c86 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/test/TestResult.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/test/TestResult.java @@ -30,6 +30,8 @@ import com.google.devtools.build.lib.vfs.FileSystem; import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.build.lib.view.test.TestStatus.BlazeTestStatus; +import com.google.devtools.build.lib.view.test.TestStatus.File; +import com.google.devtools.build.lib.view.test.TestStatus.PathMetadata; import com.google.devtools.build.lib.view.test.TestStatus.TestResultData; import java.util.Collection; import java.util.List; @@ -183,7 +185,6 @@ public DetailedExitCode getSystemFailure() { * (e.g., "test.log"). */ private Collection> getFiles() { - // TODO(ulfjack): Cache the set of generated files in the TestResultData. - return testAction.getTestOutputsMapping(ArtifactPathResolver.forExecRoot(execRoot), execRoot); + return testAction.testResultDataToMapping(execRoot, data); } } diff --git a/src/main/java/com/google/devtools/build/lib/analysis/test/TestRunnerAction.java b/src/main/java/com/google/devtools/build/lib/analysis/test/TestRunnerAction.java index 31a674463b9b9d..225106e4d76837 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/test/TestRunnerAction.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/test/TestRunnerAction.java @@ -45,6 +45,7 @@ import com.google.devtools.build.lib.actions.CommandLineExpansionException; import com.google.devtools.build.lib.actions.EnvironmentalExecException; import com.google.devtools.build.lib.actions.ExecException; +import com.google.devtools.build.lib.actions.FileArtifactValue; import com.google.devtools.build.lib.actions.NotifyOnActionCacheHit; import com.google.devtools.build.lib.actions.RunfilesSupplier; import com.google.devtools.build.lib.actions.SpawnExecutedEvent; @@ -74,6 +75,8 @@ import com.google.devtools.build.lib.vfs.FileSystemUtils; import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.build.lib.vfs.PathFragment; +import com.google.devtools.build.lib.view.test.TestStatus.File; +import com.google.devtools.build.lib.view.test.TestStatus.TestAttempt; import com.google.devtools.build.lib.view.test.TestStatus.TestResultData; import com.google.devtools.common.options.TriState; import com.google.protobuf.ExtensionRegistry; @@ -92,6 +95,7 @@ import java.util.function.Supplier; import javax.annotation.Nullable; + /** * An Action representing a test with the associated environment (runfiles, environment variables, * test result, etc). It consumes test executable and runfiles artifacts and produces test result @@ -366,78 +370,122 @@ public List getSpawnOutputs() { return outputs; } + public static final class TestOutputInfo { + private final String key; + private final Path path; + private final FileArtifactValue fileArtifactValue; + + public TestOutputInfo(String key, Path path, FileArtifactValue fileArtifactValue) { + this.key = key; + this.path = path; + this.fileArtifactValue = fileArtifactValue; + } + + public String getKey() { + return key; + } + + public Path getPath() { + return path; + } + + public FileArtifactValue getFileArtifactValue() { + return fileArtifactValue; + } + + } + /** * Returns the list of mappings from file name constants to output files. This method checks the * file system for existence of these output files, so it must only be used after test execution. */ // TODO(ulfjack): Instead of going to local disk here, use SpawnResult (add list of files there). - public ImmutableList> getTestOutputsMapping( - ArtifactPathResolver resolver, Path execRoot) { - ImmutableList.Builder> builder = ImmutableList.builder(); + public ImmutableList getTestOutputsMapping( + ArtifactPathResolver resolver, Path execRoot, + ImmutableMap injectedOutputs) { + ImmutableList.Builder builder = ImmutableList.builder(); if (resolver.toPath(getTestLog()).exists()) { - builder.add(Pair.of(TestFileNameConstants.TEST_LOG, resolver.toPath(getTestLog()))); + builder.add(new TestOutputInfo(TestFileNameConstants.TEST_LOG, resolver.toPath(getTestLog()), null)); } if (getCoverageData() != null && resolver.toPath(getCoverageData()).exists()) { - builder.add(Pair.of(TestFileNameConstants.TEST_COVERAGE, resolver.toPath(getCoverageData()))); + builder.add(new TestOutputInfo(TestFileNameConstants.TEST_COVERAGE, resolver.toPath(getCoverageData()), null)); } + if (execRoot != null) { ResolvedPaths resolvedPaths = resolve(execRoot); + for (Map.Entry entry : injectedOutputs.entrySet()) { + Path artifactPath = resolver.toPath(entry.getKey()); + if (artifactPath.equals(resolvedPaths.getTestStderr())) { + builder.add(new TestOutputInfo(TestFileNameConstants.TEST_STDERR, artifactPath, entry.getValue())); + } else if (artifactPath.equals(resolvedPaths.getXmlOutputPath())) { + builder.add(new TestOutputInfo(TestFileNameConstants.TEST_XML, artifactPath, entry.getValue())); + } else if (artifactPath.equals(resolvedPaths.getSplitLogsPath())) { + builder.add(new TestOutputInfo(TestFileNameConstants.SPLIT_LOGS, artifactPath, entry.getValue())); + } else if (artifactPath.equals(resolvedPaths.getTestWarningsPath())) { + builder.add(new TestOutputInfo(TestFileNameConstants.TEST_WARNINGS, artifactPath, entry.getValue())); + } else if (artifactPath.equals(resolvedPaths.getUndeclaredOutputsZipPath())) { + builder.add(new TestOutputInfo(TestFileNameConstants.UNDECLARED_OUTPUTS_ZIP, artifactPath, entry.getValue())); + } else if (artifactPath.equals(resolvedPaths.getUndeclaredOutputsManifestPath())) { + builder.add(new TestOutputInfo(TestFileNameConstants.UNDECLARED_OUTPUTS_MANIFEST, artifactPath, entry.getValue())); + } else if (artifactPath.equals(resolvedPaths.getUndeclaredOutputsAnnotationsPath())) { + builder.add(new TestOutputInfo(TestFileNameConstants.UNDECLARED_OUTPUTS_ANNOTATIONS, artifactPath, entry.getValue())); + } else if (artifactPath.equals(resolvedPaths.getUndeclaredOutputsAnnotationsPbPath())) { + builder.add(new TestOutputInfo(TestFileNameConstants.UNDECLARED_OUTPUTS_ANNOTATIONS_PB, artifactPath, entry.getValue())); + } else if (artifactPath.equals(resolvedPaths.getUnusedRunfilesLogPath())) { + builder.add(new TestOutputInfo(TestFileNameConstants.UNUSED_RUNFILES_LOG, artifactPath, entry.getValue())); + } else if (artifactPath.equals(resolvedPaths.getInfrastructureFailureFile())) { + builder.add(new TestOutputInfo(TestFileNameConstants.TEST_INFRASTRUCTURE_FAILURE, artifactPath, entry.getValue())); + } + } if (resolvedPaths.getTestStderr().exists()) { - builder.add(Pair.of(TestFileNameConstants.TEST_STDERR, resolvedPaths.getTestStderr())); + builder.add(new TestOutputInfo(TestFileNameConstants.TEST_STDERR, resolvedPaths.getTestStderr(), null)); } if (resolvedPaths.getXmlOutputPath().exists()) { - builder.add(Pair.of(TestFileNameConstants.TEST_XML, resolvedPaths.getXmlOutputPath())); + builder.add(new TestOutputInfo(TestFileNameConstants.TEST_XML, resolvedPaths.getXmlOutputPath(), null)); } if (resolvedPaths.getSplitLogsPath().exists()) { - builder.add(Pair.of(TestFileNameConstants.SPLIT_LOGS, resolvedPaths.getSplitLogsPath())); + builder.add(new TestOutputInfo(TestFileNameConstants.SPLIT_LOGS, resolvedPaths.getSplitLogsPath(), null)); } if (resolvedPaths.getTestWarningsPath().exists()) { - builder.add( - Pair.of(TestFileNameConstants.TEST_WARNINGS, resolvedPaths.getTestWarningsPath())); + builder.add(new TestOutputInfo(TestFileNameConstants.TEST_WARNINGS, resolvedPaths.getTestWarningsPath(), null)); } if (testConfiguration.getZipUndeclaredTestOutputs() && resolvedPaths.getUndeclaredOutputsZipPath().exists()) { - builder.add( - Pair.of( - TestFileNameConstants.UNDECLARED_OUTPUTS_ZIP, - resolvedPaths.getUndeclaredOutputsZipPath())); + builder.add(new TestOutputInfo(TestFileNameConstants.UNDECLARED_OUTPUTS_ZIP, resolvedPaths.getUndeclaredOutputsZipPath(), null)); } if (!testConfiguration.getZipUndeclaredTestOutputs() && resolvedPaths.getUndeclaredOutputsDir().exists()) { - builder.add( - Pair.of( - TestFileNameConstants.UNDECLARED_OUTPUTS_DIR, - resolvedPaths.getUndeclaredOutputsDir())); + builder.add(new TestOutputInfo(TestFileNameConstants.UNDECLARED_OUTPUTS_DIR, resolvedPaths.getUndeclaredOutputsDir(), null)); } if (resolvedPaths.getUndeclaredOutputsManifestPath().exists()) { - builder.add( - Pair.of( - TestFileNameConstants.UNDECLARED_OUTPUTS_MANIFEST, - resolvedPaths.getUndeclaredOutputsManifestPath())); + builder.add(new TestOutputInfo(TestFileNameConstants.UNDECLARED_OUTPUTS_MANIFEST, resolvedPaths.getUndeclaredOutputsManifestPath(), null)); } if (resolvedPaths.getUndeclaredOutputsAnnotationsPath().exists()) { - builder.add( - Pair.of( - TestFileNameConstants.UNDECLARED_OUTPUTS_ANNOTATIONS, - resolvedPaths.getUndeclaredOutputsAnnotationsPath())); + builder.add(new TestOutputInfo(TestFileNameConstants.UNDECLARED_OUTPUTS_ANNOTATIONS, resolvedPaths.getUndeclaredOutputsAnnotationsPath(), null)); } if (resolvedPaths.getUndeclaredOutputsAnnotationsPbPath().exists()) { - builder.add( - Pair.of( - TestFileNameConstants.UNDECLARED_OUTPUTS_ANNOTATIONS_PB, - resolvedPaths.getUndeclaredOutputsAnnotationsPbPath())); + builder.add(new TestOutputInfo(TestFileNameConstants.UNDECLARED_OUTPUTS_ANNOTATIONS_PB, resolvedPaths.getUndeclaredOutputsAnnotationsPbPath(), null)); } if (resolvedPaths.getUnusedRunfilesLogPath().exists()) { - builder.add( - Pair.of( - TestFileNameConstants.UNUSED_RUNFILES_LOG, - resolvedPaths.getUnusedRunfilesLogPath())); + builder.add(new TestOutputInfo(TestFileNameConstants.UNUSED_RUNFILES_LOG, resolvedPaths.getUnusedRunfilesLogPath(), null)); } if (resolvedPaths.getInfrastructureFailureFile().exists()) { - builder.add( - Pair.of( - TestFileNameConstants.TEST_INFRASTRUCTURE_FAILURE, - resolvedPaths.getInfrastructureFailureFile())); + builder.add(new TestOutputInfo(TestFileNameConstants.TEST_INFRASTRUCTURE_FAILURE, resolvedPaths.getInfrastructureFailureFile(), null)); + } + } + return builder.build(); + } + + public static ImmutableList> testResultDataToMapping(Path execRoot, TestResultData data) { + ImmutableList.Builder> builder = ImmutableList.builder(); + List attempts = data.getAttemptsList(); + if (attempts.size() > 0) { + for (File file : attempts.get(attempts.size()-1).getOutputsList()) { + Path path = null; + if (file.getPath() != null) { + path = execRoot.getRelative(file.getPath()); + } + builder.add(new Pair(file.getName(), path)); } } return builder.build(); diff --git a/src/main/java/com/google/devtools/build/lib/buildeventstream/LocalFilesArtifactUploader.java b/src/main/java/com/google/devtools/build/lib/buildeventstream/LocalFilesArtifactUploader.java index a812b14aa3838a..6d24729d07d306 100644 --- a/src/main/java/com/google/devtools/build/lib/buildeventstream/LocalFilesArtifactUploader.java +++ b/src/main/java/com/google/devtools/build/lib/buildeventstream/LocalFilesArtifactUploader.java @@ -72,5 +72,10 @@ public String apply(Path path) { } return FILE_URI_PATH_CONVERTER.apply(path); } + + @Override + public String applyForDigest(String hash, long sizeBytes) { + throw new IllegalStateException("Can't formulate file:// path for a remote artifact"); + } } } diff --git a/src/main/java/com/google/devtools/build/lib/buildeventstream/PathConverter.java b/src/main/java/com/google/devtools/build/lib/buildeventstream/PathConverter.java index b09073ff974245..b85f0b32f0e4c9 100644 --- a/src/main/java/com/google/devtools/build/lib/buildeventstream/PathConverter.java +++ b/src/main/java/com/google/devtools/build/lib/buildeventstream/PathConverter.java @@ -25,14 +25,23 @@ */ public interface PathConverter { /** An implementation that throws on every call to {@link #apply(Path)}. */ - PathConverter NO_CONVERSION = - path -> { - throw new IllegalStateException( - String.format( - "Can't convert '%s', as it has not been" - + "declared as a referenced artifact of a build event", - path.getPathString())); - }; + PathConverter NO_CONVERSION = new InvalidConverter(); + + final class InvalidConverter implements PathConverter { + @Override + public String apply(Path path) { + throw new IllegalStateException( + String.format( + "Can't convert '%s', as it has not been" + + "declared as a referenced artifact of a build event", + path.getPathString())); + } + + @Override + public String applyForDigest(String hash, long sizeBytes) { + throw new IllegalStateException("Can't convert remote digest"); + } + } /** A {@link PathConverter} that returns a path formatted as a URI with a {@code file} scheme. */ // TODO(ulfjack): Make this a static final field. @@ -43,6 +52,11 @@ public String apply(Path path) { return pathToUriString(path.getPathString()); } + @Override + public String applyForDigest(String hash, long sizeBytes) { + throw new IllegalStateException("Can't formulate file:// path for a remote artifact"); + } + /** * Returns the path encoded as an {@link URI}. * @@ -88,4 +102,6 @@ static String pathToUriString(String path) { */ @Nullable String apply(Path path); + + String applyForDigest(String hash, long sizeBytes); } diff --git a/src/main/java/com/google/devtools/build/lib/exec/StandaloneTestStrategy.java b/src/main/java/com/google/devtools/build/lib/exec/StandaloneTestStrategy.java index 885ff46774c44c..fb4a90be59f4ce 100644 --- a/src/main/java/com/google/devtools/build/lib/exec/StandaloneTestStrategy.java +++ b/src/main/java/com/google/devtools/build/lib/exec/StandaloneTestStrategy.java @@ -21,6 +21,7 @@ import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Maps; +import com.google.common.hash.HashCode; import com.google.common.io.ByteStreams; import com.google.common.util.concurrent.ListenableFuture; import com.google.devtools.build.lib.actions.ActionExecutionContext; @@ -46,6 +47,7 @@ import com.google.devtools.build.lib.analysis.test.TestResult; import com.google.devtools.build.lib.analysis.test.TestRunnerAction; import com.google.devtools.build.lib.analysis.test.TestRunnerAction.ResolvedPaths; +import com.google.devtools.build.lib.analysis.test.TestRunnerAction.TestOutputInfo; import com.google.devtools.build.lib.analysis.test.TestStrategy; import com.google.devtools.build.lib.buildeventstream.BuildEventStreamProtos; import com.google.devtools.build.lib.buildeventstream.BuildEventStreamProtos.TestResult.ExecutionInfo; @@ -64,13 +66,17 @@ import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.build.lib.vfs.PathFragment; import com.google.devtools.build.lib.view.test.TestStatus.BlazeTestStatus; +import com.google.devtools.build.lib.view.test.TestStatus.File; +import com.google.devtools.build.lib.view.test.TestStatus.PathMetadata; import com.google.devtools.build.lib.view.test.TestStatus.TestCase; import com.google.devtools.build.lib.view.test.TestStatus.TestResultData; +import com.google.devtools.build.lib.view.test.TestStatus; import java.io.Closeable; import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; import java.time.Duration; +import java.util.Collection; import java.util.List; import java.util.Map; import java.util.TreeMap; @@ -160,22 +166,18 @@ public TestRunnerSpawn createTestRunnerSpawn( private ImmutableSet createSpawnOutputs(TestRunnerAction action) { ImmutableSet.Builder builder = ImmutableSet.builder(); for (ActionInput output : action.getSpawnOutputs()) { - if (output.getExecPath().equals(action.getXmlOutputPath())) { - // HACK: Convert type of test.xml from BasicActionInput to DerivedArtifact. We want to - // inject metadata of test.xml if it is generated remotely and it's currently only possible - // to inject Artifact. - builder.add(createArtifactOutput(action, output.getExecPath())); - } else { - builder.add(output); - } + // HACK: Convert all outputs from BasicActionInput to DerivedArtifact. We want to + // inject metadata of all outputs if it is generated remotely and it's currently only possible + // to inject Artifact. + builder.add(createArtifactOutput(action, output.getExecPath())); } return builder.build(); } - private static ImmutableList> renameOutputs( + private static ImmutableList renameOutputs( ActionExecutionContext actionExecutionContext, TestRunnerAction action, - ImmutableList> testOutputs, + ImmutableList testOutputs, int attemptId) throws IOException { // Rename outputs @@ -189,12 +191,12 @@ private static ImmutableList> renameOutputs( // Get the normal test output paths, and then update them to use "attempt_N" names, and // attemptDir, before adding them to the outputs. - ImmutableList.Builder> testOutputsBuilder = new ImmutableList.Builder<>(); - for (Pair testOutput : testOutputs) { + ImmutableList.Builder testOutputsBuilder = new ImmutableList.Builder<>(); + for (TestOutputInfo testOutput : testOutputs) { // e.g. /testRoot/test.dir/file, an example we follow throughout this loop's comments. - Path testOutputPath = testOutput.getSecond(); + Path testOutputPath = testOutput.getPath(); Path destinationPath; - if (testOutput.getFirst().equals(TestFileNameConstants.TEST_LOG)) { + if (testOutput.getKey().equals(TestFileNameConstants.TEST_LOG)) { // The rename rules for the test log are different than for all the other files. destinationPath = testLog; } else { @@ -214,7 +216,7 @@ private static ImmutableList> renameOutputs( // Move to the destination. testOutputPath.renameTo(destinationPath); - testOutputsBuilder.add(Pair.of(testOutput.getFirst(), destinationPath)); + testOutputsBuilder.add(new TestOutputInfo(testOutput.getKey(), destinationPath, testOutput.getFileArtifactValue())); } return testOutputsBuilder.build(); } @@ -222,16 +224,18 @@ private static ImmutableList> renameOutputs( private StandaloneFailedAttemptResult processFailedTestAttempt( int attemptId, ActionExecutionContext actionExecutionContext, + TestMetadataHandler testMetadataHandler, TestRunnerAction action, StandaloneTestResult result) throws IOException { return processTestAttempt( - attemptId, /*isLastAttempt=*/ false, actionExecutionContext, action, result); + attemptId, /*isLastAttempt=*/ false, actionExecutionContext, testMetadataHandler, action, result); } private void finalizeTest( TestRunnerAction action, ActionExecutionContext actionExecutionContext, + TestMetadataHandler testMetadataHandler, StandaloneTestResult standaloneTestResult, List failedAttempts) throws IOException { @@ -239,6 +243,7 @@ private void finalizeTest( failedAttempts.size() + 1, /*isLastAttempt=*/ true, actionExecutionContext, + testMetadataHandler, action, standaloneTestResult); @@ -263,22 +268,25 @@ private StandaloneFailedAttemptResult processTestAttempt( int attemptId, boolean isLastAttempt, ActionExecutionContext actionExecutionContext, + TestMetadataHandler testMetadataHandler, TestRunnerAction action, StandaloneTestResult result) throws IOException { - ImmutableList> testOutputs = - action.getTestOutputsMapping( - actionExecutionContext.getPathResolver(), actionExecutionContext.getExecRoot()); + ImmutableMap generatedOutputs = testMetadataHandler.getInjectedFiles(); + ImmutableList testOutputs = action.getTestOutputsMapping( + actionExecutionContext.getPathResolver(), actionExecutionContext.getExecRoot(), + generatedOutputs); + if (!isLastAttempt) { testOutputs = renameOutputs(actionExecutionContext, action, testOutputs, attemptId); } // Recover the test log path, which may have been renamed, and add it to the data builder. Path renamedTestLog = null; - for (Pair pair : testOutputs) { - if (TestFileNameConstants.TEST_LOG.equals(pair.getFirst())) { + for (TestOutputInfo testOutputInfo : testOutputs) { + if (TestFileNameConstants.TEST_LOG.equals(testOutputInfo.getKey())) { Preconditions.checkState(renamedTestLog == null, "multiple test_log matches"); - renamedTestLog = pair.getSecond(); + renamedTestLog = testOutputInfo.getPath(); } } @@ -297,13 +305,31 @@ private StandaloneFailedAttemptResult processTestAttempt( dataBuilder.addFailedLogs(renamedTestLog.toString()); } + TestStatus.TestAttempt.Builder attemptDataBuilder = TestStatus.TestAttempt.newBuilder(); + ImmutableMap.Builder fileNameToMetadata = ImmutableMap.builder(); + for (TestOutputInfo testOutputInfo : testOutputs) { + File.Builder fileBuilder = File.newBuilder().setName(testOutputInfo.getKey()); + if (testOutputInfo.getFileArtifactValue() != null) { + PathMetadata md = PathMetadata.newBuilder() + .setHash(HashCode.fromBytes(testOutputInfo.getFileArtifactValue().getDigest()).toString()) + .setSizeBytes(testOutputInfo.getFileArtifactValue().getSize()).build(); + fileNameToMetadata.put(testOutputInfo.getKey(), md); + } else { + fileBuilder.setPath(testOutputInfo.getPath().toString()); + } + attemptDataBuilder.addOutputs(fileBuilder.build()); + } + attemptDataBuilder.putAllPathMetadata(fileNameToMetadata.build()); + dataBuilder.addAttempts(attemptDataBuilder.build()); + // Add the test log to the output TestResultData data = dataBuilder.build(); + ImmutableList> files = action.testResultDataToMapping(actionExecutionContext.getExecRoot(), data); actionExecutionContext .getEventHandler() .post( TestAttempt.forExecutedTestResult( - action, data, attemptId, testOutputs, result.executionInfo(), isLastAttempt)); + action, data, attemptId, files, result.executionInfo(), isLastAttempt)); processTestOutput(actionExecutionContext, data, action.getTestName(), renamedTestLog); return new StandaloneFailedAttemptResult(data); } @@ -344,6 +370,12 @@ public ActionInput getInput(String execPath) { @Nullable @Override public FileArtifactValue getMetadata(ActionInput input) throws IOException { + Artifact artifact = (Artifact) input; + if (!fileInjected(artifact)) { + // Optional test outputs may not be injected, and the base metadata handler only + // allows retrieving metadata for existing artifacts. + return null; + } return metadataHandler.getMetadata(input); } @@ -399,12 +431,17 @@ public void injectTree(SpecialArtifact output, TreeArtifactValue tree) { public boolean fileInjected(Artifact output) { return fileMetadataMap.containsKey(output); } + + public ImmutableMap getInjectedFiles() { + return ImmutableMap.copyOf(fileMetadataMap); + } } private TestAttemptContinuation beginTestAttempt( TestRunnerAction testAction, Spawn spawn, ActionExecutionContext actionExecutionContext, + @Nullable TestMetadataHandler testMetadataHandler, Path execRoot) throws IOException, InterruptedException { ResolvedPaths resolvedPaths = testAction.resolve(execRoot); @@ -418,15 +455,6 @@ private TestAttemptContinuation beginTestAttempt( Reporter.outErrForReporter(actionExecutionContext.getEventHandler()), out); } - // We use TestMetadataHandler here mainly because the one provided by actionExecutionContext - // doesn't allow to inject undeclared outputs and test.xml is undeclared by the test action. - TestMetadataHandler testMetadataHandler = null; - if (actionExecutionContext.getMetadataHandler() != null) { - testMetadataHandler = - new TestMetadataHandler( - actionExecutionContext.getMetadataHandler(), testAction.getOutputs()); - } - long startTimeMillis = actionExecutionContext.getClock().currentTimeMillis(); SpawnStrategyResolver resolver = actionExecutionContext.getContext(SpawnStrategyResolver.class); SpawnContinuation spawnContinuation; @@ -640,6 +668,7 @@ private final class StandaloneTestRunnerSpawn implements TestRunnerSpawn { private final Path tmpDir; private final Path workingDirectory; private final Path execRoot; + private final TestMetadataHandler testMetadataHandler; StandaloneTestRunnerSpawn( TestRunnerAction testAction, @@ -654,6 +683,11 @@ private final class StandaloneTestRunnerSpawn implements TestRunnerSpawn { this.tmpDir = tmpDir; this.workingDirectory = workingDirectory; this.execRoot = execRoot; + // We use TestMetadataHandler here mainly because the one provided by actionExecutionContext + // doesn't allow to inject undeclared outputs and test.xml is undeclared by the test action. + this.testMetadataHandler = + new TestMetadataHandler( + actionExecutionContext.getMetadataHandler(), testAction.getOutputs()); } @Override @@ -664,7 +698,7 @@ public ActionExecutionContext getActionExecutionContext() { @Override public TestAttemptContinuation beginExecution() throws InterruptedException, IOException { prepareFileSystem(testAction, execRoot, tmpDir, workingDirectory); - return beginTestAttempt(testAction, spawn, actionExecutionContext, execRoot); + return beginTestAttempt(testAction, spawn, actionExecutionContext, testMetadataHandler, execRoot); } @Override @@ -676,7 +710,7 @@ public int getMaxAttempts(TestAttemptResult firstTestAttemptResult) { public FailedAttemptResult finalizeFailedTestAttempt( TestAttemptResult testAttemptResult, int attempt) throws IOException { return processFailedTestAttempt( - attempt, actionExecutionContext, testAction, (StandaloneTestResult) testAttemptResult); + attempt, actionExecutionContext, testMetadataHandler, testAction, (StandaloneTestResult) testAttemptResult); } @Override @@ -684,7 +718,7 @@ public void finalizeTest( TestAttemptResult finalResult, List failedAttempts) throws IOException { StandaloneTestStrategy.this.finalizeTest( - testAction, actionExecutionContext, (StandaloneTestResult) finalResult, failedAttempts); + testAction, actionExecutionContext, testMetadataHandler, (StandaloneTestResult) finalResult, failedAttempts); } @Override diff --git a/src/main/java/com/google/devtools/build/lib/remote/ByteStreamBuildEventArtifactUploader.java b/src/main/java/com/google/devtools/build/lib/remote/ByteStreamBuildEventArtifactUploader.java index 5d6d58b7ca64aa..9ace820f02494c 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/ByteStreamBuildEventArtifactUploader.java +++ b/src/main/java/com/google/devtools/build/lib/remote/ByteStreamBuildEventArtifactUploader.java @@ -372,9 +372,14 @@ public String apply(Path path) { throw new IllegalStateException( String.format("Illegal file reference: '%s'", path.getPathString())); } + return applyForDigest(digest.getHash(), digest.getSizeBytes()); + } + + @Override + public String applyForDigest(String hash, long sizeBytes) { return String.format( "bytestream://%s/blobs/%s/%d", - remoteServerInstanceName, digest.getHash(), digest.getSizeBytes()); + remoteServerInstanceName, hash, sizeBytes); } } } diff --git a/src/main/protobuf/test_status.proto b/src/main/protobuf/test_status.proto index 72cb268607f0c0..3245793ce092ea 100644 --- a/src/main/protobuf/test_status.proto +++ b/src/main/protobuf/test_status.proto @@ -70,6 +70,21 @@ message TestCase { optional bool run = 8 [default = true]; }; +message PathMetadata { + optional string hash = 1; + optional int64 size_bytes = 2; +} + +message File { + optional string name = 1; + optional string path = 2; +} + +message TestAttempt { + repeated File outputs = 1; + map path_metadata = 2; +} + // TestResultData holds the outcome data for a single test action (A // single test rule can result in multiple actions due to sharding and // runs_per_test settings.) @@ -111,4 +126,7 @@ message TestResultData { // Additional build info optional TestCase test_case = 13; optional FailedTestCasesStatus failed_status = 14; + + // Detailed info about each attempt. + repeated TestAttempt attempts = 17; }; diff --git a/src/test/java/com/google/devtools/build/lib/runtime/BuildEventStreamerTest.java b/src/test/java/com/google/devtools/build/lib/runtime/BuildEventStreamerTest.java index 6fc94eb418ce4a..fecaaaf19e95c6 100644 --- a/src/test/java/com/google/devtools/build/lib/runtime/BuildEventStreamerTest.java +++ b/src/test/java/com/google/devtools/build/lib/runtime/BuildEventStreamerTest.java @@ -129,7 +129,16 @@ public ArtifactGroupNamer artifactGroupNamer() { @Override public PathConverter pathConverter() { - return Path::toString; + return new PathConverter() { + @Override + public String apply(Path path) { + return path.toString(); + } + @Override + public String applyForDigest(String hash, long sizeBytes) { + throw new IllegalStateException("Can't formulate file:// path for a remote artifact"); + } + }; } @Override diff --git a/src/test/java/com/google/devtools/build/lib/runtime/TestResultAggregatorTest.java b/src/test/java/com/google/devtools/build/lib/runtime/TestResultAggregatorTest.java index 9b2ae8f8f1cc80..9970f9a5ed1a13 100644 --- a/src/test/java/com/google/devtools/build/lib/runtime/TestResultAggregatorTest.java +++ b/src/test/java/com/google/devtools/build/lib/runtime/TestResultAggregatorTest.java @@ -251,13 +251,13 @@ private TestResultAggregator createAggregatorWithTestRuns(int testRuns) { private static TestResult testResult(TestResultData.Builder data, boolean locallyCached) { TestRunnerAction mockTestAction = mock(TestRunnerAction.class); - when(mockTestAction.getTestOutputsMapping(any(), any())).thenReturn(ImmutableList.of()); + when(mockTestAction.getTestOutputsMapping(any(), any(), any())).thenReturn(ImmutableList.of()); return new TestResult(mockTestAction, data.build(), locallyCached, /*systemFailure=*/ null); } private static TestResult shardedTestResult(TestResultData.Builder data, int shardNum) { TestRunnerAction mockTestAction = mock(TestRunnerAction.class); - when(mockTestAction.getTestOutputsMapping(any(), any())).thenReturn(ImmutableList.of()); + when(mockTestAction.getTestOutputsMapping(any(), any(), any())).thenReturn(ImmutableList.of()); when(mockTestAction.getShardNum()).thenReturn(shardNum); return new TestResult(mockTestAction, data.build(), /*cached=*/ false, /*systemFailure=*/ null); }