CB-510: SessionReaper wrapper tests (0% -> 86.7%)
SessionReaper had no tests at all. Its TTL *policy* was already well covered (SessionManager.reapIdle, 6 cases in SessionManagerTest); what was untested was the thread wrapper around it — idempotent start/stop and whether the loop actually runs and actually stops. Observed through an injected clock rather than by sleeping and hoping: reapIdle reads nowNanos exactly once per call, so the tick count IS the iteration count. Waits are bounded polls, not fixed sleeps, and nothing asserts an exact timing-derived number — flaky counts would be worse than no test. 321 tests (was 318); line coverage 66.9% -> 67.9%. Drafted by an opencode worker on the new local-vLLM profile (branch worker/cb-510-session-reaper-test-cd1793-1). Its structure and setup were good and it was honest that it could not run mvn. But its third test asserted NOTHING — it started the reaper, slept, stopped it, and relied on "no throw", with a comment claiming that proved the loop had run. It did not: verified by sabotage, all three of its tests passed against a start() replaced with an immediate return. Rewritten so the assertions can fail for the right reason. Same sabotage now fails 2 of 3 (the third only pins stop()-before-start(), where "does not throw" genuinely is the contract). Uncomfortably on the nose given this task began as a hunt for tests that do not mean anything.
This commit is contained in:
@@ -0,0 +1,131 @@
|
||||
package dev.ltms.bridged.session;
|
||||
|
||||
import dev.ltms.bridged.config.BridgedConfig;
|
||||
import dev.ltms.bridged.guard.SubscriptionGuard;
|
||||
import dev.ltms.bridged.herdr.AgentControl;
|
||||
import dev.ltms.bridged.herdr.FakeHerdr;
|
||||
import dev.ltms.bridged.herdr.WorkspaceControl;
|
||||
import dev.ltms.bridged.worker.ClaudeCodeLauncher;
|
||||
import org.junit.jupiter.api.Test;
|
||||
|
||||
import java.util.List;
|
||||
import java.util.Map;
|
||||
import java.util.Set;
|
||||
import java.util.concurrent.atomic.AtomicLong;
|
||||
|
||||
import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
|
||||
import static org.junit.jupiter.api.Assertions.assertEquals;
|
||||
import static org.junit.jupiter.api.Assertions.assertTrue;
|
||||
|
||||
/**
|
||||
* Wrapper-behaviour tests for {@link SessionReaper} (the thread lifecycle). The TTL policy itself
|
||||
* (SessionManager.reapIdle) is covered by SessionManagerTest and is deliberately not retested here.
|
||||
* A real SessionManager is used, built the same way the rest of this package's tests do.
|
||||
*/
|
||||
class SessionReaperTest {
|
||||
|
||||
private static final long IDLE_TTL_SECONDS = 60;
|
||||
private static final long SHORT_INTERVAL_MILLIS = 20;
|
||||
|
||||
private static ClaudeCodeLauncher launcher() {
|
||||
FakeHerdr herdr = new FakeHerdr();
|
||||
BridgedConfig.Worker cfg = new BridgedConfig.Worker(
|
||||
"ltms-local", "http://gx00.gw:8000", "coder", null, "BRIDGED_WORKER_TOKEN",
|
||||
List.of("ccs", "ltms-local"), "tab", "bridged-workers",
|
||||
"worker: {profile} #{n}", null, null, null);
|
||||
return new ClaudeCodeLauncher(new AgentControl(herdr), new WorkspaceControl(herdr),
|
||||
new SubscriptionGuard(Set.of("gx00.gw")), Map.of(cfg.profile(), cfg), cfg.profile(), _ -> null);
|
||||
}
|
||||
|
||||
/** A manager on the fake worktree seam — these tests never touch a real git checkout. */
|
||||
private static SessionManager sessionManager() {
|
||||
return new SessionManager(launcher(), new FakeWorktrees());
|
||||
}
|
||||
|
||||
private static SessionReaper reaper() {
|
||||
return new SessionReaper(sessionManager(), IDLE_TTL_SECONDS, SHORT_INTERVAL_MILLIS);
|
||||
}
|
||||
|
||||
/**
|
||||
* A double {@code start()} must leave exactly one live loop, so a single {@code stop()} still
|
||||
* silences it. Asserting only "no throw" would pass against a reaper that never started at
|
||||
* all — and against one that started twice — which is the entire point of the guard.
|
||||
*/
|
||||
@Test
|
||||
void startIsIdempotent() throws InterruptedException {
|
||||
AtomicLong ticks = new AtomicLong();
|
||||
SessionReaper reaper = new SessionReaper(countingManager(ticks),
|
||||
IDLE_TTL_SECONDS, SHORT_INTERVAL_MILLIS);
|
||||
|
||||
assertDoesNotThrow(() -> {
|
||||
reaper.start();
|
||||
reaper.start();
|
||||
}, "a second start() must not throw");
|
||||
assertTrue(awaitTicks(ticks, 2), "the loop is running after a double start()");
|
||||
|
||||
// One stop() for two start() calls: if the second start had spawned its own loop, a
|
||||
// surviving thread would keep the counter climbing past this point.
|
||||
reaper.stop();
|
||||
Thread.sleep(SHORT_INTERVAL_MILLIS * 4);
|
||||
long settled = ticks.get();
|
||||
Thread.sleep(SHORT_INTERVAL_MILLIS * 4);
|
||||
assertEquals(settled, ticks.get(),
|
||||
"a single stop() must silence the reaper even after two start() calls");
|
||||
}
|
||||
|
||||
/** A manager whose clock counts reads — every {@code reapIdle} reads it exactly once. */
|
||||
private static SessionManager countingManager(AtomicLong ticks) {
|
||||
return new SessionManager(launcher(), new FakeWorktrees(), () -> {
|
||||
ticks.incrementAndGet();
|
||||
return System.nanoTime();
|
||||
});
|
||||
}
|
||||
|
||||
/** Bounded wait for the loop to tick at least {@code n} times; avoids fixed-sleep flakiness. */
|
||||
private static boolean awaitTicks(AtomicLong ticks, long n) throws InterruptedException {
|
||||
long deadline = System.currentTimeMillis() + 2000;
|
||||
while (ticks.get() < n && System.currentTimeMillis() < deadline) {
|
||||
Thread.sleep(10);
|
||||
}
|
||||
return ticks.get() >= n;
|
||||
}
|
||||
|
||||
@Test
|
||||
void stopIsIdempotentAndSafeBeforeStart() {
|
||||
SessionReaper reaper = reaper();
|
||||
|
||||
assertDoesNotThrow(reaper::stop, "stop() before start() must not throw");
|
||||
assertDoesNotThrow(reaper::stop, "a second stop() must not throw");
|
||||
}
|
||||
|
||||
/**
|
||||
* The loop must actually iterate, and {@code stop()} must actually end it.
|
||||
*
|
||||
* <p>Observed through an injected clock rather than by sleeping and hoping: every
|
||||
* {@code reapIdle} call reads {@code nowNanos} exactly once, so the tick count <em>is</em> the
|
||||
* iteration count. Asserting merely "nothing threw" would pass even if {@code start()} were a
|
||||
* no-op, which is the whole behaviour under test.
|
||||
*/
|
||||
@Test
|
||||
void theLoopRunsRepeatedlyAndStopEndsIt() throws InterruptedException {
|
||||
AtomicLong ticks = new AtomicLong();
|
||||
SessionReaper reaper = new SessionReaper(countingManager(ticks),
|
||||
IDLE_TTL_SECONDS, SHORT_INTERVAL_MILLIS);
|
||||
|
||||
reaper.start();
|
||||
// Bounded wait rather than a fixed sleep + exact count: proves repetition without pinning
|
||||
// a timing-derived number that would flake on a loaded machine.
|
||||
boolean iterated = awaitTicks(ticks, 2);
|
||||
long whileRunning = ticks.get();
|
||||
reaper.stop();
|
||||
assertTrue(iterated,
|
||||
"the reaper loop must iterate repeatedly; observed " + whileRunning + " tick(s)");
|
||||
|
||||
// After stop() the loop must go quiet. Allow one in-flight iteration to finish, then
|
||||
// confirm the count has stopped advancing.
|
||||
Thread.sleep(SHORT_INTERVAL_MILLIS * 4);
|
||||
long settled = ticks.get();
|
||||
Thread.sleep(SHORT_INTERVAL_MILLIS * 4);
|
||||
assertEquals(settled, ticks.get(), "stop() must end the loop, not just flag it");
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user