Skip to content

Commit

Permalink
Add an ImportantOutputHandler method for test outputs.
Browse files Browse the repository at this point in the history
PiperOrigin-RevId: 651932342
Change-Id: If4ff258171345793626bb846ef068da3790fe369
  • Loading branch information
justinhorvitz authored and copybara-github committed Jul 13, 2024
1 parent 41c96be commit 4d62c8f
Show file tree
Hide file tree
Showing 5 changed files with 37 additions and 7 deletions.
1 change: 1 addition & 0 deletions src/main/java/com/google/devtools/build/lib/actions/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -471,6 +471,7 @@ java_library(
":artifacts",
"//src/main/java/com/google/devtools/build/lib/skyframe:detailed_exceptions",
"//src/main/java/com/google/devtools/build/lib/util:detailed_exit_code",
"//src/main/java/com/google/devtools/build/lib/vfs",
"//src/main/java/com/google/devtools/build/lib/vfs:pathfragment",
"//src/main/protobuf:failure_details_java_proto",
"//third_party:guava",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,10 @@
import com.google.devtools.build.lib.server.FailureDetails.FailureDetail;
import com.google.devtools.build.lib.skyframe.DetailedException;
import com.google.devtools.build.lib.util.DetailedExitCode;
import com.google.devtools.build.lib.vfs.Path;
import com.google.devtools.build.lib.vfs.PathFragment;
import java.time.Duration;
import java.util.List;
import java.util.Map;

/** Context to be informed of top-level outputs and their runfiles. */
Expand Down Expand Up @@ -68,6 +71,27 @@ ImmutableMap<String, ActionInput> processRunfilesAndGetLostArtifacts(
InputMetadataProvider metadataProvider)
throws ImportantOutputException, InterruptedException;

/**
* Informs this handler of outputs from a completed test attempt.
*
* <p>The given paths are under the exec root and are backed by an {@link
* com.google.devtools.build.lib.vfs.OutputService#createActionFileSystem action filesystem} if
* applicable.
*
* <p>Test outputs should never be lost. Test actions are not shareable across servers (see {@link
* Actions#dependsOnBuildId}), so outputs passed to this method come from a just-executed test
* action.
*/
void processTestOutputs(List<Path> testOutputs)
throws ImportantOutputException, InterruptedException;

/**
* A threshold to pass to {@link
* com.google.devtools.build.lib.profiler.GoogleAutoProfilerUtils#logged(String, Duration)} for
* profiling {@link ImportantOutputHandler} operations.
*/
Duration LOG_THRESHOLD = Duration.ofMillis(100);

/** Represents an exception encountered during processing of important outputs. */
final class ImportantOutputException extends Exception implements DetailedException {
private final FailureDetail failureDetail;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -177,15 +177,16 @@ interface TestRunnerSpawn {

/** Rename the output files if the test attempt failed, and post the test attempt result. */
FailedAttemptResult finalizeFailedTestAttempt(TestAttemptResult testAttemptResult, int attempt)
throws IOException;
throws IOException, ExecException, InterruptedException;

/** Post the final test result based on the last attempt and the list of failed attempts. */
void finalizeTest(
TestAttemptResult lastTestAttemptResult, List<FailedAttemptResult> failedAttempts)
throws IOException;
throws IOException, ExecException, InterruptedException;

/** Post the final test result based on the last attempt and the list of failed attempts. */
void finalizeCancelledTest(List<FailedAttemptResult> failedAttempts) throws IOException;
void finalizeCancelledTest(List<FailedAttemptResult> failedAttempts)
throws IOException, ExecException, InterruptedException;

/**
* Return a {@link TestRunnerSpawn} object if test fallback is enabled, or {@code null}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,6 @@
import com.google.devtools.build.skyframe.SkyKey;
import com.google.devtools.build.skyframe.SkyValue;
import com.google.devtools.build.skyframe.SkyframeLookupResult;
import java.time.Duration;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.HashSet;
Expand Down Expand Up @@ -141,8 +140,6 @@ EventReportingArtifacts createSucceeded(
throws InterruptedException;
}

private static final Duration IMPORTANT_OUTPUT_HANDLER_LOGGING_THRESHOLD = Duration.ofMillis(100);

private final PathResolverFactory pathResolverFactory;
private final Completor<ValueT, ResultT, KeyT> completor;
private final SkyframeActionExecutor skyframeActionExecutor;
Expand Down Expand Up @@ -542,7 +539,7 @@ private Reset informImportantOutputHandler(
try (var ignored =
GoogleAutoProfilerUtils.logged(
"Informing important output handler of top-level outputs for " + label,
IMPORTANT_OUTPUT_HANDLER_LOGGING_THRESHOLD)) {
ImportantOutputHandler.LOG_THRESHOLD)) {
lostOutputs =
importantOutputHandler.processOutputsAndGetLostArtifacts(
key.topLevelArtifactContext().expandFilesets()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,10 @@
import com.google.devtools.build.lib.exec.ModuleActionContextRegistry;
import com.google.devtools.build.lib.runtime.BlazeModule;
import com.google.devtools.build.lib.runtime.CommandEnvironment;
import com.google.devtools.build.lib.vfs.Path;
import com.google.devtools.build.lib.vfs.PathFragment;
import java.io.IOException;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.function.Function;
Expand Down Expand Up @@ -93,6 +95,11 @@ public ImmutableMap<String, ActionInput> processRunfilesAndGetLostArtifacts(
InputMetadataProvider metadataProvider) {
return getLostOutputs(runfiles.values(), expander, metadataProvider);
}

@Override
public void processTestOutputs(List<Path> testOutputs) {
throw new UnsupportedOperationException();
}
});
}

Expand Down

0 comments on commit 4d62c8f

Please sign in to comment.