Skip to content

Commit

Permalink
Create TestUtils methods to generate unique temp dirs, deprecate the …
Browse files Browse the repository at this point in the history
…current ones that are decidedly non-unique.

Unique temp dirs may take longer to set up, but not as long as dealing with flaky tests.

PiperOrigin-RevId: 345428489
  • Loading branch information
larsrc-google authored and copybara-github committed Dec 3, 2020
1 parent 504a5b7 commit 41b3ab6
Show file tree
Hide file tree
Showing 9 changed files with 73 additions and 27 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@

import com.google.devtools.build.lib.testutil.TestUtils;
import java.io.File;
import java.io.IOException;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.JUnit4;
Expand All @@ -28,8 +29,8 @@ public class ShardingEnvironmentTest {

@SuppressWarnings({"ResultOfMethodCallIgnored"})
@Test
public void testTouchShardingFile() {
File shardFile = new File(TestUtils.tmpDirFile(), "shard_file_123");
public void testTouchShardingFile() throws IOException {
File shardFile = TestUtils.createUniqueTmpDir(null).getChild("shard_file_123").getPathFile();
assertThat(shardFile.exists()).isFalse();
try {
ShardingEnvironment.touchShardFile(shardFile);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@
import com.google.devtools.build.lib.vfs.Root;
import com.google.devtools.build.lib.vfs.util.FileSystems;
import com.google.devtools.common.options.OptionsParser;
import java.io.FileNotFoundException;
import java.io.IOException;
import java.util.Arrays;
import java.util.Collection;
Expand Down Expand Up @@ -217,8 +218,7 @@ boolean succeeded() {

@Before
public void setUp() throws Exception {
testRoot = FileSystems.getNativeFileSystem().getPath(TestUtils.tmpDir()).getRelative("test");
testRoot.deleteTreesBelow();
testRoot = TestUtils.createUniqueTmpDir(FileSystems.getNativeFileSystem());
outErr = new FileOutErr(testRoot.getRelative("stdout"), testRoot.getRelative("stderr"));
}

Expand Down Expand Up @@ -400,6 +400,13 @@ public void tearDown() throws Exception {
if (executorServiceForCleanup != null) {
executorServiceForCleanup.shutdownNow();
}
if (testRoot != null) {
try {
testRoot.deleteTree();
} catch (FileNotFoundException e) {
// This can happen if one of the dynamic threads are still cleaning up. No big deal.
}
}
}

/** Constructs a new spawn with a custom mnemonic and execution info. */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -640,10 +640,7 @@ public void waitFor() throws InterruptedException {
public void interruptWaitsForProcessExit() throws Exception {
assumeTrue(OS.getCurrent() != OS.WINDOWS);

File tempDirFile = TestUtils.makeTempDir();
tempDirFile.deleteOnExit();
FileSystem fs = new JavaIoFileSystem(DigestHashFunction.SHA256);
Path tempDir = fs.getPath(tempDirFile.getPath());
Path tempDir = TestUtils.createUniqueTmpDir(new JavaIoFileSystem(DigestHashFunction.SHA256));

LocalSpawnRunner runner =
new LocalSpawnRunner(
Expand Down Expand Up @@ -852,10 +849,7 @@ private Path copyCpuTimeSpenderIntoExecRoot(Path execRoot) throws IOException {
}

private Path getTemporaryRoot(FileSystem fs, String name) throws IOException {
File tempDirFile = TestUtils.makeTempDir();
tempDirFile.deleteOnExit();

Path tempDirPath = fs.getPath(tempDirFile.getPath());
Path tempDirPath = TestUtils.createUniqueTmpDir(fs);
assertThat(tempDirPath.exists()).isTrue();

Path root = tempDirPath.getRelative(name);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,7 @@ public class SandboxfsSandboxedSpawnTest {
@Before
public final void setupTestDirs() throws IOException {
FileSystem fileSystem = new InMemoryFileSystem(DigestHashFunction.SHA256);
testRoot = fileSystem.getPath(TestUtils.tmpDir());
testRoot.createDirectoryAndParents();
testRoot = TestUtils.createUniqueTmpDir(fileSystem);

workspaceDir = testRoot.getRelative("workspace");
workspaceDir.createDirectory();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ private void checkLinuxSandboxStatistics(Duration userTimeToSpend, Duration syst
Long.toString(userTimeToSpend.getSeconds()),
Long.toString(systemTimeToSpend.getSeconds()));

Path outputDir = testFS.getPath(TestUtils.makeTempDir().getCanonicalPath());
Path outputDir = TestUtils.createUniqueTmpDir(testFS);
Path statisticsFilePath = outputDir.getRelative("stats.out");

List<String> fullCommandLine =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ private void checkProcessWrapperStatistics(Duration userTimeToSpend, Duration sy
Long.toString(userTimeToSpend.getSeconds()),
Long.toString(systemTimeToSpend.getSeconds()));

Path outputDir = testFS.getPath(TestUtils.makeTempDir().getCanonicalPath());
Path outputDir = TestUtils.createUniqueTmpDir(testFS);
Path statisticsFilePath = outputDir.getRelative("stats.out");

List<String> fullCommandLine =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,7 @@ public final class ExecutionStatisticsTest {
@Before
public final void createFileSystem() throws Exception {
FileSystem testFS = new InMemoryFileSystem(DigestHashFunction.SHA256);
workingDir = testFS.getPath(TestUtils.makeTempDir().getCanonicalPath());
workingDir.createDirectoryAndParents();
workingDir = TestUtils.createUniqueTmpDir(testFS);
}

private Path createExecutionStatisticsProtoFile(
Expand Down
3 changes: 3 additions & 0 deletions src/test/java/com/google/devtools/build/lib/testutil/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,9 @@ java_library(
java_library(
name = "TestUtils",
srcs = ["TestUtils.java"],
deps = [
"//src/main/java/com/google/devtools/build/lib/vfs",
],
)

java_library(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,10 @@

package com.google.devtools.build.lib.testutil;

import com.google.devtools.build.lib.vfs.DigestHashFunction;
import com.google.devtools.build.lib.vfs.FileSystem;
import com.google.devtools.build.lib.vfs.JavaIoFileSystem;
import com.google.devtools.build.lib.vfs.Path;
import java.io.ByteArrayInputStream;
import java.io.ByteArrayOutputStream;
import java.io.File;
Expand Down Expand Up @@ -49,8 +53,14 @@ public static ThreadPoolExecutor getPool() {
return POOL;
}

/**
* Returns the path to a fixed temporary directory, with back-slashes turned into slashes. The
* directory is guaranteed to exist and be unique for the test <em>target</em>. Since test
* <em>cases</em> may run in parallel, prefer using {@link #createUniqueTmpDir} instead, which
* also guarantees that the directory is empty.
*/
public static String tmpDir() {
return tmpDirFile().getAbsolutePath().replaceAll("\\\\", "/");
return tmpDirFile().getAbsolutePath().replace('\\', '/');
}

static String getUserValue(String key) {
Expand All @@ -61,10 +71,48 @@ static String getUserValue(String key) {
return value;
}

/**
* Returns a fixed temporary directory, guaranteed to exist and be unique for the test
* <em>target</em>. Since test <em>cases</em> may run in parallel, prefer using {@link
* #createUniqueTmpDir} instead, which also guarantees that the directory is empty.
*/
public static File tmpDirFile() {
File tmpDir;
File tmpDir = tmpDirRoot();

// Ensure tmpDir exists
if (!tmpDir.isDirectory()) {
tmpDir.mkdirs();
}
return tmpDir;
}

/**
* Creates a unique and empty temporary directory.
*
* @param fileSystem The file system the directory should be created on. If null, uses the Java
* file system.
* @return A newly created directory, extremely likely to be unique.
*/
public static Path createUniqueTmpDir(FileSystem fileSystem) throws IOException {
if (fileSystem == null) {
fileSystem = new JavaIoFileSystem(DigestHashFunction.SHA256);
}
File tmpDirRoot = tmpDirRoot();
Path path = fileSystem.getPath(tmpDirRoot.getPath()).getRelative(UUID.randomUUID().toString());
path.createDirectoryAndParents();
return path;
}

// Flag value specified in environment?
/**
* Creates a unique and empty temporary directory and returns the path, with backslashes turned
* into slashes.
*/
public static String createUniqueTmpDirString() throws IOException {
return createUniqueTmpDir(null).getPathString().replace('\\', '/');
}

private static File tmpDirRoot() {
File tmpDir; // Flag value specified in environment?
String tmpDirStr = getUserValue("TEST_TMPDIR");

if (tmpDirStr != null && tmpDirStr.length() > 0) {
Expand All @@ -82,15 +130,10 @@ public static File tmpDirFile() {
tmpDir = new File(tmpDir, username);
tmpDir = new File(tmpDir, "tmp");
}

// Ensure tmpDir exists
if (!tmpDir.isDirectory()) {
tmpDir.mkdirs();
}
return tmpDir;
}

public static File makeTempDir() throws IOException {
public static File makeTmpDir() throws IOException {
File dir = File.createTempFile(TestUtils.class.getName(), ".temp", tmpDirFile());
if (!dir.delete()) {
throw new IOException("Cannot remove a temporary file " + dir);
Expand Down

0 comments on commit 41b3ab6

Please sign in to comment.