From da7bd97e3c7acdca0dde14a9f6341ee78b7a8cda Mon Sep 17 00:00:00 2001 From: Sean Derrow Date: Sun, 4 Oct 2026 22:12:56 -0400 Subject: [PATCH 1/2] test: make Group error isolation test deterministic "Should pass error on failure" asserted an exact cross-key interleaving ([6] before [3]) that only held if a 40ms setTimeout fired inside a 100ms minTime window. Under the redis projects the Group is still local but timers are real, so an event-loop stall reordered the keys and failed the test. Assert the contract instead: hold key A's first job with a deferred, then show B's failure reaches its caller and C runs both jobs while A is held, so keys don't share capacity, minTime, or failures. A then drains in order. Co-Authored-By: Claude Opus 5.5 (1M context) --- test/group.test.ts | 63 +++++++++++++++++++++++++--------------------- 1 file changed, 35 insertions(+), 28 deletions(-) diff --git a/test/group.test.ts b/test/group.test.ts index 513ab04..278b249 100644 --- a/test/group.test.ts +++ b/test/group.test.ts @@ -3,7 +3,7 @@ import sleep from "../src/sleep"; import Bottleneck from "./bottleneck"; import { useFakeClock } from "./helpers/clock"; import { defined } from "./helpers/defined"; -import { test, waitForState } from "./helpers/test-api"; +import { deferred, test, waitForState } from "./helpers/test-api"; useFakeClock(); @@ -134,35 +134,42 @@ describe("Group", () => { }); expect(Object.keys(group.limiters)).toStrictEqual([]); - const results: unknown[][] = []; - - const job = async (...result: unknown[]) => { - results.push(result); - await sleep(50); + const ranA: unknown[][] = []; + const ranC: unknown[][] = []; + const runA = async (...result: unknown[]) => { + ranA.push(result); + }; + const runC = async (...result: unknown[]) => { + ranC.push(result); }; - group.key("A").schedule(job, 1, 2); - group.key("A").schedule(job, 3); - group.key("A").schedule(job, 4); - // Fire-and-forget: the rejection must be recorded concurrently with the - // other scheduled jobs; awaiting it inline would delay the schedules below - // and change the ordering under test. - group - .key("B") - .schedule(() => Promise.reject(new Error(failureMessage))) - .catch((err) => { - results.push(["CAUGHT", err.message]); - }); - setTimeout(() => { - group.key("C").schedule(job, 6); - group.key("C").schedule(job, 7); - }, 40); - - // Scheduled last on key "A", so it runs once all other jobs are done and - // acts as the completion barrier; assertion failures reject the promise. - await group.key("A").schedule(async () => { - expect(results).toStrictEqual([[1, 2], ["CAUGHT", failureMessage], [6], [3], [7], [4]]); - }); + // Hold key A's first job open. Whatever runs on B and C meanwhile proves + // a Group's keys don't share capacity, minTime, or failures. + const holdA = deferred(); + const a1 = group.key("A").schedule( + async (...result: unknown[]) => { + ranA.push(result); + await holdA.signal; + }, + 1, + 2, + ); + const a3 = group.key("A").schedule(runA, 3); + const a4 = group.key("A").schedule(runA, 4); + await waitForState(() => expect(ranA).toStrictEqual([[1, 2]])); + + // The failure reaches its caller... + await expect( + group.key("B").schedule(() => Promise.reject(new Error(failureMessage))), + ).rejects.toThrow(failureMessage); + // ...and doesn't disturb the other keys. + await Promise.all([group.key("C").schedule(runC, 6), group.key("C").schedule(runC, 7)]); + expect(ranC).toStrictEqual([[6], [7]]); + expect(ranA).toStrictEqual([[1, 2]]); + + holdA.release(); + await Promise.all([a1, a3, a4]); + expect(ranA).toStrictEqual([[1, 2], [3], [4]]); }); test("Should update its timeout", async ({ makeGroup }) => { From 2356669bcefcd6d5fce5393c3f43828433ae6d79 Mon Sep 17 00:00:00 2001 From: Sean Derrow Date: Sun, 4 Oct 2026 22:56:17 -0400 Subject: [PATCH 2/2] add back --- test/group.test.ts | 52 +++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 51 insertions(+), 1 deletion(-) diff --git a/test/group.test.ts b/test/group.test.ts index 278b249..bc2cc67 100644 --- a/test/group.test.ts +++ b/test/group.test.ts @@ -144,7 +144,9 @@ describe("Group", () => { }; // Hold key A's first job open. Whatever runs on B and C meanwhile proves - // a Group's keys don't share capacity, minTime, or failures. + // a Group's keys don't share capacity or failures. (Holding A's slot can't + // show minTime isolation: C would run under a shared minTime clock too, + // just later. The fake-clock test below covers that.) const holdA = deferred(); const a1 = group.key("A").schedule( async (...result: unknown[]) => { @@ -172,6 +174,54 @@ describe("Group", () => { expect(ranA).toStrictEqual([[1, 2], [3], [4]]); }); + // Each key has its own minTime clock, so keys interleave in time: C[6] runs + // at 40 while A's next dispatch is gated until 100, then A[3] (100), C[7] + // (140), A[4] (200). The job sleeps 50 < minTime, so A[3] waits on minTime, + // not on concurrency. The exact interleaving needs virtual time; under real + // timers a stall reorders the keys. Gated on DATASTORE (the condition + // useFakeClock() checks), not isFakeClock(): timers are not installed yet at + // test-collection time. + test.runIf(process.env.DATASTORE == null)( + "Should interleave keys by their own minTime", + async ({ makeGroup }) => { + const failureMessage = "SOMETHING BLEW UP!!"; + const group = makeGroup({ + maxConcurrent: 1, + minTime: 100, + }); + + const results: unknown[][] = []; + + const job = async (...result: unknown[]) => { + results.push(result); + await sleep(50); + }; + + group.key("A").schedule(job, 1, 2); + group.key("A").schedule(job, 3); + group.key("A").schedule(job, 4); + // Fire-and-forget: the rejection must be recorded concurrently with the + // other scheduled jobs; awaiting it inline would delay the schedules below + // and change the ordering under test. + group + .key("B") + .schedule(() => Promise.reject(new Error(failureMessage))) + .catch((err) => { + results.push(["CAUGHT", err.message]); + }); + setTimeout(() => { + group.key("C").schedule(job, 6); + group.key("C").schedule(job, 7); + }, 40); + + // Scheduled last on key "A", so it runs once all other jobs are done and + // acts as the completion barrier; assertion failures reject the promise. + await group.key("A").schedule(async () => { + expect(results).toStrictEqual([[1, 2], ["CAUGHT", failureMessage], [6], [3], [7], [4]]); + }); + }, + ); + test("Should update its timeout", async ({ makeGroup }) => { const group1 = makeGroup({ maxConcurrent: 1,