Clean up @{Before,After}Class in lib/remote tests
These were being used to set up an executor per test class. Instead, each test
should have its own executor in order to reduce the odds of state leaking across
tests and poisoning results.
PiperOrigin-RevId: 299959388
diff --git a/src/test/java/com/google/devtools/build/lib/remote/ByteStreamBuildEventArtifactUploaderTest.java b/src/test/java/com/google/devtools/build/lib/remote/ByteStreamBuildEventArtifactUploaderTest.java
index 5affb4b..771bc05 100644
--- a/src/test/java/com/google/devtools/build/lib/remote/ByteStreamBuildEventArtifactUploaderTest.java
+++ b/src/test/java/com/google/devtools/build/lib/remote/ByteStreamBuildEventArtifactUploaderTest.java
@@ -74,9 +74,7 @@
import java.util.concurrent.Executors;
import java.util.concurrent.TimeUnit;
import org.junit.After;
-import org.junit.AfterClass;
import org.junit.Before;
-import org.junit.BeforeClass;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.JUnit4;
@@ -90,7 +88,7 @@
private static final DigestUtil DIGEST_UTIL = new DigestUtil(DigestHashFunction.SHA256);
private final MutableHandlerRegistry serviceRegistry = new MutableHandlerRegistry();
- private static ListeningScheduledExecutorService retryService;
+ private ListeningScheduledExecutorService retryService;
private Server server;
private ManagedChannel channel;
@@ -101,11 +99,6 @@
private final Path execRoot = fs.getPath("/execroot");
private ArtifactRoot outputRoot;
- @BeforeClass
- public static void beforeEverything() {
- retryService = MoreExecutors.listeningDecorator(Executors.newScheduledThreadPool(1));
- }
-
@Before
public final void setUp() throws Exception {
MockitoAnnotations.initMocks(this);
@@ -126,6 +119,8 @@
outputRoot = ArtifactRoot.asDerivedRoot(execRoot, execRoot.getRelative("out"));
outputRoot.getRoot().asPath().createDirectoryAndParents();
+
+ retryService = MoreExecutors.listeningDecorator(Executors.newScheduledThreadPool(1));
}
@After
@@ -134,17 +129,16 @@
// on different threads than the tearDown.
withEmptyMetadata.detach(prevContext);
+ retryService.shutdownNow();
+ retryService.awaitTermination(
+ com.google.devtools.build.lib.testutil.TestUtils.WAIT_TIMEOUT_SECONDS, TimeUnit.SECONDS);
+
channel.shutdownNow();
channel.awaitTermination(5, TimeUnit.SECONDS);
server.shutdownNow();
server.awaitTermination();
}
- @AfterClass
- public static void afterEverything() {
- retryService.shutdownNow();
- }
-
@Before
public void setup() {
MockitoAnnotations.initMocks(this);
diff --git a/src/test/java/com/google/devtools/build/lib/remote/ByteStreamUploaderTest.java b/src/test/java/com/google/devtools/build/lib/remote/ByteStreamUploaderTest.java
index 8592412..8bed33c 100644
--- a/src/test/java/com/google/devtools/build/lib/remote/ByteStreamUploaderTest.java
+++ b/src/test/java/com/google/devtools/build/lib/remote/ByteStreamUploaderTest.java
@@ -73,9 +73,7 @@
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicInteger;
import org.junit.After;
-import org.junit.AfterClass;
import org.junit.Before;
-import org.junit.BeforeClass;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.JUnit4;
@@ -95,7 +93,7 @@
private static final String INSTANCE_NAME = "foo";
private final MutableHandlerRegistry serviceRegistry = new MutableHandlerRegistry();
- private static ListeningScheduledExecutorService retryService;
+ private ListeningScheduledExecutorService retryService;
private Server server;
private ManagedChannel channel;
@@ -104,11 +102,6 @@
@Mock private Retrier.Backoff mockBackoff;
- @BeforeClass
- public static void beforeEverything() {
- retryService = MoreExecutors.listeningDecorator(Executors.newScheduledThreadPool(1));
- }
-
@Before
public final void setUp() throws Exception {
MockitoAnnotations.initMocks(this);
@@ -120,6 +113,9 @@
withEmptyMetadata =
TracingMetadataUtils.contextWithMetadata(
"none", "none", DIGEST_UTIL.asActionKey(Digest.getDefaultInstance()));
+
+ retryService = MoreExecutors.listeningDecorator(Executors.newScheduledThreadPool(1));
+
// Needs to be repeated in every test that uses the timeout setting, since the tests run
// on different threads than the setUp.
prevContext = withEmptyMetadata.attach();
@@ -131,17 +127,16 @@
// on different threads than the tearDown.
withEmptyMetadata.detach(prevContext);
+ retryService.shutdownNow();
+ retryService.awaitTermination(
+ com.google.devtools.build.lib.testutil.TestUtils.WAIT_TIMEOUT_SECONDS, TimeUnit.SECONDS);
+
channel.shutdownNow();
channel.awaitTermination(5, TimeUnit.SECONDS);
server.shutdownNow();
server.awaitTermination();
}
- @AfterClass
- public static void afterEverything() {
- retryService.shutdownNow();
- }
-
@Test
public void singleBlobUploadShouldWork() throws Exception {
Context prevContext = withEmptyMetadata.attach();
diff --git a/src/test/java/com/google/devtools/build/lib/remote/GrpcCacheClientTest.java b/src/test/java/com/google/devtools/build/lib/remote/GrpcCacheClientTest.java
index 240947d..b2d8278 100644
--- a/src/test/java/com/google/devtools/build/lib/remote/GrpcCacheClientTest.java
+++ b/src/test/java/com/google/devtools/build/lib/remote/GrpcCacheClientTest.java
@@ -100,13 +100,12 @@
import java.io.InputStream;
import java.util.List;
import java.util.concurrent.Executors;
+import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.concurrent.atomic.AtomicInteger;
import java.util.function.Supplier;
import org.junit.After;
-import org.junit.AfterClass;
import org.junit.Before;
-import org.junit.BeforeClass;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.JUnit4;
@@ -130,12 +129,7 @@
private Server fakeServer;
private Context withEmptyMetadata;
private Context prevContext;
- private static ListeningScheduledExecutorService retryService;
-
- @BeforeClass
- public static void beforeEverything() {
- retryService = MoreExecutors.listeningDecorator(Executors.newScheduledThreadPool(1));
- }
+ private ListeningScheduledExecutorService retryService;
@Before
public final void setUp() throws Exception {
@@ -160,21 +154,23 @@
withEmptyMetadata =
TracingMetadataUtils.contextWithMetadata(
"none", "none", DIGEST_UTIL.asActionKey(Digest.getDefaultInstance()));
+ retryService = MoreExecutors.listeningDecorator(Executors.newScheduledThreadPool(1));
+
prevContext = withEmptyMetadata.attach();
}
@After
public void tearDown() throws Exception {
withEmptyMetadata.detach(prevContext);
+
+ retryService.shutdownNow();
+ retryService.awaitTermination(
+ com.google.devtools.build.lib.testutil.TestUtils.WAIT_TIMEOUT_SECONDS, TimeUnit.SECONDS);
+
fakeServer.shutdownNow();
fakeServer.awaitTermination();
}
- @AfterClass
- public static void afterEverything() {
- retryService.shutdownNow();
- }
-
private static class CallCredentialsInterceptor implements ClientInterceptor {
private final CallCredentials credentials;
@@ -218,6 +214,7 @@
try (InputStream in = scratch.resolve(authTlsOptions.googleCredentials).getInputStream()) {
creds = GoogleAuthUtils.newCallCredentials(in, authTlsOptions.googleAuthScopes);
}
+
RemoteRetrier retrier =
TestUtils.newRemoteRetrier(
backoffSupplier, RemoteRetrier.RETRIABLE_GRPC_ERRORS, retryService);
diff --git a/src/test/java/com/google/devtools/build/lib/remote/GrpcRemoteExecutionClientTest.java b/src/test/java/com/google/devtools/build/lib/remote/GrpcRemoteExecutionClientTest.java
index fab371c..737e62b 100644
--- a/src/test/java/com/google/devtools/build/lib/remote/GrpcRemoteExecutionClientTest.java
+++ b/src/test/java/com/google/devtools/build/lib/remote/GrpcRemoteExecutionClientTest.java
@@ -97,12 +97,11 @@
import io.grpc.util.MutableHandlerRegistry;
import java.util.Set;
import java.util.concurrent.Executors;
+import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicInteger;
import javax.annotation.Nullable;
import org.junit.After;
-import org.junit.AfterClass;
import org.junit.Before;
-import org.junit.BeforeClass;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.JUnit4;
@@ -129,7 +128,7 @@
private RemoteSpawnRunner client;
private FileOutErr outErr;
private Server fakeServer;
- private static ListeningScheduledExecutorService retryService;
+ private ListeningScheduledExecutorService retryService;
private static final OutputFile DUMMY_OUTPUT =
OutputFile.newBuilder()
@@ -141,11 +140,6 @@
.build())
.build();
- @BeforeClass
- public static void beforeEverything() {
- retryService = MoreExecutors.listeningDecorator(Executors.newScheduledThreadPool(1));
- }
-
@Before
public final void setUp() throws Exception {
String fakeServerName = "fake server for " + getClass();
@@ -226,6 +220,7 @@
Maps.immutableEntry("CacheKey1", "CacheValue1"),
Maps.immutableEntry("CacheKey2", "CacheValue2"));
+ retryService = MoreExecutors.listeningDecorator(Executors.newScheduledThreadPool(1));
RemoteRetrier retrier =
TestUtils.newRemoteRetrier(
() -> new ExponentialBackoff(remoteOptions),
@@ -282,15 +277,14 @@
@After
public void tearDown() throws Exception {
+ retryService.shutdownNow();
+ retryService.awaitTermination(
+ com.google.devtools.build.lib.testutil.TestUtils.WAIT_TIMEOUT_SECONDS, TimeUnit.SECONDS);
+
fakeServer.shutdownNow();
fakeServer.awaitTermination();
}
- @AfterClass
- public static void afterEverything() {
- retryService.shutdownNow();
- }
-
@Test
public void cacheHit() throws Exception {
serviceRegistry.addService(
diff --git a/src/test/java/com/google/devtools/build/lib/remote/RemoteCacheTests.java b/src/test/java/com/google/devtools/build/lib/remote/RemoteCacheTests.java
index 0606c1f..0851f57 100644
--- a/src/test/java/com/google/devtools/build/lib/remote/RemoteCacheTests.java
+++ b/src/test/java/com/google/devtools/build/lib/remote/RemoteCacheTests.java
@@ -60,6 +60,7 @@
import com.google.devtools.build.lib.remote.util.InMemoryCacheClient;
import com.google.devtools.build.lib.remote.util.Utils;
import com.google.devtools.build.lib.remote.util.Utils.InMemoryOutput;
+import com.google.devtools.build.lib.testutil.TestUtils;
import com.google.devtools.build.lib.util.io.FileOutErr;
import com.google.devtools.build.lib.util.io.RecordingOutErr;
import com.google.devtools.build.lib.vfs.DigestHashFunction;
@@ -79,9 +80,9 @@
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ConcurrentMap;
import java.util.concurrent.Executors;
-import org.junit.AfterClass;
+import java.util.concurrent.TimeUnit;
+import org.junit.After;
import org.junit.Before;
-import org.junit.BeforeClass;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.JUnit4;
@@ -101,12 +102,7 @@
private final DigestUtil digestUtil = new DigestUtil(DigestHashFunction.SHA256);
private FakeActionInputFileCache fakeFileCache;
- private static ListeningScheduledExecutorService retryService;
-
- @BeforeClass
- public static void beforeEverything() {
- retryService = MoreExecutors.listeningDecorator(Executors.newScheduledThreadPool(1));
- }
+ private ListeningScheduledExecutorService retryService;
@Before
public void setUp() throws Exception {
@@ -117,11 +113,13 @@
fakeFileCache = new FakeActionInputFileCache(execRoot);
artifactRoot = ArtifactRoot.asDerivedRoot(execRoot, execRoot.getChild("outputs"));
artifactRoot.getRoot().asPath().createDirectoryAndParents();
+ retryService = MoreExecutors.listeningDecorator(Executors.newScheduledThreadPool(1));
}
- @AfterClass
- public static void afterEverything() {
+ @After
+ public void afterEverything() throws InterruptedException {
retryService.shutdownNow();
+ retryService.awaitTermination(TestUtils.WAIT_TIMEOUT_SECONDS, TimeUnit.SECONDS);
}
@Test
diff --git a/src/test/java/com/google/devtools/build/lib/remote/RemoteRetrierTest.java b/src/test/java/com/google/devtools/build/lib/remote/RemoteRetrierTest.java
index 6d687bc..2ed56f1 100644
--- a/src/test/java/com/google/devtools/build/lib/remote/RemoteRetrierTest.java
+++ b/src/test/java/com/google/devtools/build/lib/remote/RemoteRetrierTest.java
@@ -25,15 +25,16 @@
import com.google.devtools.build.lib.remote.Retrier.Backoff;
import com.google.devtools.build.lib.remote.Retrier.Sleeper;
import com.google.devtools.build.lib.remote.options.RemoteOptions;
+import com.google.devtools.build.lib.testutil.TestUtils;
import com.google.devtools.common.options.Options;
import io.grpc.Status;
import io.grpc.StatusRuntimeException;
import java.time.Duration;
import java.util.concurrent.Executors;
+import java.util.concurrent.TimeUnit;
import java.util.function.Supplier;
-import org.junit.AfterClass;
+import org.junit.After;
import org.junit.Before;
-import org.junit.BeforeClass;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.JUnit4;
@@ -50,21 +51,18 @@
}
private RemoteRetrierTest.Foo fooMock;
- private static ListeningScheduledExecutorService retryService;
-
- @BeforeClass
- public static void beforeEverything() {
- retryService = MoreExecutors.listeningDecorator(Executors.newScheduledThreadPool(1));
- }
+ private ListeningScheduledExecutorService retryService;
@Before
public void setUp() {
+ retryService = MoreExecutors.listeningDecorator(Executors.newScheduledThreadPool(1));
fooMock = Mockito.mock(RemoteRetrierTest.Foo.class);
}
- @AfterClass
- public static void afterEverything() {
+ @After
+ public void tearDown() throws InterruptedException {
retryService.shutdownNow();
+ retryService.awaitTermination(TestUtils.WAIT_TIMEOUT_SECONDS, TimeUnit.SECONDS);
}
@Test
diff --git a/src/test/java/com/google/devtools/build/lib/remote/RemoteServerCapabilitiesTest.java b/src/test/java/com/google/devtools/build/lib/remote/RemoteServerCapabilitiesTest.java
index 357446c..989abae 100644
--- a/src/test/java/com/google/devtools/build/lib/remote/RemoteServerCapabilitiesTest.java
+++ b/src/test/java/com/google/devtools/build/lib/remote/RemoteServerCapabilitiesTest.java
@@ -50,10 +50,9 @@
import io.grpc.stub.StreamObserver;
import io.grpc.util.MutableHandlerRegistry;
import java.util.concurrent.Executors;
+import java.util.concurrent.TimeUnit;
import org.junit.After;
-import org.junit.AfterClass;
import org.junit.Before;
-import org.junit.BeforeClass;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.JUnit4;
@@ -65,12 +64,7 @@
private final MutableHandlerRegistry serviceRegistry = new MutableHandlerRegistry();
private final String fakeServerName = "fake server for " + getClass();
private Server fakeServer;
- private static ListeningScheduledExecutorService retryService;
-
- @BeforeClass
- public static void beforeEverything() {
- retryService = MoreExecutors.listeningDecorator(Executors.newScheduledThreadPool(1));
- }
+ private ListeningScheduledExecutorService retryService;
@Before
public final void setUp() throws Exception {
@@ -80,19 +74,19 @@
.directExecutor()
.build()
.start();
+ retryService = MoreExecutors.listeningDecorator(Executors.newScheduledThreadPool(1));
}
@After
public void tearDown() throws Exception {
+ retryService.shutdownNow();
+ retryService.awaitTermination(
+ com.google.devtools.build.lib.testutil.TestUtils.WAIT_TIMEOUT_SECONDS, TimeUnit.SECONDS);
+
fakeServer.shutdownNow();
fakeServer.awaitTermination();
}
- @AfterClass
- public static void afterEverything() {
- retryService.shutdownNow();
- }
-
/** Capture the request headers from a client. Useful for testing metadata propagation. */
private static class RequestHeadersValidator implements ServerInterceptor {
@Override
diff --git a/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnRunnerTest.java b/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnRunnerTest.java
index 8bd2f57..4b1e120 100644
--- a/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnRunnerTest.java
+++ b/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnRunnerTest.java
@@ -76,6 +76,7 @@
import com.google.devtools.build.lib.remote.options.RemoteOutputsMode;
import com.google.devtools.build.lib.remote.util.DigestUtil;
import com.google.devtools.build.lib.remote.util.FakeSpawnExecutionContext;
+import com.google.devtools.build.lib.testutil.TestUtils;
import com.google.devtools.build.lib.util.ExitCode;
import com.google.devtools.build.lib.util.io.FileOutErr;
import com.google.devtools.build.lib.vfs.DigestHashFunction;
@@ -91,10 +92,10 @@
import java.nio.charset.StandardCharsets;
import java.util.List;
import java.util.concurrent.Executors;
+import java.util.concurrent.TimeUnit;
import javax.annotation.Nullable;
-import org.junit.AfterClass;
+import org.junit.After;
import org.junit.Before;
-import org.junit.BeforeClass;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.JUnit4;
@@ -109,7 +110,7 @@
private static final ImmutableMap<String, String> NO_CACHE =
ImmutableMap.of(ExecutionRequirements.NO_CACHE, "");
- private static ListeningScheduledExecutorService retryService;
+ private ListeningScheduledExecutorService retryService;
private Path execRoot;
private Path logDir;
@@ -129,11 +130,6 @@
private final String simpleActionId =
"eb45b20cc979d504f96b9efc9a08c48103c6f017afa09c0df5c70a5f92a98ea8";
- @BeforeClass
- public static void beforeEverything() {
- retryService = MoreExecutors.listeningDecorator(Executors.newScheduledThreadPool(1));
- }
-
@Before
public final void setUp() throws Exception {
MockitoAnnotations.initMocks(this);
@@ -151,11 +147,14 @@
outErr = new FileOutErr(stdout, stderr);
remoteOptions = Options.getDefaults(RemoteOptions.class);
+
+ retryService = MoreExecutors.listeningDecorator(Executors.newScheduledThreadPool(1));
}
- @AfterClass
- public static void afterEverything() {
+ @After
+ public void afterEverything() throws InterruptedException {
retryService.shutdownNow();
+ retryService.awaitTermination(TestUtils.WAIT_TIMEOUT_SECONDS, TimeUnit.SECONDS);
}
@Test
diff --git a/src/test/java/com/google/devtools/build/lib/remote/RetrierTest.java b/src/test/java/com/google/devtools/build/lib/remote/RetrierTest.java
index 6ec0c87..dbbd5b7 100644
--- a/src/test/java/com/google/devtools/build/lib/remote/RetrierTest.java
+++ b/src/test/java/com/google/devtools/build/lib/remote/RetrierTest.java
@@ -29,15 +29,16 @@
import com.google.devtools.build.lib.remote.Retrier.CircuitBreaker.State;
import com.google.devtools.build.lib.remote.Retrier.CircuitBreakerException;
import com.google.devtools.build.lib.remote.Retrier.ZeroBackoff;
+import com.google.devtools.build.lib.testutil.TestUtils;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.Executors;
+import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicInteger;
import java.util.function.Predicate;
import java.util.function.Supplier;
import javax.annotation.concurrent.ThreadSafe;
-import org.junit.AfterClass;
+import org.junit.After;
import org.junit.Before;
-import org.junit.BeforeClass;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.JUnit4;
@@ -56,22 +57,20 @@
private static final Predicate<Exception> RETRY_ALL = (e) -> true;
private static final Predicate<Exception> RETRY_NONE = (e) -> false;
- private static ListeningScheduledExecutorService retryService;
-
- @BeforeClass
- public static void beforeEverything() {
- retryService = MoreExecutors.listeningDecorator(Executors.newScheduledThreadPool(1));
- }
+ private ListeningScheduledExecutorService retryService;
@Before
public void setup() {
MockitoAnnotations.initMocks(this);
when(alwaysOpen.state()).thenReturn(State.ACCEPT_CALLS);
+
+ retryService = MoreExecutors.listeningDecorator(Executors.newScheduledThreadPool(1));
}
- @AfterClass
- public static void afterEverything() {
+ @After
+ public void afterEverything() throws InterruptedException {
retryService.shutdownNow();
+ retryService.awaitTermination(TestUtils.WAIT_TIMEOUT_SECONDS, TimeUnit.SECONDS);
}
@Test
diff --git a/src/test/java/com/google/devtools/build/lib/remote/downloader/GrpcRemoteDownloaderTest.java b/src/test/java/com/google/devtools/build/lib/remote/downloader/GrpcRemoteDownloaderTest.java
index e4e6134..740cba6 100644
--- a/src/test/java/com/google/devtools/build/lib/remote/downloader/GrpcRemoteDownloaderTest.java
+++ b/src/test/java/com/google/devtools/build/lib/remote/downloader/GrpcRemoteDownloaderTest.java
@@ -63,10 +63,9 @@
import java.util.Map;
import java.util.Optional;
import java.util.concurrent.Executors;
+import java.util.concurrent.TimeUnit;
import org.junit.After;
-import org.junit.AfterClass;
import org.junit.Before;
-import org.junit.BeforeClass;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.JUnit4;
@@ -82,12 +81,7 @@
private Server fakeServer;
private Context withEmptyMetadata;
private Context prevContext;
- private static ListeningScheduledExecutorService retryService;
-
- @BeforeClass
- public static void beforeEverything() {
- retryService = MoreExecutors.listeningDecorator(Executors.newScheduledThreadPool(1));
- }
+ private ListeningScheduledExecutorService retryService;
@Before
public final void setUp() throws Exception {
@@ -101,21 +95,24 @@
withEmptyMetadata =
TracingMetadataUtils.contextWithMetadata(
"none", "none", DIGEST_UTIL.asActionKey(Digest.getDefaultInstance()));
+
+ retryService = MoreExecutors.listeningDecorator(Executors.newScheduledThreadPool(1));
+
prevContext = withEmptyMetadata.attach();
}
@After
public void tearDown() throws Exception {
withEmptyMetadata.detach(prevContext);
+
+ retryService.shutdownNow();
+ retryService.awaitTermination(
+ com.google.devtools.build.lib.testutil.TestUtils.WAIT_TIMEOUT_SECONDS, TimeUnit.SECONDS);
+
fakeServer.shutdownNow();
fakeServer.awaitTermination();
}
- @AfterClass
- public static void afterEverything() {
- retryService.shutdownNow();
- }
-
private GrpcRemoteDownloader newDownloader(RemoteCacheClient cacheClient) throws IOException {
final RemoteOptions remoteOptions = Options.getDefaults(RemoteOptions.class);
final RemoteRetrier retrier =
