test fix

Author: bluestreak01

core/src/main/java/io/questdb/client/cutlass/qwp/client/sf/cursor/SenderErrorDispatcher.java

@@ -215,12 +215,16 @@ private void dispatchLoop() {
                 // Closed-check at the loop head will catch the rest.
                 continue;
             }
+            // Increment before invoking the handler: observers using a
+            // CountDownLatch in the handler must be able to read the
+            // updated counter once their latch fires. With the increment
+            // after, the handler-released observer races the dispatcher
+            // and can see totalDelivered short by one.
+            totalDelivered.incrementAndGet();
             try {
                 handler.onError(err);
             } catch (Throwable t) {
                 LOG.error("SenderErrorHandler threw on {}: {}", err, t.getMessage(), t);
-            } finally {
-                totalDelivered.incrementAndGet();
             }
         }
     }

core/src/test/java/io/questdb/client/test/cutlass/qwp/client/InitialConnectRetryTest.java

@@ -32,7 +32,6 @@
 import java.io.IOException;
 import java.nio.ByteBuffer;
 import java.nio.ByteOrder;
-import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicLong;
 
 /**
@@ -45,22 +44,30 @@ public class InitialConnectRetryTest {
     private static final int TEST_PORT = 19_700 + (int) (System.nanoTime() % 100);
 
     @Test
-    public void testWithoutRetryFailsImmediately() {
-        // No server on this port. With initial_connect_retry off (default),
-        // fromConfig must throw without sitting around for the cap.
-        int port = TEST_PORT + 1;
-        long t0 = System.nanoTime();
-        try (Sender ignored = Sender.fromConfig("ws::addr=localhost:" + port + ";")) {
-            Assert.fail("expected immediate connect failure");
+    public void testWithRetryGivesUpAfterCap() {
+        // No server. With retry on, fromConfig must run the retry loop and
+        // ultimately throw with the connectWithRetry-shaped message that
+        // names the elapsed budget and attempt count. The actual budget
+        // honoring is observable through that message — we don't need a
+        // wall-clock check.
+        int port = TEST_PORT + 3;
+        String cfg = "ws::addr=127.0.0.1:" + port
+                + ";initial_connect_retry=true"
+                + ";reconnect_max_duration_millis=400"
+                + ";reconnect_initial_backoff_millis=10"
+                + ";reconnect_max_backoff_millis=50;";
+        try (Sender ignored = Sender.fromConfig(cfg)) {
+            Assert.fail("expected give-up after cap");
         } catch (Exception expected) {
-            long elapsedMs = (System.nanoTime() - t0) / 1_000_000L;
-            Assert.assertTrue("must fail fast (took " + elapsedMs + " ms)",
-                    elapsedMs < 2_000L);
+            String msg = expected.getMessage();
+            Assert.assertNotNull("error must have a message", msg);
+            Assert.assertTrue("error must come from the retry loop: " + msg,
+                    msg.contains("initial connect") && msg.contains("attempts"));
         }
     }
 
     @Test
-    public void testWithRetrySucceedsWhenServerComesUpInTime() throws Exception {
+    public void testWithRetrySucceedsWhenServerComesUpInTime() {
         // initial_connect_retry=true; we open the sender BEFORE starting
         // the server, then start the server in a background thread after
         // a short delay. The retry loop should see the server come up and
@@ -79,7 +86,7 @@ public void testWithRetrySucceedsWhenServerComesUpInTime() throws Exception {
         starter.setDaemon(true);
         starter.start();
         try {
-            String cfg = "ws::addr=localhost:" + port
+            String cfg = "ws::addr=127.0.0.1:" + port
                     + ";initial_connect_retry=true"
                     + ";reconnect_max_duration_millis=5000"
                     + ";reconnect_initial_backoff_millis=50"
@@ -99,30 +106,30 @@ public void testWithRetrySucceedsWhenServerComesUpInTime() throws Exception {
     }
 
     @Test
-    public void testWithRetryGivesUpAfterCap() {
-        // No server. With retry on but a tight cap, fromConfig should
-        // throw within the cap window (with some slack).
-        int port = TEST_PORT + 3;
-        long t0 = System.nanoTime();
-        String cfg = "ws::addr=localhost:" + port
-                + ";initial_connect_retry=true"
-                + ";reconnect_max_duration_millis=400"
-                + ";reconnect_initial_backoff_millis=10"
-                + ";reconnect_max_backoff_millis=50;";
-        try (Sender ignored = Sender.fromConfig(cfg)) {
-            Assert.fail("expected give-up after cap");
+    public void testWithoutRetryFailsImmediately() {
+        // No server on this port. With initial_connect_retry off (default),
+        // fromConfig must throw on the first connect failure rather than enter
+        // the retry loop. We assert the structural shape of the error: the
+        // raw "Failed to connect" message from buildAndConnect, NOT the
+        // "initial connect ... attempts" message connectWithRetry produces.
+        int port = TEST_PORT + 1;
+        // Use the IPv4 literal so the test doesn't pay first-call
+        // getaddrinfo("localhost") cost on Windows (1-2 s cold lookup).
+        try (Sender ignored = Sender.fromConfig("ws::addr=127.0.0.1:" + port + ";")) {
+            Assert.fail("expected immediate connect failure");
         } catch (Exception expected) {
-            long elapsedMs = (System.nanoTime() - t0) / 1_000_000L;
-            Assert.assertTrue("must give up around the cap (took " + elapsedMs + " ms)",
-                    elapsedMs >= 300L && elapsedMs < 3_000L);
             String msg = expected.getMessage();
-            Assert.assertTrue("error must mention startup retry: " + msg,
-                    msg != null && (msg.contains("initial connect")
-                            || msg.contains("Failed to connect")));
+            Assert.assertNotNull("error must have a message", msg);
+            Assert.assertTrue("error must be the raw connect-refused: " + msg,
+                    msg.contains("Failed to connect"));
+            Assert.assertFalse("error must NOT mention the retry loop: " + msg,
+                    msg.contains("attempts"));
         }
     }
 
-    /** Acks every binary frame so the sender's flush completes. */
+    /**
+     * Acks every binary frame so the sender's flush completes.
+     */
     private static class AckHandler implements TestWebSocketServer.WebSocketServerHandler {
         private final AtomicLong nextSeq = new AtomicLong(0);