From 19fc15ebbf6c63fcce90a038e91c5ec726852848 Mon Sep 17 00:00:00 2001 From: larsrc Date: Mon, 22 Feb 2021 04:53:32 -0800 Subject: [PATCH] Create helper method for sandbox tests, transform existing tests into using it. This CL should not change the test functionality (in one test I changed where some out-of-execroot files live). RELNOTES: None. PiperOrigin-RevId: 358794463 --- .../java/com/google/devtools/build/lib/BUILD | 1 + .../build/lib/worker/WorkerExecRootTest.java | 337 +++++++++++++----- 2 files changed, 243 insertions(+), 95 deletions(-) diff --git a/src/test/java/com/google/devtools/build/lib/BUILD b/src/test/java/com/google/devtools/build/lib/BUILD index b19e1d0b26af24..f2c2310cdab7af 100644 --- a/src/test/java/com/google/devtools/build/lib/BUILD +++ b/src/test/java/com/google/devtools/build/lib/BUILD @@ -585,6 +585,7 @@ java_test( "//src/main/java/com/google/devtools/build/lib/worker", "//src/main/java/com/google/devtools/common/options:options_internal", "//src/main/protobuf:worker_protocol_java_proto", + "//src/test/java/com/google/devtools/build/lib/actions/util", "//src/test/java/com/google/devtools/build/lib/events:testutil", "//src/test/java/com/google/devtools/build/lib/testutil", "//src/test/java/com/google/devtools/build/lib/testutil:JunitUtils", diff --git a/src/test/java/com/google/devtools/build/lib/worker/WorkerExecRootTest.java b/src/test/java/com/google/devtools/build/lib/worker/WorkerExecRootTest.java index 1ee67be78f0760..35ce642712cb7b 100644 --- a/src/test/java/com/google/devtools/build/lib/worker/WorkerExecRootTest.java +++ b/src/test/java/com/google/devtools/build/lib/worker/WorkerExecRootTest.java @@ -15,8 +15,8 @@ import static com.google.common.truth.Truth.assertThat; -import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; +import com.google.devtools.build.lib.actions.util.ActionsTestUtil; import com.google.devtools.build.lib.sandbox.SandboxHelpers.SandboxInputs; import com.google.devtools.build.lib.sandbox.SandboxHelpers.SandboxOutputs; import com.google.devtools.build.lib.testutil.TestUtils; @@ -28,7 +28,11 @@ import com.google.devtools.build.lib.vfs.inmemoryfs.InMemoryFileSystem; import java.io.IOException; import java.nio.charset.Charset; +import java.util.ArrayList; import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.stream.Collectors; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; @@ -59,20 +63,18 @@ public final void setupTestDirs() throws IOException { @Test public void cleanFileSystem() throws Exception { + SandboxHelper sandboxHelper = + new SandboxHelper(workspaceDir, execRoot) + .addAndCreateInputFile("worker.sh", "worker.sh", "#!/bin/bash") + .addOutput("very/output.txt") + .addWorkerFile("worker.sh"); Path workerSh = workspaceDir.getRelative("worker.sh"); - FileSystemUtils.writeContentAsLatin1(workerSh, "#!/bin/bash"); - - SandboxInputs inputs = - new SandboxInputs( - ImmutableMap.of(PathFragment.create("worker.sh"), workerSh), - ImmutableSet.of(), - ImmutableMap.of()); - SandboxOutputs outputs = - SandboxOutputs.create( - ImmutableSet.of(PathFragment.create("very/output.txt")), ImmutableSet.of()); - ImmutableSet workerFiles = ImmutableSet.of(PathFragment.create("worker.sh")); + WorkerExecRoot workerExecRoot = new WorkerExecRoot(execRoot); - workerExecRoot.createFileSystem(workerFiles, inputs, outputs); + workerExecRoot.createFileSystem( + sandboxHelper.getWorkerFiles(), + sandboxHelper.getSandboxInputs(), + sandboxHelper.getSandboxOutputs()); // Pretend to do some work inside the execRoot. execRoot.getRelative("tempdir").createDirectory(); @@ -82,7 +84,10 @@ public void cleanFileSystem() throws Exception { FileSystemUtils.writeContentAsLatin1(workerSh, "#!/bin/sh"); // Reuse the same execRoot. - workerExecRoot.createFileSystem(workerFiles, inputs, outputs); + workerExecRoot.createFileSystem( + sandboxHelper.getWorkerFiles(), + sandboxHelper.getSandboxInputs(), + sandboxHelper.getSandboxOutputs()); assertThat(execRoot.getRelative("worker.sh").exists()).isTrue(); assertThat( @@ -98,24 +103,22 @@ public void cleanFileSystem() throws Exception { public void createsAndCleansInputSymlinks() throws Exception { // Simulate existing symlinks in the exec root to check that `WorkerExecRoot` correctly deletes // the unnecessary ones and updates the ones that don't point to the right target. - FileSystemUtils.ensureSymbolicLink(execRoot.getRelative("dir/input_symlink_1"), "old_content"); - FileSystemUtils.ensureSymbolicLink(execRoot.getRelative("dir/input_symlink_2"), "unchanged"); - FileSystemUtils.ensureSymbolicLink(execRoot.getRelative("dir/input_symlink_3"), "whatever"); - - SandboxInputs inputs = - new SandboxInputs( - ImmutableMap.of(), - ImmutableSet.of(), - ImmutableMap.of( - PathFragment.create("dir/input_symlink_1"), PathFragment.create("new_content"), - PathFragment.create("dir/input_symlink_2"), PathFragment.create("unchanged"))); - SandboxOutputs outputs = SandboxOutputs.create(ImmutableSet.of(), ImmutableSet.of()); - ImmutableSet workerFiles = ImmutableSet.of(); + SandboxHelper sandboxHelper = + new SandboxHelper(workspaceDir, execRoot) + .createSymlink("dir/input_symlink_1", "old_content") + .createSymlink("dir/input_symlink_2", "unchanged") + .createSymlink("dir/input_symlink_3", "whatever") + .addSymlink("dir/input_symlink_1", "new_content") + .addSymlink("dir/input_symlink_2", "unchanged"); + WorkerExecRoot workerExecRoot = new WorkerExecRoot(execRoot); // This should update the `input_symlink_{1,2,3}` according to `SandboxInputs`, i.e., update the // first/second (alternatively leave the second unchanged) and delete the third. - workerExecRoot.createFileSystem(workerFiles, inputs, outputs); + workerExecRoot.createFileSystem( + sandboxHelper.getWorkerFiles(), + sandboxHelper.getSandboxInputs(), + sandboxHelper.getSandboxOutputs()); assertThat(execRoot.getRelative("dir/input_symlink_1").readSymbolicLink()) .isEqualTo(PathFragment.create("new_content")); @@ -126,23 +129,20 @@ public void createsAndCleansInputSymlinks() throws Exception { @Test public void createsOutputDirs() throws Exception { - SandboxInputs inputs = - new SandboxInputs(ImmutableMap.of(), ImmutableSet.of(), ImmutableMap.of()); - SandboxOutputs outputs = - SandboxOutputs.create( - ImmutableSet.of( - PathFragment.create("dir/foo/bar_kt.jar"), - PathFragment.create("dir/foo/bar_kt.jdeps"), - PathFragment.create("dir/foo/bar_kt-sources.jar")), - ImmutableSet.of( - PathFragment.create("dir/foo/_kotlinc/bar_kt_jvm/bar_kt_sourcegenfiles"), - PathFragment.create("dir/foo/_kotlinc/bar_kt_jvm/bar_kt_classes"), - PathFragment.create("dir/foo/_kotlinc/bar_kt_jvm/bar_kt_temp"), - PathFragment.create("dir/foo/_kotlinc/bar_kt_jvm/bar_kt_generated_classes"))); - ImmutableSet workerFiles = ImmutableSet.of(); + SandboxHelper sandboxHelper = + new SandboxHelper(workspaceDir, execRoot) + .addOutput("dir/foo/bar_kt.jar") + .addOutput("dir/foo/bar_kt.jdeps") + .addOutput("dir/foo/bar_kt-sources.jar") + .addOutputDir("dir/foo/_kotlinc/bar_kt_jvm/bar_kt_sourcegenfiles") + .addOutputDir("dir/foo/_kotlinc/bar_kt_jvm/bar_kt_classes") + .addOutputDir("dir/foo/_kotlinc/bar_kt_jvm/bar_kt_temp") + .addOutputDir("dir/foo/_kotlinc/bar_kt_jvm/bar_kt_generated_classes"); WorkerExecRoot workerExecRoot = new WorkerExecRoot(execRoot); - - workerExecRoot.createFileSystem(workerFiles, inputs, outputs); + workerExecRoot.createFileSystem( + sandboxHelper.getWorkerFiles(), + sandboxHelper.getSandboxInputs(), + sandboxHelper.getSandboxOutputs()); assertThat(execRoot.getRelative("dir/foo/_kotlinc/bar_kt_jvm/bar_kt_sourcegenfiles").exists()) .isTrue(); @@ -159,25 +159,22 @@ public void workspaceFilesAreNotDeleted() throws Exception { // We want to check that `WorkerExecRoot` deletes pre-existing symlinks if they're not in the // `SandboxInputs`, but it does not delete the files that the symlinks point to (i.e., the files // in the workspace directory). - Path neededWorkspaceFile = workspaceDir.getRelative("needed_file"); - FileSystemUtils.writeContentAsLatin1(neededWorkspaceFile, "needed workspace content"); - Path otherWorkspaceFile = workspaceDir.getRelative("other_file"); - FileSystemUtils.writeContentAsLatin1(otherWorkspaceFile, "other workspace content"); - - FileSystemUtils.ensureSymbolicLink(execRoot.getRelative("needed_file"), neededWorkspaceFile); - FileSystemUtils.ensureSymbolicLink(execRoot.getRelative("other_file"), otherWorkspaceFile); - - SandboxInputs inputs = - new SandboxInputs( - ImmutableMap.of(PathFragment.create("needed_file"), neededWorkspaceFile), - ImmutableSet.of(), - ImmutableMap.of()); - SandboxOutputs outputs = SandboxOutputs.create(ImmutableSet.of(), ImmutableSet.of()); - ImmutableSet workerFiles = ImmutableSet.of(); + + SandboxHelper sandboxHelper = + new SandboxHelper(workspaceDir, execRoot) + .addInputFile("needed_file", "needed_file") + .createWorkspaceDirFile("needed_file", "needed workspace content") + .createWorkspaceDirFile("other_file", "other workspace content") + .createSymlink("needed_file", neededWorkspaceFile.getPathString()) + .createSymlink("other_file", otherWorkspaceFile.getPathString()); + WorkerExecRoot workerExecRoot = new WorkerExecRoot(execRoot); - workerExecRoot.createFileSystem(workerFiles, inputs, outputs); + workerExecRoot.createFileSystem( + sandboxHelper.getWorkerFiles(), + sandboxHelper.getSandboxInputs(), + sandboxHelper.getSandboxOutputs()); assertThat(execRoot.getRelative("needed_file").readSymbolicLink()) .isEqualTo(neededWorkspaceFile.asFragment()); @@ -193,19 +190,19 @@ public void workspaceFilesAreNotDeleted() throws Exception { public void recreatesEmptyFiles() throws Exception { // Simulate existing non-empty file in the exec root to check that `WorkerExecRoot` will clear // the contents as requested by `SandboxInputs`. - FileSystemUtils.writeContentAsLatin1(execRoot.getRelative("some_file"), "some content"); - - HashMap inputs = new HashMap<>(); - inputs.put(PathFragment.create("some_file"), null); - SandboxInputs sandboxInputs = new SandboxInputs(inputs, ImmutableSet.of(), ImmutableMap.of()); - SandboxOutputs outputs = SandboxOutputs.create(ImmutableSet.of(), ImmutableSet.of()); - ImmutableSet workerFiles = ImmutableSet.of(); - WorkerExecRoot workerExecRoot = new WorkerExecRoot(execRoot); - // This is interesting, because the filepath is a key in `SandboxInputs`, but its value is // `null`, which means "create an empty file". So after `createFileSystem` the file should be // empty. - workerExecRoot.createFileSystem(workerFiles, sandboxInputs, outputs); + SandboxHelper sandboxHelper = + new SandboxHelper(workspaceDir, execRoot) + .createExecRootFile("some_file", "some content") + .addInputFile("some_file", null); + + WorkerExecRoot workerExecRoot = new WorkerExecRoot(sandboxHelper.execRoot); + workerExecRoot.createFileSystem( + sandboxHelper.getWorkerFiles(), + sandboxHelper.getSandboxInputs(), + sandboxHelper.getSandboxOutputs()); assertThat( FileSystemUtils.readContent( @@ -215,37 +212,31 @@ public void recreatesEmptyFiles() throws Exception { @Test public void createsAndDeletesSiblingExternalRepoFiles() throws Exception { - Path fooRepoDir = testRoot.getRelative("external_dir/foo"); - fooRepoDir.createDirectoryAndParents(); + // With the sibling repository layout, external repository source files are no longer symlinked + // under /external//. Instead, they become *siblings* of the main + // workspace files in that they're placed at /..//. Simulate this + // layout and check if inputs are correctly created and irrelevant symlinks are deleted. - fooRepoDir.getRelative("bar").createDirectory(); + Path fooRepoDir = workspaceDir.getRelative("external_dir/foo"); Path input1 = fooRepoDir.getRelative("bar/input1"); - FileSystemUtils.writeContentAsLatin1(input1, "This is input1."); Path input2 = fooRepoDir.getRelative("input2"); - FileSystemUtils.writeContentAsLatin1(input1, "This is input2."); Path random = fooRepoDir.getRelative("bar/random"); - FileSystemUtils.writeContentAsLatin1(input1, "This is random."); - // With the sibling repository layout, external repository source files are no longer symlinked - // under /external//. Instead, they become *siblings* of the main - // workspace files in that they're placed at /..//. Simulate this - // layout and check if inputs are correctly created and irrelevant symlinks are deleted. - FileSystemUtils.ensureSymbolicLink(execRoot.getRelative("../foo/bar/input1"), input1); - FileSystemUtils.ensureSymbolicLink(execRoot.getRelative("../foo/bar/random"), random); - - SandboxInputs inputs = - new SandboxInputs( - ImmutableMap.of( - PathFragment.create("../foo/bar/input1"), - input1, - PathFragment.create("../foo/input2"), - input2), - ImmutableSet.of(), - ImmutableMap.of()); - SandboxOutputs outputs = SandboxOutputs.create(ImmutableSet.of(), ImmutableSet.of()); - ImmutableSet workerFiles = ImmutableSet.of(); + SandboxHelper sandboxHelper = + new SandboxHelper(workspaceDir, execRoot) + .createWorkspaceDirFile("external_dir/foo/bar/input1", "This is input1.") + .createWorkspaceDirFile("external_dir/foo/input2", "This is input2.") + .createWorkspaceDirFile("external_dir/foo/bar/random", "This is random.") + .createSymlink("../foo/bar/input1", input1.getPathString()) + .createSymlink("../foo/bar/random", random.getPathString()) + .addInputFile("../foo/bar/input1", input1.getPathString()) + .addInputFile("../foo/input2", input2.getPathString()); + WorkerExecRoot workerExecRoot = new WorkerExecRoot(execRoot); - workerExecRoot.createFileSystem(workerFiles, inputs, outputs); + workerExecRoot.createFileSystem( + sandboxHelper.getWorkerFiles(), + sandboxHelper.getSandboxInputs(), + sandboxHelper.getSandboxOutputs()); assertThat(execRoot.getRelative("../foo/bar/input1").readSymbolicLink()) .isEqualTo(input1.asFragment()); @@ -253,4 +244,160 @@ public void createsAndDeletesSiblingExternalRepoFiles() throws Exception { .isEqualTo(input2.asFragment()); assertThat(execRoot.getRelative("bar/random").exists()).isFalse(); } + + /** + * Helper class that sets up a sandbox in a more comprehensible way. Handles setting up + * SandboxInputs and SandboxOutputs as well as creating related files. + */ + private static class SandboxHelper { + /** Map from workdir-relative input path to optional real file path. */ + private final Map inputs = new HashMap<>(); + + private final Map virtualInputs = new HashMap<>(); + private final Map symlinks = new HashMap<>(); + private final Map workerFiles = new HashMap<>(); + private final List outputFiles = new ArrayList<>(); + private final List outputDirs = new ArrayList<>(); + + private final Path workspaceDir; + private final Path execRoot; + + public SandboxHelper(Path workspaceDir, Path execRoot) { + this.workspaceDir = workspaceDir; + this.execRoot = execRoot; + } + + /** + * Adds a regular input file at execRootPath under execRoot, with the real file at workspacePath + * under workspaceDir. + */ + public SandboxHelper addInputFile(String execRootPath, String workspacePath) { + inputs.put( + PathFragment.create(execRootPath), + workspacePath != null ? workspaceDir.getRelative(workspacePath) : null); + return this; + } + + /** + * Adds a regular input file at execRootPath under execRoot, with the real file at workspacePath + * under workspaceDir. The real file gets created immediately and filled with {@code contents}. + */ + public SandboxHelper addAndCreateInputFile( + String execRootPath, String workspacePath, String contents) throws IOException { + addInputFile(execRootPath, workspacePath); + Path absPath = workspaceDir.getRelative(workspacePath); + absPath.getParentDirectory().createDirectoryAndParents(); + FileSystemUtils.writeContentAsLatin1(absPath, contents); + return this; + } + + /** Adds a virtual input with some contents. */ + public SandboxHelper addAndCreateVirtualInput(String execRootPath, String contents) { + virtualInputs.put(PathFragment.create(execRootPath), contents); + return this; + } + + /** Adds a symlink to the inputs. */ + public SandboxHelper addSymlink(String execRootPath, String linkTo) { + symlinks.put(PathFragment.create(execRootPath), PathFragment.create(linkTo)); + return this; + } + + /** Adds an output file without creating it. */ + public SandboxHelper addOutput(String execRootPath) { + outputFiles.add(PathFragment.create(execRootPath)); + return this; + } + + /** Adds an output directory without creating it. */ + public SandboxHelper addOutputDir(String execRootPath) { + outputDirs.add( + PathFragment.create(execRootPath.endsWith("/") ? execRootPath : execRootPath + "/")); + return this; + } + + /** Adds a worker file that is created under workspaceDir and referenced under execRoot. */ + public SandboxHelper addWorkerFile(String execRootPath) { + Path absPath = workspaceDir.getRelative(execRootPath); + workerFiles.put(PathFragment.create(execRootPath), absPath); + return this; + } + + /** + * Adds a worker file that is created under workspaceDir and referenced under execRoot. Writes + * the content under workspaceDir. + */ + public SandboxHelper addAndCreateWorkerFile(String execRootPath, String contents) + throws IOException { + addWorkerFile(execRootPath); + Path absPath = workspaceDir.getRelative(execRootPath); + absPath.getParentDirectory().createDirectoryAndParents(); + FileSystemUtils.writeContentAsLatin1(absPath, contents); + return this; + } + + /** Creates a file with {@code contents} at {@code relPath} under execRoot. */ + public SandboxHelper createExecRootFile(String execRootPath, String contents) + throws IOException { + Path absPath = execRoot.getRelative(execRootPath); + absPath.getParentDirectory().createDirectoryAndParents(); + FileSystemUtils.writeContentAsLatin1(absPath, contents); + return this; + } + + /** Creates a file with {@code contents} at {@code relPath} under execRoot. */ + public SandboxHelper createWorkspaceDirFile(String workspaceDirPath, String contents) + throws IOException { + Path absPath = workspaceDir.getRelative(workspaceDirPath); + absPath.getParentDirectory().createDirectoryAndParents(); + FileSystemUtils.writeContentAsLatin1(absPath, contents); + return this; + } + + /** + * Creates a symlink from within the execroot. The destination is just what's written into the + * symlink and thus relative to the created symlink. + */ + public SandboxHelper createSymlink(String execRootPath, String relativeDestination) + throws IOException { + Path fromPath = execRoot.getRelative(execRootPath); + FileSystemUtils.ensureSymbolicLink(fromPath, relativeDestination); + return this; + } + + /** + * Creates a symlink from within the execroot, and creates the target file with the given + * contents. The destination is just what's written into the symlink and thus relative to the + * created symlink. + */ + public SandboxHelper createSymlinkWithContents( + String execRootPath, String relativeDestination, String contents) throws IOException { + createSymlink(execRootPath, relativeDestination); + Path fromPath = execRoot.getRelative(execRootPath); + Path toPath = fromPath.getRelative(relativeDestination); + toPath.getParentDirectory().createDirectoryAndParents(); + FileSystemUtils.writeContentAsLatin1(toPath, contents); + return this; + } + + public SandboxInputs getSandboxInputs() { + return new SandboxInputs( + this.inputs, + virtualInputs.entrySet().stream() + .map( + entry -> + ActionsTestUtil.createVirtualActionInput(entry.getKey(), entry.getValue())) + .collect(Collectors.toSet()), + symlinks); + } + + public SandboxOutputs getSandboxOutputs() { + return SandboxOutputs.create( + ImmutableSet.copyOf(this.outputFiles), ImmutableSet.copyOf(this.outputDirs)); + } + + public ImmutableSet getWorkerFiles() { + return ImmutableSet.copyOf(workerFiles.keySet()); + } + } }