diff --git a/test/cluster-coordination.test.js b/test/cluster-coordination.test.js index 7b8ebba..9f9be96 100644 --- a/test/cluster-coordination.test.js +++ b/test/cluster-coordination.test.js @@ -51,11 +51,10 @@ describe("Cluster coordination", () => { test("Should chain local and distributed limiters (total concurrency)", async ({ 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 })); + const limiter2 = makeLimiter({ id: "limiter2", maxConcurrent: 1, datastore: "local" }); + const limiter3 = makeLimiter({ id: "limiter3", maxConcurrent: 2, datastore: "local" }); limiter2.on("error", (err) => console.log(err)); limiter2.chain(rootLimiter); @@ -105,11 +104,10 @@ describe("Cluster coordination", () => { test("Should chain local and distributed limiters (partial concurrency)", async ({ harness: h, makeLimiter, - track, }) => { const rootLimiter = makeLimiter({ maxConcurrent: 2 }); - const limiter2 = track(new Bottleneck({ maxConcurrent: 1 })); - const limiter3 = track(new Bottleneck({ maxConcurrent: 2 })); + const limiter2 = makeLimiter({ maxConcurrent: 1, datastore: "local" }); + const limiter3 = makeLimiter({ maxConcurrent: 2, datastore: "local" }); limiter2.chain(rootLimiter); limiter3.chain(rootLimiter); @@ -155,16 +153,13 @@ describe("Cluster coordination", () => { expect(h.log).toHaveCallOrder([[1], [4], [5], [2], [6], [3]]); }); - test("Should use the limiter ID to build Redis keys", async ({ makeLimiter, track }) => { + test("Should use the limiter ID to build Redis keys", async ({ makeLimiter }) => { const rootLimiter = makeLimiter(); const randomId = rootLimiter._randomIndex(); - const limiter = track( - new Bottleneck({ - id: randomId, - datastore: process.env.DATASTORE, - clearDatastore: true, - }), - ); + const limiter = makeLimiter({ + id: randomId, + clearDatastore: true, + }); await limiter.ready(); const keys = limiterKeys(limiter); @@ -173,10 +168,8 @@ describe("Cluster coordination", () => { expect(deleted).toEqual(5); }); - test("Should not fail when Redis data is missing", async ({ track }) => { - const limiter = track( - new Bottleneck({ datastore: process.env.DATASTORE, clearDatastore: true }), - ); + test("Should not fail when Redis data is missing", async ({ makeLimiter }) => { + const limiter = makeLimiter({ clearDatastore: true }); const runningBefore = await limiter.running(); expect(runningBefore).toEqual(0); @@ -190,10 +183,8 @@ describe("Cluster coordination", () => { expect(countRecreated).toBeGreaterThan(0); }); - test("Should parse SETTINGS_KEY_NOT_FOUND from Redis", async ({ track }) => { - const limiter = track( - new Bottleneck({ datastore: process.env.DATASTORE, clearDatastore: true }), - ); + test("Should parse SETTINGS_KEY_NOT_FOUND from Redis", async ({ makeLimiter }) => { + const limiter = makeLimiter({ clearDatastore: true }); await limiter.ready(); await deleteKeys(limiter); @@ -204,10 +195,10 @@ describe("Cluster coordination", () => { expect(err.message).toMatch(SETTINGS_KEY_NOT_FOUND); }); - test("Should re-register when client registration is missing in Redis", async ({ track }) => { - const limiter = track( - new Bottleneck({ datastore: process.env.DATASTORE, clearDatastore: true }), - ); + test("Should re-register when client registration is missing in Redis", async ({ + makeLimiter, + }) => { + const limiter = makeLimiter({ clearDatastore: true }); await limiter.ready(); const clientLastSeenKey = limiterKeys(limiter)[7]; @@ -218,10 +209,8 @@ describe("Cluster coordination", () => { expect(await runCommand(limiter, "zscore", [clientLastSeenKey, clientId])).not.toBeNull(); }); - test("Should parse UNKNOWN_CLIENT from Redis", async ({ track }) => { - const limiter = track( - new Bottleneck({ datastore: process.env.DATASTORE, clearDatastore: true }), - ); + test("Should parse UNKNOWN_CLIENT from Redis", async ({ makeLimiter }) => { + const limiter = makeLimiter({ clearDatastore: true }); await limiter.ready(); const clientLastSeenKey = limiterKeys(limiter)[7]; @@ -236,33 +225,25 @@ describe("Cluster coordination", () => { test("Should drop all jobs in the Cluster when entering blocked mode", async ({ 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, - }), - ); + const limiter1 = makeLimiter({ + id: "blocked", + trackDoneStatus: true, + clearDatastore: true, + maxConcurrent: 1, + minTime: 50, + highWater: 2, + strategy: Bottleneck.strategy.BLOCK, + }); const client_num_queued_key = limiterKeys(limiter1)[5]; await limiter1.ready(); - const limiter2 = track( - new Bottleneck({ - id: "blocked", - trackDoneStatus: true, - datastore: process.env.DATASTORE, - clearDatastore: false, - }), - ); + const limiter2 = makeLimiter({ + id: "blocked", + trackDoneStatus: true, + clearDatastore: false, + }); await limiter2.ready(); // Fire jobs 1-2 on limiter1, then wait for enqueued(limiter1) so both @@ -340,28 +321,22 @@ describe("Cluster coordination", () => { expect(h.log).toHaveCallOrder([[1]]); }); - test("Should pass messages to all limiters in Cluster", async ({ makeLimiter, track }) => { + test("Should pass messages to all limiters in Cluster", async ({ makeLimiter }) => { const rootLimiter = makeLimiter({ maxConcurrent: 1, minTime: 100, id: "super-duper", }); - 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 limiter1 = makeLimiter({ + maxConcurrent: 1, + minTime: 100, + id: "super-duper", + }); + const limiter2 = makeLimiter({ + maxConcurrent: 1, + minTime: 100, + id: "nope", + }); const received = []; rootLimiter.on("message", (msg) => { @@ -387,15 +362,13 @@ describe("Cluster coordination", () => { }); test("Should pass messages to correct limiter after Group re-instantiations", async ({ - track, + makeGroup, }) => { - const group = track( - new Bottleneck.Group({ - maxConcurrent: 1, - minTime: 100, - datastore: process.env.DATASTORE, - }), - ); + const group = makeGroup({ + maxConcurrent: 1, + minTime: 100, + datastore: process.env.DATASTORE, + }); const received = []; await new Promise((resolve, _reject) => { @@ -432,17 +405,15 @@ describe("Cluster coordination", () => { 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. + // "Au revoir!" PUBLISH reply before closing. makeGroup's teardown + // disconnect(false) would destroy the socket mid-flight and reject that pending command. group.disconnect(); }); - test("Should have a default key TTL when using Groups", async ({ track }) => { - const group = track( - new Bottleneck.Group({ - datastore: process.env.DATASTORE, - }), - ); + test("Should have a default key TTL when using Groups", async ({ makeGroup }) => { + const group = makeGroup({ + datastore: process.env.DATASTORE, + }); await group.key("one").ready(); const limiter = group.key("one"); @@ -452,16 +423,14 @@ describe("Cluster coordination", () => { expect(ttl).toBeLessThanOrEqual(305); }); - test("Should support Groups and expire Redis keys", async ({ makeLimiter, track }) => { + test("Should support Groups and expire Redis keys", async ({ makeLimiter, makeGroup }) => { const rootLimiter = makeLimiter(); - const group = track( - new Bottleneck.Group({ - datastore: process.env.DATASTORE, - clearDatastore: true, - minTime: 50, - timeout: 200, - }), - ); + const group = makeGroup({ + datastore: process.env.DATASTORE, + clearDatastore: true, + minTime: 50, + timeout: 200, + }); const t0 = Date.now(); const results = {}; @@ -538,17 +507,15 @@ describe("Cluster coordination", () => { expect(Object.keys(group.connection.limiters).length).toEqual(0); }); - test("Should not recreate a key when running heartbeat", async ({ harness: h, track }) => { - const group = track( - 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", async ({ harness: h, makeGroup }) => { + const group = makeGroup({ + datastore: process.env.DATASTORE, + clearDatastore: true, + maxConcurrent: 50, + minTime: 50, + timeout: 300, + heartbeatInterval: 5, + }); const key = "heartbeat"; const limiter = group.key(key); @@ -562,31 +529,27 @@ describe("Cluster coordination", () => { test("Should delete Redis key when manually deleting a group key", async ({ harness: h, - track, + makeGroup, }) => { // Bump timeout (and the corresponding waitForState 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 = 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 group1 = makeGroup({ + datastore: process.env.DATASTORE, + clearDatastore: true, + maxConcurrent: 50, + minTime: 50, + timeout: groupTimeout, + }); + const group2 = makeGroup({ + datastore: process.env.DATASTORE, + clearDatastore: true, + maxConcurrent: 50, + minTime: 50, + timeout: groupTimeout, + }); const key = "deleted"; const limiter = group1.key(key); // only for countKeys() use @@ -623,7 +586,7 @@ describe("Cluster coordination", () => { test("Should delete Redis keys from a group even when the local limiter is not present", async ({ harness: h, - track, + makeGroup, }) => { // groupTimeout pulls double duty here: it sets the redis-side TTL // (must not expire before group2.deleteKey runs), and it gates @@ -632,24 +595,20 @@ describe("Cluster coordination", () => { // the keys could TTL-expire before deleteKey ran. Refreshing the TTL // explicitly via running() right before deleteKey decouples the two. 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 group1 = makeGroup({ + datastore: process.env.DATASTORE, + clearDatastore: true, + maxConcurrent: 50, + minTime: 50, + timeout: groupTimeout, + }); + const group2 = makeGroup({ + 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 @@ -688,27 +647,23 @@ describe("Cluster coordination", () => { expect(group2.keys().length).toEqual(0); }); - test("Should returns all Group keys in the cluster", async ({ track }) => { + test("Should returns all Group keys in the cluster", async ({ makeGroup }) => { // 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 = 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 group1 = makeGroup({ + datastore: process.env.DATASTORE, + clearDatastore: true, + id: "same", + timeout: 30000, + }); + const group2 = makeGroup({ + 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); @@ -721,51 +676,22 @@ describe("Cluster coordination", () => { expect((await group1.clusterKeys()).sort()).toEqual(both.sort()); expect((await group1.clusterKeys()).sort()).toEqual(both.sort()); - const group3 = track(new Bottleneck.Group({ datastore: "local" })); + const group3 = makeGroup({ datastore: "local" }); expect(await group3.clusterKeys()).toEqual([]); }); - test("Should queue up the least busy limiter", async ({ 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, - }), - ); + test("Should queue up the least busy limiter", async ({ harness: h, makeLimiter }) => { + const busyOpts = { + clearDatastore: true, + id: "busy", + timeout: 3000, + maxConcurrent: 3, + trackDoneStatus: true, + }; + const limiter1 = makeLimiter(busyOpts); + const limiter2 = makeLimiter(busyOpts); + const limiter3 = makeLimiter(busyOpts); + const limiter4 = makeLimiter(busyOpts); await limiter1.schedule({ id: "1" }, h.promise, null, "A"); await limiter2.schedule({ id: "2" }, h.promise, null, "B"); @@ -836,47 +762,21 @@ describe("Cluster coordination", () => { expect(calls.slice(9, 11).sort()).toEqual([2, 3]); }); - test("Should pass the remaining capacity to other limiters", async ({ 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, - }), - ); + test("Should pass the remaining capacity to other limiters", async ({ + harness: h, + makeLimiter, + }) => { + const busyOpts = { + clearDatastore: true, + id: "busy", + timeout: 3000, + maxConcurrent: 3, + trackDoneStatus: true, + }; + const limiter1 = makeLimiter(busyOpts); + const limiter2 = makeLimiter(busyOpts); + const limiter3 = makeLimiter(busyOpts); + const limiter4 = makeLimiter(busyOpts); let t3, t4; await limiter1.schedule({ id: "1" }, h.promise, null, "A"); @@ -950,38 +850,18 @@ describe("Cluster coordination", () => { test("Should take the capacity and blacklist if the priority limiter is not responding", async ({ harness: h, - track, + makeLimiter, }) => { - 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, - }), - ); + const crashOpts = { + clearDatastore: true, + id: "crash", + timeout: 3000, + maxConcurrent: 1, + trackDoneStatus: true, + }; + const limiter1 = makeLimiter(crashOpts); + const limiter2 = makeLimiter(crashOpts); + const limiter3 = makeLimiter(crashOpts); await limiter1.schedule({ id: "1" }, h.promise, null, "A"); await limiter2.schedule({ id: "2" }, h.promise, null, "B"); diff --git a/test/cluster.test.js b/test/cluster.test.js index 2f1becd..69fbedc 100644 --- a/test/cluster.test.js +++ b/test/cluster.test.js @@ -1,6 +1,5 @@ import { describe, expect } from "vitest"; import { test, waitForState, deferred, enqueued } from "./helpers/test-api.js"; -const Bottleneck = require("./bottleneck"); const Scripts = require("../src/cluster/Scripts.js"); const assert = require("assert"); @@ -46,16 +45,13 @@ describe("Cluster-only", () => { test("Should allow passing a limiter's connection to a new limiter", async ({ harness: h, makeLimiter, - track, }) => { const rootLimiter = makeLimiter(); rootLimiter.connection.id = "some-id"; - const limiter = track( - new Bottleneck({ - minTime: 50, - connection: rootLimiter.connection, - }), - ); + const limiter = makeLimiter({ + minTime: 50, + connection: rootLimiter.connection, + }); await Promise.all([rootLimiter.ready(), limiter.ready()]); expect(limiter.connection.id).toEqual("some-id"); @@ -72,16 +68,14 @@ describe("Cluster-only", () => { test("Should allow passing a limiter's connection to a new Group", async ({ harness: h, makeLimiter, - track, + makeGroup, }) => { const rootLimiter = makeLimiter(); rootLimiter.connection.id = "some-id"; - const group = track( - new Bottleneck.Group({ - minTime: 50, - connection: rootLimiter.connection, - }), - ); + const group = makeGroup({ + minTime: 50, + connection: rootLimiter.connection, + }); const limiter1 = group.key("A"); const limiter2 = group.key("B"); @@ -103,25 +97,21 @@ describe("Cluster-only", () => { test("Should allow passing a Group's connection to a new limiter", async ({ harness: h, makeLimiter, - track, + makeGroup, }) => { const rootLimiter = makeLimiter(); - const group = track( - new Bottleneck.Group({ - minTime: 50, - datastore: process.env.DATASTORE, - clearDatastore: true, - }), - ); + const group = makeGroup({ + minTime: 50, + datastore: process.env.DATASTORE, + clearDatastore: true, + }); group.connection.id = "some-id"; const limiter1 = group.key("A"); - const limiter2 = track( - new Bottleneck({ - minTime: 50, - connection: group.connection, - }), - ); + const limiter2 = makeLimiter({ + minTime: 50, + connection: group.connection, + }); await Promise.all([limiter1.ready(), limiter2.ready()]); expect(limiter1.connection.id).toEqual("some-id"); @@ -140,25 +130,21 @@ describe("Cluster-only", () => { test("Should allow passing a Group's connection to a new Group", async ({ harness: h, makeLimiter, - track, + makeGroup, }) => { const rootLimiter = makeLimiter(); - const group1 = track( - new Bottleneck.Group({ - minTime: 50, - datastore: process.env.DATASTORE, - clearDatastore: true, - }), - ); + const group1 = makeGroup({ + minTime: 50, + datastore: process.env.DATASTORE, + clearDatastore: true, + }); group1.connection.id = "some-id"; - const group2 = track( - new Bottleneck.Group({ - minTime: 50, - connection: group1.connection, - clearDatastore: true, - }), - ); + const group2 = makeGroup({ + minTime: 50, + connection: group1.connection, + clearDatastore: true, + }); const limiter1 = group1.key("AAA"); const limiter2 = group1.key("BBB"); @@ -245,7 +231,6 @@ describe("Cluster-only", () => { test("Should compute reservoir increased based on number of missed intervals", async ({ makeLimiter, - track, }) => { const settings = { id: "missed-intervals", @@ -270,7 +255,7 @@ describe("Cluster-only", () => { // 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 = track(new Bottleneck({ ...settings, datastore: process.env.DATASTORE })); + const limiter2 = makeLimiter({ ...settings }); await limiter2.ready(); // process_tick uses Date.now() from JS (see RedisDatastore.runScript), @@ -294,7 +279,7 @@ describe("Cluster-only", () => { expect(reservoir).toBeLessThanOrEqual(64); }); - test("Should migrate from 2.8.0", async ({ makeLimiter, track }) => { + test("Should migrate from 2.8.0", async ({ makeLimiter }) => { // 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. @@ -313,12 +298,9 @@ describe("Cluster-only", () => { ]), runCommand(rootLimiter, "hset", [settings_key, "lastReservoirRefresh", ""]), ]); - const limiter2 = track( - new Bottleneck({ - id: "migrate", - datastore: process.env.DATASTORE, - }), - ); + const limiter2 = makeLimiter({ + id: "migrate", + }); await limiter2.ready(); const values = await runCommand(rootLimiter, "hmget", [ settings_key, @@ -352,24 +334,17 @@ describe("Cluster-only", () => { ]); }); - test("Should keep track of each client's queue length", async ({ - harness: h, - makeLimiter, - track, - }) => { + test("Should keep track of each client's queue length", async ({ harness: h, makeLimiter }) => { 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 limiter2 = makeLimiter({ + id: "queues", + maxConcurrent: 1, + trackDoneStatus: true, + }); const client_num_queued_key = limiterKeys(rootLimiter)[5]; const clientId1 = rootLimiter._store.clientId; const clientId2 = limiter2._store.clientId; @@ -409,11 +384,11 @@ describe("Cluster-only", () => { expect(await rootLimiter.clusterQueued()).toEqual(0); }); - test("Should publish capacity increases", async ({ harness: h, makeLimiter, track }) => { + test("Should publish capacity increases", async ({ harness: h, makeLimiter }) => { const rootLimiter = makeLimiter({ maxConcurrent: 2 }); await rootLimiter.ready(); - const limiter2 = track(new Bottleneck({ datastore: process.env.DATASTORE })); + const limiter2 = makeLimiter(); await limiter2.ready(); // Use deferredPromise instead of slowPromise(100) for jobs 1 and 2. @@ -447,7 +422,6 @@ describe("Cluster-only", () => { test("Should publish capacity changes on reservoir changes", async ({ harness: h, makeLimiter, - track, }) => { const rootLimiter = makeLimiter({ maxConcurrent: 2, @@ -455,11 +429,7 @@ describe("Cluster-only", () => { }); await rootLimiter.ready(); - const limiter2 = track( - new Bottleneck({ - datastore: process.env.DATASTORE, - }), - ); + const limiter2 = makeLimiter(); await limiter2.ready(); const held = deferred(); @@ -482,24 +452,17 @@ describe("Cluster-only", () => { expect(h.log).toHaveCallOrder([[0], [1], [2], [3]]); }); - test("Should remove track job data and remove lost jobs", async ({ - harness: h, - makeLimiter, - track, - }) => { + test("Should remove track job data and remove lost jobs", async ({ harness: h, makeLimiter }) => { // 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 rootLimiter = makeLimiter({ id: "lost" }, { expectErrors: true }); const clientId = rootLimiter._store.clientId; - const limiter1 = track(new Bottleneck({ datastore: process.env.DATASTORE })); - const limiter2 = track( - new Bottleneck({ - id: "lost", - datastore: process.env.DATASTORE, - heartbeatInterval: 150, - }), - ); + const limiter1 = makeLimiter(); + const limiter2 = makeLimiter({ + id: "lost", + heartbeatInterval: 150, + }); const getData = (limiter) => { expect(limiterKeys(limiter).length).toEqual(8); // Asserting, to remember to edit this test when keys change const [ @@ -601,7 +564,7 @@ describe("Cluster-only", () => { expect(numExpirations).toEqual(4); }); - test("Should clear unresponsive clients", async ({ makeLimiter, track }) => { + test("Should clear unresponsive clients", async ({ makeLimiter }) => { const rootLimiter = makeLimiter({ id: "unresponsive", maxConcurrent: 1, @@ -619,12 +582,9 @@ describe("Cluster-only", () => { // default options, clientTimeout=10000 and process_tick can't clean // up within the 5s test window. await rootLimiter.ready(); - const limiter2 = track( - new Bottleneck({ - id: "unresponsive", - datastore: process.env.DATASTORE, - }), - ); + const limiter2 = makeLimiter({ + id: "unresponsive", + }); await limiter2.ready(); await Promise.all([rootLimiter.running(), limiter2.running()]); @@ -661,7 +621,6 @@ describe("Cluster-only", () => { test("Should not clear unresponsive clients with unexpired running jobs", async ({ harness: h, makeLimiter, - track, }) => { const rootLimiter = makeLimiter({ id: "unresponsive-unexpired", @@ -674,12 +633,9 @@ describe("Cluster-only", () => { // limiter2's defaults. Constructing limiter2 up-front races init.lua and // settings can adopt the wrong values. await rootLimiter.ready(); - const limiter2 = track( - new Bottleneck({ - id: "unresponsive-unexpired", - datastore: process.env.DATASTORE, - }), - ); + const limiter2 = makeLimiter({ + id: "unresponsive-unexpired", + }); await limiter2.ready(); const client_running_key = limiterKeys(limiter2)[4]; @@ -724,7 +680,6 @@ describe("Cluster-only", () => { test("Should clear unresponsive clients after last jobs are expired", async ({ harness: h, makeLimiter, - track, }) => { const rootLimiter = makeLimiter({ id: "unresponsive-expired", @@ -736,12 +691,9 @@ describe("Cluster-only", () => { // Sequence init so rootLimiter's clientTimeout/heartbeatInterval win over // limiter2's defaults. await rootLimiter.ready(); - const limiter2 = track( - new Bottleneck({ - id: "unresponsive-expired", - datastore: process.env.DATASTORE, - }), - ); + const limiter2 = makeLimiter({ + id: "unresponsive-expired", + }); await limiter2.ready(); const client_running_key = limiterKeys(limiter2)[4]; @@ -793,7 +745,7 @@ describe("Cluster-only", () => { expect(await numClients()).toEqual([1, 1, 1, 1]); }); - test("Should use shared settings", async ({ harness: h, makeLimiter, track }) => { + test("Should use shared settings", async ({ harness: h, makeLimiter }) => { const rootLimiter = makeLimiter({ maxConcurrent: 2 }); const settings_key = limiterKeys(rootLimiter)[0]; @@ -803,7 +755,7 @@ describe("Cluster-only", () => { // both up-front and awaiting them together races init.lua executions and // produces flaky reads. await rootLimiter.ready(); - const limiter2 = track(new Bottleneck({ maxConcurrent: 1, datastore: process.env.DATASTORE })); + const limiter2 = makeLimiter({ maxConcurrent: 1 }); await limiter2.ready(); const maxConcurrent = await runCommand(rootLimiter, "hget", [settings_key, "maxConcurrent"]); expect(maxConcurrent).toEqual("2"); @@ -816,18 +768,15 @@ describe("Cluster-only", () => { expect(h.log).toHaveCallOrder([[1], [2]]); }); - test("Should clear previous settings", async ({ harness: h, makeLimiter, track }) => { + test("Should clear previous settings", async ({ harness: h, makeLimiter }) => { const rootLimiter = makeLimiter({ maxConcurrent: 2 }); const settings_key = limiterKeys(rootLimiter)[0]; await rootLimiter.ready(); - const limiter2 = track( - new Bottleneck({ - maxConcurrent: 1, - datastore: process.env.DATASTORE, - clearDatastore: true, - }), - ); + const limiter2 = makeLimiter({ + maxConcurrent: 1, + clearDatastore: true, + }); await limiter2.ready(); // Verify the actual cleared setting in redis directly — this is the // contract being tested. Avoids dependence on slowPromise wall-clock diff --git a/test/group.test.js b/test/group.test.js index d478baa..7b65327 100644 --- a/test/group.test.js +++ b/test/group.test.js @@ -7,14 +7,12 @@ const Bottleneck = require("./bottleneck"); useFakeClock(); describe("Group", () => { - test("Should create limiters", async ({ track }) => { + test("Should create limiters", async ({ makeGroup }) => { expect.hasAssertions(); - const group = track( - new Bottleneck.Group({ - maxConcurrent: 1, - minTime: 100, - }), - ); + const group = makeGroup({ + maxConcurrent: 1, + minTime: 100, + }); const results = []; @@ -53,13 +51,11 @@ describe("Group", () => { }); }); - test("Should set up the limiter IDs (default)", ({ track }) => { - const group = track( - new Bottleneck.Group({ - maxConcurrent: 1, - minTime: 100, - }), - ); + test("Should set up the limiter IDs (default)", ({ makeGroup }) => { + const group = makeGroup({ + maxConcurrent: 1, + minTime: 100, + }); expect(group.key("A").id).toStrictEqual("group-key-A"); expect(group.key("B").id).toStrictEqual("group-key-B"); @@ -73,14 +69,12 @@ describe("Group", () => { expect(ids.sort()).toStrictEqual(["group-key-A", "group-key-B", "group-key-XYZ"]); }); - test("Should set up the limiter IDs (custom)", ({ track }) => { - const group = track( - new Bottleneck.Group({ - maxConcurrent: 1, - minTime: 100, - id: "custom-id", - }), - ); + test("Should set up the limiter IDs (custom)", ({ makeGroup }) => { + const group = makeGroup({ + 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"); @@ -94,14 +88,12 @@ describe("Group", () => { expect(ids.sort()).toStrictEqual(["custom-id-A", "custom-id-B", "custom-id-XYZ"]); }); - test("Should pass new limiter to 'created' event", async ({ makeLimiter, track }) => { + test("Should pass new limiter to 'created' event", async ({ makeLimiter, makeGroup }) => { const limiter = makeLimiter(); - const group = track( - new Bottleneck.Group({ - maxConcurrent: 1, - minTime: 100, - }), - ); + const group = makeGroup({ + maxConcurrent: 1, + minTime: 100, + }); const keys = []; const ids = []; @@ -129,14 +121,12 @@ describe("Group", () => { await limiter.ready(); }); - test("Should pass error on failure", async ({ track }) => { + test("Should pass error on failure", async ({ makeGroup }) => { const failureMessage = "SOMETHING BLEW UP!!"; - const group = track( - new Bottleneck.Group({ - maxConcurrent: 1, - minTime: 100, - }), - ); + const group = makeGroup({ + maxConcurrent: 1, + minTime: 100, + }); expect(Object.keys(group.limiters)).toStrictEqual([]); const results = []; @@ -170,20 +160,16 @@ describe("Group", () => { }); }); - test("Should update its timeout", async ({ track }) => { - const group1 = track( - new Bottleneck.Group({ - maxConcurrent: 1, - minTime: 100, - }), - ); - const group2 = track( - new Bottleneck.Group({ - maxConcurrent: 1, - minTime: 100, - timeout: 5000, - }), - ); + test("Should update its timeout", async ({ makeGroup }) => { + const group1 = makeGroup({ + maxConcurrent: 1, + minTime: 100, + }); + const group2 = makeGroup({ + maxConcurrent: 1, + minTime: 100, + timeout: 5000, + }); expect(group1.timeout).toStrictEqual(300000); expect(group2.timeout).toStrictEqual(5000); @@ -195,13 +181,11 @@ describe("Group", () => { expect(group2.timeout).toStrictEqual(456); }); - test("Should update its limiter options", ({ track }) => { - const group = track( - new Bottleneck.Group({ - maxConcurrent: 1, - minTime: 100, - }), - ); + test("Should update its limiter options", ({ makeGroup }) => { + const group = makeGroup({ + maxConcurrent: 1, + minTime: 100, + }); const limiter1 = group.key("AAA"); expect(limiter1._store.storeOptions.minTime).toStrictEqual(100); @@ -213,12 +197,10 @@ describe("Group", () => { expect(limiter2._store.storeOptions.minTime).toStrictEqual(200); }); - test("Should support keys(), limiters(), deleteKey()", async ({ harness: h, track }) => { - const group1 = track( - new Bottleneck.Group({ - maxConcurrent: 1, - }), - ); + test("Should support keys(), limiters(), deleteKey()", async ({ harness: h, makeGroup }) => { + const group1 = makeGroup({ + maxConcurrent: 1, + }); const KEY_A = "AAA"; const KEY_B = "BBB"; @@ -246,13 +228,11 @@ describe("Group", () => { expect(group1.keys().length).toStrictEqual(1); }); - test("Should call autocleanup", async ({ makeLimiter, track }) => { + test("Should call autocleanup", async ({ makeLimiter, makeGroup }) => { const KEY = "test-key"; - const group = track( - new Bottleneck.Group({ - maxConcurrent: 1, - }), - ); + const group = makeGroup({ + maxConcurrent: 1, + }); group.updateSettings({ timeout: 500 }); const limiter = makeLimiter({ id: "something", timeout: group.timeout }); diff --git a/test/helpers/test-api.js b/test/helpers/test-api.js index 6597e01..0d9e6c6 100644 --- a/test/helpers/test-api.js +++ b/test/helpers/test-api.js @@ -3,6 +3,8 @@ import { isFakeClock } from "./clock.js"; import { createTaskFns } from "./job-tasks.js"; import makeLimiterHelper from "./limiter.js"; +const Bottleneck = require("../bottleneck"); + export { waitForState } from "./wait-for-state.js"; export { deferred } from "./job-tasks.js"; @@ -162,6 +164,18 @@ export const test = baseTest.extend({ async makeLimiter({ track }, use) { await use((opts, meta) => track(makeLimiterHelper(opts, meta))); }, + async makeGroup({ track }, use) { + await use((opts) => track(new Bottleneck.Group(opts ?? {}))); + }, + async makeConnection({ track }, use) { + await use((opts) => { + const Connection = + process.env.DATASTORE === "ioredis" + ? Bottleneck.IORedisConnection + : Bottleneck.RedisConnection; + return track(new Connection(opts)); + }); + }, async limiter({ makeLimiter, limiterOptions, limiterMeta }, use) { await use(makeLimiter(limiterOptions, limiterMeta)); }, diff --git a/test/ioredis.test.js b/test/ioredis.test.js index cc59706..932558e 100644 --- a/test/ioredis.test.js +++ b/test/ioredis.test.js @@ -1,6 +1,5 @@ import { describe, expect } from "vitest"; import { test } from "./helpers/test-api.js"; -const Bottleneck = require("./bottleneck"); const Redis = require("ioredis"); const buildClientOptions = require("./redis-client-options"); @@ -37,9 +36,12 @@ describe("ioredis-only", () => { expect(limiter._store.connection.client.nodes().length).toBeGreaterThanOrEqual(0); }); - test("Should connect in Redis Cluster mode with premade client", ({ makeLimiter, track }) => { + test("Should connect in Redis Cluster mode with premade client", ({ + makeLimiter, + makeConnection, + }) => { const client = new Redis.Cluster(""); - track(new Bottleneck.IORedisConnection({ client })); + makeConnection({ client }); const limiter = makeLimiter({ maxConcurrent: 2, clientOptions: {}, @@ -55,13 +57,15 @@ describe("ioredis-only", () => { expect(limiter._store.connection.client.nodes().length).toBeGreaterThanOrEqual(0); }); - test("Should accept existing connections", async ({ harness: h, makeLimiter, track }) => { - const connection = track( - new Bottleneck.IORedisConnection({ - Redis, - clientOptions: buildClientOptions("ioredis"), - }), - ); + test("Should accept existing connections", async ({ + harness: h, + makeLimiter, + makeConnection, + }) => { + const connection = makeConnection({ + Redis, + clientOptions: buildClientOptions("ioredis"), + }); connection.id = "super-connection"; const limiter = makeLimiter({ minTime: 50, @@ -82,11 +86,15 @@ describe("ioredis-only", () => { await Promise.all([expect(p1).resolves.toEqual([1]), expect(p2).resolves.toEqual([2])]); }); - test("Should accept existing redis clients", async ({ harness: h, makeLimiter, track }) => { + test("Should accept existing redis clients", async ({ + harness: h, + makeLimiter, + makeConnection, + }) => { const client = new Redis(buildClientOptions("ioredis")); client.id = "super-client"; - const connection = track(new Bottleneck.IORedisConnection({ client })); + const connection = makeConnection({ client }); connection.id = "super-connection"; const limiter = makeLimiter({ minTime: 50, @@ -108,17 +116,18 @@ describe("ioredis-only", () => { await Promise.all([expect(p1).resolves.toEqual([1]), expect(p2).resolves.toEqual([2])]); }); - test("Should trigger error events on the shared connection", ({ makeLimiter, track }) => { + test("Should trigger error events on the shared connection", ({ + makeLimiter, + makeConnection, + }) => { expect.hasAssertions(); return new Promise((resolve, reject) => { - const connection = track( - new Bottleneck.IORedisConnection({ - Redis, - clientOptions: { - port: 1, - }, - }), - ); + const connection = makeConnection({ + Redis, + clientOptions: { + port: 1, + }, + }); let fired = false; const limiter = makeLimiter({ connection }); connection.on("error", (_err) => { diff --git a/test/node_redis.test.js b/test/node_redis.test.js index 260f547..dae0c54 100644 --- a/test/node_redis.test.js +++ b/test/node_redis.test.js @@ -1,6 +1,5 @@ import { describe, expect } from "vitest"; import { test } from "./helpers/test-api.js"; -const Bottleneck = require("./bottleneck"); const Redis = require("redis"); const buildClientOptions = require("./redis-client-options"); @@ -14,13 +13,15 @@ describe("node_redis-only", () => { expect(limiter.datastore).toStrictEqual("redis"); }); - test("Should accept existing connections", async ({ harness: h, makeLimiter, track }) => { - const connection = track( - new Bottleneck.RedisConnection({ - Redis, - clientOptions: buildClientOptions("redis"), - }), - ); + test("Should accept existing connections", async ({ + harness: h, + makeLimiter, + makeConnection, + }) => { + const connection = makeConnection({ + Redis, + clientOptions: buildClientOptions("redis"), + }); connection.id = "super-connection"; const limiter = makeLimiter({ minTime: 50, @@ -41,12 +42,16 @@ describe("node_redis-only", () => { await Promise.all([expect(p1).resolves.toEqual([1]), expect(p2).resolves.toEqual([2])]); }); - test("Should accept existing redis clients", async ({ harness: h, makeLimiter, track }) => { + test("Should accept existing redis clients", async ({ + harness: h, + makeLimiter, + makeConnection, + }) => { const client = Redis.createClient(buildClientOptions("redis")); client.id = "super-client"; await client.connect(); - const connection = track(new Bottleneck.RedisConnection({ client })); + const connection = makeConnection({ client }); connection.id = "super-connection"; const limiter = makeLimiter({ minTime: 50, @@ -68,20 +73,21 @@ describe("node_redis-only", () => { await Promise.all([expect(p1).resolves.toEqual([1]), expect(p2).resolves.toEqual([2])]); }); - test("Should trigger error events on the shared connection", ({ makeLimiter, track }) => { + test("Should trigger error events on the shared connection", ({ + makeLimiter, + makeConnection, + }) => { expect.hasAssertions(); return new Promise((resolve, reject) => { - const connection = track( - new Bottleneck.RedisConnection({ - Redis, - clientOptions: { - socket: { - port: 1, - reconnectStrategy: () => false, - }, + const connection = makeConnection({ + Redis, + clientOptions: { + socket: { + port: 1, + reconnectStrategy: () => false, }, - }), - ); + }, + }); connection.ready.catch(() => {}); let fired = false; const limiter = makeLimiter({ connection }, { expectErrors: true });