From b09a6d2df8ba5eab8d68cd6b37a904e21ca7df03 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Thu, 22 Oct 2026 23:15:43 +0200 Subject: [PATCH] test: fail a test whose code caught and logged a missing mock export A module mock that leaves out an export the code under test reaches throws at the call, and where that code catches and logs (a best-effort tail, a fail-soft read) the test went on green while the path it meant to cover never ran. The Apple Health import tests mocked the rollup module without its fold window, so the import tail's rollup fold and cache invalidation never ran in any of them; four route test files mocked the logging context or the rollups without what the measurement tail reaches. Both setup files now watch the console and fail the test in which such a line was logged. The ECG import test keeps the real rollup module and stubs only the fold, and the four unit mocks gain what they lacked. --- src/__tests__/swallowed-mock-errors.test.ts | 27 ++++++++++ src/app/api/import/__tests__/route.test.ts | 3 +- .../api/import/csv/__tests__/route.test.ts | 3 +- .../__tests__/arrival-wiring.test.ts | 6 ++- .../by-external-ids/__tests__/route.test.ts | 1 + .../apple-health-import-ecg.test.ts | 8 ++- tests/integration/environment-setup.ts | 5 ++ tests/support/swallowed-mock-errors.ts | 52 +++++++++++++++++++ vitest.setup.ts | 6 +++ 9 files changed, 107 insertions(+), 4 deletions(-) create mode 100644 src/__tests__/swallowed-mock-errors.test.ts create mode 100644 tests/support/swallowed-mock-errors.ts diff --git a/src/__tests__/swallowed-mock-errors.test.ts b/src/__tests__/swallowed-mock-errors.test.ts new file mode 100644 index 000000000..671b6cbcf --- /dev/null +++ b/src/__tests__/swallowed-mock-errors.test.ts @@ -0,0 +1,27 @@ +/** + * The setup files fail a test that logged a caught "missing mock export" + * error (`tests/support/swallowed-mock-errors.ts`). Pinned in both + * directions, so the check cannot be a no-op. + */ +import { describe, expect, it } from "vitest"; + +import { watchSwallowedMockErrors } from "../../tests/support/swallowed-mock-errors"; + +describe("the swallowed mock-export check", () => { + it("fails on a caught missing-export error that was only logged", () => { + const check = watchSwallowedMockErrors(); + console.warn( + "[import] Rollup recompute failed", + new Error( + '[vitest] No "ROLLUP_FOLD_WINDOW_MS" export is defined on the "@/lib/rollups/measurement-rollups" mock. Did you forget to return it from "vi.mock"?', + ), + ); + expect(check).toThrow(/ROLLUP_FOLD_WINDOW_MS/); + }); + + it("lets every other log line through", () => { + const check = watchSwallowedMockErrors(); + console.warn("[import] Rollup recompute failed", new Error("db down")); + expect(check).not.toThrow(); + }); +}); diff --git a/src/app/api/import/__tests__/route.test.ts b/src/app/api/import/__tests__/route.test.ts index ef9e668fe..0c334a71e 100644 --- a/src/app/api/import/__tests__/route.test.ts +++ b/src/app/api/import/__tests__/route.test.ts @@ -29,7 +29,8 @@ vi.mock("@/lib/auth/audit", () => ({ auditLog: vi.fn().mockResolvedValue(undefined), })); -vi.mock("@/lib/logging/context", () => ({ +vi.mock("@/lib/logging/context", async (importOriginal) => ({ + ...(await importOriginal()), annotate: vi.fn(), })); diff --git a/src/app/api/import/csv/__tests__/route.test.ts b/src/app/api/import/csv/__tests__/route.test.ts index 8ce841f24..7dd1431a7 100644 --- a/src/app/api/import/csv/__tests__/route.test.ts +++ b/src/app/api/import/csv/__tests__/route.test.ts @@ -50,7 +50,8 @@ vi.mock("@/lib/auth/audit", () => ({ auditLog: vi.fn().mockResolvedValue(undefined), })); -vi.mock("@/lib/logging/context", () => ({ +vi.mock("@/lib/logging/context", async (importOriginal) => ({ + ...(await importOriginal()), annotate: vi.fn(), })); diff --git a/src/app/api/measurements/__tests__/arrival-wiring.test.ts b/src/app/api/measurements/__tests__/arrival-wiring.test.ts index ee77a756a..ded4e7f73 100644 --- a/src/app/api/measurements/__tests__/arrival-wiring.test.ts +++ b/src/app/api/measurements/__tests__/arrival-wiring.test.ts @@ -52,7 +52,10 @@ vi.mock("@/lib/db", () => { }); vi.mock("@/lib/auth/audit", () => ({ auditLog: vi.fn() })); -vi.mock("@/lib/logging/context", () => ({ annotate: vi.fn() })); +vi.mock("@/lib/logging/context", async (importOriginal) => ({ + ...(await importOriginal()), + annotate: vi.fn(), +})); vi.mock("@/lib/logging/fire-and-forget", () => ({ fireAndForget: (promise: Promise) => void promise.catch(() => {}), })); @@ -83,6 +86,7 @@ vi.mock("@/lib/illness/safety-floor-check", () => ({ })); vi.mock("@/lib/rollups/measurement-rollups", () => ({ recomputeBucketsForMeasurement: vi.fn(async () => {}), + recomputeBucketsForKeys: vi.fn(async () => {}), collapseToTypeDayKeys: vi.fn( (rows: Array<{ type: string; measuredAt: Date }>) => rows, ), diff --git a/src/app/api/measurements/by-external-ids/__tests__/route.test.ts b/src/app/api/measurements/by-external-ids/__tests__/route.test.ts index afb091c8e..5cfee6105 100644 --- a/src/app/api/measurements/by-external-ids/__tests__/route.test.ts +++ b/src/app/api/measurements/by-external-ids/__tests__/route.test.ts @@ -41,6 +41,7 @@ vi.mock("@/lib/rollups/measurement-rollups", async () => { return { collapseToTypeDayKeys: actual.collapseToTypeDayKeys, recomputeBucketsForMeasurement: vi.fn().mockResolvedValue(undefined), + recomputeBucketsForKeys: vi.fn().mockResolvedValue(undefined), }; }); vi.mock("@/lib/logging/transports", () => ({ emitIfSampled: vi.fn() })); diff --git a/tests/integration/apple-health-import-ecg.test.ts b/tests/integration/apple-health-import-ecg.test.ts index 2e5ad1c15..2bd581f7d 100644 --- a/tests/integration/apple-health-import-ecg.test.ts +++ b/tests/integration/apple-health-import-ecg.test.ts @@ -31,7 +31,13 @@ vi.mock("@/lib/jobs/boss-instance", () => ({ getGlobalBoss: vi.fn(() => null), })); -vi.mock("@/lib/rollups/measurement-rollups", () => ({ +// The real module with only the fold stubbed, so the import's tail (its fold +// window, the cache invalidation, the status re-warm) runs as it does in +// production. +vi.mock("@/lib/rollups/measurement-rollups", async (importOriginal) => ({ + ...(await importOriginal< + typeof import("@/lib/rollups/measurement-rollups") + >()), recomputeUserRollups: vi.fn(async () => {}), })); diff --git a/tests/integration/environment-setup.ts b/tests/integration/environment-setup.ts index 601b15715..ac64aff72 100644 --- a/tests/integration/environment-setup.ts +++ b/tests/integration/environment-setup.ts @@ -1,6 +1,7 @@ import { afterEach, beforeEach, inject, vi } from "vitest"; import { quarantineEntryFor } from "../quarantine/quarantine"; +import { watchSwallowedMockErrors } from "../support/swallowed-mock-errors"; import { prepareWorkerDatabase } from "./worker-database"; // Each worker runs against its own clone of the migrated template (see @@ -30,10 +31,14 @@ process.env.SESSION_SECRET = "integration-test-session-secret-32-bytes"; * the worker BEFORE any application module reads it at import time, and a * static import here would pull part of the application tree in first. */ +/** A swallowed "missing mock export" error fails its test (see the module). */ +const checkSwallowedMockErrors = watchSwallowedMockErrors(); + afterEach(async () => { const { settleBackgroundTasks } = await import("@/lib/logging/background-tasks"); await settleBackgroundTasks(); + checkSwallowedMockErrors(); }); /** The flaky-test quarantine, as in the unit setup (`vitest.setup.ts`). */ diff --git a/tests/support/swallowed-mock-errors.ts b/tests/support/swallowed-mock-errors.ts new file mode 100644 index 000000000..ae250eafe --- /dev/null +++ b/tests/support/swallowed-mock-errors.ts @@ -0,0 +1,52 @@ +/** + * A module mock that leaves out an export the code under test reaches throws + * `No "X" export is defined on the "Y" mock` at the call. Where the code + * catches and logs that (a best-effort tail, a fail-soft read), the test went + * on green while the code path it meant to cover never ran: the Apple Health + * import tests mocked the rollup module without its fold window, and the + * import's cache invalidation never ran in any of them. + * + * `watchSwallowedMockErrors` wraps the console once per worker and returns + * the check a setup file runs after each test: it throws, failing that test, + * when such a line was logged during it. + */ +const MISSING_MOCK_EXPORT = /No "[^"]+" export is defined on the "[^"]+" mock/; + +function textOf(arg: unknown): string { + if (arg instanceof Error) return `${arg.message} ${String(arg.cause ?? "")}`; + return typeof arg === "string" ? arg : ""; +} + +const SEEN = Symbol.for("healthlog.tests.swallowedMockErrors"); + +export function watchSwallowedMockErrors(): () => void { + // A setup file runs once per test file, and a worker runs many files: the + // console is wrapped once per worker, and every file shares what it saw. + const slot = globalThis as unknown as { [SEEN]?: string[] }; + let seen = slot[SEEN]; + if (!seen) { + const lines: string[] = []; + seen = lines; + slot[SEEN] = lines; + for (const level of ["log", "info", "warn", "error"] as const) { + const original = console[level].bind(console); + console[level] = (...args: unknown[]) => { + for (const arg of args) { + const match = MISSING_MOCK_EXPORT.exec(textOf(arg)); + if (match) lines.push(match[0]); + } + original(...args); + }; + } + } + const lines = seen; + return () => { + if (lines.length === 0) return; + const found = [...new Set(lines)]; + lines.length = 0; + throw new Error( + "a module mock lacks an export the code under test reached, and the " + + `error was caught and logged instead of failing: ${found.join("; ")}`, + ); + }; +} diff --git a/vitest.setup.ts b/vitest.setup.ts index 7b397bdaa..cb9053f6e 100644 --- a/vitest.setup.ts +++ b/vitest.setup.ts @@ -21,6 +21,7 @@ import { afterEach, beforeEach, vi } from "vitest"; import { settleBackgroundTasks } from "@/lib/logging/background-tasks"; import { quarantineEntryFor } from "./tests/quarantine/quarantine"; +import { watchSwallowedMockErrors } from "./tests/support/swallowed-mock-errors"; import { primeMessages } from "@/lib/i18n/load-locale"; import enMessages from "./messages/en.json"; @@ -41,8 +42,13 @@ primeMessages("en", enMessages); * console, which would have thrown away a real signal and left the * non-determinism in place. See `src/lib/logging/background-tasks.ts`. */ +const checkSwallowedMockErrors = watchSwallowedMockErrors(); + afterEach(async () => { await settleBackgroundTasks(); + // A caught "missing mock export" error fails the test it happened in + // (`tests/support/swallowed-mock-errors.ts`). + checkSwallowedMockErrors(); }); /**