junit4 to junit5 migration. step 2 - modules servicetalk-http-api a…

…nd `servicetalk-http-netty` (#1521)

Motivation:

    JUnit 5 leverages features from Java 8 or later, such as lambda functions, making tests more powerful and easier to maintain.
    JUnit 5 has added some very useful new features for describing, organizing, and executing tests. For instance, tests get better display names and can be organized hierarchically.
    JUnit 5 is organized into multiple libraries, so only the features you need are imported into your project. With build systems such as Maven and Gradle, including the right libraries is easy.
    JUnit 5 can use more than one extension at a time, which JUnit 4 could not (only one runner could be used at a time). This means you can easily combine the Spring extension with other extensions (such as your own custom extension).

migrating-from-junit-4-to-junit-5-important-differences-and-benefits

Modifications:
Updated unit tests

Result:
modulesservicetalk-http-api and servicetalk-http-netty run tests using junit5 engine

servicetalk-concurrent-api/src/testFixtures/java/io/servicetalk/concurrent/api/VerificationTestUtils.java

@@ -16,8 +16,8 @@
 package io.servicetalk.concurrent.api;
 
 import org.hamcrest.Matcher;
-import org.junit.Assert;
-import org.junit.function.ThrowingRunnable;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.function.Executable;
 
 import javax.annotation.Nullable;
 
@@ -59,12 +59,12 @@ public static void expectThrowable(final Runnable runnable, final Matcher<Throwa
 
     @SuppressWarnings("unchecked")
     public static <T1 extends Throwable, T2 extends Throwable> T1 assertThrows(
-            Class<T1> expectedClass, @Nullable Class<T2> optionalWrapperClass, ThrowingRunnable runnable) {
+            Class<T1> expectedClass, @Nullable Class<T2> optionalWrapperClass, Executable executable) {
         if (optionalWrapperClass == null) {
-            return Assert.assertThrows(expectedClass, runnable);
+            return Assertions.assertThrows(expectedClass, executable);
         }
         try {
-            runnable.run();
+            executable.execute();
         } catch (Throwable cause) {
             if (expectedClass.isInstance(cause)) {
                 return (T1) cause;

servicetalk-concurrent-internal/src/test/java/io/servicetalk/concurrent/internal/DelayedCancellableTest.java

@@ -24,7 +24,7 @@
 
 import java.util.concurrent.ExecutorService;
 
-import static io.servicetalk.concurrent.internal.ServiceTalkTestTimeout.DEFAULT_TIMEOUT_SECONDS;
+import static io.servicetalk.concurrent.internal.TestTimeoutConstants.DEFAULT_TIMEOUT_SECONDS;
 import static java.util.concurrent.Executors.newCachedThreadPool;
 import static java.util.concurrent.TimeUnit.SECONDS;
 import static org.mockito.Mockito.mock;

servicetalk-concurrent-internal/src/test/java/io/servicetalk/concurrent/internal/DelayedSubscriptionTest.java

@@ -27,7 +27,7 @@
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Future;
 
-import static io.servicetalk.concurrent.internal.ServiceTalkTestTimeout.DEFAULT_TIMEOUT_SECONDS;
+import static io.servicetalk.concurrent.internal.TestTimeoutConstants.DEFAULT_TIMEOUT_SECONDS;
 import static java.util.concurrent.Executors.newCachedThreadPool;
 import static java.util.concurrent.TimeUnit.SECONDS;
 import static org.hamcrest.MatcherAssert.assertThat;
@@ -136,7 +136,7 @@ public void concurrentRequestAndSwap() throws Exception {
 
     private void doConcurrentRequestAndSwap() throws InterruptedException, ExecutionException {
         DelayedSubscription ds = new DelayedSubscription();
-        Subscription s = new CountingSubscription();
+        CountingSubscription s = new CountingSubscription();
         CyclicBarrier barrier = new CyclicBarrier(2);
         Future<Void> requester = executor.submit(() -> {
             for (int i = 0; i < 10_000; i++) {
@@ -154,7 +154,7 @@ private void doConcurrentRequestAndSwap() throws InterruptedException, Execution
         });
         swapper.get();
         requester.get();
-        assertThat("Unexpected items requested.", ((CountingSubscription) s).requested(), is(10_000));
+        assertThat("Unexpected items requested.", s.requested(), is(10_000));
     }
 
     private static class CountingSubscription implements Subscription {

servicetalk-concurrent-internal/src/testFixtures/java/io/servicetalk/concurrent/internal/ServiceTalkTestTimeout.java

@@ -33,28 +33,26 @@
 import java.util.concurrent.TimeoutException;
 import javax.annotation.Nullable;
 
-import static java.lang.Boolean.parseBoolean;
 import static java.util.Objects.requireNonNull;
 import static java.util.concurrent.TimeUnit.SECONDS;
 
 /**
  * Standard timeout shared by test classes. The {@link #lookForStuckThread} setting is ignored.
  */
 public final class ServiceTalkTestTimeout extends Timeout {
-    public static final boolean CI = parseBoolean(System.getenv("CI"));
-    public static final int DEFAULT_TIMEOUT_SECONDS = CI ? 30 : 10;
-    public static final String THREAD_PREFIX = "Time-limited test";
+
+    private static final String THREAD_PREFIX = "Time-limited test";
     private final Runnable onTimeout;
 
     public ServiceTalkTestTimeout() {
-        this(DEFAULT_TIMEOUT_SECONDS, SECONDS);
+        this(TestTimeoutConstants.DEFAULT_TIMEOUT_SECONDS, SECONDS);
     }
 
     public ServiceTalkTestTimeout(long timeout, TimeUnit unit) {
         this(timeout, unit, () -> { });
     }
 
-    public ServiceTalkTestTimeout(long timeout, TimeUnit unit, Runnable onTimeout) {
+    private ServiceTalkTestTimeout(long timeout, TimeUnit unit, Runnable onTimeout) {
         super(timeout, unit);
         this.onTimeout = requireNonNull(onTimeout);
     }

servicetalk-concurrent-internal/src/testFixtures/java/io/servicetalk/concurrent/internal/TestTimeoutConstants.java

@@ -0,0 +1,26 @@
+/*
+ * Copyright © 2021 Apple Inc. and the ServiceTalk project authors
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package io.servicetalk.concurrent.internal;
+
+import static java.lang.Boolean.parseBoolean;
+
+public final class TestTimeoutConstants {
+    public static final boolean CI = parseBoolean(System.getenv("CI"));
+    public static final int DEFAULT_TIMEOUT_SECONDS = CI ? 30 : 10;
+
+    private TestTimeoutConstants() {
+    }
+}

servicetalk-grpc-netty/src/test/java/io/servicetalk/grpc/netty/ProtocolCompatibilityTest.java

@@ -97,7 +97,7 @@
 import static io.servicetalk.concurrent.api.Processors.newSingleProcessor;
 import static io.servicetalk.concurrent.api.Single.succeeded;
 import static io.servicetalk.concurrent.api.SourceAdapters.fromSource;
-import static io.servicetalk.concurrent.internal.ServiceTalkTestTimeout.DEFAULT_TIMEOUT_SECONDS;
+import static io.servicetalk.concurrent.internal.TestTimeoutConstants.DEFAULT_TIMEOUT_SECONDS;
 import static io.servicetalk.encoding.api.Identity.identity;
 import static io.servicetalk.encoding.netty.ContentCodings.gzipDefault;
 import static io.servicetalk.grpc.api.GrpcExecutionStrategies.defaultStrategy;

servicetalk-http-api/build.gradle

@@ -43,15 +43,21 @@ dependencies {
   testImplementation project(":servicetalk-concurrent-test-internal")
   testImplementation project(":servicetalk-test-resources")
   testImplementation project(":servicetalk-transport-netty-internal")
-  testImplementation "junit:junit:$junitVersion"
+  testImplementation "org.junit.jupiter:junit-jupiter-api:$junit5Version"
+  testImplementation "org.junit.jupiter:junit-jupiter-params:$junit5Version"
   testImplementation "org.hamcrest:hamcrest-library:$hamcrestVersion"
   testImplementation "org.mockito:mockito-core:$mockitoCoreVersion"
+  testImplementation "org.mockito:mockito-junit-jupiter:$mockitoCoreVersion"
 
   testFixturesImplementation testFixtures(project(":servicetalk-concurrent-internal"))
   testFixturesImplementation testFixtures(project(":servicetalk-transport-netty-internal"))
   testFixturesImplementation project(":servicetalk-buffer-netty")
   testFixturesImplementation "com.google.code.findbugs:jsr305:$jsr305Version"
-  testFixturesImplementation "junit:junit:$junitVersion"
+  testFixturesImplementation "org.junit.jupiter:junit-jupiter-api:$junit5Version"
+  testFixturesImplementation "org.junit.jupiter:junit-jupiter-params:$junit5Version"
   testFixturesImplementation "org.hamcrest:hamcrest-library:$hamcrestVersion"
   testFixturesImplementation "org.mockito:mockito-core:$mockitoCoreVersion"
+  testFixturesImplementation "org.mockito:mockito-junit-jupiter:$mockitoCoreVersion"
+
+  testRuntimeOnly "org.junit.jupiter:junit-jupiter-engine:$junit5Version"
 }

servicetalk-http-api/src/test/java/io/servicetalk/http/api/AbstractBlockingStreamingHttpRequesterTest.java

@@ -22,11 +22,9 @@
 import io.servicetalk.concurrent.api.Single;
 import io.servicetalk.concurrent.api.TestPublisher;
 import io.servicetalk.concurrent.api.TestSubscription;
-import io.servicetalk.concurrent.internal.ServiceTalkTestTimeout;
 
-import org.junit.Before;
-import org.junit.Rule;
-import org.junit.Test;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
 import org.mockito.Mock;
 import org.mockito.MockitoAnnotations;
 
@@ -42,15 +40,13 @@
 import static io.servicetalk.http.api.HttpProtocolVersion.HTTP_1_1;
 import static io.servicetalk.http.api.HttpResponseStatus.OK;
 import static java.nio.charset.StandardCharsets.US_ASCII;
-import static org.junit.Assert.assertArrayEquals;
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertTrue;
+import static org.junit.jupiter.api.Assertions.assertArrayEquals;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertTrue;
 import static org.mockito.Mockito.when;
 
 public abstract class AbstractBlockingStreamingHttpRequesterTest {
-    @Rule
-    public final ServiceTalkTestTimeout timeout = new ServiceTalkTestTimeout();
     @Mock
     private HttpExecutionContext mockExecutionCtx;
     @Mock
@@ -76,16 +72,16 @@ protected interface TestHttpRequester {
         boolean isClosed();
     }
 
-    @Before
-    public void setup() {
+    @BeforeEach
+    void setup() {
         MockitoAnnotations.initMocks(this);
         when(mockExecutionCtx.executor()).thenReturn(immediate());
         when(mockCtx.executionContext()).thenReturn(mockExecutionCtx);
         when(mockIterable.iterator()).thenReturn(mockIterator);
     }
 
     @Test
-    public void asyncToSyncNoPayload() throws Exception {
+    void asyncToSyncNoPayload() throws Exception {
         StreamingHttpRequester asyncRequester = newAsyncRequester(reqRespFactory, mockExecutionCtx,
                 (strategy, req) -> succeeded(reqRespFactory.ok()));
         BlockingStreamingHttpRequester syncRequester = toBlockingStreamingRequester(asyncRequester);
@@ -96,7 +92,7 @@ public void asyncToSyncNoPayload() throws Exception {
     }
 
     @Test
-    public void asyncToSyncWithPayload() throws Exception {
+    void asyncToSyncWithPayload() throws Exception {
         StreamingHttpRequester asyncRequester = newAsyncRequester(reqRespFactory, mockExecutionCtx,
                 (strategy, req) -> succeeded(reqRespFactory.ok().payloadBody(from(allocator.fromAscii("hello")))));
         BlockingStreamingHttpRequester syncRequester = toBlockingStreamingRequester(asyncRequester);
@@ -111,7 +107,7 @@ public void asyncToSyncWithPayload() throws Exception {
     }
 
     @Test
-    public void asyncToSyncWithPayloadInputStream() throws Exception {
+    void asyncToSyncWithPayloadInputStream() throws Exception {
         String expectedPayload = "hello";
         byte[] expectedPayloadBytes = expectedPayload.getBytes(US_ASCII);
         StreamingHttpRequester asyncRequester = newAsyncRequester(reqRespFactory, mockExecutionCtx,
@@ -130,7 +126,7 @@ public void asyncToSyncWithPayloadInputStream() throws Exception {
     }
 
     @Test
-    public void asyncToSyncClose() throws Exception {
+    void asyncToSyncClose() throws Exception {
         StreamingHttpRequester asyncRequester = newAsyncRequester(reqRespFactory, mockExecutionCtx,
                 (strategy, req) -> failed(new IllegalStateException("shouldn't be called!")));
         BlockingStreamingHttpRequester syncRequester = toBlockingStreamingRequester(asyncRequester);
@@ -139,7 +135,7 @@ public void asyncToSyncClose() throws Exception {
     }
 
     @Test
-    public void asyncToSyncCancelPropagated() throws Exception {
+    void asyncToSyncCancelPropagated() throws Exception {
         StreamingHttpRequester asyncRequester = newAsyncRequester(reqRespFactory, mockExecutionCtx,
                 (strategy, req) -> succeeded(reqRespFactory.ok().payloadBody(publisher)));
         TestSubscription subscription = new TestSubscription();

servicetalk-http-api/src/test/java/io/servicetalk/http/api/AbstractConditionalHttpFilterTest.java

@@ -1,5 +1,5 @@
 /*
- * Copyright © 2018 Apple Inc. and the ServiceTalk project authors
+ * Copyright © 2018, 2021 Apple Inc. and the ServiceTalk project authors
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -22,13 +22,10 @@
 import io.servicetalk.concurrent.api.AsyncContextMap;
 import io.servicetalk.concurrent.api.Completable;
 import io.servicetalk.concurrent.api.Single;
-import io.servicetalk.concurrent.internal.ServiceTalkTestTimeout;
-import io.servicetalk.transport.netty.internal.ExecutionContextRule;
+import io.servicetalk.transport.netty.internal.ExecutionContextExtension;
 
-import org.junit.ClassRule;
-import org.junit.Rule;
-import org.junit.Test;
-import org.junit.rules.Timeout;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.RegisterExtension;
 
 import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.concurrent.atomic.AtomicReference;
@@ -40,22 +37,22 @@
 import static io.servicetalk.concurrent.api.Single.succeeded;
 import static io.servicetalk.concurrent.api.SourceAdapters.toSource;
 import static io.servicetalk.http.api.HttpExecutionStrategies.defaultStrategy;
-import static io.servicetalk.transport.netty.internal.ExecutionContextRule.cached;
+import static io.servicetalk.transport.netty.internal.ExecutionContextExtension.cached;
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.equalTo;
 import static org.hamcrest.Matchers.is;
 
 public abstract class AbstractConditionalHttpFilterTest {
-    protected static final String FILTERED_HEADER = "X-Filtered";
+    private static final String FILTERED_HEADER = "X-Filtered";
 
     protected static final StreamingHttpRequestResponseFactory REQ_RES_FACTORY =
             new DefaultStreamingHttpRequestResponseFactory(DEFAULT_ALLOCATOR, DefaultHttpHeadersFactory.INSTANCE,
                     HttpProtocolVersion.HTTP_1_1);
 
-    protected static final AsyncContextMap.Key<String> CTX_KEY = newKey("test-key");
+    private static final AsyncContextMap.Key<String> CTX_KEY = newKey("test-key");
 
-    @ClassRule
-    public static final ExecutionContextRule TEST_CTX = cached();
+    @RegisterExtension
+    static final ExecutionContextExtension TEST_CTX = cached();
 
     protected static final Predicate<StreamingHttpRequest> TEST_REQ_PREDICATE = req -> {
         AsyncContext.put(CTX_KEY, "in-predicate");
@@ -66,9 +63,6 @@ public abstract class AbstractConditionalHttpFilterTest {
             TEST_REQ_HANDLER = (req, resFactory) -> succeeded(resFactory.ok()
             .setHeader(FILTERED_HEADER, req.headers().get(FILTERED_HEADER, "false")));
 
-    @Rule
-    public final Timeout timeout = new ServiceTalkTestTimeout();
-
     protected static StreamingHttpRequest markFiltered(StreamingHttpRequest req) {
         return req.setHeader(FILTERED_HEADER, "true");
     }
@@ -93,17 +87,17 @@ protected static HttpExecutionContext testHttpExecutionContext() {
     }
 
     @Test
-    public void predicateAccepts() throws Exception {
+    void predicateAccepts() throws Exception {
         testFilter(true);
     }
 
     @Test
-    public void predicateRejects() throws Exception {
+    void predicateRejects() throws Exception {
         testFilter(false);
     }
 
     @Test
-    public void contextIsPreserved() throws Exception {
+    void contextIsPreserved() throws Exception {
         final StreamingHttpRequest req = REQ_RES_FACTORY.get("/reject");
         AsyncContext.put(CTX_KEY, "in-test");
         AtomicReference<String> ctxKeyValueInNextFilter = new AtomicReference<>();
@@ -128,14 +122,14 @@ private void testFilter(boolean expectAccepted) throws Exception {
     }
 
     @Test
-    public void closeAsyncImpactsBoth() throws Exception {
+    void closeAsyncImpactsBoth() throws Exception {
         AtomicBoolean closed = new AtomicBoolean();
         returnConditionallyFilteredResource(closed).closeAsync().toFuture().get();
         assertThat(closed.get(), is(true));
     }
 
     @Test
-    public void closeAsyncGracefullyImpactsBoth() throws Exception {
+    void closeAsyncGracefullyImpactsBoth() throws Exception {
         AtomicBoolean closed = new AtomicBoolean();
         returnConditionallyFilteredResource(closed).closeAsyncGracefully().toFuture().get();
         assertThat(closed.get(), is(true));