Skip to content

Commit

Permalink
Promote the undeclared test outputs to an artifact.
Browse files Browse the repository at this point in the history
The undeclared outputs can be produced in zipped or unzipped form, depending on the value of --zip_undeclared_test_outputs; test-setup.sh is responsible for the zipping. To simplify the code, the TestRunnerAction is declared as unconditionally producing an output directory, which will either contain a single zip file, a bunch of unzipped files, or be empty if no undeclared outputs were produced.

This change is required to make it possible to download the undeclared test outputs in unzipped form when building without the bytes.

Related to #17884.

PiperOrigin-RevId: 535188928
Change-Id: I94e72fe8840fa2274b66a2c8463cf90b4533c3a6
  • Loading branch information
tjgq authored and copybara-github committed May 25, 2023
1 parent 90a6a2f commit 9181f32
Show file tree
Hide file tree
Showing 3 changed files with 25 additions and 11 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -765,6 +765,10 @@ public Artifact getPackageRelativeTreeArtifact(PathFragment relative, ArtifactRo
return getTreeArtifact(getPackageDirectory().getRelative(relative), root);
}

public Artifact getPackageRelativeTreeArtifact(String relative, ArtifactRoot root) {
return getPackageRelativeTreeArtifact(PathFragment.create(relative), root);
}

