Skip to content

Commit cce3001

Browse files
committed
test(qwp): assert slot distinctness, not wrapper identity, in pool borrow tests
SenderPool.borrow() allocates a fresh PooledSender wrapper on every call, so assertNotSame on the wrappers is unconditionally true and proves nothing -- it stays green even if the pool double-lends a single slot to concurrent borrowers. Commits d6bdbdf and e77b4fc fixed this exact pattern in testBrokenSenderIsNotReturnedToPool and SenderPoolErrorSafetyTest via slotOf(); two more instances remained. SenderPoolTest.testPoolBuildsRequestedNumberOfSenders: compare the underlying slots pairwise via slotOf() and assert totalSize() == 3, so the test actually verifies what its name claims -- three concurrent borrowers each hold their own sender. SenderPoolSfTest.testTwoConcurrentSfSendersGetDistinctSlots: the slot-dir assertions prove the pool created two slots on disk, but not that the two borrowed handles wrap different ones; replace the vacuous wrapper comparison with slotOf() distinctness (helper added, mirroring SenderPoolTest and SenderPoolErrorSafetyTest). QueryImplResetTest's assertNotSame on leases is left as-is: it is an intentional contract pin (fresh lease wrapper per borrow) paired with the load-bearing assertSame on the shared QueryImpl.
1 parent 8393065 commit cce3001

2 files changed

Lines changed: 31 additions & 5 deletions

File tree

core/src/test/java/io/questdb/client/test/impl/SenderPoolSfTest.java

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,12 @@ public void testTwoConcurrentSfSendersGetDistinctSlots() throws Exception {
108108
PooledSender a = pool.borrow();
109109
PooledSender b = pool.borrow();
110110
try {
111-
Assert.assertNotSame("two borrows must be distinct wrappers", a, b);
111+
// borrow() allocates a fresh PooledSender wrapper on every
112+
// call, so comparing the wrappers is vacuously true.
113+
// Distinctness of the two borrowed senders lives in the
114+
// underlying slots (mirrors SenderPoolTest.slotOf usage).
115+
Assert.assertNotSame("two borrows must hold distinct slots",
116+
slotOf(a), slotOf(b));
112117
Assert.assertTrue("slot default-0 must exist", Files.exists(slot("default-0")));
113118
Assert.assertTrue("slot default-1 must exist", Files.exists(slot("default-1")));
114119
Assert.assertEquals("exactly two slot dirs", 2, countSlotDirs());
@@ -1947,6 +1952,16 @@ private static Object getField(Object target, String name) throws Exception {
19471952
return f.get(target);
19481953
}
19491954

1955+
// Reads the package-private PooledSender.slot -- the identity that the pool
1956+
// actually recycles. Wrapper identity is useless for aliasing checks because
1957+
// borrow() allocates a fresh wrapper every call (mirrors SenderPoolTest and
1958+
// SenderPoolErrorSafetyTest).
1959+
private static Object slotOf(PooledSender pooledWrapper) throws Exception {
1960+
Field f = PooledSender.class.getDeclaredField("slot");
1961+
f.setAccessible(true);
1962+
return f.get(pooledWrapper);
1963+
}
1964+
19501965
private static void invokeDiscardBroken(SenderPool pool, PooledSender ps) throws Exception {
19511966
Method m = SenderPool.class.getDeclaredMethod("discardBroken", PooledSender.class);
19521967
m.setAccessible(true);

core/src/test/java/io/questdb/client/test/impl/SenderPoolTest.java

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -196,14 +196,25 @@ public void testExhaustionTimeoutThrows() {
196196
}
197197

198198
@Test
199-
public void testPoolBuildsRequestedNumberOfSenders() {
199+
public void testPoolBuildsRequestedNumberOfSenders() throws Exception {
200200
try (SenderPool pool = new SenderPool(DEAD_HTTP_CONFIG, 3, 3, 1_000, Long.MAX_VALUE, Long.MAX_VALUE)) {
201201
Sender a = pool.borrow();
202202
Sender b = pool.borrow();
203203
Sender c = pool.borrow();
204-
Assert.assertNotSame(a, b);
205-
Assert.assertNotSame(b, c);
206-
Assert.assertNotSame(a, c);
204+
// borrow() allocates a fresh PooledSender wrapper on every call, so
205+
// comparing the wrappers is vacuously true -- it would stay green
206+
// even if the pool double-lent a single slot to all three borrowers.
207+
// The property this test exists for -- three concurrent borrowers
208+
// each hold their own sender -- lives in the underlying slots, so
209+
// compare those (same reasoning as testBrokenSenderIsNotReturnedToPool).
210+
Object slotA = slotOf(a);
211+
Object slotB = slotOf(b);
212+
Object slotC = slotOf(c);
213+
Assert.assertNotSame("a and b must not share a slot", slotA, slotB);
214+
Assert.assertNotSame("b and c must not share a slot", slotB, slotC);
215+
Assert.assertNotSame("a and c must not share a slot", slotA, slotC);
216+
Assert.assertEquals("pool must build exactly the requested number of senders",
217+
3, pool.totalSize());
207218
a.close();
208219
b.close();
209220
c.close();

0 commit comments

Comments
 (0)