Skip to content

Commit

Permalink
Refactor test action preparation
Browse files Browse the repository at this point in the history
Move all deletion / directory creation to prepareFileSystem.

PiperOrigin-RevId: 245374483
  • Loading branch information
ulfjack authored and copybara-github committed Apr 26, 2019
1 parent 48c3be8 commit fb04c5b
Show file tree
Hide file tree
Showing 4 changed files with 28 additions and 19 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -923,6 +923,10 @@ public Path getXmlOutputPath() {
return getPath(xmlOutputPath);
}

public Path getCoverageDirectory() {
return getPath(TestRunnerAction.this.getCoverageDirectory());
}

public Path getCoverageDataPath() {
return getPath(getCoverageData().getExecPath());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,6 @@ public TestRunnerSpawn createTestRunnerSpawn(
TestRunnerAction action, ActionExecutionContext actionExecutionContext)
throws ExecException, InterruptedException {
Path execRoot = actionExecutionContext.getExecRoot();
Path coverageDir = execRoot.getRelative(action.getCoverageDirectory());
Path runfilesDir =
getLocalRunfilesDirectory(
action,
Expand Down Expand Up @@ -143,7 +142,7 @@ public TestRunnerSpawn createTestRunnerSpawn(
ImmutableList.copyOf(action.getSpawnOutputs()),
localResourceUsage);
return new StandaloneTestRunnerSpawn(
action, actionExecutionContext, spawn, tmpDir, coverageDir, workingDirectory, execRoot);
action, actionExecutionContext, spawn, tmpDir, workingDirectory, execRoot);
}

private StandaloneFailedAttemptResult processFailedTestAttempt(
Expand Down Expand Up @@ -454,7 +453,6 @@ private final class StandaloneTestRunnerSpawn implements TestRunnerSpawn {
private final ActionExecutionContext actionExecutionContext;
private final Spawn spawn;
private final Path tmpDir;
private final Path coverageDir;
private final Path workingDirectory;
private final Path execRoot;

Expand All @@ -463,14 +461,12 @@ private final class StandaloneTestRunnerSpawn implements TestRunnerSpawn {
ActionExecutionContext actionExecutionContext,
Spawn spawn,
Path tmpDir,
Path coverageDir,
Path workingDirectory,
Path execRoot) {
this.testAction = testAction;
this.actionExecutionContext = actionExecutionContext;
this.spawn = spawn;
this.tmpDir = tmpDir;
this.coverageDir = coverageDir;
this.workingDirectory = workingDirectory;
this.execRoot = execRoot;
}
Expand All @@ -483,7 +479,7 @@ public ActionExecutionContext getActionExecutionContext() {
@Override
public TestAttemptContinuation beginExecution()
throws InterruptedException, IOException, ExecException {
prepareFileSystem(testAction, tmpDir, coverageDir, workingDirectory);
prepareFileSystem(testAction, actionExecutionContext.getExecRoot(), tmpDir, workingDirectory);
return beginTestAttempt(testAction, spawn, actionExecutionContext, execRoot);
}

Expand Down
27 changes: 18 additions & 9 deletions src/main/java/com/google/devtools/build/lib/exec/TestStrategy.java
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
import com.google.devtools.build.lib.analysis.test.TestConfiguration;
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.TestTargetExecutionSettings;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.events.Event;
Expand Down Expand Up @@ -63,24 +64,32 @@ public abstract class TestStrategy implements TestActionContext {
* not result in stale files.
*/
protected void prepareFileSystem(
TestRunnerAction testAction, Path tmpDir, Path coverageDir, Path workingDirectory)
TestRunnerAction testAction, Path execRoot, Path tmpDir, Path workingDirectory)
throws IOException {
if (tmpDir != null) {
recreateDirectory(tmpDir);
}
if (workingDirectory != null) {
workingDirectory.createDirectoryAndParents();
}

ResolvedPaths resolvedPaths = testAction.resolve(execRoot);
if (testAction.isCoverageMode()) {
recreateDirectory(coverageDir);
recreateDirectory(resolvedPaths.getCoverageDirectory());
}
recreateDirectory(tmpDir);
workingDirectory.createDirectoryAndParents();

resolvedPaths.getBaseDir().createDirectoryAndParents();
resolvedPaths.getUndeclaredOutputsDir().createDirectoryAndParents();
resolvedPaths.getUndeclaredOutputsAnnotationsDir().createDirectoryAndParents();
resolvedPaths.getSplitLogsDir().createDirectoryAndParents();
}

/**
* Ensures that all directories used to run test are in the correct state and their content will
* not result in stale files. Only use this if no local tmp and working directory are required.
*/
protected void prepareFileSystem(TestRunnerAction testAction, Path coverageDir)
throws IOException {
if (testAction.isCoverageMode()) {
recreateDirectory(coverageDir);
}
protected void prepareFileSystem(TestRunnerAction testAction, Path execRoot) throws IOException {
prepareFileSystem(testAction, execRoot, null, null);
}

/** Removes directory if it exists and recreates it. */
Expand Down
8 changes: 4 additions & 4 deletions src/test/shell/bazel/bazel_test_test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -717,12 +717,12 @@ EOF
bazel test -s //dir:test &> $TEST_log || fail "expected success"

# Check that the undeclared outputs directory doesn't exist.
outputs_dir=bazel-testlogs/dir/test/test.outputs/
[ ! -d $outputs_dir ] || fail "$outputs_dir was present after test"
outputs_zip=bazel-testlogs/dir/test/test.outputs/outputs.zip
[ ! -e $outputs_zip ] || fail "$outputs_zip was present after test"

# Check that the undeclared outputs manifest directory doesn't exist.
outputs_manifest_dir=bazel-testlogs/dir/test/test.outputs_manifest/
[ ! -d $outputs_manifest_dir ] || fail "$outputs_manifest_dir was present after test"
outputs_manifest=bazel-testlogs/dir/test/test.outputs_manifest/MANIFEST
[ ! -d $outputs_manifest ] || fail "$outputs_manifest was present after test"
}

function test_test_with_nobuild_runfile_manifests() {
Expand Down

0 comments on commit fb04c5b

Please sign in to comment.