/**
* Creates an artifact in a directory that is unique to the rule, thus guaranteeing that it never
* clashes with artifacts created by other rules.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -434,6 +434,9 @@ private TestParams createTestAction(int shards)
}
}

Artifact undeclaredOutputsDir =
ruleContext.getPackageRelativeTreeArtifact(dir.getRelative("test.outputs"), root);

boolean cancelConcurrentTests =
testConfiguration.runsPerTestDetectsFlakes()
&& testConfiguration.cancelConcurrentTests();
Expand All @@ -452,6 +455,7 @@ private TestParams createTestAction(int shards)
cacheStatus,
coverageArtifact,
coverageDirectory,
undeclaredOutputsDir,
testProperties,
runfilesSupport
.getActionEnvironment()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,8 +95,11 @@
// Not final so that we can mock it in tests.
public class TestRunnerAction extends AbstractAction
implements NotifyOnActionCacheHit, CommandAction {

public static final PathFragment COVERAGE_TMP_ROOT = PathFragment.create("_coverage");

private static final String UNDECLARED_OUTPUTS_ZIP_NAME = "outputs.zip";

// Used for selecting subset of testcase / testmethods.
private static final String TEST_BRIDGE_TEST_FILTER_ENV = "TESTBRIDGE_TEST_ONLY";

Expand All @@ -118,8 +121,6 @@ public class TestRunnerAction extends AbstractAction
@Nullable private final PathFragment shExecutable;
private final PathFragment splitLogsPath;
private final PathFragment splitLogsDir;
private final PathFragment undeclaredOutputsDir;
private final PathFragment undeclaredOutputsZipPath;
private final PathFragment undeclaredOutputsAnnotationsDir;
private final PathFragment undeclaredOutputsManifestPath;
private final PathFragment undeclaredOutputsAnnotationsPath;
Expand All @@ -136,6 +137,7 @@ public class TestRunnerAction extends AbstractAction

private final Artifact coverageData;
@Nullable private final Artifact coverageDirectory;
private final Artifact undeclaredOutputsDir;
private final TestTargetProperties testProperties;
private final TestTargetExecutionSettings executionSettings;
private final int shardNum;
Expand Down Expand Up @@ -196,6 +198,7 @@ private static ImmutableSet<Artifact> nonNullAsSet(Artifact... artifacts) {
Artifact cacheStatus,
Artifact coverageArtifact,
@Nullable Artifact coverageDirectory,
Artifact undeclaredOutputsDir,
TestTargetProperties testProperties,
ActionEnvironment extraTestEnv,
TestTargetExecutionSettings executionSettings,
Expand All @@ -209,7 +212,11 @@ private static ImmutableSet<Artifact> nonNullAsSet(Artifact... artifacts) {
NestedSetBuilder<Artifact> lcovMergerFilesToRun,
RunfilesSupplier lcovMergerRunfilesSupplier,
PackageSpecificationProvider networkAllowlist) {
super(owner, inputs, nonNullAsSet(testLog, cacheStatus, coverageArtifact, coverageDirectory));
super(
owner,
inputs,
nonNullAsSet(
testLog, cacheStatus, coverageArtifact, coverageDirectory, undeclaredOutputsDir));
Preconditions.checkState((collectCoverageScript == null) == (coverageArtifact == null));
this.runfilesSupplier = runfilesSupplier;
this.testSetupScript = testSetupScript;
Expand All @@ -221,6 +228,7 @@ private static ImmutableSet<Artifact> nonNullAsSet(Artifact... artifacts) {
this.cacheStatus = cacheStatus;
this.coverageData = coverageArtifact;
this.coverageDirectory = coverageDirectory;
this.undeclaredOutputsDir = undeclaredOutputsDir;
this.shardNum = shardNum;
this.runNumber = runNumber;
this.testProperties = checkNotNull(testProperties);
Expand All @@ -243,8 +251,6 @@ private static ImmutableSet<Artifact> nonNullAsSet(Artifact... artifacts) {
this.splitLogsDir = baseDir.getChild("test.raw_splitlogs");
// See note in {@link #getSplitLogsPath} on the choice of file name.
this.splitLogsPath = splitLogsDir.getChild("test.splitlogs");
this.undeclaredOutputsDir = baseDir.getChild("test.outputs");
this.undeclaredOutputsZipPath = undeclaredOutputsDir.getChild("outputs.zip");
this.undeclaredOutputsAnnotationsDir = baseDir.getChild("test.outputs_manifest");
this.undeclaredOutputsManifestPath = undeclaredOutputsAnnotationsDir.getChild("MANIFEST");
this.undeclaredOutputsAnnotationsPath = undeclaredOutputsAnnotationsDir.getChild("ANNOTATIONS");
Expand Down Expand Up @@ -293,7 +299,7 @@ private static ImmutableSet<Artifact> nonNullAsSet(Artifact... artifacts) {
// Note that splitLogsPath points to a file inside the splitLogsDir so it's not
// necessary to delete it explicitly.
splitLogsDir,
undeclaredOutputsDir,
getUndeclaredOutputsDir(),
undeclaredOutputsAnnotationsDir,
baseDir.getRelative("test_attempts"));
}
Expand Down Expand Up @@ -355,7 +361,7 @@ public List<ActionInput> getSpawnOutputs() {
if (testConfiguration.getZipUndeclaredTestOutputs()) {
outputs.add(ActionInputHelper.fromPath(getUndeclaredOutputsZipPath()));
} else {
outputs.add(ActionInputHelper.fromPathToDirectory(getUndeclaredOutputsDir()));
outputs.add(undeclaredOutputsDir);
}
outputs.add(ActionInputHelper.fromPath(getUndeclaredOutputsManifestPath()));
outputs.add(ActionInputHelper.fromPath(getUndeclaredOutputsAnnotationsPath()));
Expand Down Expand Up @@ -807,12 +813,12 @@ public PathFragment getSplitLogsPath() {
}

public PathFragment getUndeclaredOutputsDir() {
return undeclaredOutputsDir;
return undeclaredOutputsDir.getExecPath();
}

/** Returns path to the optional zip file of undeclared test outputs. */
public PathFragment getUndeclaredOutputsZipPath() {
return undeclaredOutputsZipPath;
return getUndeclaredOutputsDir().getChild(UNDECLARED_OUTPUTS_ZIP_NAME);
}

/** Returns path to the undeclared output manifest file. */
Expand Down Expand Up @@ -1069,12 +1075,12 @@ public Path getSplitLogsDir() {

/** Returns path to the optional zip file of undeclared test outputs. */
public Path getUndeclaredOutputsZipPath() {
return getPath(undeclaredOutputsZipPath);
return getUndeclaredOutputsDir().getChild(UNDECLARED_OUTPUTS_ZIP_NAME);
}

/** Returns path to the directory to hold undeclared test outputs. */
public Path getUndeclaredOutputsDir() {
return getPath(undeclaredOutputsDir);
return getPath(undeclaredOutputsDir.getExecPath());
}

/** Returns path to the directory to hold undeclared output annotations parts. */
Expand Down

0 comments on commit 9181f32

Please sign in to comment.