From d27a2b3df98f5a190185fded4132ea240e081fa9 Mon Sep 17 00:00:00 2001 From: Chris Vest Date: Thu, 19 Aug 2021 13:28:32 +0200 Subject: [PATCH] Backport some fixes and cleanups for DefaultPromiseTest (#11600) Motivation: These cleanups were done in another PR but were not directly related to that PR. This extracts those changes and backports them to 4.1. Modification: * Remove the use of mocking in DefaultPromiseTest. * Fix a few warnings. * Make `testStackOverFlowChainedFuturesB` test with the right listener chain. Result: Cleaner code. --- .../util/concurrent/DefaultPromiseTest.java | 97 +++++++++++++++---- 1 file changed, 79 insertions(+), 18 deletions(-) diff --git a/common/src/test/java/io/netty/util/concurrent/DefaultPromiseTest.java b/common/src/test/java/io/netty/util/concurrent/DefaultPromiseTest.java index 5e91803d49..b5f1034214 100644 --- a/common/src/test/java/io/netty/util/concurrent/DefaultPromiseTest.java +++ b/common/src/test/java/io/netty/util/concurrent/DefaultPromiseTest.java @@ -22,11 +22,12 @@ import io.netty.util.internal.logging.InternalLoggerFactory; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.Timeout; -import org.mockito.Mockito; +import org.junit.jupiter.api.function.Executable; import java.util.HashMap; import java.util.Map; import java.util.concurrent.BlockingQueue; +import java.util.concurrent.Callable; import java.util.concurrent.CancellationException; import java.util.concurrent.CompletionException; import java.util.concurrent.CountDownLatch; @@ -37,17 +38,15 @@ import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicInteger; import static java.lang.Math.max; -import static org.hamcrest.MatcherAssert.assertThat; -import static org.hamcrest.Matchers.instanceOf; -import static org.hamcrest.Matchers.lessThan; +import static org.assertj.core.api.Assertions.assertThat; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertSame; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assertions.fail; -@SuppressWarnings("unchecked") public class DefaultPromiseTest { private static final InternalLogger logger = InternalLoggerFactory.getInstance(DefaultPromiseTest.class); private static int stackOverflowDepth; @@ -62,6 +61,7 @@ public class DefaultPromiseTest { } } + @SuppressWarnings("InfiniteRecursion") private static void findStackOverflowDepth() { ++stackOverflowDepth; findStackOverflowDepth(); @@ -71,38 +71,99 @@ public class DefaultPromiseTest { return max(stackOverflowDepth << 1, stackOverflowDepth); } + private static class RejectingEventExecutor extends AbstractEventExecutor { + @Override + public boolean isShuttingDown() { + return false; + } + + @Override + public Future shutdownGracefully(long quietPeriod, long timeout, TimeUnit unit) { + return null; + } + + @Override + public Future terminationFuture() { + return null; + } + + @Override + public void shutdown() { + } + + @Override + public boolean isShutdown() { + return false; + } + + @Override + public boolean isTerminated() { + return false; + } + + @Override + public boolean awaitTermination(long timeout, TimeUnit unit) throws InterruptedException { + return false; + } + + @Override + public ScheduledFuture schedule(Runnable command, long delay, TimeUnit unit) { + return fail("Cannot schedule commands"); + } + + @Override + public ScheduledFuture schedule(Callable callable, long delay, TimeUnit unit) { + return fail("Cannot schedule commands"); + } + + @Override + public ScheduledFuture scheduleAtFixedRate(Runnable command, long initialDelay, long period, TimeUnit unit) { + return fail("Cannot schedule commands"); + } + + @Override + public ScheduledFuture scheduleWithFixedDelay(Runnable command, long initialDelay, long delay, + TimeUnit unit) { + return fail("Cannot schedule commands"); + } + + @Override + public boolean inEventLoop(Thread thread) { + return false; + } + + @Override + public void execute(Runnable command) { + fail("Cannot schedule commands"); + } + } + @Test public void testCancelDoesNotScheduleWhenNoListeners() { - EventExecutor executor = Mockito.mock(EventExecutor.class); - Mockito.when(executor.inEventLoop()).thenReturn(false); + EventExecutor executor = new RejectingEventExecutor(); Promise promise = new DefaultPromise(executor); assertTrue(promise.cancel(false)); - Mockito.verify(executor, Mockito.never()).execute(Mockito.any(Runnable.class)); assertTrue(promise.isCancelled()); } @Test public void testSuccessDoesNotScheduleWhenNoListeners() { - EventExecutor executor = Mockito.mock(EventExecutor.class); - Mockito.when(executor.inEventLoop()).thenReturn(false); + EventExecutor executor = new RejectingEventExecutor(); Object value = new Object(); Promise promise = new DefaultPromise(executor); promise.setSuccess(value); - Mockito.verify(executor, Mockito.never()).execute(Mockito.any(Runnable.class)); assertSame(value, promise.getNow()); } @Test public void testFailureDoesNotScheduleWhenNoListeners() { - EventExecutor executor = Mockito.mock(EventExecutor.class); - Mockito.when(executor.inEventLoop()).thenReturn(false); + EventExecutor executor = new RejectingEventExecutor(); Exception cause = new Exception(); Promise promise = new DefaultPromise(executor); promise.setFailure(cause); - Mockito.verify(executor, Mockito.never()).execute(Mockito.any(Runnable.class)); assertSame(cause, promise.cause()); } @@ -124,7 +185,7 @@ public class DefaultPromiseTest { public void testCancellationExceptionIsReturnedAsCause() throws Exception { final Promise promise = new DefaultPromise<>(ImmediateEventExecutor.INSTANCE); assertTrue(promise.cancel(false)); - assertThat(promise.cause(), instanceOf(CancellationException.class)); + assertThat(promise.cause()).isInstanceOf(CancellationException.class); } @Test @@ -273,7 +334,7 @@ public class DefaultPromiseTest { promise.getKey().start(); final long start = System.nanoTime(); promise.getValue().awaitUninterruptibly(wait, TimeUnit.NANOSECONDS); - assertThat(System.nanoTime() - start, lessThan(wait)); + assertThat(System.nanoTime() - start).isLessThan(wait); } } finally { if (executor != null) { @@ -388,9 +449,9 @@ public class DefaultPromiseTest { final CountDownLatch latch = new CountDownLatch(promiseChainLength); if (runTestInExecutorThread) { - executor.execute(() -> testStackOverFlowChainedFuturesA(executor, p, latch)); + executor.execute(() -> testStackOverFlowChainedFuturesB(executor, p, latch)); } else { - testStackOverFlowChainedFuturesA(executor, p, latch); + testStackOverFlowChainedFuturesB(executor, p, latch); } assertTrue(latch.await(2, TimeUnit.SECONDS));