From 12eb8a7280f3bb6da528332668af502436029871 Mon Sep 17 00:00:00 2001 From: Sean Derrow Date: Sat, 11 Jul 2026 17:03:07 -0400 Subject: [PATCH 1/2] fix(tests): reduce flakiness --- .gitignore | 1 + .oxfmtrc.json | 2 +- .oxlintrc.json | 3 +- test/batcher.test.js | 59 +-- test/cluster-coordination.test.js | 800 ++++++++++++++++-------------- test/cluster.test.js | 558 +++++++++++---------- test/general-traffic.test.js | 187 ++++--- test/general.test.js | 314 ++++++------ test/global-setup/redis.ts | 39 +- test/group.test.js | 144 +++--- test/helpers/call-log.js | 80 --- test/helpers/clock.js | 46 ++ test/helpers/job-tasks.js | 28 +- test/helpers/job-tracking.js | 34 -- test/helpers/test-api.js | 181 +++++++ test/helpers/wait-for-state.js | 77 ++- test/ioredis.test.js | 83 ++-- test/node_redis.test.js | 73 ++- test/priority.test.js | 107 ++-- test/promises.test.js | 98 ++-- test/retries.test.js | 44 +- test/stop.test.js | 50 +- vitest.config.ts | 10 +- 23 files changed, 1614 insertions(+), 1404 deletions(-) delete mode 100644 test/helpers/call-log.js create mode 100644 test/helpers/clock.js delete mode 100644 test/helpers/job-tracking.js create mode 100644 test/helpers/test-api.js diff --git a/.gitignore b/.gitignore index 1b555a1..0363169 100644 --- a/.gitignore +++ b/.gitignore @@ -5,3 +5,4 @@ test.js dist/ .env .env.* +.context/ \ No newline at end of file diff --git a/.oxfmtrc.json b/.oxfmtrc.json index 482f1e1..f9f39b6 100644 --- a/.oxfmtrc.json +++ b/.oxfmtrc.json @@ -16,5 +16,5 @@ "sortPackageJson": { "sortScripts": true }, - "ignorePatterns": ["dist/**"] + "ignorePatterns": ["dist/**", ".context/**"] } diff --git a/.oxlintrc.json b/.oxlintrc.json index 8166798..9a74ee9 100644 --- a/.oxlintrc.json +++ b/.oxlintrc.json @@ -13,7 +13,7 @@ "options": { "reportUnusedDisableDirectives": "error" }, - "ignorePatterns": ["**/.gitignore", "**/node_modules", "dist/**", "test.ts"], + "ignorePatterns": ["**/.gitignore", "**/node_modules", "dist/**", ".context/**", "test.ts"], "rules": { "no-unused-vars": [ "warn", @@ -32,6 +32,7 @@ "no-await-in-loop": "off", "no-underscore-dangle": "off", // Disabled because it's a common pattern in the codebase "no-array-constructor": "error", + "prefer-arrow-callback": "warn", "typescript/no-explicit-any": "off", // Disabled because it's a common pattern in the codebase "typescript/no-require-imports": "off", // Disabled because it's a common pattern in the codebase "typescript/no-unsafe-function-type": "error", diff --git a/test/batcher.test.js b/test/batcher.test.js index 4068116..beaa19a 100644 --- a/test/batcher.test.js +++ b/test/batcher.test.js @@ -1,21 +1,14 @@ -import { describe, it, expect, afterEach } from "vitest"; +import { useFakeClock, wait } from "./helpers/clock.js"; +import { test, describe, expect } from "./helpers/test-api.js"; const Bottleneck = require("./bottleneck"); -const wait = function (ms) { - return new Promise(function (resolve) { - setTimeout(resolve, ms); - }); -}; - -describe("Batcher", function () { - let limiter; - - afterEach(function () { - if (limiter) return limiter.disconnect(false); - }); +// Batcher is datastore-independent, so this file only runs in the `local` +// project (excluded from the redis projects in vitest.config.ts) and always +// gets the fake clock — timing assertions below are exact virtual times. +useFakeClock(); - it("Should batch by time and size", async function () { - limiter = new Bottleneck(); +describe("Batcher", () => { + test("Should batch by time and size", async function () { const batcher = new Bottleneck.Batcher({ maxTime: 100, maxSize: 3 }); const batches = []; const batchTimes = []; @@ -32,12 +25,11 @@ describe("Batcher", function () { [1, 2, 3], [4, 5], ]); - expect(batchTimes[0] - t0).toBeLessThan(20); - expect(batchTimes[1] - batchTimes[0]).toBeGreaterThanOrEqual(95); + expect(batchTimes[0] - t0).toBe(0); + expect(batchTimes[1] - batchTimes[0]).toBe(100); }); - it("Should batch by time", async function () { - limiter = new Bottleneck(); + test("Should batch by time", async function () { const batcher = new Bottleneck.Batcher({ maxTime: 100 }); const batches = []; const batchTimes = []; @@ -51,7 +43,7 @@ describe("Batcher", function () { await Promise.all([batcher.add(1), batcher.add(2)]); expect(batches).toStrictEqual([[1, 2]]); - expect(batchTimes[0] - t0).toBeGreaterThanOrEqual(95); + expect(batchTimes[0] - t0).toBe(100); const t1 = Date.now(); await Promise.all([batcher.add(3), batcher.add(4)]); @@ -60,11 +52,10 @@ describe("Batcher", function () { [1, 2], [3, 4], ]); - expect(batchTimes[1] - t1).toBeGreaterThanOrEqual(95); + expect(batchTimes[1] - t1).toBe(100); }); - it("Should batch by size", async function () { - limiter = new Bottleneck(); + test("Should batch by size", async function () { const batcher = new Bottleneck.Batcher({ maxSize: 2 }); const batches = []; @@ -82,8 +73,7 @@ describe("Batcher", function () { ]); }); - it("Should stagger flushes", async function () { - limiter = new Bottleneck(); + test("Should stagger flushes", async function () { const batcher = new Bottleneck.Batcher({ maxTime: 100, maxSize: 3 }); const batches = []; const batchTimes = []; @@ -100,19 +90,10 @@ describe("Batcher", function () { await Promise.all([p1, p2]); expect(batches).toStrictEqual([[1, 2]]); - const elapsed = batchTimes[0] - t0; - // Lower bound is the contract: the flush MUST wait for maxTime=100ms - // since adding p2 mid-window must not reset (or shorten) the flush - // timer. The upper bound is just a sanity check — under sustained - // event-loop pressure (parallel test files, redis containers booting, - // GC) setTimeout can drift well past maxTime+40ms; the original 140ms - // upper bound was flaky for that reason. - expect(elapsed).toBeGreaterThanOrEqual(95); - expect(elapsed).toBeLessThan(1000); + expect(batchTimes[0] - t0).toBe(100); }); - it("Should force then stagger flushes", async function () { - limiter = new Bottleneck(); + test("Should force then stagger flushes", async function () { const batcher = new Bottleneck.Batcher({ maxTime: 100, maxSize: 3 }); const batches = []; const batchTimes = []; @@ -125,7 +106,7 @@ describe("Batcher", function () { const t0 = Date.now(); await Promise.all([batcher.add(1), batcher.add(2), batcher.add(3)]); expect(batches).toStrictEqual([[1, 2, 3]]); - expect(batchTimes[0] - t0).toBeLessThan(20); + expect(batchTimes[0] - t0).toBe(0); const t1 = Date.now(); const p4 = batcher.add(4); @@ -137,8 +118,6 @@ describe("Batcher", function () { [1, 2, 3], [4, 5], ]); - const elapsed = batchTimes[1] - t1; - expect(elapsed).toBeGreaterThanOrEqual(95); - expect(elapsed).toBeLessThan(140); + expect(batchTimes[1] - t1).toBe(100); }); }); diff --git a/test/cluster-coordination.test.js b/test/cluster-coordination.test.js index f3b70b1..c3df38a 100644 --- a/test/cluster-coordination.test.js +++ b/test/cluster-coordination.test.js @@ -1,10 +1,10 @@ -import { describe, it, afterEach, expect } from "vitest"; -import { createJobHarness } from "./helpers/job-tracking.js"; -import { waitForState } from "./helpers/wait-for-state.js"; -const makeLimiter = require("./helpers/limiter"); +import { test, describe, expect, waitForState, deferred } from "./helpers/test-api.js"; const Bottleneck = require("./bottleneck"); const Scripts = require("../src/cluster/Scripts.js"); +// Causality policy (Workstream B): observe product-timer effects via waitForState +// and state counts — never assert wall-clock bounds around real network time. + const limiterKeys = function (limiter) { return Scripts.allKeys(limiter._store.originalId); }; @@ -22,122 +22,128 @@ const runningOrExecuting = function (limiter) { return counts.RUNNING + counts.EXECUTING; }; -describe("Cluster coordination", function () { +describe("Cluster coordination", () => { if (process.env.DATASTORE !== "redis" && process.env.DATASTORE !== "ioredis") { throw new Error("DATASTORE must be redis or ioredis"); } - let rootLimiter; - - afterEach(function () { - return rootLimiter.disconnect(false); - }); - it("Should chain local and distributed limiters (total concurrency)", function () { - const h = createJobHarness(); - rootLimiter = makeLimiter({ id: "limiter1", maxConcurrent: 3 }); - const limiter2 = new Bottleneck({ id: "limiter2", maxConcurrent: 1 }); - const limiter3 = new Bottleneck({ id: "limiter3", maxConcurrent: 2 }); + test("Should chain local and distributed limiters (total concurrency)", async function ({ + harness: h, + makeLimiter, + track, + }) { + const rootLimiter = makeLimiter({ id: "limiter1", maxConcurrent: 3 }); + const limiter2 = track(new Bottleneck({ id: "limiter2", maxConcurrent: 1 })); + const limiter3 = track(new Bottleneck({ id: "limiter3", maxConcurrent: 2 })); limiter2.on("error", (err) => console.log(err)); - limiter2.chain(rootLimiter); limiter3.chain(rootLimiter); - return Promise.all([ - limiter2.schedule(h.slowPromise, 100, null, 1), - limiter2.schedule(h.slowPromise, 100, null, 2), - limiter2.schedule(h.slowPromise, 100, null, 3), - limiter3.schedule(h.slowPromise, 100, null, 4), - limiter3.schedule(h.slowPromise, 100, null, 5), - limiter3.schedule(h.slowPromise, 100, null, 6), - ]) - .then(function () { - return h.flushLimiter(rootLimiter); - }) - .then(function (results) { - h.checkDuration(300); - h.checkResultsOrder([[1], [4], [5], [2], [6], [3]]); - - // Lower bounds = real contract (chained gating waited the expected - // minimum). Upper bounds catch logic bugs that would over-wait by - // multiples of the expected step. The +700ms ceiling above the floor - // absorbs a single connectTimeout+retry cycle (~555ms; see - // test/redis-client-options.js) plus event-loop / Redis-roundtrip / - // Docker-daemon latency under parallel load, while still catching the - // previous heartbeat-stall regression which manifested as +5000ms - // over the floor. - expect(results.calls[0].time).toBeGreaterThanOrEqual(100); - expect(results.calls[0].time).toBeLessThan(800); - expect(results.calls[1].time).toBeGreaterThanOrEqual(100); - expect(results.calls[1].time).toBeLessThan(800); - expect(results.calls[2].time).toBeGreaterThanOrEqual(100); - expect(results.calls[2].time).toBeLessThan(800); - - expect(results.calls[3].time).toBeGreaterThanOrEqual(200); - expect(results.calls[3].time).toBeLessThan(900); - expect(results.calls[4].time).toBeGreaterThanOrEqual(200); - expect(results.calls[4].time).toBeLessThan(900); - - expect(results.calls[5].time).toBeGreaterThanOrEqual(300); - expect(results.calls[5].time).toBeLessThan(1000); - }); + await Promise.all([rootLimiter.ready(), limiter2.ready(), limiter3.ready()]); + + const sig1 = deferred(); + const sig2 = deferred(); + const sig3 = deferred(); + const sig4 = deferred(); + const sig5 = deferred(); + const sig6 = deferred(); + + const p1 = limiter2.schedule(h.deferredPromise, sig1.signal, null, 1); + const p2 = limiter2.schedule(h.deferredPromise, sig2.signal, null, 2); + const p3 = limiter2.schedule(h.deferredPromise, sig3.signal, null, 3); + const p4 = limiter3.schedule(h.deferredPromise, sig4.signal, null, 4); + const p5 = limiter3.schedule(h.deferredPromise, sig5.signal, null, 5); + const p6 = limiter3.schedule(h.deferredPromise, sig6.signal, null, 6); + + await waitForState(function () { + expect(rootLimiter.counts().EXECUTING).toBe(3); + expect(limiter2.counts().QUEUED).toBe(2); + expect(limiter3.counts().QUEUED).toBe(1); + }); + + sig1.release(); + sig4.release(); + sig5.release(); + await waitForState(function () { + expect(h.log.mock.calls.length).toBe(3); + }); + + sig2.release(); + sig6.release(); + await waitForState(function () { + expect(h.log.mock.calls.length).toBe(5); + }); + + sig3.release(); + await Promise.all([p1, p2, p3, p4, p5, p6]); + await h.flushLimiter(rootLimiter); + expect(h.log).toHaveCallOrder([[1], [4], [5], [2], [6], [3]]); }); - it("Should chain local and distributed limiters (partial concurrency)", function () { - const h = createJobHarness(); - rootLimiter = makeLimiter({ maxConcurrent: 2 }); - const limiter2 = new Bottleneck({ maxConcurrent: 1 }); - const limiter3 = new Bottleneck({ maxConcurrent: 2 }); + test("Should chain local and distributed limiters (partial concurrency)", async function ({ + harness: h, + makeLimiter, + track, + }) { + const rootLimiter = makeLimiter({ maxConcurrent: 2 }); + const limiter2 = track(new Bottleneck({ maxConcurrent: 1 })); + const limiter3 = track(new Bottleneck({ maxConcurrent: 2 })); limiter2.chain(rootLimiter); limiter3.chain(rootLimiter); - return Promise.all([ - limiter2.schedule(h.slowPromise, 100, null, 1), - limiter2.schedule(h.slowPromise, 100, null, 2), - limiter2.schedule(h.slowPromise, 100, null, 3), - limiter3.schedule(h.slowPromise, 100, null, 4), - limiter3.schedule(h.slowPromise, 100, null, 5), - limiter3.schedule(h.slowPromise, 100, null, 6), - ]) - .then(function () { - return h.flushLimiter(rootLimiter); - }) - .then(function (results) { - h.checkResultsOrder([[1], [4], [5], [2], [6], [3]]); - - // Lower bounds prove the chained gates worked (each wave waits for - // the previous to release a slot in rootLimiter). Upper bounds catch - // catastrophic stalls but must tolerate event-loop / Redis-roundtrip - // jitter under load. +700ms above the floor absorbs a single - // connectTimeout+retry cycle (~555ms; see test/redis-client-options.js) - // and is wide enough to ignore a one-off connection timeout while - // still catching the previous heartbeat-stall regression (+5000ms). - expect(results.calls[0].time).toBeGreaterThanOrEqual(100); - expect(results.calls[0].time).toBeLessThan(800); - expect(results.calls[1].time).toBeGreaterThanOrEqual(100); - expect(results.calls[1].time).toBeLessThan(800); - - expect(results.calls[2].time).toBeGreaterThanOrEqual(200); - expect(results.calls[2].time).toBeLessThan(900); - expect(results.calls[3].time).toBeGreaterThanOrEqual(200); - expect(results.calls[3].time).toBeLessThan(900); - - expect(results.calls[4].time).toBeGreaterThanOrEqual(300); - expect(results.calls[4].time).toBeLessThan(1000); - expect(results.calls[5].time).toBeGreaterThanOrEqual(300); - expect(results.calls[5].time).toBeLessThan(1000); - }); + await Promise.all([rootLimiter.ready(), limiter2.ready(), limiter3.ready()]); + + const sig1 = deferred(); + const sig2 = deferred(); + const sig3 = deferred(); + const sig4 = deferred(); + const sig5 = deferred(); + const sig6 = deferred(); + + const p1 = limiter2.schedule(h.deferredPromise, sig1.signal, null, 1); + const p2 = limiter2.schedule(h.deferredPromise, sig2.signal, null, 2); + const p3 = limiter2.schedule(h.deferredPromise, sig3.signal, null, 3); + const p4 = limiter3.schedule(h.deferredPromise, sig4.signal, null, 4); + const p5 = limiter3.schedule(h.deferredPromise, sig5.signal, null, 5); + const p6 = limiter3.schedule(h.deferredPromise, sig6.signal, null, 6); + + await waitForState(function () { + expect(rootLimiter.counts().EXECUTING).toBe(2); + expect(limiter2.counts().QUEUED).toBe(2); + expect(limiter3.counts().QUEUED).toBe(1); + }); + + sig1.release(); + sig4.release(); + sig5.release(); + await waitForState(function () { + expect(h.log.mock.calls.length).toBe(3); + }); + + sig2.release(); + sig6.release(); + await waitForState(function () { + expect(h.log.mock.calls.length).toBe(5); + }); + + sig3.release(); + await Promise.all([p1, p2, p3, p4, p5, p6]); + await h.flushLimiter(rootLimiter); + expect(h.log).toHaveCallOrder([[1], [4], [5], [2], [6], [3]]); }); - it("Should use the limiter ID to build Redis keys", function () { - rootLimiter = makeLimiter(); + test("Should use the limiter ID to build Redis keys", function ({ makeLimiter, track }) { + const rootLimiter = makeLimiter(); const randomId = rootLimiter._randomIndex(); - const limiter = new Bottleneck({ - id: randomId, - datastore: process.env.DATASTORE, - clearDatastore: true, - }); + const limiter = track( + new Bottleneck({ + id: randomId, + datastore: process.env.DATASTORE, + clearDatastore: true, + }), + ); return limiter .ready() @@ -148,13 +154,13 @@ describe("Cluster coordination", function () { }) .then(function (deleted) { expect(deleted).toEqual(5); - return limiter.disconnect(false); }); }); - it("Should not fail when Redis data is missing", function () { - rootLimiter = makeLimiter(); - const limiter = new Bottleneck({ datastore: process.env.DATASTORE, clearDatastore: true }); + test("Should not fail when Redis data is missing", function ({ track }) { + const limiter = track( + new Bottleneck({ datastore: process.env.DATASTORE, clearDatastore: true }), + ); return limiter .running() @@ -176,36 +182,42 @@ describe("Cluster coordination", function () { }) .then(function (count) { expect(count).toBeGreaterThan(0); - return limiter.disconnect(false); }); }); - it("Should drop all jobs in the Cluster when entering blocked mode", function () { - const h = createJobHarness(); - rootLimiter = makeLimiter(); - const limiter1 = new Bottleneck({ - id: "blocked", - trackDoneStatus: true, - datastore: process.env.DATASTORE, - clearDatastore: true, - - maxConcurrent: 1, - minTime: 50, - highWater: 2, - strategy: Bottleneck.strategy.BLOCK, - }); + test("Should drop all jobs in the Cluster when entering blocked mode", function ({ + harness: h, + makeLimiter, + track, + }) { + const rootLimiter = makeLimiter(); + const limiter1 = track( + new Bottleneck({ + id: "blocked", + trackDoneStatus: true, + datastore: process.env.DATASTORE, + clearDatastore: true, + + maxConcurrent: 1, + minTime: 50, + highWater: 2, + strategy: Bottleneck.strategy.BLOCK, + }), + ); let limiter2; const client_num_queued_key = limiterKeys(limiter1)[5]; return limiter1 .ready() .then(function () { - limiter2 = new Bottleneck({ - id: "blocked", - trackDoneStatus: true, - datastore: process.env.DATASTORE, - clearDatastore: false, - }); + limiter2 = track( + new Bottleneck({ + id: "blocked", + trackDoneStatus: true, + datastore: process.env.DATASTORE, + clearDatastore: false, + }), + ); return limiter2.ready(); }) .then(function () { @@ -239,17 +251,14 @@ describe("Cluster coordination", function () { // connect-retry on the ioredis client adds ~500ms to register; // doExecute's setTimeout(0) drifts when the event loop is busy // serving other parallel test workers). - return waitForState( - function () { - const c1 = limiter1.counts(); - expect(c1.RECEIVED).toBe(0); - expect(c1.QUEUED).toBe(0); - expect(c1.RUNNING).toBe(0); - expect(c1.EXECUTING).toBe(0); - expect(c1.DONE).toBe(1); - }, - { timeout: 5000 }, - ); + return waitForState(function () { + const c1 = limiter1.counts(); + expect(c1.RECEIVED).toBe(0); + expect(c1.QUEUED).toBe(0); + expect(c1.RUNNING).toBe(0); + expect(c1.EXECUTING).toBe(0); + expect(c1.DONE).toBe(1); + }); }) .then(function () { const counts1 = limiter1.counts(); @@ -269,30 +278,32 @@ describe("Cluster coordination", function () { return h.flushLimiter(rootLimiter); }) .then(function (_results) { - h.checkResultsOrder([[1]]); - - return Promise.all([limiter1.disconnect(false), limiter2.disconnect(false)]); + expect(h.log).toHaveCallOrder([[1]]); }); }); - it("Should pass messages to all limiters in Cluster", function () { - rootLimiter = makeLimiter({ + test("Should pass messages to all limiters in Cluster", function ({ makeLimiter, track }) { + const rootLimiter = makeLimiter({ maxConcurrent: 1, minTime: 100, id: "super-duper", }); - const limiter1 = new Bottleneck({ - maxConcurrent: 1, - minTime: 100, - id: "super-duper", - datastore: process.env.DATASTORE, - }); - const limiter2 = new Bottleneck({ - maxConcurrent: 1, - minTime: 100, - id: "nope", - datastore: process.env.DATASTORE, - }); + const limiter1 = track( + new Bottleneck({ + maxConcurrent: 1, + minTime: 100, + id: "super-duper", + datastore: process.env.DATASTORE, + }), + ); + const limiter2 = track( + new Bottleneck({ + maxConcurrent: 1, + minTime: 100, + id: "nope", + datastore: process.env.DATASTORE, + }), + ); const received = []; rootLimiter.on("message", (msg) => { @@ -305,11 +316,9 @@ describe("Cluster coordination", function () { received.push(3, msg); }); - return Promise.all([rootLimiter.ready(), limiter2.ready()]) + return Promise.all([rootLimiter.ready(), limiter1.ready(), limiter2.ready()]) .then(function () { limiter1.publish(555); - // Poll for delivery instead of a fixed setTimeout — pub/sub round-trips - // can exceed a tight 150ms window under load. return waitForState(function () { expect(received.length).toBeGreaterThanOrEqual(4); }); @@ -321,13 +330,16 @@ describe("Cluster coordination", function () { }); }); - it("Should pass messages to correct limiter after Group re-instantiations", function () { - rootLimiter = makeLimiter(); - const group = new Bottleneck.Group({ - maxConcurrent: 1, - minTime: 100, - datastore: process.env.DATASTORE, - }); + test("Should pass messages to correct limiter after Group re-instantiations", function ({ + track, + }) { + const group = track( + new Bottleneck.Group({ + maxConcurrent: 1, + minTime: 100, + datastore: process.env.DATASTORE, + }), + ); const received = []; return new Promise(function (resolve, _reject) { @@ -366,15 +378,19 @@ describe("Cluster coordination", function () { }) .then(function () { expect(received).toEqual(["1", "Bonjour!", "2", "Comment allez-vous?", "3", "Au revoir!"]); + // Semantic, not cleanup: flush=true gracefully drains the un-awaited + // "Au revoir!" PUBLISH reply before closing. track's disconnect(false) + // would destroy the socket mid-flight and reject that pending command. group.disconnect(); }); }); - it("Should have a default key TTL when using Groups", function () { - rootLimiter = makeLimiter(); - const group = new Bottleneck.Group({ - datastore: process.env.DATASTORE, - }); + test("Should have a default key TTL when using Groups", function ({ track }) { + const group = track( + new Bottleneck.Group({ + datastore: process.env.DATASTORE, + }), + ); return group .key("one") @@ -387,20 +403,19 @@ describe("Cluster coordination", function () { .then(function (ttl) { expect(ttl).toBeGreaterThanOrEqual(290); expect(ttl).toBeLessThanOrEqual(305); - }) - .then(function () { - return group.disconnect(false); }); }); - it("Should support Groups and expire Redis keys", function () { - rootLimiter = makeLimiter(); - const group = new Bottleneck.Group({ - datastore: process.env.DATASTORE, - clearDatastore: true, - minTime: 50, - timeout: 200, - }); + test("Should support Groups and expire Redis keys", function ({ makeLimiter, track }) { + const rootLimiter = makeLimiter(); + const group = track( + new Bottleneck.Group({ + datastore: process.env.DATASTORE, + clearDatastore: true, + minTime: 50, + timeout: 200, + }), + ); let limiter1; let limiter2; let limiter3; @@ -478,21 +493,20 @@ describe("Cluster coordination", function () { expect(counts).toEqual([0, 0, 0]); expect(group.keys().length).toEqual(0); expect(Object.keys(group.connection.limiters).length).toEqual(0); - return group.disconnect(false); }); }); - it("Should not recreate a key when running heartbeat", function () { - const h = createJobHarness(); - rootLimiter = makeLimiter(); - const group = new Bottleneck.Group({ - datastore: process.env.DATASTORE, - clearDatastore: true, - maxConcurrent: 50, - minTime: 50, - timeout: 300, - heartbeatInterval: 5, - }); + test("Should not recreate a key when running heartbeat", function ({ harness: h, track }) { + const group = track( + new Bottleneck.Group({ + datastore: process.env.DATASTORE, + clearDatastore: true, + maxConcurrent: 50, + minTime: 50, + timeout: 300, + heartbeatInterval: 5, + }), + ); const key = "heartbeat"; const limiter = group.key(key); @@ -510,32 +524,36 @@ describe("Cluster coordination", function () { }) .then(function (count) { expect(count).toEqual(0); - return group.disconnect(false); }); }); - it("Should delete Redis key when manually deleting a group key", function () { - const h = createJobHarness(); - rootLimiter = makeLimiter(); + test("Should delete Redis key when manually deleting a group key", function ({ + harness: h, + track, + }) { // Bump timeout (and the corresponding h.waitFor below) so autocleanup // doesn't race with the initial schedule under stress. Original 300ms // gave a 150ms autocleanup interval that could fire before init.lua // settled when redis was slow. - const groupTimeout = 2000; - const group1 = new Bottleneck.Group({ - datastore: process.env.DATASTORE, - clearDatastore: true, - maxConcurrent: 50, - minTime: 50, - timeout: groupTimeout, - }); - const group2 = new Bottleneck.Group({ - datastore: process.env.DATASTORE, - clearDatastore: true, - maxConcurrent: 50, - minTime: 50, - timeout: groupTimeout, - }); + const groupTimeout = 5000; + const group1 = track( + new Bottleneck.Group({ + datastore: process.env.DATASTORE, + clearDatastore: true, + maxConcurrent: 50, + minTime: 50, + timeout: groupTimeout, + }), + ); + const group2 = track( + new Bottleneck.Group({ + datastore: process.env.DATASTORE, + clearDatastore: true, + maxConcurrent: 50, + minTime: 50, + timeout: groupTimeout, + }), + ); const key = "deleted"; const limiter = group1.key(key); // only for countKeys() use @@ -547,6 +565,13 @@ describe("Cluster coordination", function () { .then(function () { expect(group1.keys().length).toEqual(1); expect(group2.keys().length).toEqual(1); + return group1.key(key).running(); + }) + .then(function () { + // Call deleteKey ONCE and assert its return value — retrying a delete + // until it returns true would mask a regression where the first call + // wrongly returns false. group1 holds the local instance, so true is + // guaranteed structurally (instance != null short-circuits). return group1.deleteKey(key); }) .then(function (deleted) { @@ -569,34 +594,38 @@ describe("Cluster coordination", function () { .then(function () { expect(group1.keys().length).toEqual(0); expect(group2.keys().length).toEqual(0); - return Promise.all([group1.disconnect(false), group2.disconnect(false)]); }); }); - it("Should delete Redis keys from a group even when the local limiter is not present", function () { - const h = createJobHarness(); - rootLimiter = makeLimiter(); + test("Should delete Redis keys from a group even when the local limiter is not present", function ({ + harness: h, + track, + }) { // groupTimeout pulls double duty here: it sets the redis-side TTL // (must not expire before group2.deleteKey runs), and it gates // autocleanup interval (timeout/2). 2000ms was enough for autocleanup // to fire within the waitFor window, but tight enough that under stress // the keys could TTL-expire before deleteKey ran. Refreshing the TTL // explicitly via running() right before deleteKey decouples the two. - const groupTimeout = 2000; - const group1 = new Bottleneck.Group({ - datastore: process.env.DATASTORE, - clearDatastore: true, - maxConcurrent: 50, - minTime: 50, - timeout: groupTimeout, - }); - const group2 = new Bottleneck.Group({ - datastore: process.env.DATASTORE, - clearDatastore: true, - maxConcurrent: 50, - minTime: 50, - timeout: groupTimeout, - }); + const groupTimeout = 5000; + const group1 = track( + new Bottleneck.Group({ + datastore: process.env.DATASTORE, + clearDatastore: true, + maxConcurrent: 50, + minTime: 50, + timeout: groupTimeout, + }), + ); + const group2 = track( + new Bottleneck.Group({ + datastore: process.env.DATASTORE, + clearDatastore: true, + maxConcurrent: 50, + minTime: 50, + timeout: groupTimeout, + }), + ); const key = "deleted-cluster-wide"; const limiter = group1.key(key); // only for countKeys() use @@ -605,11 +634,20 @@ describe("Cluster coordination", function () { .then(function () { expect(group1.keys().length).toEqual(1); expect(group2.keys().length).toEqual(0); - // Refresh the redis-side TTL before deleteKey. Any operation that - // hits refresh_expiration in lua resets the timer to groupTimeout. return group1.key(key).running(); }) .then(function () { + // The keys were written through group1's connection; poll read-only + // existence before the cross-group delete so a slow write can't turn + // this into a false failure... + return waitForState(async function () { + expect(await countKeys(limiter)).toBeGreaterThan(0); + }); + }) + .then(function () { + // ...then call deleteKey ONCE and assert its return value. group2 has + // no local instance, so the value reflects the redis DEL — retrying + // until true would mask a regression where it wrongly returns false. return group2.deleteKey(key); }) .then(function (deleted) { @@ -631,28 +669,30 @@ describe("Cluster coordination", function () { .then(function () { expect(group1.keys().length).toEqual(0); expect(group2.keys().length).toEqual(0); - return Promise.all([group1.disconnect(false), group2.disconnect(false)]); }); }); - it("Should returns all Group keys in the cluster", async function () { - rootLimiter = makeLimiter(); + test("Should returns all Group keys in the cluster", async function ({ track }) { // Use a long timeout so redis-side TTLs cannot expire mid-test under load. // Original 3000ms was tight enough that a slow run (cumulative redis latency) // could let keys expire before the assertions, then autocleanup would prune // them from instances and group.keys() would surprisingly return []. - const group1 = new Bottleneck.Group({ - datastore: process.env.DATASTORE, - clearDatastore: true, - id: "same", - timeout: 30000, - }); - const group2 = new Bottleneck.Group({ - datastore: process.env.DATASTORE, - clearDatastore: true, - id: "same", - timeout: 30000, - }); + const group1 = track( + new Bottleneck.Group({ + datastore: process.env.DATASTORE, + clearDatastore: true, + id: "same", + timeout: 30000, + }), + ); + const group2 = track( + new Bottleneck.Group({ + datastore: process.env.DATASTORE, + clearDatastore: true, + id: "same", + timeout: 30000, + }), + ); const keys1 = ["lorem", "ipsum", "dolor", "sit", "amet", "consectetur"]; const keys2 = ["adipiscing", "elit"]; const both = keys1.concat(keys2); @@ -665,48 +705,51 @@ describe("Cluster coordination", function () { expect((await group1.clusterKeys()).sort()).toEqual(both.sort()); expect((await group1.clusterKeys()).sort()).toEqual(both.sort()); - const group3 = new Bottleneck.Group({ datastore: "local" }); + const group3 = track(new Bottleneck.Group({ datastore: "local" })); expect(await group3.clusterKeys()).toEqual([]); - - await group1.disconnect(false); - await group2.disconnect(false); }); - it("Should queue up the least busy limiter", async function () { - const h = createJobHarness(); - rootLimiter = makeLimiter(); - const limiter1 = new Bottleneck({ - datastore: process.env.DATASTORE, - clearDatastore: true, - id: "busy", - timeout: 3000, - maxConcurrent: 3, - trackDoneStatus: true, - }); - const limiter2 = new Bottleneck({ - datastore: process.env.DATASTORE, - clearDatastore: true, - id: "busy", - timeout: 3000, - maxConcurrent: 3, - trackDoneStatus: true, - }); - const limiter3 = new Bottleneck({ - datastore: process.env.DATASTORE, - clearDatastore: true, - id: "busy", - timeout: 3000, - maxConcurrent: 3, - trackDoneStatus: true, - }); - const limiter4 = new Bottleneck({ - datastore: process.env.DATASTORE, - clearDatastore: true, - id: "busy", - timeout: 3000, - maxConcurrent: 3, - trackDoneStatus: true, - }); + test("Should queue up the least busy limiter", async function ({ harness: h, track }) { + const limiter1 = track( + new Bottleneck({ + datastore: process.env.DATASTORE, + clearDatastore: true, + id: "busy", + timeout: 3000, + maxConcurrent: 3, + trackDoneStatus: true, + }), + ); + const limiter2 = track( + new Bottleneck({ + datastore: process.env.DATASTORE, + clearDatastore: true, + id: "busy", + timeout: 3000, + maxConcurrent: 3, + trackDoneStatus: true, + }), + ); + const limiter3 = track( + new Bottleneck({ + datastore: process.env.DATASTORE, + clearDatastore: true, + id: "busy", + timeout: 3000, + maxConcurrent: 3, + trackDoneStatus: true, + }), + ); + const limiter4 = track( + new Bottleneck({ + datastore: process.env.DATASTORE, + clearDatastore: true, + id: "busy", + timeout: 3000, + maxConcurrent: 3, + trackDoneStatus: true, + }), + ); let resolve1, resolve2, resolve3, resolve4, resolve5, resolve6, resolve7; const p1 = new Promise(function (resolve, _reject) { @@ -754,12 +797,9 @@ describe("Cluster coordination", function () { // D/E/F/G queue, regardless of how long the submit round-trips take. // We release A after the QUEUED assertions so it finishes before B, // preserving the original [1, 4, 5, 6, 7, 2, 3] completion order. - let releaseA; - const aSignal = new Promise(function (r) { - releaseA = r; - }); + const sigA = deferred(); - await limiter1.submit({ id: "A" }, h.deferredJob, aSignal, null, 1, resolve1); + await limiter1.submit({ id: "A" }, h.deferredJob, sigA.signal, null, 1, resolve1); // B and C must finish after D/E/F/G. Use generous durations so the test // is robust to redis round-trip delays between releaseA() and G's completion. await limiter1.submit({ id: "B" }, h.slowJob, 1000, null, 2, resolve2); @@ -776,7 +816,7 @@ describe("Cluster coordination", function () { expect(limiter3.counts().QUEUED).toEqual(2); expect(limiter4.counts().QUEUED).toEqual(2); - releaseA(); + sigA.release(); await Promise.all([p1, p2, p3, p4, p5, p6, p7]); @@ -805,48 +845,52 @@ describe("Cluster coordination", function () { expect(calls.slice(5, 7).sort()).toEqual([4, 5]); expect(calls.slice(7, 9).sort()).toEqual([6, 7]); expect(calls.slice(9, 11).sort()).toEqual([2, 3]); - - await limiter1.disconnect(false); - await limiter2.disconnect(false); - await limiter3.disconnect(false); - await limiter4.disconnect(false); }); - it("Should pass the remaining capacity to other limiters", async function () { - const h = createJobHarness(); - rootLimiter = makeLimiter(); - const limiter1 = new Bottleneck({ - datastore: process.env.DATASTORE, - clearDatastore: true, - id: "busy", - timeout: 3000, - maxConcurrent: 3, - trackDoneStatus: true, - }); - const limiter2 = new Bottleneck({ - datastore: process.env.DATASTORE, - clearDatastore: true, - id: "busy", - timeout: 3000, - maxConcurrent: 3, - trackDoneStatus: true, - }); - const limiter3 = new Bottleneck({ - datastore: process.env.DATASTORE, - clearDatastore: true, - id: "busy", - timeout: 3000, - maxConcurrent: 3, - trackDoneStatus: true, - }); - const limiter4 = new Bottleneck({ - datastore: process.env.DATASTORE, - clearDatastore: true, - id: "busy", - timeout: 3000, - maxConcurrent: 3, - trackDoneStatus: true, - }); + test("Should pass the remaining capacity to other limiters", async function ({ + harness: h, + track, + }) { + const limiter1 = track( + new Bottleneck({ + datastore: process.env.DATASTORE, + clearDatastore: true, + id: "busy", + timeout: 3000, + maxConcurrent: 3, + trackDoneStatus: true, + }), + ); + const limiter2 = track( + new Bottleneck({ + datastore: process.env.DATASTORE, + clearDatastore: true, + id: "busy", + timeout: 3000, + maxConcurrent: 3, + trackDoneStatus: true, + }), + ); + const limiter3 = track( + new Bottleneck({ + datastore: process.env.DATASTORE, + clearDatastore: true, + id: "busy", + timeout: 3000, + maxConcurrent: 3, + trackDoneStatus: true, + }), + ); + const limiter4 = track( + new Bottleneck({ + datastore: process.env.DATASTORE, + clearDatastore: true, + id: "busy", + timeout: 3000, + maxConcurrent: 3, + trackDoneStatus: true, + }), + ); let t3, t4; let resolve1, resolve2, resolve3, resolve4, resolve5; @@ -889,11 +933,15 @@ describe("Cluster coordination", function () { // submits can take >50ms, so limiter1's job sometimes finished // before the asserts ran — capacity freed, limiter3's queued job // dispatched, and `limiter3.counts().QUEUED` flipped from 1 to 0. - let releaseFirst; - const firstSignal = new Promise(function (r) { - releaseFirst = r; - }); - await limiter1.submit({ id: "A", weight: 2 }, h.deferredJob, firstSignal, null, 1, resolve1); + const sigFirst = deferred(); + await limiter1.submit( + { id: "A", weight: 2 }, + h.deferredJob, + sigFirst.signal, + null, + 1, + resolve1, + ); await limiter2.submit({ id: "C" }, h.slowJob, 550, null, 2, resolve2); expect(runningOrExecuting(limiter1)).toEqual(1); @@ -909,7 +957,7 @@ describe("Cluster coordination", function () { // Release limiter1's job; capacity opens up; queued jobs dispatch. // Order is preserved because deferredJob's calls.push fires when the // signal resolves (matching slowJob's timing semantics). - releaseFirst(); + sigFirst.release(); await Promise.all([p1, p2, p3, p4, p5]); @@ -919,47 +967,49 @@ describe("Cluster coordination", function () { // so L3’s score stays strictly below L4’s until work runs — the first grant after // releaseFirst() targets L3, then FIFO on L4 gives [4] before [5]. Call-log order // must remain [[3],[4],[5]]; this is not the symmetric F/G case in "least busy limiter". - h.checkResultsOrder([["A"], ["B"], ["C"], ["D"], [1], [3], [4], [5], [2]]); + expect(h.log).toHaveCallOrder([["A"], ["B"], ["C"], ["D"], [1], [3], [4], [5], [2]]); // limiter3's job 3 and limiter4's job 4 are both 50ms slowJobs that start // back-to-back; they should finish near-simultaneously. The 15ms // ceiling was too tight under parallel testcontainer load — 100ms // still proves "near-simultaneous" while absorbing event-loop jitter. expect(Math.abs(t3 - t4)).toBeLessThan(100); - - await limiter1.disconnect(false); - await limiter2.disconnect(false); - await limiter3.disconnect(false); - await limiter4.disconnect(false); }); - it("Should take the capacity and blacklist if the priority limiter is not responding", async function () { - const h = createJobHarness(); - rootLimiter = makeLimiter(); - const limiter1 = new Bottleneck({ - datastore: process.env.DATASTORE, - clearDatastore: true, - id: "crash", - timeout: 3000, - maxConcurrent: 1, - trackDoneStatus: true, - }); - const limiter2 = new Bottleneck({ - datastore: process.env.DATASTORE, - clearDatastore: true, - id: "crash", - timeout: 3000, - maxConcurrent: 1, - trackDoneStatus: true, - }); - const limiter3 = new Bottleneck({ - datastore: process.env.DATASTORE, - clearDatastore: true, - id: "crash", - timeout: 3000, - maxConcurrent: 1, - trackDoneStatus: true, - }); + test("Should take the capacity and blacklist if the priority limiter is not responding", async function ({ + harness: h, + track, + }) { + const limiter1 = track( + new Bottleneck({ + datastore: process.env.DATASTORE, + clearDatastore: true, + id: "crash", + timeout: 3000, + maxConcurrent: 1, + trackDoneStatus: true, + }), + ); + const limiter2 = track( + new Bottleneck({ + datastore: process.env.DATASTORE, + clearDatastore: true, + id: "crash", + timeout: 3000, + maxConcurrent: 1, + trackDoneStatus: true, + }), + ); + const limiter3 = track( + new Bottleneck({ + datastore: process.env.DATASTORE, + clearDatastore: true, + id: "crash", + timeout: 3000, + maxConcurrent: 1, + trackDoneStatus: true, + }), + ); await limiter1.schedule({ id: "1" }, h.promise, null, "A"); await limiter2.schedule({ id: "2" }, h.promise, null, "B"); @@ -988,10 +1038,6 @@ describe("Cluster coordination", function () { await limiter2.disconnect(false); await Promise.all([p1, p3]); - h.checkResultsOrder([["A"], ["B"], ["C"], [4], [6]]); - - await limiter1.disconnect(false); - await limiter2.disconnect(false); - await limiter3.disconnect(false); + expect(h.log).toHaveCallOrder([["A"], ["B"], ["C"], [4], [6]]); }); }); diff --git a/test/cluster.test.js b/test/cluster.test.js index c53771b..9c7d7ef 100644 --- a/test/cluster.test.js +++ b/test/cluster.test.js @@ -1,11 +1,13 @@ -import { describe, it, afterEach, expect } from "vitest"; -import { createJobHarness } from "./helpers/job-tracking.js"; -import { waitForState } from "./helpers/wait-for-state.js"; -const makeLimiter = require("./helpers/limiter"); +import { test, describe, expect, waitForState, deferred } from "./helpers/test-api.js"; const Bottleneck = require("./bottleneck"); const Scripts = require("../src/cluster/Scripts.js"); const assert = require("assert"); +// Causality policy (Workstream B): observe product-timer effects via waitForState +// and state counts — never assert wall-clock bounds around real network time. +// Job duration is never an assertion; use deferredJob/deferredPromise with explicit +// release. "Must NOT happen" uses bounded sleeps only after positive preconditions. + const limiterKeys = function (limiter) { return Scripts.allKeys(limiter._store.originalId); }; @@ -18,26 +20,21 @@ const sumWeights = function (weights) { }, 0); }; -describe("Cluster-only", function () { +describe("Cluster-only", () => { if (process.env.DATASTORE !== "redis" && process.env.DATASTORE !== "ioredis") { throw new Error("DATASTORE must be redis or ioredis"); } - let rootLimiter; - - afterEach(function () { - return rootLimiter.disconnect(false); - }); - it("Should return a promise for ready()", function () { - rootLimiter = makeLimiter({ maxConcurrent: 2 }); + test("Should return a promise for ready()", function ({ makeLimiter }) { + const rootLimiter = makeLimiter({ maxConcurrent: 2 }); const ready = rootLimiter.ready(); expect(ready).toBeInstanceOf(Promise); return ready; }); - it("Should return clients", function () { - rootLimiter = makeLimiter({ maxConcurrent: 2 }); + test("Should return clients", function ({ makeLimiter }) { + const rootLimiter = makeLimiter({ maxConcurrent: 2 }); return rootLimiter.ready().then(function (clients) { expect(Object.keys(clients)).toEqual(["client", "subscriber"]); @@ -45,8 +42,8 @@ describe("Cluster-only", function () { }); }); - it("Should return a promise when disconnecting", function () { - rootLimiter = makeLimiter({ maxConcurrent: 2 }); + test("Should return a promise when disconnecting", function ({ makeLimiter }) { + const rootLimiter = makeLimiter({ maxConcurrent: 2 }); const disconnected = rootLimiter.disconnect(); expect(disconnected).toBeInstanceOf(Promise); @@ -55,14 +52,19 @@ describe("Cluster-only", function () { }); }); - it("Should allow passing a limiter's connection to a new limiter", function () { - const h = createJobHarness(); - rootLimiter = makeLimiter(); + test("Should allow passing a limiter's connection to a new limiter", function ({ + harness: h, + makeLimiter, + track, + }) { + const rootLimiter = makeLimiter(); rootLimiter.connection.id = "some-id"; - const limiter = new Bottleneck({ - minTime: 50, - connection: rootLimiter.connection, - }); + const limiter = track( + new Bottleneck({ + minTime: 50, + connection: rootLimiter.connection, + }), + ); return Promise.all([rootLimiter.ready(), limiter.ready()]) .then(function () { @@ -78,18 +80,23 @@ describe("Cluster-only", function () { return h.flushLimiter(rootLimiter); }) .then(function (_results) { - h.checkResultsOrder([[1], [2]]); + expect(h.log).toHaveCallOrder([[1], [2]]); }); }); - it("Should allow passing a limiter's connection to a new Group", function () { - const h = createJobHarness(); - rootLimiter = makeLimiter(); + test("Should allow passing a limiter's connection to a new Group", function ({ + harness: h, + makeLimiter, + track, + }) { + const rootLimiter = makeLimiter(); rootLimiter.connection.id = "some-id"; - const group = new Bottleneck.Group({ - minTime: 50, - connection: rootLimiter.connection, - }); + const group = track( + new Bottleneck.Group({ + minTime: 50, + connection: rootLimiter.connection, + }), + ); const limiter1 = group.key("A"); const limiter2 = group.key("B"); @@ -110,25 +117,32 @@ describe("Cluster-only", function () { return h.flushLimiter(rootLimiter); }) .then(function (_results) { - h.checkResultsOrder([[1], [2], [3]]); + expect(h.log).toHaveCallOrder([[1], [2], [3]]); }); }); - it("Should allow passing a Group's connection to a new limiter", function () { - const h = createJobHarness(); - rootLimiter = makeLimiter(); - const group = new Bottleneck.Group({ - minTime: 50, - datastore: process.env.DATASTORE, - clearDatastore: true, - }); + test("Should allow passing a Group's connection to a new limiter", function ({ + harness: h, + makeLimiter, + track, + }) { + const rootLimiter = makeLimiter(); + const group = track( + new Bottleneck.Group({ + minTime: 50, + datastore: process.env.DATASTORE, + clearDatastore: true, + }), + ); group.connection.id = "some-id"; const limiter1 = group.key("A"); - const limiter2 = new Bottleneck({ - minTime: 50, - connection: group.connection, - }); + const limiter2 = track( + new Bottleneck({ + minTime: 50, + connection: group.connection, + }), + ); return Promise.all([limiter1.ready(), limiter2.ready()]) .then(function () { @@ -146,26 +160,32 @@ describe("Cluster-only", function () { return h.flushLimiter(rootLimiter); }) .then(function (_results) { - h.checkResultsOrder([[1], [2]]); - return group.disconnect(); + expect(h.log).toHaveCallOrder([[1], [2]]); }); }); - it("Should allow passing a Group's connection to a new Group", function () { - const h = createJobHarness(); - rootLimiter = makeLimiter(); - const group1 = new Bottleneck.Group({ - minTime: 50, - datastore: process.env.DATASTORE, - clearDatastore: true, - }); + test("Should allow passing a Group's connection to a new Group", function ({ + harness: h, + makeLimiter, + track, + }) { + const rootLimiter = makeLimiter(); + const group1 = track( + new Bottleneck.Group({ + minTime: 50, + datastore: process.env.DATASTORE, + clearDatastore: true, + }), + ); group1.connection.id = "some-id"; - const group2 = new Bottleneck.Group({ - minTime: 50, - connection: group1.connection, - clearDatastore: true, - }); + const group2 = track( + new Bottleneck.Group({ + minTime: 50, + connection: group1.connection, + clearDatastore: true, + }), + ); const limiter1 = group1.key("AAA"); const limiter2 = group1.key("BBB"); @@ -196,13 +216,12 @@ describe("Cluster-only", function () { return h.flushLimiter(rootLimiter); }) .then(function (_results) { - h.checkResultsOrder([[1], [2], [3], [4]]); - return group1.disconnect(); + expect(h.log).toHaveCallOrder([[1], [2], [3], [4]]); }); }); - it("Should not have a key TTL by default for standalone limiters", function () { - rootLimiter = makeLimiter(); + test("Should not have a key TTL by default for standalone limiters", function ({ makeLimiter }) { + const rootLimiter = makeLimiter(); return rootLimiter .ready() @@ -215,8 +234,8 @@ describe("Cluster-only", function () { }); }); - it("Should allow timeout setting for standalone limiters", function () { - rootLimiter = makeLimiter({ timeout: 5 * 60 * 1000 }); + test("Should allow timeout setting for standalone limiters", function ({ makeLimiter }) { + const rootLimiter = makeLimiter({ timeout: 5 * 60 * 1000 }); return rootLimiter .ready() @@ -230,8 +249,10 @@ describe("Cluster-only", function () { }); }); - it("Should set TTL on all keys including client_* keys after register_client", async function () { - rootLimiter = makeLimiter({ timeout: 5 * 60 * 1000 }); + test("Should set TTL on all keys including client_* keys after register_client", async function ({ + makeLimiter, + }) { + const rootLimiter = makeLimiter({ timeout: 5 * 60 * 1000 }); await rootLimiter.ready(); @@ -265,7 +286,10 @@ describe("Cluster-only", function () { } }); - it("Should compute reservoir increased based on number of missed intervals", async function () { + test("Should compute reservoir increased based on number of missed intervals", async function ({ + makeLimiter, + track, + }) { const settings = { id: "missed-intervals", clearDatastore: false, @@ -274,7 +298,7 @@ describe("Cluster-only", function () { reservoirIncreaseAmount: 2, timeout: 2000, }; - rootLimiter = makeLimiter({ ...settings }); + const rootLimiter = makeLimiter({ ...settings }); await rootLimiter.ready(); expect(await rootLimiter.currentReservoir()).toEqual(2); @@ -289,7 +313,7 @@ describe("Cluster-only", function () { // duration. By doing the hset and the read back-to-back as the very // last steps, the only Δ between shift-time and read-time is one hset // round trip (typically <10ms, well under the 100ms interval). - const limiter2 = new Bottleneck({ ...settings, datastore: process.env.DATASTORE }); + const limiter2 = track(new Bottleneck({ ...settings, datastore: process.env.DATASTORE })); await limiter2.ready(); // process_tick uses Date.now() from JS (see RedisDatastore.runScript), @@ -311,16 +335,14 @@ describe("Cluster-only", function () { const reservoir = await rootLimiter.currentReservoir(); expect(reservoir).toBeGreaterThanOrEqual(62); expect(reservoir).toBeLessThanOrEqual(64); - - await limiter2.disconnect(); }); - it("Should migrate from 2.8.0", function () { + test("Should migrate from 2.8.0", function ({ makeLimiter, track }) { // Bound the expected timestamps to the test window — not a wall-clock-from-now // window that depends on test runtime under load. lastReservoirIncrease is // preserved from rootLimiter's init (hsetnx), so the bound must precede that too. const testStart = Date.now(); - rootLimiter = makeLimiter({ id: "migrate" }); + const rootLimiter = makeLimiter({ id: "migrate" }); const settings_key = limiterKeys(rootLimiter)[0]; let limiter2; @@ -339,10 +361,12 @@ describe("Cluster-only", function () { ]); }) .then(function () { - limiter2 = new Bottleneck({ - id: "migrate", - datastore: process.env.DATASTORE, - }); + limiter2 = track( + new Bottleneck({ + id: "migrate", + datastore: process.env.DATASTORE, + }), + ); return limiter2.ready(); }) .then(function () { @@ -378,25 +402,27 @@ describe("Cluster-only", function () { "", "", ]); - }) - .then(function () { - return limiter2.disconnect(false); }); }); - it("Should keep track of each client's queue length", async function () { - const h = createJobHarness(); - rootLimiter = makeLimiter({ - id: "queues", - maxConcurrent: 1, - trackDoneStatus: true, - }); - const limiter2 = new Bottleneck({ - datastore: process.env.DATASTORE, + test("Should keep track of each client's queue length", async function ({ + harness: h, + makeLimiter, + track, + }) { + const rootLimiter = makeLimiter({ id: "queues", maxConcurrent: 1, trackDoneStatus: true, }); + const limiter2 = track( + new Bottleneck({ + datastore: process.env.DATASTORE, + id: "queues", + maxConcurrent: 1, + trackDoneStatus: true, + }), + ); const client_num_queued_key = limiterKeys(rootLimiter)[5]; const clientId1 = rootLimiter._store.clientId; const clientId2 = limiter2._store.clientId; @@ -404,7 +430,9 @@ describe("Cluster-only", function () { await rootLimiter.ready(); await limiter2.ready(); - const p0 = rootLimiter.schedule({ id: 0 }, h.slowPromise, 100, null, 0); + const job0 = deferred(); + + const p0 = rootLimiter.schedule({ id: 0 }, h.deferredPromise, job0.signal, null, 0); await rootLimiter._submitLock.schedule(() => Promise.resolve()); const p1 = rootLimiter.schedule({ id: 1 }, h.promise, null, 1); @@ -424,6 +452,7 @@ describe("Cluster-only", function () { expect(await rootLimiter.clusterQueued()).toEqual(3); + job0.release(); await Promise.all([p0, p1, p2, p3]); const queuedB = await runCommand(rootLimiter, "hgetall", [client_num_queued_key]); expect(rootLimiter.counts().QUEUED).toEqual(0); @@ -434,19 +463,17 @@ describe("Cluster-only", function () { expect(limiter2.counts().DONE).toEqual(1); expect(await rootLimiter.clusterQueued()).toEqual(0); - - return limiter2.disconnect(false); }); - it("Should publish capacity increases", function () { - const h = createJobHarness(); - rootLimiter = makeLimiter({ maxConcurrent: 2 }); + test("Should publish capacity increases", function ({ harness: h, makeLimiter, track }) { + const rootLimiter = makeLimiter({ maxConcurrent: 2 }); let limiter2; + let p3; return rootLimiter .ready() .then(function () { - limiter2 = new Bottleneck({ datastore: process.env.DATASTORE }); + limiter2 = track(new Bottleneck({ datastore: process.env.DATASTORE })); return limiter2.ready(); }) .then(function () { @@ -460,44 +487,44 @@ describe("Cluster-only", function () { // then release them (after a fixed wait that preserves the // original ~200ms total duration so capacity-published-to-limiter2 // semantics are still exercised end-to-end). - let releaseJobs; - const jobsSignal = new Promise(function (r) { - releaseJobs = r; - }); - rootLimiter.schedule({ id: 1 }, h.deferredPromise, jobsSignal, null, 1); - rootLimiter.schedule({ id: 2 }, h.deferredPromise, jobsSignal, null, 2); + const jobs = deferred(); + rootLimiter.schedule({ id: 1 }, h.deferredPromise, jobs.signal, null, 1); + rootLimiter.schedule({ id: 2 }, h.deferredPromise, jobs.signal, null, 2); return rootLimiter .schedule({ id: 0, weight: 0 }, h.promise, null, 0) .then(function () { - return h.wait(100); + return rootLimiter._submitLock.schedule(() => Promise.resolve()); + }) + .then(function () { + expect(rootLimiter.counts().EXECUTING).toEqual(2); + p3 = limiter2.schedule({ id: 3 }, h.promise, null, 3); + // Drain limiter2's lock — job 3 was submitted on limiter2, so only + // its own _submitLock guarantees the registration reached redis. + return limiter2._submitLock.schedule(() => Promise.resolve()); }) .then(function () { - releaseJobs(); + expect(limiter2.counts().EXECUTING).toEqual(0); + jobs.release(); }); }) .then(function () { - return limiter2.schedule({ id: 3 }, h.slowPromise, 100, null, 3); + return p3; }) .then(function () { return h.flushLimiter(rootLimiter); }) .then(function (_results) { - h.checkResultsOrder([[0], [1], [2], [3]]); - // Lower bound ~200ms is the contract: job 0 (instant) + ~100ms - // hold for jobs 1/2 + ~100ms for job 3 to dispatch on limiter2 - // after capacity opens. We don't tightly upper-bound here — under - // load, redis round trips + capacity pubsub can stretch this past - // the original implicit 1200ms cap. - expect(h.results().elapsed).toBeGreaterThanOrEqual(195); - - return limiter2.disconnect(false); + expect(h.log).toHaveCallOrder([[0], [1], [2], [3]]); }); }); - it("Should publish capacity changes on reservoir changes", function () { - const h = createJobHarness(); - rootLimiter = makeLimiter({ + test("Should publish capacity changes on reservoir changes", function ({ + harness: h, + makeLimiter, + track, + }) { + const rootLimiter = makeLimiter({ maxConcurrent: 2, reservoir: 2, }); @@ -507,31 +534,36 @@ describe("Cluster-only", function () { return rootLimiter .ready() .then(function () { - limiter2 = new Bottleneck({ - datastore: process.env.DATASTORE, - }); + limiter2 = track( + new Bottleneck({ + datastore: process.env.DATASTORE, + }), + ); return limiter2.ready(); }) .then(function () { - rootLimiter.schedule({ id: 1 }, h.slowPromise, 100, null, 1); - rootLimiter.schedule({ id: 2 }, h.slowPromise, 100, null, 2); + const held = deferred(); + rootLimiter.schedule({ id: 1 }, h.deferredPromise, held.signal, null, 1); + rootLimiter.schedule({ id: 2 }, h.deferredPromise, held.signal, null, 2); - return rootLimiter.schedule({ id: 0, weight: 0 }, h.promise, null, 0); - }) - .then(function () { - p3 = limiter2.schedule({ id: 3, weight: 2 }, h.slowPromise, 100, null, 3); - return rootLimiter.currentReservoir(); - }) - .then(function (reservoir) { - expect(reservoir).toEqual(0); - return rootLimiter.updateSettings({ reservoir: 1 }); - }) - .then(function () { - return rootLimiter.incrementReservoir(1); - }) - .then(function (reservoir) { - expect(reservoir).toEqual(2); - return p3; + return rootLimiter + .schedule({ id: 0, weight: 0 }, h.promise, null, 0) + .then(function () { + return rootLimiter.currentReservoir(); + }) + .then(function (reservoir) { + expect(reservoir).toEqual(0); + p3 = limiter2.schedule({ id: 3, weight: 2 }, h.promise, null, 3); + return rootLimiter.updateSettings({ reservoir: 1 }); + }) + .then(function () { + return rootLimiter.incrementReservoir(1); + }) + .then(function (reservoir) { + expect(reservoir).toEqual(2); + held.release(); + return p3; + }); }) .then(function (result) { expect(result).toEqual([3]); @@ -542,32 +574,28 @@ describe("Cluster-only", function () { return h.flushLimiter(rootLimiter, { weight: 0 }); }) .then(function (_results) { - h.checkResultsOrder([[0], [1], [2], [3]]); - // Upper bound is generous: a dropped node-redis connection retries - // in-flight commands after reconnect; with connectTimeout=500ms and - // one retry cycle the delay can reach ~1500ms on a loaded testcontainer. - // The ordering assertion above is the real semantic check; checkDuration - // just guards against a completely silent pub/sub channel (>5s delay). - h.checkDuration(210, 10, 2000); - }) - .then(function (_data) { - return limiter2.disconnect(false); + expect(h.log).toHaveCallOrder([[0], [1], [2], [3]]); }); }); - it("Should remove track job data and remove lost jobs", function () { + test("Should remove track job data and remove lost jobs", function ({ + harness: h, + makeLimiter, + track, + }) { // Capture before any limiter is constructed; redis-side timestamps may be // assigned during rootLimiter's init via hsetnx (see init.lua). const testStart = Date.now(); - const h = createJobHarness(); - rootLimiter = makeLimiter({ id: "lost" }, { expectErrors: true }); + const rootLimiter = makeLimiter({ id: "lost" }, { expectErrors: true }); const clientId = rootLimiter._store.clientId; - const limiter1 = new Bottleneck({ datastore: process.env.DATASTORE }); - const limiter2 = new Bottleneck({ - id: "lost", - datastore: process.env.DATASTORE, - heartbeatInterval: 150, - }); + const limiter1 = track(new Bottleneck({ datastore: process.env.DATASTORE })); + const limiter2 = track( + new Bottleneck({ + id: "lost", + datastore: process.env.DATASTORE, + heartbeatInterval: 150, + }), + ); const getData = function (limiter) { expect(limiterKeys(limiter).length).toEqual(8); // Asserting, to remember to edit this test when keys change const [ @@ -592,6 +620,8 @@ describe("Cluster-only", function () { runCommand(limiter1, "zrange", [client_last_seen_key, "0", "-1", "withscores"]), ]); }; + const job1 = deferred(); + let p1; let numExpirations = 0; const errorHandler = function (err) { if (err.message.indexOf("This job timed out") === 0) { @@ -602,21 +632,27 @@ describe("Cluster-only", function () { return ( Promise.all([rootLimiter.ready(), limiter1.ready(), limiter2.ready()]) .then(function () { - // No expiration, it should not be removed - // oxlint-disable-next-line no-unused-expressions - (h.pNoErrVal(rootLimiter.schedule({ weight: 1 }, h.slowPromise, 150, null, 1), 1), - // Expiration present, these jobs should be removed automatically - rootLimiter - .schedule({ expiration: 50, weight: 2 }, h.slowPromise, 75, null, 2) - .catch(errorHandler)); + const never = new Promise(function () {}); + // No expiration, it should not be removed. Held until after the + // disconnect below, then released so we keep the completion-with-value + // coverage: the task resolves locally (free.lua fails post-disconnect + // and is swallowed), so redis still shows it as running. + p1 = h.pNoErrVal( + rootLimiter.schedule({ weight: 1 }, h.deferredPromise, job1.signal, null, 1), + 1, + ); + // Expiration present, these jobs should be removed automatically rootLimiter - .schedule({ expiration: 50, weight: 3 }, h.slowPromise, 75, null, 3) + .schedule({ expiration: 50, weight: 2 }, h.deferredPromise, never, null, 2) .catch(errorHandler); rootLimiter - .schedule({ expiration: 50, weight: 4 }, h.slowPromise, 75, null, 4) + .schedule({ expiration: 50, weight: 3 }, h.deferredPromise, never, null, 3) .catch(errorHandler); rootLimiter - .schedule({ expiration: 50, weight: 5 }, h.slowPromise, 75, null, 5) + .schedule({ expiration: 50, weight: 4 }, h.deferredPromise, never, null, 4) + .catch(errorHandler); + rootLimiter + .schedule({ expiration: 50, weight: 5 }, h.deferredPromise, never, null, 5) .catch(errorHandler); return rootLimiter._submitLock.schedule(() => Promise.resolve(true)); @@ -627,6 +663,10 @@ describe("Cluster-only", function () { .then(function () { return rootLimiter.disconnect(false); }) + .then(function () { + job1.release(); + return p1; + }) // Poll for the post-cleanup state instead of asserting an intermediate // snapshot in the narrow window between dispatch and the 50ms expiration // timers firing — under event-loop stress that window can effectively @@ -672,14 +712,11 @@ describe("Cluster-only", function () { expect(numExpirations).toEqual(4); }) - .then(function () { - return Promise.all([limiter1.disconnect(false), limiter2.disconnect(false)]); - }) ); }); - it("Should clear unresponsive clients", async function () { - rootLimiter = makeLimiter({ + test("Should clear unresponsive clients", async function ({ makeLimiter, track }) { + const rootLimiter = makeLimiter({ id: "unresponsive", maxConcurrent: 1, timeout: 1000, @@ -696,10 +733,12 @@ describe("Cluster-only", function () { // default options, clientTimeout=10000 and process_tick can't clean // up within the 5s test window. await rootLimiter.ready(); - const limiter2 = new Bottleneck({ - id: "unresponsive", - datastore: process.env.DATASTORE, - }); + const limiter2 = track( + new Bottleneck({ + id: "unresponsive", + datastore: process.env.DATASTORE, + }), + ); await limiter2.ready(); await Promise.all([rootLimiter.running(), limiter2.running()]); @@ -721,24 +760,24 @@ describe("Cluster-only", function () { // Poll for cleanup. Cleanup happens in process_tick.lua, triggered by // limiter operations. Each poll calls running() which fires process_tick. - await waitForState( - async function () { - await rootLimiter.running(); - const counts = await numClients(); - expect(counts[0]).toBe(1); - expect(counts[1]).toBe(1); - expect(counts[2]).toBe(1); - expect(counts[3]).toBe(1); - }, - { timeout: 5000 }, - ); + await waitForState(async function () { + await rootLimiter.running(); + const counts = await numClients(); + expect(counts[0]).toBe(1); + expect(counts[1]).toBe(1); + expect(counts[2]).toBe(1); + expect(counts[3]).toBe(1); + }); expect(await numClients()).toEqual([1, 1, 1, 1]); }); - it("Should not clear unresponsive clients with unexpired running jobs", async function () { - const h = createJobHarness(); - rootLimiter = makeLimiter({ + test("Should not clear unresponsive clients with unexpired running jobs", async function ({ + harness: h, + makeLimiter, + track, + }) { + const rootLimiter = makeLimiter({ id: "unresponsive-unexpired", maxConcurrent: 1, timeout: 1000, @@ -749,10 +788,12 @@ describe("Cluster-only", function () { // limiter2's defaults. Constructing limiter2 up-front races init.lua and // settings can adopt the wrong values. await rootLimiter.ready(); - const limiter2 = new Bottleneck({ - id: "unresponsive-unexpired", - datastore: process.env.DATASTORE, - }); + const limiter2 = track( + new Bottleneck({ + id: "unresponsive-unexpired", + datastore: process.env.DATASTORE, + }), + ); await limiter2.ready(); const client_running_key = limiterKeys(limiter2)[4]; @@ -767,26 +808,39 @@ describe("Cluster-only", function () { runCommand(limiter2, "zcard", [client_last_seen_key]), ]); - const job = rootLimiter.schedule(h.slowPromise, 500, null, 1); + const held = deferred(); + const job = rootLimiter.schedule(h.deferredPromise, held.signal, null, 1); - await h.wait(300); + await waitForState(async function () { + expect(await limiter2.running()).toEqual(1); + expect(await numClients()).toEqual([2, 2, 2, 2]); + }); - // running() triggers process_tick and that will attempt to remove client 1 - // but it shouldn't do it because it has a running job - expect(await limiter2.running()).toEqual(1); + // Wait until client 1's last_seen is observably stale (> clientTimeout). + // rootLimiter is idle (heartbeatInterval 2000) so nothing refreshes it; + // the zscore read goes straight to redis and does not run process_tick. + const clientId1 = rootLimiter._store.clientId; + await waitForState(async function () { + const score = await runCommand(limiter2, "zscore", [client_last_seen_key, clientId1]); + expect(Date.now() - parseFloat(score)).toBeGreaterThan(200); + }); + // running() fires process_tick, which now sees client 1 as unresponsive — + // it must NOT reap a client that still has an unexpired running job. + expect(await limiter2.running()).toEqual(1); expect(await numClients()).toEqual([2, 2, 2, 2]); - await job; - + held.release(); + expect(await job).toEqual([1]); expect(await limiter2.running()).toEqual(0); - - await limiter2.disconnect(false); }); - it("Should clear unresponsive clients after last jobs are expired", async function () { - const h = createJobHarness(); - rootLimiter = makeLimiter({ + test("Should clear unresponsive clients after last jobs are expired", async function ({ + harness: h, + makeLimiter, + track, + }) { + const rootLimiter = makeLimiter({ id: "unresponsive-expired", maxConcurrent: 1, timeout: 1000, @@ -796,10 +850,12 @@ describe("Cluster-only", function () { // Sequence init so rootLimiter's clientTimeout/heartbeatInterval win over // limiter2's defaults. await rootLimiter.ready(); - const limiter2 = new Bottleneck({ - id: "unresponsive-expired", - datastore: process.env.DATASTORE, - }); + const limiter2 = track( + new Bottleneck({ + id: "unresponsive-expired", + datastore: process.env.DATASTORE, + }), + ); await limiter2.ready(); const client_running_key = limiterKeys(limiter2)[4]; @@ -814,11 +870,13 @@ describe("Cluster-only", function () { runCommand(limiter2, "zcard", [client_last_seen_key]), ]); - const job = rootLimiter.schedule({ expiration: 250 }, h.slowPromise, 300, null, 1); - await h.wait(100); // wait for it to register + const never = new Promise(function () {}); + const job = rootLimiter.schedule({ expiration: 250 }, h.deferredPromise, never, null, 1); - expect(await rootLimiter.running()).toEqual(1); - expect(await numClients()).toEqual([2, 2, 2, 2]); + await waitForState(async function () { + expect(await rootLimiter.running()).toEqual(1); + expect(await numClients()).toEqual([2, 2, 2, 2]); + }); let dropped = false; try { @@ -836,27 +894,21 @@ describe("Cluster-only", function () { // Poll instead of relying on a fixed wait — under load the cleanup might // need more than 200ms wall-clock, and a fixed wait either fails (too short) // or wastes time (too long). Each poll calls running() which fires process_tick. - await waitForState( - async function () { - await limiter2.running(); - const counts = await numClients(); - expect(counts[0]).toBe(1); - expect(counts[1]).toBe(1); - expect(counts[2]).toBe(1); - expect(counts[3]).toBe(1); - }, - { timeout: 5000 }, - ); + await waitForState(async function () { + await limiter2.running(); + const counts = await numClients(); + expect(counts[0]).toBe(1); + expect(counts[1]).toBe(1); + expect(counts[2]).toBe(1); + expect(counts[3]).toBe(1); + }); expect(await limiter2.running()).toEqual(0); expect(await numClients()).toEqual([1, 1, 1, 1]); - - await limiter2.disconnect(false); }); - it("Should use shared settings", function () { - const h = createJobHarness(); - rootLimiter = makeLimiter({ maxConcurrent: 2 }); + test("Should use shared settings", function ({ harness: h, makeLimiter, track }) { + const rootLimiter = makeLimiter({ maxConcurrent: 2 }); let limiter2; const settings_key = limiterKeys(rootLimiter)[0]; @@ -868,7 +920,7 @@ describe("Cluster-only", function () { return rootLimiter .ready() .then(function () { - limiter2 = new Bottleneck({ maxConcurrent: 1, datastore: process.env.DATASTORE }); + limiter2 = track(new Bottleneck({ maxConcurrent: 1, datastore: process.env.DATASTORE })); return limiter2.ready(); }) .then(function () { @@ -877,8 +929,8 @@ describe("Cluster-only", function () { .then(function (maxConcurrent) { expect(maxConcurrent).toEqual("2"); return Promise.all([ - limiter2.schedule(h.slowPromise, 100, null, 1), - limiter2.schedule(h.slowPromise, 100, null, 2), + limiter2.schedule(h.promise, null, 1), + limiter2.schedule(h.promise, null, 2), ]); }) .then(function () { @@ -888,24 +940,25 @@ describe("Cluster-only", function () { return h.flushLimiter(rootLimiter); }) .then(function (_results) { - h.checkResultsOrder([[1], [2]]); + expect(h.log).toHaveCallOrder([[1], [2]]); }); }); - it("Should clear previous settings", function () { - const h = createJobHarness(); - rootLimiter = makeLimiter({ maxConcurrent: 2 }); + test("Should clear previous settings", function ({ harness: h, makeLimiter, track }) { + const rootLimiter = makeLimiter({ maxConcurrent: 2 }); let limiter2; const settings_key = limiterKeys(rootLimiter)[0]; return rootLimiter .ready() .then(function () { - limiter2 = new Bottleneck({ - maxConcurrent: 1, - datastore: process.env.DATASTORE, - clearDatastore: true, - }); + limiter2 = track( + new Bottleneck({ + maxConcurrent: 1, + datastore: process.env.DATASTORE, + clearDatastore: true, + }), + ); return limiter2.ready(); }) .then(function () { @@ -916,10 +969,15 @@ describe("Cluster-only", function () { }) .then(function (maxConcurrent) { expect(maxConcurrent).toEqual("1"); - return Promise.all([ - rootLimiter.schedule(h.slowPromise, 100, null, 1), - rootLimiter.schedule(h.slowPromise, 100, null, 2), - ]); + const job1 = deferred(); + const p1 = rootLimiter.schedule(h.deferredPromise, job1.signal, null, 1); + const p2 = rootLimiter.schedule(h.slowPromise, 100, null, 2); + return waitForState(function () { + expect(rootLimiter.counts().EXECUTING).toEqual(1); + }).then(function () { + job1.release(); + return Promise.all([p1, p2]); + }); }) .then(function () { return limiter2.disconnect(false); @@ -928,18 +986,18 @@ describe("Cluster-only", function () { return h.flushLimiter(rootLimiter); }) .then(function (_results) { - h.checkResultsOrder([[1], [2]]); + expect(h.log).toHaveCallOrder([[1], [2]]); }); }); - it("Should safely handle connection failures", function () { + test("Should safely handle connection failures", function ({ makeLimiter }) { expect.hasAssertions(); // node-redis v4+ uses a nested socket option shape; ioredis stays flat. const failingOptions = process.env.DATASTORE === "redis" ? { socket: { port: 1, reconnectStrategy: () => false } } : { port: 1 }; - rootLimiter = makeLimiter({ clientOptions: failingOptions }, { expectErrors: true }); + const rootLimiter = makeLimiter({ clientOptions: failingOptions }, { expectErrors: true }); return new Promise(function (resolve, reject) { rootLimiter.on("error", function (err) { diff --git a/test/general-traffic.test.js b/test/general-traffic.test.js index abfa3e1..a65e496 100644 --- a/test/general-traffic.test.js +++ b/test/general-traffic.test.js @@ -1,24 +1,21 @@ -import { describe, it, afterEach, expect } from "vitest"; -import { createJobHarness } from "./helpers/job-tracking.js"; -import { waitForState } from "./helpers/wait-for-state.js"; -const makeLimiter = require("./helpers/limiter"); +import { useFakeClock, useRealClockForThisTest } from "./helpers/clock.js"; +import { test, describe, expect, waitForState, deferred } from "./helpers/test-api.js"; const path = require("path"); const util = require("util"); const execFile = util.promisify(require("child_process").execFile); -describe("General traffic", function () { - let limiter; - - afterEach(function () { - if (limiter == null) return; - return limiter.disconnect(false); - }); +useFakeClock(); +describe("General traffic", () => { describe("High water limit", function () { - it("Should support highWater set to 0", function () { - const h = createJobHarness(); - limiter = makeLimiter({ maxConcurrent: 1, minTime: 0, highWater: 0, rejectOnDrop: false }); + test("Should support highWater set to 0", function ({ harness: h, makeLimiter }) { + const limiter = makeLimiter({ + maxConcurrent: 1, + minTime: 0, + highWater: 0, + rejectOnDrop: false, + }); const first = h.pNoErrVal(limiter.schedule(h.slowPromise, 50, null, 1), 1); h.pNoErrVal(limiter.schedule(h.slowPromise, 50, null, 2), 2); @@ -30,14 +27,18 @@ describe("General traffic", function () { return h.flushLimiter(limiter, { weight: 0 }); }) .then(function (_results) { - h.checkDuration(50); - h.checkResultsOrder([[1]]); + expect(h).toHaveFinalCallAt(50); + expect(h.log).toHaveCallOrder([[1]]); }); }); - it("Should support highWater set to 1", async function () { - const h = createJobHarness(); - limiter = makeLimiter({ maxConcurrent: 1, minTime: 0, highWater: 1, rejectOnDrop: false }); + test("Should support highWater set to 1", async function ({ harness: h, makeLimiter }) { + const limiter = makeLimiter({ + maxConcurrent: 1, + minTime: 0, + highWater: 1, + rejectOnDrop: false, + }); await limiter.ready(); // Track how many jobs have actually been committed to the queue by @@ -64,11 +65,8 @@ describe("General traffic", function () { // submits can exceed slowPromise(50)'s window — job 1 finishes // before jobs 3/4 commit, jobs 2/3 dispatch, and the test sees 3 // results instead of the expected 2. - let releaseFirst; - const firstSignal = new Promise(function (r) { - releaseFirst = r; - }); - const first = h.pNoErrVal(limiter.schedule(h.deferredPromise, firstSignal, null, 1), 1); + const primer = deferred(); + const first = h.pNoErrVal(limiter.schedule(h.deferredPromise, primer.signal, null, 1), 1); // Wait until the primer is running. Once it occupies the running // slot at maxConcurrent=1, no subsequent job can be dispatched @@ -86,17 +84,19 @@ describe("General traffic", function () { expect(committed).toBe(4); }); - releaseFirst(); + primer.release(); await Promise.all([first, last]); await h.flushLimiter(limiter, { weight: 0 }); - h.checkResultsOrder([[1], [4]]); + expect(h.log).toHaveCallOrder([[1], [4]]); }); }); describe("Weight", function () { - it("Should not add jobs with a weight above the maxConcurrent", function () { - const h = createJobHarness(); - limiter = makeLimiter({ maxConcurrent: 2 }); + test("Should not add jobs with a weight above the maxConcurrent", function ({ + harness: h, + makeLimiter, + }) { + const limiter = makeLimiter({ maxConcurrent: 2 }); h.pNoErrVal(limiter.schedule({ weight: 1 }, h.promise, null, 1), 1); h.pNoErrVal(limiter.schedule({ weight: 2 }, h.promise, null, 2), 2); @@ -110,14 +110,13 @@ describe("General traffic", function () { return h.flushLimiter(limiter); }) .then(function (_results) { - h.checkDuration(0); - h.checkResultsOrder([[1], [2]]); + expect(h).toHaveFinalCallAt(0); + expect(h.log).toHaveCallOrder([[1], [2]]); }); }); - it("Should support custom job weights", function () { - const h = createJobHarness(); - limiter = makeLimiter({ maxConcurrent: 2 }); + test("Should support custom job weights", function ({ harness: h, makeLimiter }) { + const limiter = makeLimiter({ maxConcurrent: 2 }); // Await all 5 schedule promises before h.flushLimiter(limiter); otherwise the weight: 0 // job's slowPromise may not have settled by the time h.flushLimiter(limiter) reads calls[]. @@ -132,14 +131,13 @@ describe("General traffic", function () { return h.flushLimiter(limiter); }) .then(function (_results) { - h.checkDuration(400); - h.checkResultsOrder([[1], [2], [3], [4], [5]]); + expect(h).toHaveFinalCallAt(400); + expect(h.log).toHaveCallOrder([[1], [2], [3], [4], [5]]); }); }); - it("Should overflow at the correct rate", function () { - const h = createJobHarness(); - limiter = makeLimiter({ + test("Should overflow at the correct rate", function ({ harness: h, makeLimiter }) { + const limiter = makeLimiter({ maxConcurrent: 2, reservoir: 3, }); @@ -185,8 +183,8 @@ describe("General traffic", function () { .then(function (_results) { expect(calledDepleted).toEqual(3); expect(limiter.queued()).toEqual(1); - h.checkDuration(250); - h.checkResultsOrder([[1], [2]]); + expect(h).toHaveFinalCallAt(250); + expect(h.log).toHaveCallOrder([[1], [2]]); return limiter.currentReservoir(); }) .then(function (reservoir) { @@ -208,9 +206,8 @@ describe("General traffic", function () { }); describe("Expiration", function () { - it("Should cancel jobs", { timeout: 20000 }, function () { - const h = createJobHarness(); - limiter = makeLimiter({ maxConcurrent: 2 }); + test("Should cancel jobs", { timeout: 20000 }, function ({ harness: h, makeLimiter }) { + const limiter = makeLimiter({ maxConcurrent: 2 }); const t0 = Date.now(); // Hold j1 with deferredPromise instead of slowPromise(150). Reason: @@ -220,14 +217,17 @@ describe("General traffic", function () { // expiration catch runs the running===1 assertion, making running===0 // (because j1 was freed too). With a deferredPromise we release j1 // explicitly inside the catch chain, after verifying running===1. - let releaseJ1; - const j1Signal = new Promise(function (r) { - releaseJ1 = r; - }); + const holdJ1 = deferred(); return Promise.all([ h.pNoErrVal( - limiter.schedule({ id: "very-slow-no-expiration" }, h.deferredPromise, j1Signal, null, 1), + limiter.schedule( + { id: "very-slow-no-expiration" }, + h.deferredPromise, + holdJ1.signal, + null, + 1, + ), 1, ), @@ -252,7 +252,7 @@ describe("General traffic", function () { // meaningful interval, without depending on a fixed timer that // can race event-loop jitter. return h.wait(100).then(function () { - releaseJ1(); + holdJ1.release(); }); }), ]) @@ -271,8 +271,8 @@ describe("General traffic", function () { }); describe("Pubsub", function () { - it("Should pass strings", function () { - limiter = makeLimiter({ maxConcurrent: 2 }); + test("Should pass strings", function ({ makeLimiter }) { + const limiter = makeLimiter({ maxConcurrent: 2 }); return new Promise((resolve, reject) => { limiter.on("message", function (msg) { @@ -288,8 +288,8 @@ describe("General traffic", function () { }); }); - it("Should pass objects", function () { - limiter = makeLimiter({ maxConcurrent: 2 }); + test("Should pass objects", function ({ makeLimiter }) { + const limiter = makeLimiter({ maxConcurrent: 2 }); const obj = { array: ["abc", true], num: 235.59, @@ -311,9 +311,8 @@ describe("General traffic", function () { }); describe("Reservoir Refresh", function () { - it("Should auto-refresh the reservoir", function () { - const h = createJobHarness(); - limiter = makeLimiter({ + test("Should auto-refresh the reservoir", function ({ harness: h, makeLimiter }) { + const limiter = makeLimiter({ reservoir: 8, reservoirRefreshInterval: 150, reservoirRefreshAmount: 5, @@ -336,7 +335,7 @@ describe("General traffic", function () { return h.flushLimiter(limiter, { weight: 0, priority: 9 }); }) .then(function (results) { - h.checkResultsOrder([[1], [2], [3], [4], [5]]); + expect(h.log).toHaveCallOrder([[1], [2], [3], [4], [5]]); // The contract is "`depleted` fires when the reservoir reaches 0". // We get >=2 fires reliably: // 1) j5 (weight 5) dispatching after the t=300 refresh @@ -353,14 +352,13 @@ describe("General traffic", function () { // Jobs 4 and 5 must wait for refreshes; that lower bound proves the // refresh gate worked. Asserting current reservoir or a tight upper // bound (checkDuration(300)) races a third refresh at t=450ms. - expect(results.calls[3].time).toBeGreaterThanOrEqual(145); - expect(results.calls[4].time).toBeGreaterThanOrEqual(295); + expect(results).toHaveCallAt(3, 150); + expect(results).toHaveCallAt(4, 300); }); }); - it("Should allow staggered X by Y type usage", function () { - const h = createJobHarness(); - limiter = makeLimiter({ + test("Should allow staggered X by Y type usage", function ({ harness: h, makeLimiter }) { + const limiter = makeLimiter({ reservoir: 2, reservoirRefreshInterval: 150, reservoirRefreshAmount: 2, @@ -377,18 +375,19 @@ describe("General traffic", function () { return h.flushLimiter(limiter, { weight: 0, priority: 9 }); }) .then(function (results) { - h.checkResultsOrder([[1], [2], [3], [4]]); + expect(h.log).toHaveCallOrder([[1], [2], [3], [4]]); // Jobs 3 and 4 must wait for the reservoir refresh at t=150ms; that // lower bound proves the gate worked. Asserting the *current* // reservoir is 0 races a possible second refresh at t=300 — and // checkDuration(150) is too tight under load. - expect(results.calls[2].time).toBeGreaterThanOrEqual(145); - expect(results.calls[3].time).toBeGreaterThanOrEqual(145); + expect(results).toHaveCallAt(2, 150); + expect(results).toHaveCallAt(3, 150); }); }); - it("Should keep process alive until queue is empty", async function () { - limiter = makeLimiter(); + test("Should keep process alive until queue is empty", async function ({ makeLimiter }) { + useRealClockForThisTest(); + const limiter = makeLimiter(); const fixturePath = path.resolve(__dirname, "fixtures/keep-alive/refreshKeepAlive.mjs"); const { stdout, stderr } = await execFile(process.execPath, [fixturePath], { timeout: 10000, @@ -420,9 +419,8 @@ describe("General traffic", function () { }); describe("Reservoir Increase", function () { - it("Should auto-increase the reservoir", async function () { - const h = createJobHarness(); - limiter = makeLimiter({ + test("Should auto-increase the reservoir", async function ({ harness: h, makeLimiter }) { + const limiter = makeLimiter({ reservoir: 3, reservoirIncreaseInterval: 150, reservoirIncreaseAmount: 5, @@ -443,19 +441,19 @@ describe("General traffic", function () { ]); const results = await h.flushLimiter(limiter, { weight: 0, priority: 9 }); - h.checkResultsOrder([[1], [2], [3], [4], [5]]); + expect(h.log).toHaveCallOrder([[1], [2], [3], [4], [5]]); expect(calledDepleted).toEqual(1); - // Jobs 3, 4, 5 must each wait for an increase tick (150/300/450ms); - // the lower bound proves the gate worked. The current-reservoir read - // and a tight checkDuration both race the next increase tick at 600ms. - expect(results.calls[2].time).toBeGreaterThanOrEqual(145); - expect(results.calls[3].time).toBeGreaterThanOrEqual(295); - expect(results.calls[4].time).toBeGreaterThanOrEqual(445); + // Jobs 3, 4, 5 must each wait for an increase tick (150/300/450ms). + expect(results).toHaveCallAt(2, 150); + expect(results).toHaveCallAt(3, 300); + expect(results).toHaveCallAt(4, 450); }); - it("Should auto-increase the reservoir up to a maximum", async function () { - const h = createJobHarness(); - limiter = makeLimiter({ + test("Should auto-increase the reservoir up to a maximum", async function ({ + harness: h, + makeLimiter, + }) { + const limiter = makeLimiter({ reservoir: 3, reservoirIncreaseInterval: 150, reservoirIncreaseAmount: 5, @@ -477,19 +475,15 @@ describe("General traffic", function () { ]); const results = await h.flushLimiter(limiter, { weight: 0, priority: 9 }); - h.checkResultsOrder([[1], [2], [3], [4], [5]]); + expect(h.log).toHaveCallOrder([[1], [2], [3], [4], [5]]); expect(calledDepleted).toEqual(1); - // Lower-bound timing assertions are the actual contract — the reservoir - // value at end is racy because additional increase ticks fire even after - // the last job dispatches. - expect(results.calls[2].time).toBeGreaterThanOrEqual(145); - expect(results.calls[3].time).toBeGreaterThanOrEqual(295); - expect(results.calls[4].time).toBeGreaterThanOrEqual(445); + expect(results).toHaveCallAt(2, 150); + expect(results).toHaveCallAt(3, 300); + expect(results).toHaveCallAt(4, 450); }); - it("Should allow staggered X by Y type usage", function () { - const h = createJobHarness(); - limiter = makeLimiter({ + test("Should allow staggered X by Y type usage", function ({ harness: h, makeLimiter }) { + const limiter = makeLimiter({ reservoir: 2, reservoirIncreaseInterval: 150, reservoirIncreaseAmount: 2, @@ -517,17 +511,18 @@ describe("General traffic", function () { return h.flushLimiter(limiter, { weight: 0, priority: 9 }); }) .then(function (results) { - h.checkResultsOrder([[1], [2], [3], [4]]); + expect(h.log).toHaveCallOrder([[1], [2], [3], [4]]); // Jobs 3 and 4 must wait for the reservoir refill at t=150ms; lower // bound proves the refill gate worked. No upper bound — under load // dispatch latency adds to the wait time but doesn't violate the contract. - expect(results.calls[2].time).toBeGreaterThanOrEqual(145); - expect(results.calls[3].time).toBeGreaterThanOrEqual(145); + expect(results).toHaveCallAt(2, 150); + expect(results).toHaveCallAt(3, 150); }); }); - it("Should keep process alive until queue is empty", async function () { - limiter = makeLimiter(); + test("Should keep process alive until queue is empty", async function ({ makeLimiter }) { + useRealClockForThisTest(); + const limiter = makeLimiter(); const fixturePath = path.resolve(__dirname, "fixtures/keep-alive/increaseKeepAlive.mjs"); const { stdout, stderr } = await execFile(process.execPath, [fixturePath], { timeout: 10000, diff --git a/test/general.test.js b/test/general.test.js index 234a609..30d42f3 100644 --- a/test/general.test.js +++ b/test/general.test.js @@ -1,31 +1,24 @@ -import { describe, it, afterEach, expect } from "vitest"; -import { createJobHarness } from "./helpers/job-tracking.js"; -import { waitForState } from "./helpers/wait-for-state.js"; -const makeLimiter = require("./helpers/limiter"); +import { useFakeClock } from "./helpers/clock.js"; +import { test, describe, expect, waitForState, deferred } from "./helpers/test-api.js"; const Bottleneck = require("./bottleneck"); -describe("General", function () { - let limiter; +useFakeClock(); - afterEach(function () { - if (limiter == null) return; - return limiter.disconnect(false); - }); - - it("Should prompt to upgrade", function () { - limiter = makeLimiter(); +describe("General", () => { + test("Should prompt to upgrade", function ({ makeLimiter }) { + const limiter = makeLimiter(); expect(() => { const _limiter = new Bottleneck(1, 250); }).toThrow(/Bottleneck v2 takes a single object argument/); }); - it("Should allow null capacity", async function () { - limiter = makeLimiter({ id: "null", minTime: 0 }); + test("Should allow null capacity", async function ({ makeLimiter }) { + const limiter = makeLimiter({ id: "null", minTime: 0 }); await expect(limiter.updateSettings({ minTime: 10 })).resolves.toBe(limiter); }); - it("Should keep scope", async function () { - limiter = makeLimiter({ maxConcurrent: 1 }); + test("Should keep scope", async function ({ makeLimiter }) { + const limiter = makeLimiter({ maxConcurrent: 1 }); class Job { constructor() { @@ -41,10 +34,12 @@ describe("General", function () { expect(await limiter.wrap(job.action.bind(job))(2)).toEqual(7); }); - it("Should pass multiple arguments back even on errors when using submit()", function () { + test("Should pass multiple arguments back even on errors when using submit()", function ({ + harness: h, + makeLimiter, + }) { expect.hasAssertions(); - const h = createJobHarness(); - limiter = makeLimiter({ maxConcurrent: 1 }); + const limiter = makeLimiter({ maxConcurrent: 1 }); return new Promise(function (resolve, reject) { limiter.submit(h.job, new Error("welp"), 1, 2, function (err, x, y) { @@ -60,8 +55,8 @@ describe("General", function () { }); }); - it("Should expose the Events library", function () { - limiter = makeLimiter(); + test("Should expose the Events library", function ({ makeLimiter }) { + const limiter = makeLimiter(); class Hello { constructor() { @@ -92,25 +87,24 @@ describe("General", function () { }); describe("Counts and statuses", function () { - it("Should check() and return the queued count with and without a priority value", async function () { - const h = createJobHarness(); - limiter = makeLimiter({ maxConcurrent: 1, minTime: 100 }); + test("Should check() and return the queued count with and without a priority value", async function ({ + harness: h, + makeLimiter, + }) { + const limiter = makeLimiter({ maxConcurrent: 1, minTime: 100 }); // Hold job 1 with a deferred promise so it never finishes until we // explicitly release it. Otherwise the prior `slowJob, 50` could finish // before all 4 submits complete (each submit adds Redis RTT) and a // queued job dispatches, making `queued()` count race the minTime gate. - let release1; - const hold1 = new Promise(function (resolve) { - release1 = resolve; - }); + const hold1 = deferred(); expect(await limiter.check()).toEqual(true); expect(limiter.queued()).toEqual(0); expect(await limiter.clusterQueued()).toEqual(0); - await limiter.submit({ id: 1 }, h.deferredJob, hold1, null, 1, h.noErrVal(1)); + await limiter.submit({ id: 1 }, h.deferredJob, hold1.signal, null, 1, h.noErrVal(1)); expect(limiter.queued()).toEqual(0); // It's already running expect(await limiter.check()).toEqual(false); @@ -139,49 +133,41 @@ describe("General", function () { expect(limiter.queued(1)).toEqual(1); expect(limiter.queued(5)).toEqual(3); - release1(); + hold1.release(); await h.flushLimiter(limiter); expect(limiter.queued()).toEqual(0); expect(await limiter.clusterQueued()).toEqual(0); - h.checkResultsOrder([[1], [5], [2], [3], [4]]); + expect(h.log).toHaveCallOrder([[1], [5], [2], [3], [4]]); }); - it("Should return the running and done counts", async function () { - const h = createJobHarness(); - limiter = makeLimiter({ maxConcurrent: 5, minTime: 0 }); + test("Should return the running and done counts", async function ({ harness: h, makeLimiter }) { + const limiter = makeLimiter({ maxConcurrent: 5, minTime: 0 }); // Held jobs let the test observe each (running, done) checkpoint // deterministically. With slowPromise(100) the first checkpoint // (running=5) raced Redis RTT — an early job could transition to // DONE before the running()/done() round-trip returned, dropping // running to 4. - let release1, release2, release3; - const hold1 = new Promise(function (r) { - release1 = r; - }); - const hold2 = new Promise(function (r) { - release2 = r; - }); - const hold3 = new Promise(function (r) { - release3 = r; - }); + const hold1 = deferred(); + const hold2 = deferred(); + const hold3 = deferred(); const [running0, done0] = await Promise.all([limiter.running(), limiter.done()]); expect(running0).toEqual(0); expect(done0).toEqual(0); - limiter.submit({ weight: 1, id: 1 }, h.deferredJob, hold1, null, 1, h.noErrVal(1)); - limiter.submit({ weight: 3, id: 2 }, h.deferredJob, hold2, null, 2, h.noErrVal(2)); - limiter.submit({ weight: 1, id: 3 }, h.deferredJob, hold3, null, 3, h.noErrVal(3)); + limiter.submit({ weight: 1, id: 1 }, h.deferredJob, hold1.signal, null, 1, h.noErrVal(1)); + limiter.submit({ weight: 3, id: 2 }, h.deferredJob, hold2.signal, null, 2, h.noErrVal(2)); + limiter.submit({ weight: 1, id: 3 }, h.deferredJob, hold3.signal, null, 3, h.noErrVal(3)); await limiter.schedule({ weight: 0, id: 4 }, h.promise, null); const [running1, done1] = await Promise.all([limiter.running(), limiter.done()]); expect(running1).toEqual(5); expect(done1).toEqual(0); - release1(); - release3(); + hold1.release(); + hold3.release(); await waitForState(async function () { const [r, d] = await Promise.all([limiter.running(), limiter.done()]); expect(r).toBe(3); @@ -192,7 +178,7 @@ describe("General", function () { expect(running2).toEqual(3); expect(done2).toEqual(2); - release2(); + hold2.release(); await waitForState(async function () { const [r, d] = await Promise.all([limiter.running(), limiter.done()]); expect(r).toBe(0); @@ -204,12 +190,11 @@ describe("General", function () { expect(done3).toEqual(5); await h.flushLimiter(limiter); - h.checkResultsOrder([[], [1], [3], [2]]); + expect(h.log).toHaveCallOrder([[], [1], [3], [2]]); }); - it("Should refuse duplicate Job IDs", async function () { - const h = createJobHarness(); - limiter = makeLimiter({ maxConcurrent: 2, minTime: 100, trackDoneStatus: true }); + test("Should refuse duplicate Job IDs", async function ({ harness: h, makeLimiter }) { + const limiter = makeLimiter({ maxConcurrent: 2, minTime: 100, trackDoneStatus: true }); try { await limiter.schedule({ id: "a" }, h.promise, null, 1); @@ -220,43 +205,45 @@ describe("General", function () { } }); - it("Should return job statuses", function () { - const h = createJobHarness(); - limiter = makeLimiter({ maxConcurrent: 2, minTime: 100 }); + test("Should return job statuses", async function ({ harness: h, makeLimiter }) { + const limiter = makeLimiter({ maxConcurrent: 2, minTime: 100 }); + await limiter.ready(); expect(limiter.counts()).toEqual({ RECEIVED: 0, QUEUED: 0, RUNNING: 0, EXECUTING: 0 }); - h.pNoErrVal(limiter.schedule({ weight: 1, id: 1 }, h.slowPromise, 100, null, 1), 1); + const hold1 = deferred(); + h.pNoErrVal( + limiter.schedule({ weight: 1, id: 1 }, h.deferredPromise, hold1.signal, null, 1), + 1, + ); h.pNoErrVal(limiter.schedule({ weight: 1, id: 2 }, h.slowPromise, 200, null, 2), 2); h.pNoErrVal(limiter.schedule({ weight: 2, id: 3 }, h.slowPromise, 100, null, 3), 3); expect(limiter.counts()).toEqual({ RECEIVED: 3, QUEUED: 0, RUNNING: 0, EXECUTING: 0 }); - // Poll for the dispatched state instead of a fixed wait — under redis latency, - // a 50ms sleep can race with Lua dispatch. - return waitForState(function () { + await waitForState(function () { const counts = limiter.counts(); expect(counts.RECEIVED).toBe(0); expect(counts.QUEUED).toBe(1); expect(counts.RUNNING).toBe(1); expect(counts.EXECUTING).toBe(1); - }) - .then(function () { - expect(limiter.counts()).toEqual({ RECEIVED: 0, QUEUED: 1, RUNNING: 1, EXECUTING: 1 }); - expect(limiter.jobStatus(1)).toEqual("EXECUTING"); - expect(limiter.jobStatus(2)).toEqual("RUNNING"); - expect(limiter.jobStatus(3)).toEqual("QUEUED"); + }); - return h.flushLimiter(limiter); - }) - .then(function (_results) { - h.checkDuration(400); - h.checkResultsOrder([[1], [2], [3]]); - }); + expect(limiter.counts()).toEqual({ RECEIVED: 0, QUEUED: 1, RUNNING: 1, EXECUTING: 1 }); + expect(limiter.jobStatus(1)).toEqual("EXECUTING"); + expect(limiter.jobStatus(2)).toEqual("RUNNING"); + expect(limiter.jobStatus(3)).toEqual("QUEUED"); + + hold1.release(); + await h.flushLimiter(limiter); + expect(h).toHaveFinalCallAt(400); + expect(h.log).toHaveCallOrder([[1], [2], [3]]); }); - it("Should return job statuses, including DONE", async function () { - const h = createJobHarness(); - limiter = makeLimiter({ maxConcurrent: 2, minTime: 100, trackDoneStatus: true }); + test("Should return job statuses, including DONE", async function ({ + harness: h, + makeLimiter, + }) { + const limiter = makeLimiter({ maxConcurrent: 2, minTime: 100, trackDoneStatus: true }); expect(limiter.counts()).toEqual({ RECEIVED: 0, @@ -274,11 +261,11 @@ describe("General", function () { // resolved first (state would jump straight to {DONE:1, EXECUTING:1 // (job 2 — was RUNNING for one microtask), QUEUED:1}). Holding job 1 // with deferredPromise eliminates the race. - let release1; - const hold1 = new Promise(function (resolve) { - release1 = resolve; - }); - h.pNoErrVal(limiter.schedule({ weight: 1, id: 1 }, h.deferredPromise, hold1, null, 1), 1); + const hold1 = deferred(); + h.pNoErrVal( + limiter.schedule({ weight: 1, id: 1 }, h.deferredPromise, hold1.signal, null, 1), + 1, + ); h.pNoErrVal(limiter.schedule({ weight: 1, id: 2 }, h.slowPromise, 200, null, 2), 2); h.pNoErrVal(limiter.schedule({ weight: 2, id: 3 }, h.slowPromise, 100, null, 3), 3); expect(limiter.counts()).toEqual({ @@ -309,7 +296,7 @@ describe("General", function () { expect(limiter.jobStatus(2)).toEqual("RUNNING"); expect(limiter.jobStatus(3)).toEqual("QUEUED"); - release1(); + hold1.release(); await waitForState(function () { const counts = limiter.counts(); @@ -340,12 +327,11 @@ describe("General", function () { EXECUTING: 0, DONE: 4, }); - h.checkResultsOrder([[1], [2], [3]]); + expect(h.log).toHaveCallOrder([[1], [2], [3]]); }); - it("Should return jobs for a status", async function () { - const h = createJobHarness(); - limiter = makeLimiter({ maxConcurrent: 2, minTime: 100, trackDoneStatus: true }); + test("Should return jobs for a status", async function ({ harness: h, makeLimiter }) { + const limiter = makeLimiter({ maxConcurrent: 2, minTime: 100, trackDoneStatus: true }); expect(limiter.counts()).toEqual({ RECEIVED: 0, @@ -361,12 +347,9 @@ describe("General", function () { // races with minTime=100 — the moment job 2 dispatches is the same // instant job 1 finishes, so the {DONE:0, EXECUTING:1, RUNNING:1} // window may not exist depending on microtask order. - let release1; - const hold1 = new Promise(function (resolve) { - release1 = resolve; - }); + const hold1 = deferred(); - limiter.submit({ weight: 1, id: 1 }, h.deferredJob, hold1, null, 1, h.noErrVal(1)); + limiter.submit({ weight: 1, id: 1 }, h.deferredJob, hold1.signal, null, 1, h.noErrVal(1)); h.pNoErrVal(limiter.schedule({ weight: 1, id: 2 }, h.slowPromise, 200, null, 2), 2); h.pNoErrVal(limiter.schedule({ weight: 2, id: 3 }, h.slowPromise, 100, null, 3), 3); expect(limiter.counts()).toEqual({ @@ -400,9 +383,9 @@ describe("General", function () { expect(limiter.jobs("RUNNING")).toEqual(["2"]); expect(limiter.jobs("QUEUED")).toEqual(["3"]); - release1(); + hold1.release(); - // After release1, job 1 transitions to DONE and frees a slot. Job 2 is + // After hold1.release(), job 1 transitions to DONE and frees a slot. Job 2 is // already in RUNNING and immediately moves to EXECUTING. Wait for that // to complete to avoid catching the brief in-between RUNNING=1 state. await waitForState(function () { @@ -432,12 +415,12 @@ describe("General", function () { EXECUTING: 0, DONE: 4, }); - h.checkResultsOrder([[1], [2], [3]]); + expect(h.log).toHaveCallOrder([[1], [2], [3]]); }); - it("Should trigger events on status changes", function () { - const h = createJobHarness(); - limiter = makeLimiter({ maxConcurrent: 2, minTime: 100, trackDoneStatus: true }); + test("Should trigger events on status changes", async function ({ harness: h, makeLimiter }) { + const limiter = makeLimiter({ maxConcurrent: 2, minTime: 100, trackDoneStatus: true }); + await limiter.ready(); let onReceived = 0; let onQueued = 0; let onScheduled = 0; @@ -472,7 +455,11 @@ describe("General", function () { DONE: 0, }); - h.pNoErrVal(limiter.schedule({ weight: 1, id: 1 }, h.slowPromise, 100, null, 1), 1); + const hold1 = deferred(); + h.pNoErrVal( + limiter.schedule({ weight: 1, id: 1 }, h.deferredPromise, hold1.signal, null, 1), + 1, + ); h.pNoErrVal(limiter.schedule({ weight: 1, id: 2 }, h.slowPromise, 200, null, 2), 2); h.pNoErrVal(limiter.schedule({ weight: 2, id: 3 }, h.slowPromise, 100, null, 3), 3); expect(limiter.counts()).toEqual({ @@ -485,71 +472,64 @@ describe("General", function () { expect([onReceived, onQueued, onScheduled, onExecuting, onDone]).toEqual([3, 0, 0, 0, 0]); - // Poll for the dispatched state. Include DONE=0 in the predicate to avoid - // racing the JOB_1 → DONE transition that can happen in the same - // microtask JOB_2 dispatches. - return waitForState(function () { + await waitForState(function () { const counts = limiter.counts(); expect(counts.RECEIVED).toBe(0); expect(counts.QUEUED).toBe(1); expect(counts.RUNNING).toBe(1); expect(counts.EXECUTING).toBe(1); expect(counts.DONE).toBe(0); - }) - .then(function () { - expect(limiter.counts()).toEqual({ - RECEIVED: 0, - QUEUED: 1, - RUNNING: 1, - EXECUTING: 1, - DONE: 0, - }); - expect([onReceived, onQueued, onScheduled, onExecuting, onDone]).toEqual([3, 3, 2, 1, 0]); - - return waitForState(function () { - const counts = limiter.counts(); - expect(counts.RECEIVED).toBe(0); - expect(counts.QUEUED).toBe(1); - expect(counts.RUNNING).toBe(0); - expect(counts.EXECUTING).toBe(1); - expect(counts.DONE).toBe(1); - }); - }) - .then(function () { - expect(limiter.counts()).toEqual({ - RECEIVED: 0, - QUEUED: 1, - RUNNING: 0, - EXECUTING: 1, - DONE: 1, - }); - expect(limiter.jobs("DONE")).toEqual(["1"]); - expect(limiter.jobs("EXECUTING")).toEqual(["2"]); - expect(limiter.jobs("QUEUED")).toEqual(["3"]); - expect([onReceived, onQueued, onScheduled, onExecuting, onDone]).toEqual([3, 3, 2, 2, 1]); + }); - return h.flushLimiter(limiter); - }) - .then(function (_results) { - expect(limiter.counts()).toEqual({ - RECEIVED: 0, - QUEUED: 0, - RUNNING: 0, - EXECUTING: 0, - DONE: 4, - }); - expect([onReceived, onQueued, onScheduled, onExecuting, onDone]).toEqual([4, 4, 4, 4, 4]); - // The real contract is the event count + ordering; total wall-clock - // duration depends on minTime + slowPromise sums plus any Redis RTT - // and is too noisy to bound tightly. - h.checkResultsOrder([[1], [2], [3]]); - }); + expect(limiter.counts()).toEqual({ + RECEIVED: 0, + QUEUED: 1, + RUNNING: 1, + EXECUTING: 1, + DONE: 0, + }); + expect([onReceived, onQueued, onScheduled, onExecuting, onDone]).toEqual([3, 3, 2, 1, 0]); + + hold1.release(); + + await waitForState(function () { + const counts = limiter.counts(); + expect(counts.RECEIVED).toBe(0); + expect(counts.QUEUED).toBe(1); + expect(counts.RUNNING).toBe(0); + expect(counts.EXECUTING).toBe(1); + expect(counts.DONE).toBe(1); + }); + + expect(limiter.counts()).toEqual({ + RECEIVED: 0, + QUEUED: 1, + RUNNING: 0, + EXECUTING: 1, + DONE: 1, + }); + expect(limiter.jobs("DONE")).toEqual(["1"]); + expect(limiter.jobs("EXECUTING")).toEqual(["2"]); + expect(limiter.jobs("QUEUED")).toEqual(["3"]); + expect([onReceived, onQueued, onScheduled, onExecuting, onDone]).toEqual([3, 3, 2, 2, 1]); + + await h.flushLimiter(limiter); + + expect(limiter.counts()).toEqual({ + RECEIVED: 0, + QUEUED: 0, + RUNNING: 0, + EXECUTING: 0, + DONE: 4, + }); + expect([onReceived, onQueued, onScheduled, onExecuting, onDone]).toEqual([4, 4, 4, 4, 4]); + expect(h.log).toHaveCallOrder([[1], [2], [3]]); }); }); describe("Events", function () { - it("Should return itself", function () { - limiter = makeLimiter({ id: "test-limiter" }); + test("Should return itself", function ({ makeLimiter }) { + const limiter = makeLimiter({ id: "test-limiter" }); const returned = limiter.on("ready", function () {}); // The contract is that `.on()` returns the limiter itself for chaining; @@ -558,9 +538,8 @@ describe("General", function () { expect(returned.id).toEqual(limiter.id); }); - it("Should fire events on empty queue", function () { - const h = createJobHarness(); - limiter = makeLimiter({ maxConcurrent: 1, minTime: 100 }); + test("Should fire events on empty queue", function ({ harness: h, makeLimiter }) { + const limiter = makeLimiter({ maxConcurrent: 1, minTime: 100 }); let calledEmpty = 0; let calledIdle = 0; let calledDepleted = 0; @@ -589,8 +568,8 @@ describe("General", function () { return limiter.submit({ id: 4 }, h.slowJob, 50, null, 4, null); }) .then(function () { - h.checkDuration(250); - h.checkResultsOrder([[1], [2], [3]]); + expect(h).toHaveFinalCallAt(250); + expect(h.log).toHaveCallOrder([[1], [2], [3]]); expect(calledEmpty).toEqual(3); expect(calledIdle).toEqual(2); expect(calledDepleted).toEqual(0); @@ -598,9 +577,8 @@ describe("General", function () { }); }); - it("Should fire events once", function () { - const h = createJobHarness(); - limiter = makeLimiter({ maxConcurrent: 1, minTime: 100 }); + test("Should fire events once", function ({ harness: h, makeLimiter }) { + const limiter = makeLimiter({ maxConcurrent: 1, minTime: 100 }); let calledEmptyOnce = 0; let calledIdleOnce = 0; let calledEmpty = 0; @@ -635,8 +613,8 @@ describe("General", function () { return h.pNoErrVal(limiter.schedule(h.promise, null, 3), 3); }) .then(function () { - h.checkDuration(200); - h.checkResultsOrder([[1], [2], [3]]); + expect(h).toHaveFinalCallAt(200); + expect(h.log).toHaveCallOrder([[1], [2], [3]]); expect(calledEmptyOnce).toEqual(1); expect(calledIdleOnce).toEqual(1); expect(calledEmpty).toEqual(2); @@ -645,9 +623,8 @@ describe("General", function () { }); }); - it("Should support faulty event listeners", function () { - const h = createJobHarness(); - limiter = makeLimiter({ maxConcurrent: 1, minTime: 100 }, { expectErrors: true }); + test("Should support faulty event listeners", function ({ harness: h, makeLimiter }) { + const limiter = makeLimiter({ maxConcurrent: 1, minTime: 100 }, { expectErrors: true }); // We only care that the listener-thrown error eventually surfaces on // the "error" event. Counting calls is brittle under Redis-backed runs // because a connectTimeout retry (see test/redis-client-options.js) can @@ -669,9 +646,8 @@ describe("General", function () { }); }); - it("Should wait for async event listeners", function () { - const h = createJobHarness(); - limiter = makeLimiter({ maxConcurrent: 1, minTime: 100 }, { expectErrors: true }); + test("Should wait for async event listeners", function ({ harness: h, makeLimiter }) { + const limiter = makeLimiter({ maxConcurrent: 1, minTime: 100 }, { expectErrors: true }); // Match on the specific error message — under load any unrelated redis // error could fire first; we only care that "It broke!" eventually does. let fired = false; diff --git a/test/global-setup/redis.ts b/test/global-setup/redis.ts index bed2cd9..ac4cd0b 100644 --- a/test/global-setup/redis.ts +++ b/test/global-setup/redis.ts @@ -12,20 +12,45 @@ // Host/port are forwarded via process.env; worker forks inherit them // automatically because they are spawned after this setup function runs. +import type { StartedRedisContainer } from "@testcontainers/redis"; + let stop: (() => Promise) | undefined; +const START_ATTEMPTS = 3; +const RETRY_DELAY_MS = 2_000; + export async function setup(): Promise { const { RedisContainer } = await import("@testcontainers/redis"); - const container = await new RedisContainer("redis:7-alpine") - .withStartupTimeout(30_000) - .withCommand(["redis-server", "--save", "", "--appendonly", "no"]) - .start(); + // testcontainers hardcodes a 10s port-bind-inspection timeout that + // withStartupTimeout cannot override (inspect-container-util-ports-exposed.js), + // so under Docker Desktop churn .start() can time out even with margin to spare. + // Retry with a fresh builder each attempt; containers leaked by a failed + // attempt are reaped by ryuk at session end, and vitest applies no timeout + // to root globalSetup, so the worst-case ~40s here is safe. + let container: StartedRedisContainer | undefined; + for (let attempt = 1; attempt <= START_ATTEMPTS; attempt++) { + try { + container = await new RedisContainer("redis:7-alpine") + .withStartupTimeout(30_000) + .withCommand(["redis-server", "--save", "", "--appendonly", "no"]) + .start(); + break; + } catch (err) { + if (attempt === START_ATTEMPTS) throw err; + console.warn( + `[global-setup] Redis container start failed (attempt ${attempt}/${START_ATTEMPTS}): ${err}; retrying in ${RETRY_DELAY_MS}ms`, + ); + await new Promise((resolve) => setTimeout(resolve, RETRY_DELAY_MS)); + } + } + + const started = container!; - process.env.REDIS_HOST = container.getHost(); - process.env.REDIS_PORT = String(container.getPort()); + process.env.REDIS_HOST = started.getHost(); + process.env.REDIS_PORT = String(started.getPort()); - stop = () => container.stop(); + stop = () => started.stop(); } export async function teardown(): Promise { diff --git a/test/group.test.js b/test/group.test.js index 4374ef7..bac2a8d 100644 --- a/test/group.test.js +++ b/test/group.test.js @@ -1,23 +1,18 @@ -import { describe, it, afterEach, expect } from "vitest"; -import { createJobHarness } from "./helpers/job-tracking.js"; -import { waitForState } from "./helpers/wait-for-state.js"; -const makeLimiter = require("./helpers/limiter"); +import { useFakeClock } from "./helpers/clock.js"; +import { test, describe, expect, waitForState } from "./helpers/test-api.js"; const Bottleneck = require("./bottleneck"); -describe("Group", function () { - let limiter; +useFakeClock(); - afterEach(function () { - return limiter.disconnect(false); - }); - - it("Should create limiters", function () { +describe("Group", () => { + test("Should create limiters", function ({ track }) { expect.hasAssertions(); - limiter = makeLimiter(); - const group = new Bottleneck.Group({ - maxConcurrent: 1, - minTime: 100, - }); + const group = track( + new Bottleneck.Group({ + maxConcurrent: 1, + minTime: 100, + }), + ); const results = []; @@ -66,12 +61,13 @@ describe("Group", function () { }); }); - it("Should set up the limiter IDs (default)", function () { - limiter = makeLimiter(); - const group = new Bottleneck.Group({ - maxConcurrent: 1, - minTime: 100, - }); + test("Should set up the limiter IDs (default)", function ({ track }) { + const group = track( + new Bottleneck.Group({ + maxConcurrent: 1, + minTime: 100, + }), + ); expect(group.key("A").id).toStrictEqual("group-key-A"); expect(group.key("B").id).toStrictEqual("group-key-B"); @@ -85,13 +81,14 @@ describe("Group", function () { expect(ids.sort()).toStrictEqual(["group-key-A", "group-key-B", "group-key-XYZ"]); }); - it("Should set up the limiter IDs (custom)", function () { - limiter = makeLimiter(); - const group = new Bottleneck.Group({ - maxConcurrent: 1, - minTime: 100, - id: "custom-id", - }); + test("Should set up the limiter IDs (custom)", function ({ track }) { + const group = track( + new Bottleneck.Group({ + maxConcurrent: 1, + minTime: 100, + id: "custom-id", + }), + ); expect(group.key("A").id).toStrictEqual("custom-id-A"); expect(group.key("B").id).toStrictEqual("custom-id-B"); @@ -105,12 +102,14 @@ describe("Group", function () { expect(ids.sort()).toStrictEqual(["custom-id-A", "custom-id-B", "custom-id-XYZ"]); }); - it("Should pass new limiter to 'created' event", function () { - limiter = makeLimiter(); - const group = new Bottleneck.Group({ - maxConcurrent: 1, - minTime: 100, - }); + test("Should pass new limiter to 'created' event", function ({ makeLimiter, track }) { + const limiter = makeLimiter(); + const group = track( + new Bottleneck.Group({ + maxConcurrent: 1, + minTime: 100, + }), + ); const keys = []; const ids = []; @@ -139,13 +138,14 @@ describe("Group", function () { }); }); - it("Should pass error on failure", function () { + test("Should pass error on failure", function ({ track }) { const failureMessage = "SOMETHING BLEW UP!!"; - limiter = makeLimiter(); - const group = new Bottleneck.Group({ - maxConcurrent: 1, - minTime: 100, - }); + const group = track( + new Bottleneck.Group({ + maxConcurrent: 1, + minTime: 100, + }), + ); expect(Object.keys(group.limiters)).toStrictEqual([]); const results = []; @@ -186,17 +186,20 @@ describe("Group", function () { }); }); - it("Should update its timeout", function () { - limiter = makeLimiter(); - const group1 = new Bottleneck.Group({ - maxConcurrent: 1, - minTime: 100, - }); - const group2 = new Bottleneck.Group({ - maxConcurrent: 1, - minTime: 100, - timeout: 5000, - }); + test("Should update its timeout", function ({ track }) { + const group1 = track( + new Bottleneck.Group({ + maxConcurrent: 1, + minTime: 100, + }), + ); + const group2 = track( + new Bottleneck.Group({ + maxConcurrent: 1, + minTime: 100, + timeout: 5000, + }), + ); expect(group1.timeout).toStrictEqual(300000); expect(group2.timeout).toStrictEqual(5000); @@ -209,12 +212,13 @@ describe("Group", function () { }); }); - it("Should update its limiter options", function () { - limiter = makeLimiter(); - const group = new Bottleneck.Group({ - maxConcurrent: 1, - minTime: 100, - }); + test("Should update its limiter options", function ({ track }) { + const group = track( + new Bottleneck.Group({ + maxConcurrent: 1, + minTime: 100, + }), + ); const limiter1 = group.key("AAA"); expect(limiter1._store.storeOptions.minTime).toStrictEqual(100); @@ -226,12 +230,12 @@ describe("Group", function () { expect(limiter2._store.storeOptions.minTime).toStrictEqual(200); }); - it("Should support keys(), limiters(), deleteKey()", function () { - const h = createJobHarness(); - limiter = makeLimiter(); - const group1 = new Bottleneck.Group({ - maxConcurrent: 1, - }); + test("Should support keys(), limiters(), deleteKey()", function ({ harness: h, track }) { + const group1 = track( + new Bottleneck.Group({ + maxConcurrent: 1, + }), + ); const KEY_A = "AAA"; const KEY_B = "BBB"; @@ -263,13 +267,15 @@ describe("Group", function () { }); }); - it("Should call autocleanup", function () { + test("Should call autocleanup", function ({ makeLimiter, track }) { const KEY = "test-key"; - const group = new Bottleneck.Group({ - maxConcurrent: 1, - }); + const group = track( + new Bottleneck.Group({ + maxConcurrent: 1, + }), + ); group.updateSettings({ timeout: 500 }); - limiter = makeLimiter({ id: "something", timeout: group.timeout }); + const limiter = makeLimiter({ id: "something", timeout: group.timeout }); group.instances[KEY] = limiter; return group diff --git a/test/helpers/call-log.js b/test/helpers/call-log.js deleted file mode 100644 index 93ef872..0000000 --- a/test/helpers/call-log.js +++ /dev/null @@ -1,80 +0,0 @@ -import { expect } from "vitest"; - -function wait(ms) { - return new Promise(function (resolve) { - setTimeout(resolve, ms); - }); -} - -function pNoErrVal(promise, ...expected) { - return promise.then(function (actual) { - expect(actual).toEqual(expected); - }); -} - -function noErrVal(...expected) { - return function (err, ...actual) { - expect(err).toBeNull(); - expect(actual).toEqual(expected); - }; -} - -/** - * Call history + timing + assertions for integration tests. - * Pair with {@link createTaskFns} from "./job-tasks.js" or use {@link createJobHarness}. - */ -export function createCallLog() { - const start = Date.now(); - const calls = []; - - function record(err, result) { - calls.push({ err: err, result: result, time: Date.now() - start }); - } - - function getResults() { - return { - elapsed: Date.now() - start, - callsDuration: calls.length > 0 ? calls.at(-1).time : null, - calls: calls, - }; - } - - function checkResultsOrder(order) { - expect(order.length).toBe(calls.length); - for (let i = 0; i < calls.length; i++) { - expect(calls[i].result).toEqual(order[i]); - } - } - - function checkDuration(shouldBe, minBound, maxBound) { - const lo = minBound !== undefined ? minBound : 10; - const hi = maxBound !== undefined ? maxBound : 1000; - const results = getResults(); - const min = shouldBe - lo; - const max = shouldBe + hi; - expect(results.callsDuration).toBeGreaterThan(min); - expect(results.callsDuration).toBeLessThan(max); - } - - return { - record: record, - getResults: getResults, - results: getResults, - checkResultsOrder: checkResultsOrder, - checkDuration: checkDuration, - pNoErrVal: pNoErrVal, - noErrVal: noErrVal, - wait: wait, - calls: calls, - }; -} - -/** - * Schedule a barrier job and resolve with {@link createCallLog}'s snapshot (same as legacy `c.last()`). - */ -export function flushLimiter(limiter, getResults, scheduleOptions) { - const opt = scheduleOptions != null ? scheduleOptions : {}; - return limiter.schedule(opt, function () { - return Promise.resolve(getResults()); - }); -} diff --git a/test/helpers/clock.js b/test/helpers/clock.js new file mode 100644 index 0000000..2675e85 --- /dev/null +++ b/test/helpers/clock.js @@ -0,0 +1,46 @@ +import { beforeEach, afterEach, vi } from "vitest"; + +/** Real setTimeout captured at module load, before any fake-timer install. */ +export const realSetTimeout = globalThis.setTimeout; + +/** Ground truth from vitest — never track this in module state. */ +export function isFakeClock() { + return vi.isFakeTimers(); +} + +/** + * Install vitest fake timers for the `local` project (no-op when DATASTORE is + * set — never freeze time while real Redis I/O is in flight, and never fake + * timers in a fork holding a long-lived redis client whose reconnect timers + * would be discarded by useRealTimers()). + * + * Must be called at module top level before any `describe()` so hook-stack ordering + * matches setup.ts's redis flush (module beforeEach runs after setup beforeEach). + * + * Do NOT use `vi.waitFor` or `expect.poll` directly in shared test files — their sync + * auto-advance corrupts the virtual timeline under fake time. Use `waitForState` instead. + */ +export function useFakeClock() { + if (process.env.DATASTORE != null) return; + + beforeEach(() => { + vi.useFakeTimers(); + vi.setTimerTickMode("nextTimerAsync"); + }); + + afterEach(() => { + vi.useRealTimers(); + }); +} + +/** Per-test opt-out from fake timers; next test's beforeEach reinstalls. */ +export function useRealClockForThisTest() { + vi.useRealTimers(); +} + +/** Promise delay via global setTimeout (respects fake timers when installed). */ +export function wait(ms) { + return new Promise((resolve) => { + setTimeout(resolve, ms); + }); +} diff --git a/test/helpers/job-tasks.js b/test/helpers/job-tasks.js index aeb26cd..0e7639a 100644 --- a/test/helpers/job-tasks.js +++ b/test/helpers/job-tasks.js @@ -1,6 +1,22 @@ +/** + * Manually-released signal for {@link createTaskFns}'s deferredJob/deferredPromise. + * + * const d = deferred(); + * limiter.schedule(h.deferredPromise, d.signal, null, 1); + * ...observe held state... + * d.release(); + */ +export function deferred() { + let release; + const signal = new Promise((resolve) => { + release = resolve; + }); + return { signal: signal, release: release }; +} + /** * Bottleneck task functions that record into `log.record(err, result)`. - * Expects `log` from {@link createCallLog} in "./call-log.js". + * Expects a `log` with a `record(err, result)` method. */ export function createTaskFns(log) { function job(err, ...result) { @@ -11,7 +27,7 @@ export function createTaskFns(log) { function slowJob(duration, err, ...result) { const cb = result.pop(); - setTimeout(function () { + setTimeout(() => { log.record(err, result); cb.apply(null, [err].concat(result)); }, duration); @@ -19,14 +35,14 @@ export function createTaskFns(log) { function deferredJob(signal, err, ...result) { const cb = result.pop(); - signal.then(function () { + signal.then(() => { log.record(err, result); cb.apply(null, [err].concat(result)); }); } function promise(err, ...result) { - return new Promise(function (resolve, reject) { + return new Promise((resolve, reject) => { log.record(err, result); if (err === null) { return resolve(result); @@ -36,7 +52,7 @@ export function createTaskFns(log) { } function slowPromise(duration, err, ...result) { - return new Promise(function (resolve, reject) { + return new Promise((resolve, reject) => { setTimeout(function () { log.record(err, result); if (err === null) { @@ -48,7 +64,7 @@ export function createTaskFns(log) { } function deferredPromise(signal, err, ...result) { - return new Promise(function (resolve, reject) { + return new Promise((resolve, reject) => { signal.then(function () { log.record(err, result); if (err === null) { diff --git a/test/helpers/job-tracking.js b/test/helpers/job-tracking.js deleted file mode 100644 index 6e053e2..0000000 --- a/test/helpers/job-tracking.js +++ /dev/null @@ -1,34 +0,0 @@ -import { createCallLog, flushLimiter as scheduleFlush } from "./call-log.js"; -import { createTaskFns } from "./job-tasks.js"; - -export { createCallLog, flushLimiter } from "./call-log.js"; -export { createTaskFns } from "./job-tasks.js"; - -/** - * Default test helper: {@link createCallLog} + {@link createTaskFns} + `flushLimiter` bound to this log. - * For low-level use, import `createCallLog` and `createTaskFns` separately. - */ -export function createJobHarness() { - const log = createCallLog(); - const tasks = createTaskFns(log); - - return { - job: tasks.job, - slowJob: tasks.slowJob, - deferredJob: tasks.deferredJob, - promise: tasks.promise, - slowPromise: tasks.slowPromise, - deferredPromise: tasks.deferredPromise, - getResults: log.getResults, - results: log.results, - flushLimiter: function (limiter, scheduleOptions) { - return scheduleFlush(limiter, log.getResults, scheduleOptions); - }, - checkResultsOrder: log.checkResultsOrder, - checkDuration: log.checkDuration, - pNoErrVal: log.pNoErrVal, - noErrVal: log.noErrVal, - wait: log.wait, - calls: log.calls, - }; -} diff --git a/test/helpers/test-api.js b/test/helpers/test-api.js new file mode 100644 index 0000000..0ebc658 --- /dev/null +++ b/test/helpers/test-api.js @@ -0,0 +1,181 @@ +import { test as baseTest, expect as vitestExpect, vi } from "vitest"; +import { wait, isFakeClock } from "./clock.js"; +import { createTaskFns } from "./job-tasks.js"; +import makeLimiterHelper from "./limiter.js"; + +export { waitForState } from "./wait-for-state.js"; +export { deferred } from "./job-tasks.js"; + +function pNoErrVal(promise, ...expected) { + return promise.then((actual) => { + vitestExpect(actual).toEqual(expected); + }); +} + +function noErrVal(...expected) { + return function (err, ...actual) { + vitestExpect(err).toBeNull(); + vitestExpect(actual).toEqual(expected); + }; +} + +function createJobHarness() { + const start = Date.now(); + const callTimes = []; + + const record = vi.fn((_err, _result) => { + callTimes.push(Date.now() - start); + }); + + const log = { + record: record, + calls: record.mock.calls, + }; + + const tasks = createTaskFns(log); + + function getResults() { + return { + elapsed: Date.now() - start, + callsDuration: callTimes.length > 0 ? callTimes.at(-1) : null, + calls: record.mock.calls.map((call, i) => { + return { err: call[0], result: call[1], time: callTimes[i] }; + }), + }; + } + + function flushLimiter(limiter, scheduleOptions) { + const opt = scheduleOptions != null ? scheduleOptions : {}; + return limiter.schedule(opt, () => { + return Promise.resolve(getResults()); + }); + } + + return { + log: record, + job: tasks.job, + slowJob: tasks.slowJob, + deferredJob: tasks.deferredJob, + promise: tasks.promise, + slowPromise: tasks.slowPromise, + deferredPromise: tasks.deferredPromise, + getResults: getResults, + results: getResults, + flushLimiter: flushLimiter, + pNoErrVal: pNoErrVal, + noErrVal: noErrVal, + wait: wait, + callTimes: callTimes, + }; +} + +function callResultArgs(call) { + const result = call[1]; + return Array.isArray(result) ? result : [result]; +} + +vitestExpect.extend({ + toHaveCallOrder(received, expected) { + const calls = received.mock.calls; + const pass = + calls.length === expected.length && + expected.every((order, i) => this.equals(callResultArgs(calls[i]), order)); + + const message = () => + pass + ? "expected call order not to match" + : `expected call order ${this.utils.printExpected(expected)}, got ${this.utils.printReceived(calls.map(callResultArgs))}`; + + return { pass, message }; + }, + + toHaveFinalCallAt(received, expectedMs, minBound) { + const lo = minBound !== undefined ? minBound : 10; + const duration = + received.callTimes != null ? received.callTimes.at(-1) : received.getResults().callsDuration; + + if (isFakeClock()) { + const pass = duration === expectedMs; + const message = () => + pass + ? `expected final call not to be at exactly ${expectedMs}ms` + : `expected final call at exactly ${expectedMs}ms, got ${duration}ms`; + return { pass, message }; + } + + const min = expectedMs - lo; + const pass = duration != null && duration > min; + const message = () => + pass + ? `expected final call not to be after ${min}ms` + : `expected final call after ${min}ms, got ${duration}ms`; + + return { pass, message }; + }, + + toHaveCallAt(received, index, expectedMs, minBound) { + const lo = minBound !== undefined ? minBound : 5; + const time = received.calls != null ? received.calls[index]?.time : received.callTimes?.[index]; + + if (isFakeClock()) { + // Accepts N or exactly N+1: sinon fake-timers assigns callAt = now + 1 + // to a 0ms timer created INSIDE a running timer callback ("duringTick" + // quantization), which every heartbeat-driven dispatch hits via + // LocalDatastore's heartbeat setInterval -> yieldLoop() setTimeout(0). + // Interval-driven calls therefore land at exactly N+1; direct minTime + // dispatches land at exactly N. Never N-1, never N+2. + const pass = time === expectedMs || time === expectedMs + 1; + const message = () => + pass + ? `expected call ${index} not to be at ${expectedMs}ms or ${expectedMs + 1}ms` + : `expected call ${index} at ${expectedMs}ms (or +1ms fake-timer quantization), got ${time}ms`; + + return { pass, message }; + } + + const min = expectedMs - lo; + const pass = time != null && time >= min; + const message = () => + pass + ? `expected call ${index} not to be after ${min}ms` + : `expected call ${index} after ${min}ms, got ${time}ms`; + + return { pass, message }; + }, +}); + +export const test = baseTest.extend({ + // Vitest requires fixture functions to destructure their first argument — + // it parses the pattern to build the dependency graph — so the empty + // pattern is mandatory for dependency-free fixtures. + // oxlint-disable-next-line no-empty-pattern + harness: async function ({}, use) { + await use(createJobHarness()); + }, + limiterOptions: {}, + limiterMeta: {}, + // oxlint-disable-next-line no-empty-pattern + track: async function ({}, use) { + const resources = []; + await use((resource) => { + resources.push(resource); + return resource; + }); + for (let i = resources.length - 1; i >= 0; i--) { + try { + await resources[i].disconnect(false); + } catch { + // tolerate mid-test disconnects + } + } + }, + makeLimiter: async function ({ track }, use) { + await use((opts, meta) => track(makeLimiterHelper(opts, meta))); + }, + limiter: async function ({ makeLimiter, limiterOptions, limiterMeta }, use) { + await use(makeLimiter(limiterOptions, limiterMeta)); + }, +}); + +export const expect = vitestExpect; +export { describe, vi } from "vitest"; diff --git a/test/helpers/wait-for-state.js b/test/helpers/wait-for-state.js index 76e4b23..61581f7 100644 --- a/test/helpers/wait-for-state.js +++ b/test/helpers/wait-for-state.js @@ -1,19 +1,76 @@ import { vi } from "vitest"; +import { realSetTimeout } from "./clock.js"; /** - * Thin wrapper around `vi.waitFor` with project-tuned defaults. + * Poll until `callback` succeeds (does not throw). * - * Bottleneck tests poll for transient state windows (state machine transitions - * gated by `minTime`, redis pub/sub round-trips, etc.) that can be much narrower - * than `vi.waitFor`'s 50ms default polling interval. Defaulting `interval: 10` - * keeps polling tight enough to observe those windows reliably; `timeout: 2000` - * matches the previous handcrafted helper. + * Real mode: delegates to `vi.waitFor`. * - * Same shape as `vi.waitFor`: pass an `options` object to override either - * default for a specific call site. + * Fake mode: steps the clock one timer batch at a time between predicate + * evaluations. Stepping (rather than passively yielding to the nextTimerAsync + * auto-tick) is load-bearing: our next advance is issued from a microtask, + * which beats the auto-tick pump's next macroturn — so the predicate observes + * the state after EVERY timer batch. A passive real-macrotask yield loses that + * race and lets the pump fire several timers between polls, skipping the + * transient windows callers poll for. + * + * Known limits (inherent to vitest's API): + * - advanceTimersToNextTimerAsync fires ALL timers due at the same virtual + * instant atomically — a window that exists only between same-instant timers + * is unobservable. Don't write predicates that need one. + * - An async predicate that itself awaits fake timers (e.g. limiter.running()) + * parks on real macrotasks, during which the auto-tick pump may fire timers. + * Poll stable held states (deferred-hold pattern), not racing ones. + * + * Redis-backed projects get a larger default timeout: cross-instance state + * propagates over real network round-trips, and the documented Docker-proxy + * stall can shift an entire dispatch chain by ~5000ms (see + * test/redis-client-options.js) — the window must absorb one full stall with + * margin while staying below the redis projects' 15s testTimeout so the poll's + * assertion diff (not vitest's opaque timeout) reports the failure. */ -const DEFAULTS = { timeout: 2000, interval: 10 }; +const DEFAULTS = { + timeout: process.env.DATASTORE != null ? 10_000 : 2000, + interval: 10, +}; + +// Under fake time, yield a real macrotask after this many consecutive timer +// advances. A predicate that keeps failing over a dense repeating interval +// (heartbeats every 75ms across an unbounded virtual span) would otherwise +// monopolize the CPU for the entire real-time deadline. +const ADVANCES_PER_YIELD = 50; export function waitForState(callback, options) { - return vi.waitFor(callback, { ...DEFAULTS, ...options }); + const opts = { ...DEFAULTS, ...options }; + + if (!vi.isFakeTimers()) { + return vi.waitFor(callback, opts); + } + + return pollUnderFakeClock(callback, opts); +} + +async function pollUnderFakeClock(callback, opts) { + // Deadline on the real clock: Date/performance/hrtime are all faked. + const deadline = vi.getRealSystemTime() + opts.timeout; + let advances = 0; + + for (;;) { + try { + return await callback(); + } catch (err) { + // Throw the freshest error so the failure shows the final state. + if (vi.getRealSystemTime() >= deadline) { + throw err; + } + } + + if (vi.getTimerCount() > 0 && ++advances % ADVANCES_PER_YIELD !== 0) { + await vi.advanceTimersToNextTimerAsync(); + } else { + await new Promise((resolve) => { + realSetTimeout(resolve, opts.interval); + }); + } + } } diff --git a/test/ioredis.test.js b/test/ioredis.test.js index ca324ca..8d3f8e8 100644 --- a/test/ioredis.test.js +++ b/test/ioredis.test.js @@ -1,22 +1,11 @@ -import { describe, it, afterEach, expect } from "vitest"; -import { createJobHarness } from "./helpers/job-tracking.js"; -const makeLimiter = require("./helpers/limiter"); +import { test, describe, expect } from "./helpers/test-api.js"; const Bottleneck = require("./bottleneck"); const Redis = require("ioredis"); const buildClientOptions = require("./redis-client-options"); -describe("ioredis-only", function () { - if (process.env.DATASTORE !== "ioredis") { - throw new Error("DATASTORE must be ioredis"); - } - let limiter; - - afterEach(function () { - return limiter.disconnect(false); - }); - - it("Should accept ioredis lib override", function () { - limiter = makeLimiter({ +describe("ioredis-only", () => { + test("Should accept ioredis lib override", function ({ makeLimiter }) { + const limiter = makeLimiter({ maxConcurrent: 2, Redis, clientOptions: {}, @@ -31,8 +20,8 @@ describe("ioredis-only", function () { expect(limiter.datastore).toStrictEqual("ioredis"); }); - it("Should connect in Redis Cluster mode", function () { - limiter = makeLimiter({ + test("Should connect in Redis Cluster mode", function ({ makeLimiter }) { + const limiter = makeLimiter({ maxConcurrent: 2, clientOptions: {}, clusterNodes: [ @@ -47,10 +36,13 @@ describe("ioredis-only", function () { expect(limiter._store.connection.client.nodes().length).toBeGreaterThanOrEqual(0); }); - it("Should connect in Redis Cluster mode with premade client", function () { + test("Should connect in Redis Cluster mode with premade client", function ({ + makeLimiter, + track, + }) { const client = new Redis.Cluster(""); - const connection = new Bottleneck.IORedisConnection({ client }); - limiter = makeLimiter({ + track(new Bottleneck.IORedisConnection({ client })); + const limiter = makeLimiter({ maxConcurrent: 2, clientOptions: {}, clusterNodes: [ @@ -63,17 +55,17 @@ describe("ioredis-only", function () { expect(limiter.datastore).toStrictEqual("ioredis"); expect(limiter._store.connection.client.nodes().length).toBeGreaterThanOrEqual(0); - connection.disconnect(false); }); - it("Should accept existing connections", function () { - const h = createJobHarness(); - const connection = new Bottleneck.IORedisConnection({ - Redis, - clientOptions: buildClientOptions("ioredis"), - }); + test("Should accept existing connections", function ({ harness: h, makeLimiter, track }) { + const connection = track( + new Bottleneck.IORedisConnection({ + Redis, + clientOptions: buildClientOptions("ioredis"), + }), + ); connection.id = "super-connection"; - limiter = makeLimiter({ + const limiter = makeLimiter({ minTime: 50, connection, }); @@ -84,8 +76,8 @@ describe("ioredis-only", function () { return h .flushLimiter(limiter) .then(function (_results) { - h.checkResultsOrder([[1], [2]]); - h.checkDuration(50); + expect(h.log).toHaveCallOrder([[1], [2]]); + expect(h).toHaveFinalCallAt(50); expect(limiter.connection.id).toStrictEqual("super-connection"); expect(limiter.datastore).toStrictEqual("ioredis"); @@ -93,18 +85,16 @@ describe("ioredis-only", function () { }) .then(function () { expect(limiter.clients().client.status).toStrictEqual("ready"); - return connection.disconnect(); }); }); - it("Should accept existing redis clients", function () { - const h = createJobHarness(); + test("Should accept existing redis clients", function ({ harness: h, makeLimiter, track }) { const client = new Redis(buildClientOptions("ioredis")); client.id = "super-client"; - const connection = new Bottleneck.IORedisConnection({ client }); + const connection = track(new Bottleneck.IORedisConnection({ client })); connection.id = "super-connection"; - limiter = makeLimiter({ + const limiter = makeLimiter({ minTime: 50, connection, }); @@ -115,8 +105,8 @@ describe("ioredis-only", function () { return h .flushLimiter(limiter) .then(function (_results) { - h.checkResultsOrder([[1], [2]]); - h.checkDuration(50); + expect(h.log).toHaveCallOrder([[1], [2]]); + expect(h).toHaveFinalCallAt(50); expect(limiter.clients().client.id).toStrictEqual("super-client"); expect(limiter.connection.id).toStrictEqual("super-connection"); expect(limiter.datastore).toStrictEqual("ioredis"); @@ -125,21 +115,22 @@ describe("ioredis-only", function () { }) .then(function () { expect(limiter.clients().client.status).toStrictEqual("ready"); - return connection.disconnect(); }); }); - it("Should trigger error events on the shared connection", function () { + test("Should trigger error events on the shared connection", function ({ makeLimiter, track }) { expect.hasAssertions(); return new Promise(function (resolve, reject) { - const connection = new Bottleneck.IORedisConnection({ - Redis, - clientOptions: { - port: 1, - }, - }); + const connection = track( + new Bottleneck.IORedisConnection({ + Redis, + clientOptions: { + port: 1, + }, + }), + ); let fired = false; - limiter = makeLimiter({ connection }); + const limiter = makeLimiter({ connection }); connection.on("error", function (_err) { if (fired) return; fired = true; diff --git a/test/node_redis.test.js b/test/node_redis.test.js index ff11096..675c4c7 100644 --- a/test/node_redis.test.js +++ b/test/node_redis.test.js @@ -1,22 +1,11 @@ -import { describe, it, afterEach, expect } from "vitest"; -import { createJobHarness } from "./helpers/job-tracking.js"; -const makeLimiter = require("./helpers/limiter"); +import { test, describe, expect } from "./helpers/test-api.js"; const Bottleneck = require("./bottleneck"); const Redis = require("redis"); const buildClientOptions = require("./redis-client-options"); -describe("node_redis-only", function () { - if (process.env.DATASTORE !== "redis") { - throw new Error("DATASTORE must be redis"); - } - let limiter; - - afterEach(function () { - return limiter.disconnect(false); - }); - - it("Should accept node_redis lib override", function () { - limiter = makeLimiter({ +describe("node_redis-only", () => { + test("Should accept node_redis lib override", function ({ makeLimiter }) { + const limiter = makeLimiter({ maxConcurrent: 2, Redis, }); @@ -24,14 +13,15 @@ describe("node_redis-only", function () { expect(limiter.datastore).toStrictEqual("redis"); }); - it("Should accept existing connections", function () { - const h = createJobHarness(); - const connection = new Bottleneck.RedisConnection({ - Redis, - clientOptions: buildClientOptions("redis"), - }); + test("Should accept existing connections", function ({ harness: h, makeLimiter, track }) { + const connection = track( + new Bottleneck.RedisConnection({ + Redis, + clientOptions: buildClientOptions("redis"), + }), + ); connection.id = "super-connection"; - limiter = makeLimiter({ + const limiter = makeLimiter({ minTime: 50, connection, }); @@ -42,8 +32,8 @@ describe("node_redis-only", function () { return h .flushLimiter(limiter) .then(function (_results) { - h.checkResultsOrder([[1], [2]]); - h.checkDuration(50); + expect(h.log).toHaveCallOrder([[1], [2]]); + expect(h).toHaveFinalCallAt(50); expect(limiter.connection.id).toStrictEqual("super-connection"); expect(limiter.datastore).toStrictEqual("redis"); @@ -51,19 +41,17 @@ describe("node_redis-only", function () { }) .then(function () { expect(limiter.clients().client.isReady).toStrictEqual(true); - return connection.disconnect(); }); }); - it("Should accept existing redis clients", async function () { - const h = createJobHarness(); + test("Should accept existing redis clients", async function ({ harness: h, makeLimiter, track }) { const client = Redis.createClient(buildClientOptions("redis")); client.id = "super-client"; await client.connect(); - const connection = new Bottleneck.RedisConnection({ client }); + const connection = track(new Bottleneck.RedisConnection({ client })); connection.id = "super-connection"; - limiter = makeLimiter({ + const limiter = makeLimiter({ minTime: 50, connection, }); @@ -74,8 +62,8 @@ describe("node_redis-only", function () { return h .flushLimiter(limiter) .then(function (_results) { - h.checkResultsOrder([[1], [2]]); - h.checkDuration(50); + expect(h.log).toHaveCallOrder([[1], [2]]); + expect(h).toHaveFinalCallAt(50); expect(limiter.clients().client.id).toStrictEqual("super-client"); expect(limiter.connection.id).toStrictEqual("super-connection"); expect(limiter.datastore).toStrictEqual("redis"); @@ -84,25 +72,26 @@ describe("node_redis-only", function () { }) .then(function () { expect(limiter.clients().client.isReady).toStrictEqual(true); - return connection.disconnect(); }); }); - it("Should trigger error events on the shared connection", function () { + test("Should trigger error events on the shared connection", function ({ makeLimiter, track }) { expect.hasAssertions(); return new Promise(function (resolve, reject) { - const connection = new Bottleneck.RedisConnection({ - Redis, - clientOptions: { - socket: { - port: 1, - reconnectStrategy: () => false, + const connection = track( + new Bottleneck.RedisConnection({ + Redis, + clientOptions: { + socket: { + port: 1, + reconnectStrategy: () => false, + }, }, - }, - }); + }), + ); connection.ready.catch(() => {}); let fired = false; - limiter = makeLimiter({ connection }, { expectErrors: true }); + const limiter = makeLimiter({ connection }, { expectErrors: true }); connection.on("error", function (_err) { if (fired) return; fired = true; diff --git a/test/priority.test.js b/test/priority.test.js index 7032e42..9d72808 100644 --- a/test/priority.test.js +++ b/test/priority.test.js @@ -1,19 +1,12 @@ -import { describe, it, afterEach, expect } from "vitest"; -import { createJobHarness } from "./helpers/job-tracking.js"; -import { waitForState } from "./helpers/wait-for-state.js"; -const makeLimiter = require("./helpers/limiter"); +import { useFakeClock, isFakeClock } from "./helpers/clock.js"; +import { test, describe, expect, waitForState, deferred } from "./helpers/test-api.js"; const Bottleneck = require("./bottleneck"); -describe("Priority", function () { - let limiter; +useFakeClock(); - afterEach(function () { - return limiter.disconnect(false); - }); - - it("Should do basic ordering", function () { - const h = createJobHarness(); - limiter = makeLimiter({ maxConcurrent: 1, minTime: 100, rejectOnDrop: false }); +describe("Priority", () => { + test("Should do basic ordering", function ({ harness: h, makeLimiter }) { + const limiter = makeLimiter({ maxConcurrent: 1, minTime: 100, rejectOnDrop: false }); return Promise.all([ h.pNoErrVal(limiter.schedule(h.slowPromise, 50, null, 1), 1), @@ -26,14 +19,13 @@ describe("Priority", function () { return h.flushLimiter(limiter); }) .then(function (_results) { - h.checkResultsOrder([[1], [5, 6], [2], [3], [4]]); - h.checkDuration(400); + expect(h.log).toHaveCallOrder([[1], [5, 6], [2], [3], [4]]); + expect(h).toHaveFinalCallAt(400); }); }); - it("Should support LEAK", async function () { - const h = createJobHarness(); - limiter = makeLimiter({ + test("Should support LEAK", async function ({ harness: h, makeLimiter }) { + const limiter = makeLimiter({ maxConcurrent: 1, minTime: 100, highWater: 3, @@ -49,13 +41,10 @@ describe("Priority", function () { called = true; }); - let releaseFirst; - const firstSignal = new Promise(function (r) { - releaseFirst = r; - }); + const first = deferred(); const subs = [ - limiter.submit(h.deferredJob, firstSignal, null, 1, h.noErrVal(1)), + limiter.submit(h.deferredJob, first.signal, null, 1, h.noErrVal(1)), limiter.submit(h.job, null, 2, h.noErrVal(2)), limiter.submit(h.job, null, 3, h.noErrVal(3)), limiter.submit(h.job, null, 4, h.noErrVal(4)), @@ -64,17 +53,16 @@ describe("Priority", function () { limiter.submit({ priority: 9 }, h.job, null, 7, h.noErrVal(7)), ]; await Promise.all(subs); - releaseFirst(); + first.release(); return h.flushLimiter(limiter, { weight: 0 }).then(function (_results) { - h.checkResultsOrder([[1], [6], [5]]); + expect(h.log).toHaveCallOrder([[1], [6], [5]]); expect(called).toEqual(true); }); }); - it("Should support OVERFLOW", async function () { - const h = createJobHarness(); - limiter = makeLimiter({ + test("Should support OVERFLOW", async function ({ harness: h, makeLimiter }) { + const limiter = makeLimiter({ maxConcurrent: 1, minTime: 100, highWater: 2, @@ -89,13 +77,10 @@ describe("Priority", function () { called = true; }); - let releaseFirst; - const firstSignal = new Promise(function (r) { - releaseFirst = r; - }); + const first = deferred(); const subs = [ - limiter.submit(h.deferredJob, firstSignal, null, 1, h.noErrVal(1)), + limiter.submit(h.deferredJob, first.signal, null, 1, h.noErrVal(1)), limiter.submit(h.job, null, 2, h.noErrVal(2)), limiter.submit(h.job, null, 3, h.noErrVal(3)), limiter.submit(h.job, null, 4, h.noErrVal(4)), @@ -104,7 +89,7 @@ describe("Priority", function () { limiter.submit({ priority: 9 }, h.job, null, 7, h.noErrVal(7)), ]; await Promise.all(subs); - releaseFirst(); + first.release(); return limiter .updateSettings({ highWater: null }) @@ -112,14 +97,13 @@ describe("Priority", function () { return h.flushLimiter(limiter); }) .then(function (_results) { - h.checkResultsOrder([[1], [2], [3]]); + expect(h.log).toHaveCallOrder([[1], [2], [3]]); expect(called).toEqual(true); }); }); - it("Should support OVERFLOW_PRIORITY", async function () { - const h = createJobHarness(); - limiter = makeLimiter({ + test("Should support OVERFLOW_PRIORITY", async function ({ harness: h, makeLimiter }) { + const limiter = makeLimiter({ maxConcurrent: 1, minTime: 100, highWater: 2, @@ -134,13 +118,10 @@ describe("Priority", function () { called = true; }); - let releaseFirst; - const firstSignal = new Promise(function (r) { - releaseFirst = r; - }); + const first = deferred(); const subs = [ - limiter.submit(h.deferredJob, firstSignal, null, 1, h.noErrVal(1)), + limiter.submit(h.deferredJob, first.signal, null, 1, h.noErrVal(1)), limiter.submit(h.job, null, 2, h.noErrVal(2)), limiter.submit(h.job, null, 3, h.noErrVal(3)), limiter.submit(h.job, null, 4, h.noErrVal(4)), @@ -149,7 +130,7 @@ describe("Priority", function () { limiter.submit({ priority: 2 }, h.job, null, 7, h.noErrVal(7)), ]; await Promise.all(subs); - releaseFirst(); + first.release(); return limiter .updateSettings({ highWater: null }) @@ -157,15 +138,14 @@ describe("Priority", function () { return h.flushLimiter(limiter); }) .then(function (_results) { - h.checkResultsOrder([[1], [5], [6]]); + expect(h.log).toHaveCallOrder([[1], [5], [6]]); expect(called).toEqual(true); }); }); - it("Should support BLOCK", function () { + test("Should support BLOCK", function ({ harness: h, makeLimiter }) { expect.hasAssertions(); - const h = createJobHarness(); - limiter = makeLimiter({ + const limiter = makeLimiter({ maxConcurrent: 1, minTime: 100, highWater: 2, @@ -175,10 +155,7 @@ describe("Priority", function () { let called = 0; return new Promise(function (resolve) { - let releaseFirst; - const firstSignal = new Promise(function (r) { - releaseFirst = r; - }); + const first = deferred(); limiter.on("dropped", function (dropped) { expect(dropped.task).toBeTruthy(); @@ -195,43 +172,43 @@ describe("Priority", function () { expect(err).toBeInstanceOf(Bottleneck.BottleneckError); expect(err.message).toEqual("This job has been dropped by Bottleneck"); limiter.removeAllListeners("error"); - releaseFirst(); + first.release(); resolve(); }); } }); - limiter.submit(h.deferredJob, firstSignal, null, 1, h.noErrVal(1)); + limiter.submit(h.deferredJob, first.signal, null, 1, h.noErrVal(1)); limiter.submit(h.slowJob, 20, null, 2, (err) => expect(err).toBeTruthy()); limiter.submit(h.slowJob, 20, null, 3, (err) => expect(err).toBeTruthy()); limiter.submit(h.slowJob, 20, null, 4, (err) => expect(err).toBeTruthy()); }); }); - it("Should have the right priority", async function () { - const h = createJobHarness(); - limiter = makeLimiter({ maxConcurrent: 1, minTime: 100 }); + test("Should have the right priority", async function ({ harness: h, makeLimiter }) { + const limiter = makeLimiter({ maxConcurrent: 1, minTime: 100 }); let committed = 0; limiter.on("queued", function () { committed++; }); - let releaseFirst; - const firstSignal = new Promise(function (r) { - releaseFirst = r; - }); - h.pNoErrVal(limiter.schedule({ priority: 6 }, h.deferredPromise, firstSignal, null, 1), 1); + const first = deferred(); + h.pNoErrVal(limiter.schedule({ priority: 6 }, h.deferredPromise, first.signal, null, 1), 1); h.pNoErrVal(limiter.schedule({ priority: 5 }, h.promise, null, 2), 2); h.pNoErrVal(limiter.schedule({ priority: 4 }, h.promise, null, 3), 3); h.pNoErrVal(limiter.schedule({ priority: 3 }, h.promise, null, 4), 4); await waitForState(() => { expect(committed).toBe(4); }); - releaseFirst(); + first.release(); return h.flushLimiter(limiter).then(function (_results) { - expect(h.results().elapsed).toBeGreaterThanOrEqual(295); - h.checkResultsOrder([[1], [4], [3], [2]]); + if (isFakeClock()) { + expect(h.results().elapsed).toBe(400); + } else { + expect(h.results().elapsed).toBeGreaterThanOrEqual(295); + } + expect(h.log).toHaveCallOrder([[1], [4], [3], [2]]); }); }); }); diff --git a/test/promises.test.js b/test/promises.test.js index ed96a0d..5c41089 100644 --- a/test/promises.test.js +++ b/test/promises.test.js @@ -1,19 +1,12 @@ -import { describe, it, afterEach, expect } from "vitest"; -import { createJobHarness } from "./helpers/job-tracking.js"; -import { waitForState } from "./helpers/wait-for-state.js"; -const makeLimiter = require("./helpers/limiter"); +import { useFakeClock } from "./helpers/clock.js"; +import { test, describe, expect, waitForState, deferred } from "./helpers/test-api.js"; const Bottleneck = require("./bottleneck"); -describe("Promises", function () { - let limiter; +useFakeClock(); - afterEach(function () { - return limiter.disconnect(false); - }); - - it("Should support promises", function () { - const h = createJobHarness(); - limiter = makeLimiter({ maxConcurrent: 1, minTime: 100 }); +describe("Promises", () => { + test("Should support promises", function ({ harness: h, makeLimiter }) { + const limiter = makeLimiter({ maxConcurrent: 1, minTime: 100 }); limiter.submit(h.job, null, 1, 9, h.noErrVal(1, 9)); limiter.submit(h.job, null, 2, h.noErrVal(2)); @@ -21,23 +14,22 @@ describe("Promises", function () { h.pNoErrVal(limiter.schedule(h.promise, null, 4, 5), 4, 5); return h.flushLimiter(limiter).then(function (_results) { - h.checkResultsOrder([[1, 9], [2], [3], [4, 5]]); - h.checkDuration(300); + expect(h.log).toHaveCallOrder([[1, 9], [2], [3], [4, 5]]); + expect(h).toHaveFinalCallAt(300); }); }); - it("Should pass error on failure", function () { - const h = createJobHarness(); + test("Should pass error on failure", function ({ harness: h, makeLimiter }) { const failureMessage = "failed"; - limiter = makeLimiter({ maxConcurrent: 1, minTime: 100 }); + const limiter = makeLimiter({ maxConcurrent: 1, minTime: 100 }); return limiter.schedule(h.promise, new Error(failureMessage)).catch(function (err) { expect(err.message).toEqual(failureMessage); }); }); - it("Should allow non-Promise returns", function () { - limiter = makeLimiter(); + test("Should allow non-Promise returns", function ({ makeLimiter }) { + const limiter = makeLimiter(); const str = "This is a string"; return limiter @@ -47,9 +39,8 @@ describe("Promises", function () { }); }); - it("Should get rejected when rejectOnDrop is true", function () { - const h = createJobHarness(); - limiter = makeLimiter({ + test("Should get rejected when rejectOnDrop is true", function ({ harness: h, makeLimiter }) { + const limiter = makeLimiter({ maxConcurrent: 1, minTime: 0, highWater: 1, @@ -80,15 +71,17 @@ describe("Promises", function () { return h.flushLimiter(limiter); }) .then(function (_results) { - h.checkResultsOrder([[1], [2]]); - h.checkDuration(100); + expect(h.log).toHaveCallOrder([[1], [2]]); + expect(h).toHaveFinalCallAt(100); expect(dropped).toEqual(1); expect(caught).toEqual(1); }); }); - it("Should automatically wrap an exception in a rejected promise - schedule()", function () { - limiter = makeLimiter({ maxConcurrent: 1, minTime: 100 }); + test("Should automatically wrap an exception in a rejected promise - schedule()", function ({ + makeLimiter, + }) { + const limiter = makeLimiter({ maxConcurrent: 1, minTime: 100 }); return limiter .schedule(() => { @@ -102,10 +95,9 @@ describe("Promises", function () { describe("Wrap", function () { let fn; - it("Should wrap", function () { - const h = createJobHarness(); - limiter = makeLimiter({ maxConcurrent: 1, minTime: 100 }); + test.override({ limiterOptions: { maxConcurrent: 1, minTime: 100 } }); + test("Should wrap", function ({ harness: h, limiter }) { limiter.submit(h.job, null, 1, h.noErrVal(1)); limiter.submit(h.job, null, 2, h.noErrVal(2)); limiter.submit(h.job, null, 3, h.noErrVal(3)); @@ -114,14 +106,14 @@ describe("Promises", function () { h.pNoErrVal(wrapped(null, 4), 4); return h.flushLimiter(limiter).then(function (_results) { - h.checkResultsOrder([[1], [2], [3], [4]]); - h.checkDuration(300); + expect(h.log).toHaveCallOrder([[1], [2], [3], [4]]); + expect(h).toHaveFinalCallAt(300); }); }); - it("Should automatically wrap a returned value in a resolved promise", function () { - limiter = makeLimiter({ maxConcurrent: 1, minTime: 100 }); - + test("Should automatically wrap a returned value in a resolved promise", function ({ + limiter, + }) { fn = limiter.wrap(() => { return 7; }); @@ -131,9 +123,7 @@ describe("Promises", function () { }); }); - it("Should automatically wrap an exception in a rejected promise", function () { - limiter = makeLimiter({ maxConcurrent: 1, minTime: 100 }); - + test("Should automatically wrap an exception in a rejected promise", function ({ limiter }) { fn = limiter.wrap(() => { throw new Error("I will reject"); }); @@ -145,9 +135,7 @@ describe("Promises", function () { }); }); - it("Should inherit the original target for wrapped methods", function () { - limiter = makeLimiter({ maxConcurrent: 1, minTime: 100 }); - + test("Should inherit the original target for wrapped methods", function ({ limiter }) { const object = { fn: limiter.wrap(function () { return this; @@ -159,9 +147,7 @@ describe("Promises", function () { }); }); - it("Should inherit the original target on prototype methods", function () { - limiter = makeLimiter({ maxConcurrent: 1, minTime: 100 }); - + test("Should inherit the original target on prototype methods", function ({ limiter }) { class Animal { constructor(name) { this.name = name; @@ -179,10 +165,8 @@ describe("Promises", function () { }); }); - it("Should pass errors back", function () { + test("Should pass errors back", function ({ harness: h, limiter }) { const failureMessage = "BLEW UP!!!"; - const h = createJobHarness(); - limiter = makeLimiter({ maxConcurrent: 1, minTime: 100 }); const wrapped = limiter.wrap(h.promise); h.pNoErrVal(wrapped(null, 1), 1); @@ -194,22 +178,18 @@ describe("Promises", function () { return h.flushLimiter(limiter); }) .then(function (_results) { - h.checkResultsOrder([[1], [2], [3]]); - h.checkDuration(200); + expect(h.log).toHaveCallOrder([[1], [2], [3]]); + expect(h).toHaveFinalCallAt(200); }); }); - it("Should allow passing options", async function () { + test("Should allow passing options", async function ({ harness: h, makeLimiter }) { const failureMessage = "BLEW UP!!!"; - const h = createJobHarness(); - limiter = makeLimiter({ maxConcurrent: 1, minTime: 50 }); + const limiter = makeLimiter({ maxConcurrent: 1, minTime: 50 }); - let releasePrimer; - const primerHeld = new Promise(function (r) { - releasePrimer = r; - }); + const primer = deferred(); limiter.schedule(function () { - return primerHeld; + return primer.signal; }); const wrapped = limiter.wrap(h.promise); @@ -223,7 +203,7 @@ describe("Promises", function () { await waitForState(() => { expect(limiter.queued()).toBe(6); }); - releasePrimer(); + primer.release(); return job6 .catch(function (err) { @@ -231,7 +211,7 @@ describe("Promises", function () { return h.flushLimiter(limiter); }) .then(function (_results) { - h.checkResultsOrder([[5], [6], [1], [2], [3], [4]]); + expect(h.log).toHaveCallOrder([[5], [6], [1], [2], [3], [4]]); }); }); }); diff --git a/test/retries.test.js b/test/retries.test.js index 3495bef..78b2174 100644 --- a/test/retries.test.js +++ b/test/retries.test.js @@ -1,5 +1,7 @@ -import { describe, it, expect, afterEach } from "vitest"; -const makeLimiter = require("./helpers/limiter"); +import { useFakeClock, isFakeClock } from "./helpers/clock.js"; +import { test, describe, expect } from "./helpers/test-api.js"; + +useFakeClock(); const badJob = function () { return Promise.reject(new Error("boom")); @@ -8,19 +10,17 @@ const badJob = function () { const assertBackoffs = function (attemptTimes, backoffMs) { for (let i = 1; i < attemptTimes.length; i++) { const delta = attemptTimes[i] - attemptTimes[i - 1]; - expect(delta).toBeGreaterThanOrEqual(backoffMs - 5); + if (isFakeClock()) { + expect(delta).toBe(backoffMs); + } else { + expect(delta).toBeGreaterThanOrEqual(backoffMs - 5); + } } }; -describe("Retries", function () { - let limiter; - - afterEach(function () { - return limiter.disconnect(false); - }); - - it("Should retry when requested by the user (sync)", async function () { - limiter = makeLimiter({ trackDoneStatus: true }); +describe("Retries", () => { + test("Should retry when requested by the user (sync)", async function ({ makeLimiter }) { + const limiter = makeLimiter({ trackDoneStatus: true }); let failedEvents = 0; let retryEvents = 0; const attemptTimes = []; @@ -56,8 +56,8 @@ describe("Retries", function () { expect(limiter.counts().DONE).toStrictEqual(1); }); - it("Should retry when requested by the user (async)", async function () { - limiter = makeLimiter({ trackDoneStatus: true }); + test("Should retry when requested by the user (async)", async function ({ makeLimiter }) { + const limiter = makeLimiter({ trackDoneStatus: true }); let failedEvents = 0; let retryEvents = 0; const attemptTimes = []; @@ -93,8 +93,8 @@ describe("Retries", function () { expect(limiter.counts().DONE).toStrictEqual(1); }); - it("Should not retry when user returns an error (sync)", async function () { - limiter = makeLimiter({ trackDoneStatus: true }, { expectErrors: true }); + test("Should not retry when user returns an error (sync)", async function ({ makeLimiter }) { + const limiter = makeLimiter({ trackDoneStatus: true }, { expectErrors: true }); let failedEvents = 0; let retryEvents = 0; let errorEvents = 0; @@ -131,8 +131,8 @@ describe("Retries", function () { expect(limiter.counts().DONE).toStrictEqual(1); }); - it("Should not retry when user returns an error (async)", async function () { - limiter = makeLimiter({ trackDoneStatus: true }, { expectErrors: true }); + test("Should not retry when user returns an error (async)", async function ({ makeLimiter }) { + const limiter = makeLimiter({ trackDoneStatus: true }, { expectErrors: true }); let failedEvents = 0; let retryEvents = 0; let errorEvents = 0; @@ -169,8 +169,8 @@ describe("Retries", function () { expect(limiter.counts().DONE).toStrictEqual(1); }); - it("Should not retry when user returns null (sync)", async function () { - limiter = makeLimiter({ trackDoneStatus: true }); + test("Should not retry when user returns null (sync)", async function ({ makeLimiter }) { + const limiter = makeLimiter({ trackDoneStatus: true }); let failedEvents = 0; let retryEvents = 0; let caught = false; @@ -200,8 +200,8 @@ describe("Retries", function () { expect(limiter.counts().DONE).toStrictEqual(1); }); - it("Should not retry when user returns null (async)", async function () { - limiter = makeLimiter({ trackDoneStatus: true }); + test("Should not retry when user returns null (async)", async function ({ makeLimiter }) { + const limiter = makeLimiter({ trackDoneStatus: true }); let failedEvents = 0; let retryEvents = 0; let caught = false; diff --git a/test/stop.test.js b/test/stop.test.js index 0df8a3e..7839a89 100644 --- a/test/stop.test.js +++ b/test/stop.test.js @@ -1,19 +1,11 @@ -import { describe, it, afterEach, expect } from "vitest"; -import { createJobHarness } from "./helpers/job-tracking.js"; -import { waitForState } from "./helpers/wait-for-state.js"; -const makeLimiter = require("./helpers/limiter"); +import { useFakeClock } from "./helpers/clock.js"; +import { test, describe, expect, waitForState } from "./helpers/test-api.js"; -describe("Stop", function () { - let limiter; +useFakeClock(); - afterEach(function () { - if (limiter == null) return; - return limiter.disconnect(false); - }); - - it("Should stop and drop the queue", async function () { - const h = createJobHarness(); - limiter = makeLimiter({ +describe("Stop", () => { + test("Should stop and drop the queue", async function ({ harness: h, makeLimiter }) { + const limiter = makeLimiter({ maxConcurrent: 2, minTime: 100, trackDoneStatus: true, @@ -61,12 +53,11 @@ describe("Stop", function () { expect(counts.EXECUTING).toEqual(0); expect(counts.DONE).toEqual(2); - h.checkResultsOrder([[0], [1]]); + expect(h.log).toHaveCallOrder([[0], [1]]); }); - it("Should stop and let the queue finish", async function () { - const h = createJobHarness(); - limiter = makeLimiter({ + test("Should stop and let the queue finish", async function ({ harness: h, makeLimiter }) { + const limiter = makeLimiter({ maxConcurrent: 1, minTime: 100, trackDoneStatus: true, @@ -105,12 +96,11 @@ describe("Stop", function () { expect(counts.EXECUTING).toEqual(0); expect(counts.DONE).toEqual(4); - h.checkResultsOrder([[1], [2], [3]]); + expect(h.log).toHaveCallOrder([[1], [2], [3]]); }); - it("Should still resolve when rejectOnDrop is false", function () { - const h = createJobHarness(); - limiter = makeLimiter({ + test("Should still resolve when rejectOnDrop is false", function ({ harness: h, makeLimiter }) { + const limiter = makeLimiter({ maxConcurrent: 1, minTime: 100, rejectOnDrop: false, @@ -133,9 +123,11 @@ describe("Stop", function () { }); }); - it("Should not allow calling stop() twice when dropWaitingJobs=true", function () { - const h = createJobHarness(); - limiter = makeLimiter({ + test("Should not allow calling stop() twice when dropWaitingJobs=true", function ({ + harness: h, + makeLimiter, + }) { + const limiter = makeLimiter({ maxConcurrent: 1, minTime: 100, }); @@ -163,9 +155,11 @@ describe("Stop", function () { }); }); - it("Should not allow calling stop() twice when dropWaitingJobs=false", function () { - const h = createJobHarness(); - limiter = makeLimiter({ + test("Should not allow calling stop() twice when dropWaitingJobs=false", function ({ + harness: h, + makeLimiter, + }) { + const limiter = makeLimiter({ maxConcurrent: 1, minTime: 100, }); diff --git a/vitest.config.ts b/vitest.config.ts index 22ac999..9ff4e0f 100644 --- a/vitest.config.ts +++ b/vitest.config.ts @@ -7,6 +7,12 @@ const libGlobalSetup = "test/global-setup/lib.ts"; const sourceInclude = ["test/**/*.test.js"]; const sourceExclude = ["test/smoke/**", "test/memory/**"]; +// Batcher is datastore-independent (its tests never touch Redis), so running it +// under the redis projects is pure duplication — and worse, it uses fake timers, +// which must never be installed in a fork holding the long-lived redis flush +// client (a reconnect timer scheduled on the fake clock is discarded unfired by +// useRealTimers(), stranding the client). +const redisExclude = [...sourceExclude, "test/batcher.test.js"]; export default defineConfig({ test: { @@ -30,7 +36,7 @@ export default defineConfig({ root: ".", env: { DATASTORE: "ioredis" }, include: sourceInclude, - exclude: [...sourceExclude, "test/node_redis.test.js"], + exclude: [...redisExclude, "test/node_redis.test.js"], setupFiles: [setupFile], testTimeout: 15_000, hookTimeout: 30_000, @@ -42,7 +48,7 @@ export default defineConfig({ root: ".", env: { DATASTORE: "redis" }, include: sourceInclude, - exclude: [...sourceExclude, "test/ioredis.test.js"], + exclude: [...redisExclude, "test/ioredis.test.js"], setupFiles: [setupFile], testTimeout: 15_000, hookTimeout: 30_000, From 407f33611db995e2a50c506c5a1ff44de7a4e0b2 Mon Sep 17 00:00:00 2001 From: Sean Derrow Date: Sat, 11 Jul 2026 23:49:18 -0400 Subject: [PATCH 2/2] sleep vs wait --- .oxlintrc.json | 1 - .vscode/settings.json | 2 +- test/batcher.test.js | 6 +++--- test/cluster-coordination.test.js | 3 ++- test/general-traffic.test.js | 4 ++-- test/helpers/clock.js | 2 +- test/helpers/test-api.js | 3 +-- 7 files changed, 10 insertions(+), 11 deletions(-) diff --git a/.oxlintrc.json b/.oxlintrc.json index 9a74ee9..bbc595c 100644 --- a/.oxlintrc.json +++ b/.oxlintrc.json @@ -32,7 +32,6 @@ "no-await-in-loop": "off", "no-underscore-dangle": "off", // Disabled because it's a common pattern in the codebase "no-array-constructor": "error", - "prefer-arrow-callback": "warn", "typescript/no-explicit-any": "off", // Disabled because it's a common pattern in the codebase "typescript/no-require-imports": "off", // Disabled because it's a common pattern in the codebase "typescript/no-unsafe-function-type": "error", diff --git a/.vscode/settings.json b/.vscode/settings.json index 4567dda..f0941c5 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -1,5 +1,5 @@ { "editor.defaultFormatter": "oxc.oxc-vscode", "editor.formatOnSave": true, - "typescript.tsdk": "node_modules/typescript/lib" + "js/ts.tsdk.path": "node_modules/typescript/lib" } diff --git a/test/batcher.test.js b/test/batcher.test.js index beaa19a..ab30b92 100644 --- a/test/batcher.test.js +++ b/test/batcher.test.js @@ -1,4 +1,4 @@ -import { useFakeClock, wait } from "./helpers/clock.js"; +import { useFakeClock, sleep } from "./helpers/clock.js"; import { test, describe, expect } from "./helpers/test-api.js"; const Bottleneck = require("./bottleneck"); @@ -85,7 +85,7 @@ describe("Batcher", () => { const t0 = Date.now(); const p1 = batcher.add(1); - await wait(50); + await sleep(50); const p2 = batcher.add(2); await Promise.all([p1, p2]); @@ -110,7 +110,7 @@ describe("Batcher", () => { const t1 = Date.now(); const p4 = batcher.add(4); - await wait(50); + await sleep(50); const p5 = batcher.add(5); await Promise.all([p4, p5]); diff --git a/test/cluster-coordination.test.js b/test/cluster-coordination.test.js index c3df38a..694c48a 100644 --- a/test/cluster-coordination.test.js +++ b/test/cluster-coordination.test.js @@ -1,3 +1,4 @@ +import { sleep } from "./helpers/clock.js"; import { test, describe, expect, waitForState, deferred } from "./helpers/test-api.js"; const Bottleneck = require("./bottleneck"); const Scripts = require("../src/cluster/Scripts.js"); @@ -517,7 +518,7 @@ describe("Cluster coordination", () => { }) .then(function (doneCount) { expect(doneCount).toEqual(1); - return h.wait(400); + return sleep(400); }) .then(function () { return countKeys(limiter); diff --git a/test/general-traffic.test.js b/test/general-traffic.test.js index a65e496..37b7c96 100644 --- a/test/general-traffic.test.js +++ b/test/general-traffic.test.js @@ -1,4 +1,4 @@ -import { useFakeClock, useRealClockForThisTest } from "./helpers/clock.js"; +import { sleep, useFakeClock, useRealClockForThisTest } from "./helpers/clock.js"; import { test, describe, expect, waitForState, deferred } from "./helpers/test-api.js"; const path = require("path"); @@ -251,7 +251,7 @@ describe("General traffic", () => { // (`Date.now() - t0 > 145`) verifies j1 actually ran a // meaningful interval, without depending on a fixed timer that // can race event-loop jitter. - return h.wait(100).then(function () { + return sleep(100).then(function () { holdJ1.release(); }); }), diff --git a/test/helpers/clock.js b/test/helpers/clock.js index 2675e85..3bfa733 100644 --- a/test/helpers/clock.js +++ b/test/helpers/clock.js @@ -39,7 +39,7 @@ export function useRealClockForThisTest() { } /** Promise delay via global setTimeout (respects fake timers when installed). */ -export function wait(ms) { +export function sleep(ms) { return new Promise((resolve) => { setTimeout(resolve, ms); }); diff --git a/test/helpers/test-api.js b/test/helpers/test-api.js index 0ebc658..3cf35a0 100644 --- a/test/helpers/test-api.js +++ b/test/helpers/test-api.js @@ -1,5 +1,5 @@ import { test as baseTest, expect as vitestExpect, vi } from "vitest"; -import { wait, isFakeClock } from "./clock.js"; +import { isFakeClock } from "./clock.js"; import { createTaskFns } from "./job-tasks.js"; import makeLimiterHelper from "./limiter.js"; @@ -64,7 +64,6 @@ function createJobHarness() { flushLimiter: flushLimiter, pNoErrVal: pNoErrVal, noErrVal: noErrVal, - wait: wait, callTimes: callTimes, }; }