Skip to content

Commit f6200a0

Browse files
adrians5jclaude
andcommitted
test(api-scheduler-server): reword safeRemove comment to read naturally
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 6542df3 commit f6200a0

1 file changed

Lines changed: 11 additions & 12 deletions

File tree

packages/api-scheduler-server/__tests__/BreeSchedulerService.test.ts

Lines changed: 11 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -161,13 +161,13 @@ describe("BreeSchedulerService", () => {
161161
});
162162

163163
describe("safeRemove logging", () => {
164-
// safeRemove() swallows bree.stop()/remove() errors and logs them at debug level — a guard for
165-
// the race where a job fires (bree drops it) between us deciding to delete it and actually
166-
// telling bree to. That guard only runs when our `jobs` map still holds an id that bree no
167-
// longer knows about. NO public API produces that divergence: create/delete/fire all keep the
168-
// `jobs` map and bree in lockstep. So this is necessarily a WHITE-BOX test — it fabricates the
169-
// divergence by writing the private `jobs` map directly, which is the only way to exercise the
170-
// swallow-and-log branch short of making bree injectable (not worth it for defensive logging).
164+
// safeRemove() ignores errors from bree.stop()/remove() and just logs them at debug level.
165+
// It's there for the case where a job fires on its own (so bree forgets it) right before we
166+
// try to delete it. To reach that branch, our `jobs` map has to still contain an id that bree
167+
// no longer knows about — but in normal use create, delete, and firing all keep the two in
168+
// sync, so there's no way to set that up through the public API. That's why this test reaches
169+
// into the private `jobs` map directly: it's the only way to recreate the mismatch without
170+
// making bree injectable, which felt like too much for a bit of defensive logging.
171171
it("should log debug when stop/remove fail silently", async () => {
172172
await service.create({
173173
id: "schedule-1",
@@ -176,13 +176,12 @@ describe("BreeSchedulerService", () => {
176176
scheduleFor: futureDate(60_000)
177177
});
178178

179-
// Delete once: removes the job from BOTH the `jobs` map and bree.
179+
// First delete removes the job from both the `jobs` map and bree.
180180
await service.delete({ id: "schedule-1", namespace, tenant });
181181

182-
// Re-add ONLY to the private `jobs` map — bree still doesn't have it. This is the
183-
// fabricated divergence: exists() (which reads `jobs`) now returns true, so the next
184-
// delete() gets past its "does not exist" guard and reaches safeRemove(), where
185-
// bree.stop()/remove() throw (unknown job) and get swallowed + logged at debug.
182+
// Now add it back to the `jobs` map only, so bree no longer has it. exists() reads the
183+
// `jobs` map, so the next delete gets past its "does not exist" check and calls
184+
// safeRemove(), where bree throws for the unknown job and we log it at debug.
186185
(service as any).jobs.set("schedule-1", { namespace, tenant });
187186
await service.delete({ id: "schedule-1", namespace, tenant });
188187

0 commit comments

Comments
 (0)