Skip to content

Commit

Permalink
Use assertThat#instanceOf and assertThat#isNotInstanceOf [Codec-H…
Browse files Browse the repository at this point in the history
…TTP] (netty#13135)

Motivation:
We use `assertTrue(obj instanceOf Cat)` for asserting object instances. While this is correct and works, it becomes impossible to debug when the assertion fails because we have no clue at all which instance type made the assertion fail.

Modification:
To address this, we should use `assertThat#instanceOf` which will do the exact same thing but in a much cleaner way. Also, it will give us more information when it fails like why the assertion of instance check failed and what exactly we got in place of the expected one.

Result:
Cleaner and easier to debug test case failure

This PR migrates the entire Codec-HTTP module completely to the new API.
  • Loading branch information
hyperxpro authored Jan 18, 2023
1 parent c0e7141 commit 10a9490
Show file tree
Hide file tree
Showing 4 changed files with 8 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -380,7 +380,7 @@ public void testInformationalResponseKeepsPairsInSync() {
HttpContent content = ch.readInbound();
// HTTP 102 is not allowed to have content.
assertThat(content.content().readableBytes(), is(0));
assertThat(content, CoreMatchers.<HttpContent>instanceOf(LastHttpContent.class));
assertThat(content, instanceOf(LastHttpContent.class));
content.release();

assertTrue(ch.writeOutbound(new DefaultFullHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.GET, "/")));
Expand All @@ -395,7 +395,7 @@ public void testInformationalResponseKeepsPairsInSync() {
content = ch.readInbound();
// HTTP 200 has content.
assertThat(content.content().readableBytes(), is(8));
assertThat(content, CoreMatchers.<HttpContent>instanceOf(LastHttpContent.class));
assertThat(content, instanceOf(LastHttpContent.class));
content.release();

assertThat(ch.finish(), is(false));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import io.netty.channel.embedded.EmbeddedChannel;
import io.netty.util.AsciiString;
import io.netty.util.CharsetUtil;
import org.hamcrest.CoreMatchers;
import org.junit.jupiter.api.Test;

import java.util.ArrayList;
Expand Down Expand Up @@ -307,7 +308,7 @@ public void testTooLargeInitialLine() {
assertTrue(channel.writeInbound(Unpooled.copiedBuffer(requestStr, CharsetUtil.US_ASCII)));
HttpRequest request = channel.readInbound();
assertTrue(request.decoderResult().isFailure());
assertTrue(request.decoderResult().cause() instanceof TooLongHttpLineException);
assertThat(request.decoderResult().cause(), instanceOf(TooLongHttpLineException.class));
assertFalse(channel.finish());
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
import io.netty.util.ReferenceCountUtil;
import org.junit.jupiter.api.Test;

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.assertNotNull;
Expand Down Expand Up @@ -179,7 +180,7 @@ protected boolean shouldHandleUpgradeRequest(HttpRequest req) {
assertNull(channel.pipeline().get("marker"));

HttpRequest req = channel.readInbound();
assertFalse(req instanceof FullHttpRequest); // Should not be aggregated.
assertThat(req).isNotInstanceOf(FullHttpRequest.class); // Should not be aggregated.
assertTrue(req.headers().contains(HttpHeaderNames.CONNECTION, "Upgrade", false));
assertTrue(req.headers().contains(HttpHeaderNames.UPGRADE, "do-not-upgrade", false));
assertTrue(channel.readInbound() instanceof LastHttpContent);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
import java.util.Iterator;

import static io.netty.handler.codec.http.HttpVersion.HTTP_1_1;
import static org.hamcrest.CoreMatchers.instanceOf;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
Expand Down Expand Up @@ -188,7 +189,7 @@ private static void testCloseReason0(ChannelHandler... handlers) {
// expected
}
ReferenceCounted closeMessage = ch.readOutbound();
assertThat(closeMessage, CoreMatchers.instanceOf(ByteBuf.class));
assertThat(closeMessage, instanceOf(ByteBuf.class));
closeMessage.release();
assertFalse(ch.finish());
}
Expand Down

0 comments on commit 10a9490

Please sign in to comment.