From 0955acbea7ddd02c4ac06f866a981165475e9261 Mon Sep 17 00:00:00 2001
From: Idel Pivnitskiy <idel.pivnitskiy@apple.com>
Date: Tue, 12 Nov 2024 18:42:47 -0800
Subject: [PATCH] Improvements for `ConnectionCloseHeaderHandlingTest` (#3099)

Motivation:

1. `serverCloseSecondPipelinedRequestWriteAborted` test should write
only portion of the `/second` request to simulate incomplete write.
Currently, it writes the full content and then concats with `never()`.
As a result, internal state machine of the pipelined connection
(`RequestResponseCloseHandler`) thinks that we already finished the
write.
2. `tearDown()` may throw NPE if we use `assume` to skip some test
scenarios.

Modifications:

1. Write only half of the intended content.
2. Add null checks inside `tearDown()` method.
---
 .../ConnectionCloseHeaderHandlingTest.java    | 19 ++++++++++++++++---
 1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/servicetalk-http-netty/src/test/java/io/servicetalk/http/netty/ConnectionCloseHeaderHandlingTest.java b/servicetalk-http-netty/src/test/java/io/servicetalk/http/netty/ConnectionCloseHeaderHandlingTest.java
index fc0082d8d1..91fceaa81c 100644
--- a/servicetalk-http-netty/src/test/java/io/servicetalk/http/netty/ConnectionCloseHeaderHandlingTest.java
+++ b/servicetalk-http-netty/src/test/java/io/servicetalk/http/netty/ConnectionCloseHeaderHandlingTest.java
@@ -18,6 +18,7 @@
 import io.servicetalk.buffer.api.Buffer;
 import io.servicetalk.concurrent.BlockingIterator;
 import io.servicetalk.concurrent.api.Completable;
+import io.servicetalk.concurrent.api.CompositeCloseable;
 import io.servicetalk.http.api.HttpPayloadWriter;
 import io.servicetalk.http.api.HttpServerBuilder;
 import io.servicetalk.http.api.ReservedStreamingHttpConnection;
@@ -132,7 +133,7 @@ void setUp(boolean useUds, boolean viaProxy, boolean awaitRequestPayload) throws
                     HttpServers.forAddress(localAddress(0)))
                     .ioExecutor(serverCtx.ioExecutor())
                     .executor(serverCtx.executor())
-                    .enableWireLogging("servicetalk-tests-wire-logger", TRACE, Boolean.TRUE::booleanValue)
+                    .enableWireLogging("servicetalk-tests-wire-logger", TRACE, () -> true)
                     .appendConnectionAcceptorFilter(original -> new DelegatingConnectionAcceptor(original) {
                         @Override
                         public Completable accept(final ConnectionContext context) {
@@ -216,7 +217,17 @@ public Completable accept(final ConnectionContext context) {
         @AfterEach
         void tearDown() throws Exception {
             try {
-                newCompositeCloseable().appendAll(connection, client, serverContext).close();
+                CompositeCloseable closeable = newCompositeCloseable();
+                if (connection != null) {
+                    closeable.append(connection);
+                }
+                if (client != null) {
+                    closeable.append(client);
+                }
+                if (serverContext != null) {
+                    closeable.append(serverContext);
+                }
+                closeable.close();
             } finally {
                 if (proxyTunnel != null) {
                     safeClose(proxyTunnel);
@@ -392,7 +403,9 @@ void serverCloseSecondPipelinedRequestWriteAborted(boolean useUds, boolean viaPr
             String content = "request_content";
             connection.request(connection.get("/second")
                     .addHeader(CONTENT_LENGTH, valueOf(content.length()))
-                    .payloadBody(from(content).concat(never()), RAW_STRING_SERIALIZER))
+                    // Write only part of the intended payload body to simulate incomplete request:
+                    .payloadBody(from(content.substring(0, content.length() / 2)).concat(never()),
+                            RAW_STRING_SERIALIZER))
                     .whenOnError(secondRequestError::set)
                     .whenFinally(secondResponseReceived::countDown)
                     .subscribe(second -> { });