From a48b3c5af68b65cd0ea30ed934ed2950efeb6dc8 Mon Sep 17 00:00:00 2001 From: Nwokedi Chigozirim Date: Wed, 21 Oct 2026 01:19:06 +0100 Subject: [PATCH] test: add unit tests for Redis config, cleanup jobs, and webhook retry job (#607, #609, #611) tests/unit/config/redis.test.ts (#607): the ioredis client construction from config.REDIS_URL, its retry strategy (linear backoff capped at 2000ms, gives up after 10 attempts), event handler registration, and closeRedis(). tests/unit/jobs/cleanup-idempotency.test.ts (#609): the interval scheduling wrapper around cleanupIdempotencyKeys(), the deletion logic itself already has its own tests in tests/unit/services/idempotency.test.ts, scheduling, stopping, restarting, and that a rejected cleanup pass is logged rather than crashing the interval. tests/unit/jobs/cleanup-notifications.test.ts (#609): moved from tests/unit/notifications/ to the path #609 asks for. While moving it, fixed a stale mock: the real cleanupExpiredNotifications() chains .returning() after .where() to get a deletable row count, but the mock's delete chain only had a .where() terminal, so 4 of its 5 cases were failing with 'returning is not a function' both before and after the move, confirmed against upstream/main. Added .returning() to the chain. tests/unit/jobs/process-webhook-retries.test.ts (#611): covers the job's actual current responsibility, scheduling a periodic call to webhook-dispatcher.js's processWebhookRetries() and recovering from a rejected pass, plus the generation counter mechanism that prevents a tick already in flight from running again after stop(). Also, while reading process-webhook-retries.ts for #611: the file had two full, incompatible implementations of startWebhookRetryProcessor concatenated together, one Redis queue based with its own enqueue/dequeue/shouldRetry/markForRetry exports, a stray import mid-file, and a broken stopWebhookRetryProcessor referencing variables from both halves. Nothing outside the file used the first implementation's exports; kept only the second, the one server.ts actually calls, otherwise this file doesn't even parse, so no test file for it could load either. Also fixed in services/webhook-dispatcher.ts, which the job wrapper delegates to and which #611 is really asking to have its retry behavior covered: sendWebhook()'s finally block called clearTimeout(timer), but the variable holding the AbortController timeout handle is named timeout, not timer, a ReferenceError on every single webhook send attempt, success or failure, confirmed against upstream/main. Also fixed utils/logger.ts: it imported config/index.ts to read NODE_ENV, while config/index.ts imports logger.ts to log its own validation warnings/errors. That circular import left config undefined partway through evaluation, crashing any test that touches logger.ts, directly or via another module's import chain, with 'Cannot read properties of undefined (reading NODE_ENV)', confirmed against upstream/main and reproducible via npm test today. Fixed by reading process.env.NODE_ENV directly instead of through config. Disclosure: npm test still fails on this branch beyond the files listed above, from the same scaffold contamination issue already reported in full in PR #689 on this repo, several files, including course.service.ts, contain dead NestJS style scaffold code bolted onto the real Fastify implementations, causing duplicate export compile errors that crash every test file that transitively imports them. Not re-fixed here to avoid overlapping with that PR; all 4 test files this PR adds, plus the moved one, pass cleanly when run directly (29/29), which is what this issue's scope covers. --- src/jobs/process-webhook-retries.ts | 106 -------------- src/services/webhook-dispatcher.ts | 2 +- src/utils/logger.ts | 18 +-- tests/unit/config/redis.test.ts | 132 ++++++++++++++++++ tests/unit/jobs/cleanup-idempotency.test.ts | 110 +++++++++++++++ .../cleanup-notifications.test.ts | 16 ++- .../unit/jobs/process-webhook-retries.test.ts | 126 +++++++++++++++++ 7 files changed, 389 insertions(+), 121 deletions(-) create mode 100644 tests/unit/config/redis.test.ts create mode 100644 tests/unit/jobs/cleanup-idempotency.test.ts rename tests/unit/{notifications => jobs}/cleanup-notifications.test.ts (85%) create mode 100644 tests/unit/jobs/process-webhook-retries.test.ts diff --git a/src/jobs/process-webhook-retries.ts b/src/jobs/process-webhook-retries.ts index e8b8773..fd02898 100644 --- a/src/jobs/process-webhook-retries.ts +++ b/src/jobs/process-webhook-retries.ts @@ -1,106 +1,4 @@ import { logger } from "../utils/logger.js"; -import { redis } from "../config/redis.js"; - -const QUEUE_KEY = "chainlearn:retry:webhooks"; -const MAX_RETRIES = 5; -const BASE_DELAY_MS = 1000; -const MAX_DELAY_MS = 3600000; - -export interface WebhookAttempt { - id: string; - webhookId: string; - event: string; - payload: unknown; - statusCode: number | null; - errorMessage: string | null; - retryCount: number; - nextRetryAt: string | null; - succeededAt: string | null; - failedAt: string | null; -} - -function isTransientFailure(statusCode: number | null, error: string | null): boolean { - if (!statusCode) return true; - return statusCode >= 500 || statusCode === 429; -} - -function calculateNextRetry(retryCount: number): Date { - const delay = Math.min(BASE_DELAY_MS * Math.pow(2, retryCount), MAX_DELAY_MS); - const jitter = delay * (0.5 + Math.random() * 0.5); - return new Date(Date.now() + jitter); -} - -export async function enqueueWebhookRetry(attempt: WebhookAttempt): Promise { - await redis.lpush(QUEUE_KEY, JSON.stringify(attempt)); - logger.info({ webhookId: attempt.webhookId, event: attempt.event }, "Webhook retry enqueued"); -} - -export async function dequeueWebhookRetry(): Promise { - const raw = await redis.rpop(QUEUE_KEY); - if (!raw) return null; - return JSON.parse(raw) as WebhookAttempt; -} - -export function shouldRetry(attempt: WebhookAttempt): boolean { - if (attempt.retryCount >= MAX_RETRIES) return false; - if (!isTransientFailure(attempt.statusCode, attempt.errorMessage)) return false; - return true; -} - -export function markForRetry(attempt: WebhookAttempt): WebhookAttempt { - const nextRetry = calculateNextRetry(attempt.retryCount); - return { - ...attempt, - retryCount: attempt.retryCount + 1, - nextRetryAt: nextRetry.toISOString(), - }; -} - -export function markAsFailed(attempt: WebhookAttempt): WebhookAttempt { - return { - ...attempt, - failedAt: new Date().toISOString(), - }; -} - -let processorRunning = false; -let processorTimer: ReturnType | null = null; - -export async function startWebhookRetryProcessor( - processFn: (attempt: WebhookAttempt) => Promise -): Promise { - if (processorRunning) return; - processorRunning = true; - - const tick = async () => { - if (!processorRunning) return; - try { - const attempt = await dequeueWebhookRetry(); - if (attempt) { - if (!shouldRetry(attempt)) { - const failed = markAsFailed(attempt); - logger.warn( - { webhookId: failed.webhookId, retryCount: failed.retryCount }, - "Webhook permanently failed" - ); - return; - } - - const success = await processFn(attempt); - if (!success) { - const retried = markForRetry(attempt); - await enqueueWebhookRetry(retried); - } - } - } catch (err) { - logger.error({ err }, "Webhook retry processor tick failed"); - } - if (processorRunning) { - processorTimer = setTimeout(tick, 30000); - } - }; - - tick(); import { processWebhookRetries } from "../services/webhook-dispatcher.js"; let retryProcessorRunning = false; @@ -131,10 +29,6 @@ export async function startWebhookRetryProcessor(): Promise { } export function stopWebhookRetryProcessor(): void { - processorRunning = false; - if (processorTimer) { - clearTimeout(processorTimer); - processorTimer = null; retryProcessorRunning = false; retryProcessorGeneration++; if (retryProcessorTimer) { diff --git a/src/services/webhook-dispatcher.ts b/src/services/webhook-dispatcher.ts index 6415e27..9b3cec3 100644 --- a/src/services/webhook-dispatcher.ts +++ b/src/services/webhook-dispatcher.ts @@ -104,7 +104,7 @@ async function sendWebhook( logger.error({ url, err }, "Webhook dispatch failed"); return { success: false, error: err.message }; } finally { - clearTimeout(timer); + clearTimeout(timeout); } } diff --git a/src/utils/logger.ts b/src/utils/logger.ts index c3d51bd..8e4eeda 100644 --- a/src/utils/logger.ts +++ b/src/utils/logger.ts @@ -1,16 +1,18 @@ import pino from "pino"; -import { config } from "../config/index.js"; import { getRequestId } from "./request-context.js"; +// Reads process.env.NODE_ENV directly rather than importing config/index.ts: +// that module imports this one (to log its own validation warnings/errors), +// so importing config here would form a cycle — ESM leaves the not-yet- +// initialized `config` export as undefined partway through that cycle, +// which crashed every request (and every test that imports anything +// touching this module) before config had finished evaluating. +const nodeEnv = process.env.NODE_ENV ?? "development"; + export const logger = pino({ - level: - config.NODE_ENV === "production" - ? "info" - : config.NODE_ENV === "test" - ? "silent" - : "debug", + level: nodeEnv === "production" ? "info" : nodeEnv === "test" ? "silent" : "debug", transport: - config.NODE_ENV !== "production" + nodeEnv !== "production" ? { target: "pino-pretty", options: { colorize: true } } : undefined, serializers: { diff --git a/tests/unit/config/redis.test.ts b/tests/unit/config/redis.test.ts new file mode 100644 index 0000000..5b9bb09 --- /dev/null +++ b/tests/unit/config/redis.test.ts @@ -0,0 +1,132 @@ +import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; + +/** + * #607: config/redis.ts constructs a single ioredis client from + * config.REDIS_URL at import time (same memoize-on-import pattern as + * config/index.ts), so each case resets the module registry and re-imports + * with a fresh env/mock rather than relying on the already-imported client. + */ + +const mockRedisInstance = { + on: vi.fn(), + quit: vi.fn().mockResolvedValue("OK"), +}; + +const RedisConstructorMock = vi.fn(() => mockRedisInstance); + +vi.mock("ioredis", () => ({ + default: RedisConstructorMock, +})); + +vi.mock("../../../src/utils/logger.js", () => ({ + logger: { info: vi.fn(), error: vi.fn(), warn: vi.fn() }, +})); + +async function loadRedisConfig(env: Record = {}) { + vi.resetModules(); + RedisConstructorMock.mockClear(); + mockRedisInstance.on.mockClear(); + mockRedisInstance.quit.mockClear(); + + for (const [key, value] of Object.entries(env)) { + if (value === undefined) { + vi.stubEnv(key, ""); + delete process.env[key]; + } else { + vi.stubEnv(key, value); + } + } + + return import("../../../src/config/redis.js"); +} + +describe("config/redis.ts (#607)", () => { + beforeEach(() => { + vi.unstubAllEnvs(); + }); + + afterEach(() => { + vi.unstubAllEnvs(); + vi.resetModules(); + }); + + describe("client creation", () => { + it("constructs a single ioredis client from config.REDIS_URL", async () => { + await loadRedisConfig({ REDIS_URL: "redis://custom-host:6380" }); + + expect(RedisConstructorMock).toHaveBeenCalledOnce(); + expect(RedisConstructorMock).toHaveBeenCalledWith( + "redis://custom-host:6380", + expect.objectContaining({ maxRetriesPerRequest: 3 }) + ); + }); + + it("falls back to the default redis://localhost:6379 when REDIS_URL is unset", async () => { + await loadRedisConfig({ REDIS_URL: undefined }); + + expect(RedisConstructorMock).toHaveBeenCalledWith( + "redis://localhost:6379", + expect.anything() + ); + }); + + it("exports the constructed client as `redis`", async () => { + const { redis } = await loadRedisConfig(); + + expect(redis).toBe(mockRedisInstance); + }); + + it("registers error and connect event handlers on the client", async () => { + await loadRedisConfig(); + + const registeredEvents = mockRedisInstance.on.mock.calls.map((call) => call[0]); + expect(registeredEvents).toContain("error"); + expect(registeredEvents).toContain("connect"); + }); + }); + + describe("retryStrategy", () => { + it("backs off linearly, capped at 2000ms", async () => { + await loadRedisConfig(); + + const options = RedisConstructorMock.mock.calls[0]![1] as { + retryStrategy: (times: number) => number | null; + }; + + expect(options.retryStrategy(1)).toBe(200); + expect(options.retryStrategy(5)).toBe(1000); + // 8 * 200 = 1600 + expect(options.retryStrategy(8)).toBe(1600); + // 10 * 200 = 2000, exactly at the cap + expect(options.retryStrategy(10)).toBe(2000); + }); + + it("gives up (returns null) after more than 10 attempts", async () => { + await loadRedisConfig(); + + const options = RedisConstructorMock.mock.calls[0]![1] as { + retryStrategy: (times: number) => number | null; + }; + + expect(options.retryStrategy(10)).not.toBeNull(); + expect(options.retryStrategy(11)).toBeNull(); + }); + }); + + describe("closeRedis()", () => { + it("quits the underlying client", async () => { + const { closeRedis } = await loadRedisConfig(); + + await closeRedis(); + + expect(mockRedisInstance.quit).toHaveBeenCalledOnce(); + }); + + it("propagates an error from quit()", async () => { + const { closeRedis } = await loadRedisConfig(); + mockRedisInstance.quit.mockRejectedValueOnce(new Error("quit failed")); + + await expect(closeRedis()).rejects.toThrow("quit failed"); + }); + }); +}); diff --git a/tests/unit/jobs/cleanup-idempotency.test.ts b/tests/unit/jobs/cleanup-idempotency.test.ts new file mode 100644 index 0000000..92e81b1 --- /dev/null +++ b/tests/unit/jobs/cleanup-idempotency.test.ts @@ -0,0 +1,110 @@ +import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; + +/** + * #609: startIdempotencyCleanup/stopIdempotencyCleanup is a thin interval + * wrapper around middleware/idempotency.js's cleanupIdempotencyKeys(), which + * already has its own deletion-logic tests in + * tests/unit/services/idempotency.test.ts. This suite covers the job + * wrapper itself: that it schedules on an interval, can be stopped, and + * doesn't crash the process when the underlying cleanup rejects. + */ + +const mockCleanupIdempotencyKeys = vi.fn(); + +vi.mock("../../../src/middleware/idempotency.js", () => ({ + cleanupIdempotencyKeys: mockCleanupIdempotencyKeys, +})); + +vi.mock("../../../src/utils/logger.js", () => ({ + logger: { info: vi.fn(), error: vi.fn() }, +})); + +const { startIdempotencyCleanup, stopIdempotencyCleanup } = await import( + "../../../src/jobs/cleanup-idempotency.js" +); +const { logger } = await import("../../../src/utils/logger.js"); + +describe("cleanup-idempotency job (#609)", () => { + beforeEach(() => { + vi.useFakeTimers(); + vi.clearAllMocks(); + mockCleanupIdempotencyKeys.mockResolvedValue(0); + }); + + afterEach(() => { + stopIdempotencyCleanup(); + vi.useRealTimers(); + }); + + it("schedules cleanup on an hourly interval", async () => { + startIdempotencyCleanup(); + + expect(mockCleanupIdempotencyKeys).not.toHaveBeenCalled(); + + await vi.advanceTimersByTimeAsync(60 * 60 * 1000); + + expect(mockCleanupIdempotencyKeys).toHaveBeenCalledOnce(); + }); + + it("runs again on the next interval tick", async () => { + startIdempotencyCleanup(); + + await vi.advanceTimersByTimeAsync(60 * 60 * 1000); + await vi.advanceTimersByTimeAsync(60 * 60 * 1000); + + expect(mockCleanupIdempotencyKeys).toHaveBeenCalledTimes(2); + }); + + it("logs the deleted count when keys were actually deleted", async () => { + mockCleanupIdempotencyKeys.mockResolvedValue(5); + startIdempotencyCleanup(); + + await vi.advanceTimersByTimeAsync(60 * 60 * 1000); + + expect(logger.info).toHaveBeenCalledWith( + { deleted: 5 }, + "Cleaned up expired idempotency keys" + ); + }); + + it("does not log when nothing was deleted", async () => { + mockCleanupIdempotencyKeys.mockResolvedValue(0); + startIdempotencyCleanup(); + + await vi.advanceTimersByTimeAsync(60 * 60 * 1000); + + expect(logger.info).not.toHaveBeenCalled(); + }); + + it("logs and does not throw when cleanupIdempotencyKeys rejects", async () => { + mockCleanupIdempotencyKeys.mockRejectedValue(new Error("db unavailable")); + startIdempotencyCleanup(); + + await vi.advanceTimersByTimeAsync(60 * 60 * 1000); + + expect(logger.error).toHaveBeenCalledWith( + { err: expect.any(Error) }, + "Failed to clean up idempotency keys" + ); + }); + + it("stops scheduling further runs once stopped", async () => { + startIdempotencyCleanup(); + + await vi.advanceTimersByTimeAsync(60 * 60 * 1000); + expect(mockCleanupIdempotencyKeys).toHaveBeenCalledTimes(1); + + stopIdempotencyCleanup(); + + await vi.advanceTimersByTimeAsync(60 * 60 * 1000); + expect(mockCleanupIdempotencyKeys).toHaveBeenCalledTimes(1); + }); + + it("stopping before starting (or stopping twice) does not throw", () => { + expect(() => stopIdempotencyCleanup()).not.toThrow(); + + startIdempotencyCleanup(); + stopIdempotencyCleanup(); + expect(() => stopIdempotencyCleanup()).not.toThrow(); + }); +}); diff --git a/tests/unit/notifications/cleanup-notifications.test.ts b/tests/unit/jobs/cleanup-notifications.test.ts similarity index 85% rename from tests/unit/notifications/cleanup-notifications.test.ts rename to tests/unit/jobs/cleanup-notifications.test.ts index c87a98d..c7cc9c5 100644 --- a/tests/unit/notifications/cleanup-notifications.test.ts +++ b/tests/unit/jobs/cleanup-notifications.test.ts @@ -4,13 +4,14 @@ import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; const mockDeleteChain = { where: vi.fn(), + returning: vi.fn(), }; const mockDb = { delete: vi.fn(() => mockDeleteChain) }; vi.mock("../../../src/config/database.js", () => ({ db: mockDb })); vi.mock("../../../src/database/schema.js", () => ({ - notifications: { createdAt: "createdAt" }, + notifications: { createdAt: "createdAt", id: "id" }, })); vi.mock("../../../src/utils/logger.js", () => ({ logger: { info: vi.fn(), error: vi.fn() }, @@ -24,20 +25,22 @@ const { cleanupExpiredNotifications, startNotificationCleanup, stopNotificationC describe("cleanupExpiredNotifications()", () => { beforeEach(() => { vi.clearAllMocks(); + mockDeleteChain.where.mockReturnValue(mockDeleteChain); }); it("deletes notifications older than 30 days and returns the count", async () => { - mockDeleteChain.where.mockResolvedValue([{ id: "a" }, { id: "b" }]); + mockDeleteChain.returning.mockResolvedValue([{ id: "a" }, { id: "b" }]); const deleted = await cleanupExpiredNotifications(); expect(deleted).toBe(2); expect(mockDb.delete).toHaveBeenCalledOnce(); expect(mockDeleteChain.where).toHaveBeenCalledOnce(); + expect(mockDeleteChain.returning).toHaveBeenCalledOnce(); }); it("returns 0 when no notifications are older than 30 days", async () => { - mockDeleteChain.where.mockResolvedValue([]); + mockDeleteChain.returning.mockResolvedValue([]); const deleted = await cleanupExpiredNotifications(); @@ -45,7 +48,7 @@ describe("cleanupExpiredNotifications()", () => { }); it("passes a cutoff date that is approximately 30 days ago", async () => { - mockDeleteChain.where.mockResolvedValue([]); + mockDeleteChain.returning.mockResolvedValue([]); const before = Date.now(); await cleanupExpiredNotifications(); @@ -66,7 +69,7 @@ describe("cleanupExpiredNotifications()", () => { }); it("propagates database errors", async () => { - mockDeleteChain.where.mockRejectedValue(new Error("DB error")); + mockDeleteChain.returning.mockRejectedValue(new Error("DB error")); await expect(cleanupExpiredNotifications()).rejects.toThrow("DB error"); }); @@ -75,7 +78,8 @@ describe("cleanupExpiredNotifications()", () => { describe("startNotificationCleanup / stopNotificationCleanup", () => { beforeEach(() => { vi.useFakeTimers(); - mockDeleteChain.where.mockResolvedValue([]); + mockDeleteChain.where.mockReturnValue(mockDeleteChain); + mockDeleteChain.returning.mockResolvedValue([]); }); afterEach(() => { diff --git a/tests/unit/jobs/process-webhook-retries.test.ts b/tests/unit/jobs/process-webhook-retries.test.ts new file mode 100644 index 0000000..cfc6496 --- /dev/null +++ b/tests/unit/jobs/process-webhook-retries.test.ts @@ -0,0 +1,126 @@ +import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; + +/** + * #611: startWebhookRetryProcessor/stopWebhookRetryProcessor is a thin + * interval wrapper around webhook-dispatcher.js's processWebhookRetries(), + * which does the actual ready-for-retry query, dispatch, and + * success/failure bookkeeping against the database. This suite covers the + * job wrapper itself: that it schedules on an interval, can be stopped + * (including the "stop mid-flight, a tick already queued must not run" + * case via the generation counter), and doesn't crash the process when the + * underlying retry pass rejects. + */ + +const mockProcessWebhookRetries = vi.fn(); + +vi.mock("../../../src/services/webhook-dispatcher.js", () => ({ + processWebhookRetries: mockProcessWebhookRetries, +})); + +vi.mock("../../../src/utils/logger.js", () => ({ + logger: { info: vi.fn(), error: vi.fn(), warn: vi.fn() }, +})); + +const { startWebhookRetryProcessor, stopWebhookRetryProcessor } = await import( + "../../../src/jobs/process-webhook-retries.js" +); +const { logger } = await import("../../../src/utils/logger.js"); + +const POLL_INTERVAL_MS = 60_000; + +describe("process-webhook-retries job (#611)", () => { + beforeEach(() => { + vi.useFakeTimers(); + vi.clearAllMocks(); + mockProcessWebhookRetries.mockResolvedValue(undefined); + }); + + afterEach(() => { + stopWebhookRetryProcessor(); + vi.useRealTimers(); + }); + + it("runs an immediate pass on start, before the first interval tick", async () => { + await startWebhookRetryProcessor(); + + expect(mockProcessWebhookRetries).toHaveBeenCalledOnce(); + }); + + it("schedules the next pass on a 1 minute interval", async () => { + await startWebhookRetryProcessor(); + mockProcessWebhookRetries.mockClear(); + + await vi.advanceTimersByTimeAsync(POLL_INTERVAL_MS); + + expect(mockProcessWebhookRetries).toHaveBeenCalledOnce(); + }); + + it("is a no-op if already running", async () => { + await startWebhookRetryProcessor(); + mockProcessWebhookRetries.mockClear(); + + await startWebhookRetryProcessor(); + + expect(mockProcessWebhookRetries).not.toHaveBeenCalled(); + }); + + it("logs and does not throw when processWebhookRetries rejects", async () => { + mockProcessWebhookRetries.mockRejectedValueOnce(new Error("db unavailable")); + + await expect(startWebhookRetryProcessor()).resolves.not.toThrow(); + expect(logger.error).toHaveBeenCalledWith( + { err: expect.any(Error) }, + "Webhook retry processor tick failed" + ); + }); + + it("keeps polling on the interval even after a tick's pass rejected", async () => { + mockProcessWebhookRetries.mockRejectedValueOnce(new Error("transient")); + await startWebhookRetryProcessor(); + mockProcessWebhookRetries.mockClear(); + mockProcessWebhookRetries.mockResolvedValue(undefined); + + await vi.advanceTimersByTimeAsync(POLL_INTERVAL_MS); + + expect(mockProcessWebhookRetries).toHaveBeenCalledOnce(); + }); + + it("stops scheduling further passes once stopped", async () => { + await startWebhookRetryProcessor(); + mockProcessWebhookRetries.mockClear(); + + stopWebhookRetryProcessor(); + + await vi.advanceTimersByTimeAsync(POLL_INTERVAL_MS * 2); + + expect(mockProcessWebhookRetries).not.toHaveBeenCalled(); + }); + + it("allows starting again after being stopped", async () => { + await startWebhookRetryProcessor(); + stopWebhookRetryProcessor(); + mockProcessWebhookRetries.mockClear(); + + await startWebhookRetryProcessor(); + + expect(mockProcessWebhookRetries).toHaveBeenCalledOnce(); + }); + + it("a tick already scheduled before stop() does not run after stop()", async () => { + await startWebhookRetryProcessor(); + mockProcessWebhookRetries.mockClear(); + + // Advance partway into the interval so a timer is pending, then stop. + await vi.advanceTimersByTimeAsync(POLL_INTERVAL_MS - 1); + stopWebhookRetryProcessor(); + await vi.advanceTimersByTimeAsync(10_000); + + expect(mockProcessWebhookRetries).not.toHaveBeenCalled(); + }); + + it("stopping before starting (or stopping twice) does not throw", () => { + expect(() => stopWebhookRetryProcessor()).not.toThrow(); + stopWebhookRetryProcessor(); + expect(() => stopWebhookRetryProcessor()).not.toThrow(); + }); +});