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.
This commit is contained in:
Chris Vest 2021-08-19 13:28:32 +02:00
parent aa69d5b5cf
commit d27a2b3df9

View File

@ -22,11 +22,12 @@ import io.netty.util.internal.logging.InternalLoggerFactory;
import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.Test; import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.Timeout; import org.junit.jupiter.api.Timeout;
import org.mockito.Mockito; import org.junit.jupiter.api.function.Executable;
import java.util.HashMap; import java.util.HashMap;
import java.util.Map; import java.util.Map;
import java.util.concurrent.BlockingQueue; import java.util.concurrent.BlockingQueue;
import java.util.concurrent.Callable;
import java.util.concurrent.CancellationException; import java.util.concurrent.CancellationException;
import java.util.concurrent.CompletionException; import java.util.concurrent.CompletionException;
import java.util.concurrent.CountDownLatch; import java.util.concurrent.CountDownLatch;
@ -37,17 +38,15 @@ import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicInteger;
import static java.lang.Math.max; import static java.lang.Math.max;
import static org.hamcrest.MatcherAssert.assertThat; import static org.assertj.core.api.Assertions.assertThat;
import static org.hamcrest.Matchers.instanceOf;
import static org.hamcrest.Matchers.lessThan;
import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertNull;
import static org.junit.jupiter.api.Assertions.assertSame; import static org.junit.jupiter.api.Assertions.assertSame;
import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.junit.jupiter.api.Assertions.fail;
@SuppressWarnings("unchecked")
public class DefaultPromiseTest { public class DefaultPromiseTest {
private static final InternalLogger logger = InternalLoggerFactory.getInstance(DefaultPromiseTest.class); private static final InternalLogger logger = InternalLoggerFactory.getInstance(DefaultPromiseTest.class);
private static int stackOverflowDepth; private static int stackOverflowDepth;
@ -62,6 +61,7 @@ public class DefaultPromiseTest {
} }
} }
@SuppressWarnings("InfiniteRecursion")
private static void findStackOverflowDepth() { private static void findStackOverflowDepth() {
++stackOverflowDepth; ++stackOverflowDepth;
findStackOverflowDepth(); findStackOverflowDepth();
@ -71,38 +71,99 @@ public class DefaultPromiseTest {
return max(stackOverflowDepth << 1, stackOverflowDepth); 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 <V> ScheduledFuture<V> schedule(Callable<V> 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 @Test
public void testCancelDoesNotScheduleWhenNoListeners() { public void testCancelDoesNotScheduleWhenNoListeners() {
EventExecutor executor = Mockito.mock(EventExecutor.class); EventExecutor executor = new RejectingEventExecutor();
Mockito.when(executor.inEventLoop()).thenReturn(false);
Promise<Void> promise = new DefaultPromise<Void>(executor); Promise<Void> promise = new DefaultPromise<Void>(executor);
assertTrue(promise.cancel(false)); assertTrue(promise.cancel(false));
Mockito.verify(executor, Mockito.never()).execute(Mockito.any(Runnable.class));
assertTrue(promise.isCancelled()); assertTrue(promise.isCancelled());
} }
@Test @Test
public void testSuccessDoesNotScheduleWhenNoListeners() { public void testSuccessDoesNotScheduleWhenNoListeners() {
EventExecutor executor = Mockito.mock(EventExecutor.class); EventExecutor executor = new RejectingEventExecutor();
Mockito.when(executor.inEventLoop()).thenReturn(false);
Object value = new Object(); Object value = new Object();
Promise<Object> promise = new DefaultPromise<Object>(executor); Promise<Object> promise = new DefaultPromise<Object>(executor);
promise.setSuccess(value); promise.setSuccess(value);
Mockito.verify(executor, Mockito.never()).execute(Mockito.any(Runnable.class));
assertSame(value, promise.getNow()); assertSame(value, promise.getNow());
} }
@Test @Test
public void testFailureDoesNotScheduleWhenNoListeners() { public void testFailureDoesNotScheduleWhenNoListeners() {
EventExecutor executor = Mockito.mock(EventExecutor.class); EventExecutor executor = new RejectingEventExecutor();
Mockito.when(executor.inEventLoop()).thenReturn(false);
Exception cause = new Exception(); Exception cause = new Exception();
Promise<Void> promise = new DefaultPromise<Void>(executor); Promise<Void> promise = new DefaultPromise<Void>(executor);
promise.setFailure(cause); promise.setFailure(cause);
Mockito.verify(executor, Mockito.never()).execute(Mockito.any(Runnable.class));
assertSame(cause, promise.cause()); assertSame(cause, promise.cause());
} }
@ -124,7 +185,7 @@ public class DefaultPromiseTest {
public void testCancellationExceptionIsReturnedAsCause() throws Exception { public void testCancellationExceptionIsReturnedAsCause() throws Exception {
final Promise<Void> promise = new DefaultPromise<>(ImmediateEventExecutor.INSTANCE); final Promise<Void> promise = new DefaultPromise<>(ImmediateEventExecutor.INSTANCE);
assertTrue(promise.cancel(false)); assertTrue(promise.cancel(false));
assertThat(promise.cause(), instanceOf(CancellationException.class)); assertThat(promise.cause()).isInstanceOf(CancellationException.class);
} }
@Test @Test
@ -273,7 +334,7 @@ public class DefaultPromiseTest {
promise.getKey().start(); promise.getKey().start();
final long start = System.nanoTime(); final long start = System.nanoTime();
promise.getValue().awaitUninterruptibly(wait, TimeUnit.NANOSECONDS); promise.getValue().awaitUninterruptibly(wait, TimeUnit.NANOSECONDS);
assertThat(System.nanoTime() - start, lessThan(wait)); assertThat(System.nanoTime() - start).isLessThan(wait);
} }
} finally { } finally {
if (executor != null) { if (executor != null) {
@ -388,9 +449,9 @@ public class DefaultPromiseTest {
final CountDownLatch latch = new CountDownLatch(promiseChainLength); final CountDownLatch latch = new CountDownLatch(promiseChainLength);
if (runTestInExecutorThread) { if (runTestInExecutorThread) {
executor.execute(() -> testStackOverFlowChainedFuturesA(executor, p, latch)); executor.execute(() -> testStackOverFlowChainedFuturesB(executor, p, latch));
} else { } else {
testStackOverFlowChainedFuturesA(executor, p, latch); testStackOverFlowChainedFuturesB(executor, p, latch);
} }
assertTrue(latch.await(2, TimeUnit.SECONDS)); assertTrue(latch.await(2, TimeUnit.SECONDS));