Skip to content

Commit e496e62

Browse files
committed
test(qwp): scope Invariant-B test exception swallows to close-path noise only (M6)
The broad catch (Exception ignored) around whole test bodies let the Invariant-B pin tests pass vacuously: fromConfig/builder/first-flush/setup failures were swallowed, skipping every contract assertion. - ReconnectTest.testReconnectNeverGivesUpInvariantB: hoist fromConfig out of the swallow; confine the catch to sender.close() in a finally. - InitialConnectAsyncTest: add closeQuietly(Sender) and use it in testAsyncDeliversBufferedRowsWhenServerArrivesLate, testConnectionLostRetriesForeverNoTerminal, and testWasEverConnectedTrueImmediatelyInSyncMode; drop the outer catches. - testConnectionLostRetriesForeverNoTerminal additionally had a bare finally { sender.close(); } whose close-path exception could REPLACE a pending AssertionError and get swallowed by the outer catch -- i.e. a broken feature (terminal delivered, close() rethrows it) actively masked the assertion failure. closeQuietly in the finally closes that path too.
1 parent 4340371 commit e496e62

2 files changed

Lines changed: 42 additions & 16 deletions

File tree

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

Lines changed: 31 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -144,7 +144,7 @@ public void testAsyncNoServerRetriesForeverNoTerminal() throws Exception {
144144
}
145145

146146
@Test
147-
public void testAsyncDeliversBufferedRowsWhenServerArrivesLate() {
147+
public void testAsyncDeliversBufferedRowsWhenServerArrivesLate() throws Exception {
148148
// Sender opens before the server is listening. Frames are
149149
// appended to the cursor SF engine on the producer thread. The
150150
// I/O thread retries connect in the background; once the server
@@ -158,7 +158,10 @@ public void testAsyncDeliversBufferedRowsWhenServerArrivesLate() {
158158
+ ";reconnect_initial_backoff_millis=20"
159159
+ ";reconnect_max_backoff_millis=200"
160160
+ ";close_flush_timeout_millis=2000;";
161-
try (Sender sender = Sender.fromConfig(cfg)) {
161+
// fromConfig/flush/setup failures must fail the test -- only
162+
// close() teardown noise is tolerated (see closeQuietly).
163+
Sender sender = Sender.fromConfig(cfg);
164+
try {
162165
QwpWebSocketSender wss = (QwpWebSocketSender) sender;
163166
// wasEverConnected starts false in async mode — the I/O
164167
// thread has not yet completed an upgrade.
@@ -187,9 +190,9 @@ public void testAsyncDeliversBufferedRowsWhenServerArrivesLate() {
187190
Assert.assertTrue(
188191
"wasEverConnected() must flip to true after the I/O thread connects",
189192
((QwpWebSocketSender) sender).wasEverConnected());
193+
} finally {
194+
closeQuietly(sender);
190195
}
191-
} catch (Exception ignored) {
192-
// already closed
193196
}
194197
}
195198

@@ -222,7 +225,7 @@ public void testAsyncReturnsImmediatelyWithNoServer() {
222225
}
223226

224227
@Test
225-
public void testConnectionLostRetriesForeverNoTerminal() {
228+
public void testConnectionLostRetriesForeverNoTerminal() throws Exception {
226229
// INVARIANT B: after a successful connect, if the server drops, the
227230
// mid-stream reconnect must retry FOREVER -- it must NEVER surface a
228231
// connection-lost terminal on a wall-clock budget. The rows are safe in
@@ -266,15 +269,16 @@ public void testConnectionLostRetriesForeverNoTerminal() {
266269
"wasEverConnected() must remain true after the outage",
267270
((QwpWebSocketSender) sender).wasEverConnected());
268271
} finally {
269-
sender.close();
272+
// closeQuietly (not a bare close()) so a close-path exception
273+
// cannot replace a pending AssertionError from the contract
274+
// assertions above and mask a genuine failure.
275+
closeQuietly(sender);
270276
}
271-
} catch (Exception ignored) {
272-
// already closed
273277
}
274278
}
275279

276280
@Test
277-
public void testWasEverConnectedTrueImmediatelyInSyncMode() {
281+
public void testWasEverConnectedTrueImmediatelyInSyncMode() throws Exception {
278282
// Default (OFF) and SYNC modes both connect on the user thread
279283
// before fromConfig returns. wasEverConnected() must therefore
280284
// already be true the instant the sender becomes visible to the
@@ -287,13 +291,14 @@ public void testWasEverConnectedTrueImmediatelyInSyncMode() {
287291
Assert.assertTrue(server.awaitStart(5, java.util.concurrent.TimeUnit.SECONDS));
288292
String cfg = "ws::addr=localhost:" + port
289293
+ ";close_flush_timeout_millis=0;";
290-
try (Sender sender = Sender.fromConfig(cfg)) {
294+
Sender sender = Sender.fromConfig(cfg);
295+
try {
291296
Assert.assertTrue(
292297
"wasEverConnected() must be true immediately in OFF/SYNC mode",
293298
((QwpWebSocketSender) sender).wasEverConnected());
299+
} finally {
300+
closeQuietly(sender);
294301
}
295-
} catch (Exception ignored) {
296-
// already closed
297302
}
298303
}
299304

@@ -316,6 +321,20 @@ private static void awaitAtLeastOneConnectAttempt(QwpWebSocketSender wss) {
316321
}
317322
}
318323

324+
/**
325+
* Closes the sender, tolerating close-path teardown noise only. Used
326+
* instead of a broad {@code catch (Exception ignored)} around a whole
327+
* test body, which would swallow fromConfig/flush/setup failures and
328+
* let the contract assertions pass vacuously.
329+
*/
330+
private static void closeQuietly(Sender sender) {
331+
try {
332+
sender.close();
333+
} catch (Exception ignored) {
334+
// close() teardown noise only
335+
}
336+
}
337+
319338
/**
320339
* Closes the sender and tolerates either outcome:
321340
* * close() throws -- the latched terminal must mention the expected

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

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,10 @@ public void testReconnectNeverGivesUpInvariantB() throws Exception {
119119
+ ";reconnect_max_backoff_millis=50"
120120
+ ";close_flush_timeout_millis=0;";
121121
Throwable observed = null;
122-
try (Sender sender = Sender.fromConfig(cfg)) {
122+
// fromConfig/first-flush/setup failures must fail the test --
123+
// only close() teardown noise is tolerated in the finally below.
124+
Sender sender = Sender.fromConfig(cfg);
125+
try {
123126
sender.table("foo").longColumn("v", 1L).atNow();
124127
sender.flush();
125128

@@ -141,9 +144,13 @@ public void testReconnectNeverGivesUpInvariantB() throws Exception {
141144
}
142145
Thread.sleep(50);
143146
}
144-
} catch (Exception ignored) {
145-
// close() teardown noise -- the contract under test is the flush
146-
// loop above, captured in `observed`.
147+
} finally {
148+
try {
149+
sender.close();
150+
} catch (Exception ignored) {
151+
// close() teardown noise -- the contract under test is the
152+
// flush loop above, captured in `observed`.
153+
}
147154
}
148155
Assert.assertNull(
149156
"mid-stream reconnect must retry forever, not surface a terminal "

0 commit comments

Comments
 (0)