From bcdc07fe139b8e1fad3577be74efb7b303011077 Mon Sep 17 00:00:00 2001 From: Chris Vest Date: Thu, 12 Aug 2021 11:37:56 +0200 Subject: [PATCH] Improve test failure reporting of EpollSocketChannelConfigTest (#11570) Motivation: This test is inherently flaky due to file descriptor reuse. Even though we have taken steps to make it less flaky, it still fails sometimes. When it does, the error message is not very helpful. Modification: Make use of assertThrows and assertThat to get more descriptive error messages when the tests fail. Result: More meaningful messages on test failures, which may help us make the tests more resilient in the future --- .../epoll/EpollSocketChannelConfigTest.java | 36 +++++++++---------- 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/transport-native-epoll/src/test/java/io/netty/channel/epoll/EpollSocketChannelConfigTest.java b/transport-native-epoll/src/test/java/io/netty/channel/epoll/EpollSocketChannelConfigTest.java index 6f812c36592..44305e251ab 100644 --- a/transport-native-epoll/src/test/java/io/netty/channel/epoll/EpollSocketChannelConfigTest.java +++ b/transport-native-epoll/src/test/java/io/netty/channel/epoll/EpollSocketChannelConfigTest.java @@ -27,6 +27,7 @@ import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.function.Executable; import org.opentest4j.TestAbortedException; import java.net.InetSocketAddress; @@ -34,8 +35,10 @@ import java.util.Map; import java.util.Random; +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.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assertions.fail; @@ -141,34 +144,31 @@ public void testTcpQickAck() { // For this test to pass, we are relying on the sockets file descriptor not being reused after the socket is closed. // This is inherently racy, so we allow getSoLinger to throw ChannelException a few of times, but eventually we do // want to see a ClosedChannelException for the test to pass. - @RepeatedIfExceptionsTest(repeats = 4, exceptions = ChannelException.class) + @RepeatedIfExceptionsTest(repeats = 4) public void testSetOptionWhenClosed() { ch.close().syncUninterruptibly(); - try { - ch.config().setSoLinger(0); - fail(); - } catch (ChannelException e) { - assertTrue(e.getCause() instanceof ClosedChannelException); - } + ChannelException e = assertThrows(ChannelException.class, new Executable() { + @Override + public void execute() throws Throwable { + ch.config().setSoLinger(0); + } + }); + assertThat(e).hasCauseInstanceOf(ClosedChannelException.class); } // For this test to pass, we are relying on the sockets file descriptor not being reused after the socket is closed. // This is inherently racy, so we allow getSoLinger to throw ChannelException a few of times, but eventually we do // want to see a ClosedChannelException for the test to pass. - @RepeatedIfExceptionsTest(repeats = 4, exceptions = ChannelException.class) + @RepeatedIfExceptionsTest(repeats = 4) public void testGetOptionWhenClosed() { ch.close().syncUninterruptibly(); - try { - ch.config().getSoLinger(); - fail(); - } catch (ChannelException e) { - if (!(e.getCause() instanceof ClosedChannelException)) { - AssertionError error = new AssertionError( - "Expected the suppressed exception to be an instance of ClosedChannelException."); - error.addSuppressed(e); - throw error; + ChannelException e = assertThrows(ChannelException.class, new Executable() { + @Override + public void execute() throws Throwable { + ch.config().getSoLinger(); } - } + }); + assertThat(e).hasCauseInstanceOf(ClosedChannelException.class); } @Test