Clean up some unused and under-used TestUtils
getPool was reusing the same thread pool across tests, instead create new pools
for each test.
advanceCurrentTimeSeconds' name gives the illusion it actually moves the clock,
as is customary for mock clocks these days, however it actually slept, which
isn't ideal in tests either.
The rest is unused.
PiperOrigin-RevId: 418832178
diff --git a/src/test/java/com/google/devtools/build/lib/packages/GlobCacheTest.java b/src/test/java/com/google/devtools/build/lib/packages/GlobCacheTest.java
index f1b693f..b830b661 100644
--- a/src/test/java/com/google/devtools/build/lib/packages/GlobCacheTest.java
+++ b/src/test/java/com/google/devtools/build/lib/packages/GlobCacheTest.java
@@ -22,7 +22,6 @@
import com.google.devtools.build.lib.cmdline.PackageIdentifier;
import com.google.devtools.build.lib.packages.Globber.BadGlobException;
import com.google.devtools.build.lib.testutil.Scratch;
-import com.google.devtools.build.lib.testutil.TestUtils;
import com.google.devtools.build.lib.util.Pair;
import com.google.devtools.build.lib.vfs.Path;
import com.google.devtools.build.lib.vfs.PathFragment;
@@ -31,6 +30,8 @@
import java.util.Collection;
import java.util.Collections;
import java.util.List;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
import org.junit.After;
import org.junit.Before;
import org.junit.Test;
@@ -49,6 +50,7 @@
private Path packageDirectory;
private Path buildFile;
+ private ExecutorService cacheThreadPool;
private GlobCache cache;
@Before
@@ -92,7 +94,16 @@
createCache();
}
+ @After
+ public void shutDownThreadPoolIfExists() {
+ if (cacheThreadPool != null) {
+ cacheThreadPool.shutdownNow();
+ }
+ }
+
private void createCache(PathFragment... ignoredDirectories) {
+ shutDownThreadPoolIfExists();
+ cacheThreadPool = Executors.newFixedThreadPool(10);
cache =
new GlobCache(
packageDirectory,
@@ -118,7 +129,7 @@
}
},
null,
- TestUtils.getPool(),
+ cacheThreadPool,
-1,
ThreadStateReceiver.NULL_INSTANCE);
}
diff --git a/src/test/java/com/google/devtools/build/lib/packages/PackageFactoryTest.java b/src/test/java/com/google/devtools/build/lib/packages/PackageFactoryTest.java
index 1710588..d3df2e4 100644
--- a/src/test/java/com/google/devtools/build/lib/packages/PackageFactoryTest.java
+++ b/src/test/java/com/google/devtools/build/lib/packages/PackageFactoryTest.java
@@ -31,7 +31,6 @@
import com.google.devtools.build.lib.events.Event;
import com.google.devtools.build.lib.packages.PackageValidator.InvalidPackageException;
import com.google.devtools.build.lib.packages.util.PackageLoadingTestCase;
-import com.google.devtools.build.lib.testutil.TestUtils;
import com.google.devtools.build.lib.vfs.DigestHashFunction;
import com.google.devtools.build.lib.vfs.Dirent;
import com.google.devtools.build.lib.vfs.FileSystem;
@@ -47,6 +46,8 @@
import java.util.HashMap;
import java.util.List;
import java.util.Set;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
import net.starlark.java.eval.Starlark;
import net.starlark.java.syntax.ParserInput;
import net.starlark.java.syntax.StarlarkFile;
@@ -1240,29 +1241,34 @@
private static void assertGlob(
Package pkg, List<String> expected, List<String> include, List<String> exclude)
throws Exception {
- GlobCache globCache =
- new GlobCache(
- pkg.getFilename().asPath().getParentDirectory(),
- pkg.getPackageIdentifier(),
- ImmutableSet.of(),
- // a package locator that finds no packages
- new CachingPackageLocator() {
- @Override
- public Path getBuildFileForPackage(PackageIdentifier packageName) {
- return null;
- }
+ ExecutorService executorService = Executors.newFixedThreadPool(10);
+ try {
+ GlobCache globCache =
+ new GlobCache(
+ pkg.getFilename().asPath().getParentDirectory(),
+ pkg.getPackageIdentifier(),
+ ImmutableSet.of(),
+ // a package locator that finds no packages
+ new CachingPackageLocator() {
+ @Override
+ public Path getBuildFileForPackage(PackageIdentifier packageName) {
+ return null;
+ }
- @Override
- public String getBaseNameForLoadedPackage(PackageIdentifier packageName) {
- return null;
- }
- },
- null,
- TestUtils.getPool(),
- -1,
- ThreadStateReceiver.NULL_INSTANCE);
- assertThat(globCache.globUnsorted(include, exclude, false, true))
- .containsExactlyElementsIn(expected);
+ @Override
+ public String getBaseNameForLoadedPackage(PackageIdentifier packageName) {
+ return null;
+ }
+ },
+ null,
+ executorService,
+ -1,
+ ThreadStateReceiver.NULL_INSTANCE);
+ assertThat(globCache.globUnsorted(include, exclude, false, true))
+ .containsExactlyElementsIn(expected);
+ } finally {
+ executorService.shutdownNow();
+ }
}
private Path emptyFile(String path) {
diff --git a/src/test/java/com/google/devtools/build/lib/testutil/TestUtils.java b/src/test/java/com/google/devtools/build/lib/testutil/TestUtils.java
index fc06762..1fca902 100644
--- a/src/test/java/com/google/devtools/build/lib/testutil/TestUtils.java
+++ b/src/test/java/com/google/devtools/build/lib/testutil/TestUtils.java
@@ -21,35 +21,15 @@
import java.io.File;
import java.io.IOException;
import java.util.UUID;
-import java.util.concurrent.Executors;
-import java.util.concurrent.ThreadPoolExecutor;
/**
* Some static utility functions for testing.
*/
public class TestUtils {
- public static final ThreadPoolExecutor POOL =
- (ThreadPoolExecutor) Executors.newFixedThreadPool(10);
public static final UUID ZERO_UUID = UUID.fromString("00000000-0000-0000-0000-000000000000");
/**
- * Wait until the {@link System#currentTimeMillis} / 1000 advances.
- *
- * This method takes 0-1000ms to run, 500ms on average.
- */
- public static void advanceCurrentTimeSeconds() throws InterruptedException {
- long currentTimeSeconds = System.currentTimeMillis() / 1000;
- do {
- Thread.sleep(50);
- } while (currentTimeSeconds == System.currentTimeMillis() / 1000);
- }
-
- 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
@@ -99,14 +79,6 @@
return path;
}
- /**
- * 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");
@@ -129,17 +101,6 @@
return tmpDir;
}
- 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);
- }
- if (!dir.mkdir()) {
- throw new IOException("Cannot create a temporary directory " + dir);
- }
- return dir;
- }
-
public static int getRandomSeed() {
// Default value if not running under framework
int randomSeed = 301;
