From 796ad4f16eaf11a39829361184812d3999c5444a Mon Sep 17 00:00:00 2001 From: ekklesia Date: Wed, 7 Oct 2026 19:08:30 -0500 Subject: [PATCH] test: give Core's import-safety tests a timeout that holds under full verify load MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Four Core tests prove that importing a module has no side effects. Each one does a cold dynamic import() of a large Core source graph inside its test body: - doctor-import - module-http-import - cli-bootstrap-platform-owner - cli-create-initial-owner (its first test) Vitest transforms and evaluates that graph on demand, so the CPU-bound cost counts against the default 5 s per-test timeout. Ordinary test files import at the top level, outside any test timeout. There is no subprocess, shared resource, or test bug. Measured on six cores: - alone: 0.7–1.2 s; - alongside five other packages' suites, as in a full pnpm verify: 3.0–5.4 s (doctor-import reached 5,400 ms). A shared IMPORT_SAFETY_TEST_TIMEOUT_MS of 30 s (core/tests/support/ import-safety.ts) now applies to exactly these four tests. That is about five times the slowest time measured under load, the same as the repository's integration-test timeout, and still bounded, so an import that genuinely hangs fails. Assertions, coverage, the global Vitest timeout, parallelism, and production code are unchanged; there are no retries. Co-Authored-By: Claude Opus 5.5 --- .../cli-bootstrap-platform-owner.test.ts | 23 ++++++----- core/tests/cli-create-initial-owner.test.ts | 23 ++++++----- core/tests/doctor-import.test.ts | 39 +++++++++++-------- core/tests/module-http-import.test.ts | 21 ++++++---- core/tests/support/import-safety.ts | 19 +++++++++ 5 files changed, 82 insertions(+), 43 deletions(-) create mode 100644 core/tests/support/import-safety.ts diff --git a/core/tests/cli-bootstrap-platform-owner.test.ts b/core/tests/cli-bootstrap-platform-owner.test.ts index eb069bd..8adef95 100644 --- a/core/tests/cli-bootstrap-platform-owner.test.ts +++ b/core/tests/cli-bootstrap-platform-owner.test.ts @@ -1,4 +1,5 @@ import { describe, expect, it, vi } from "vitest"; +import { IMPORT_SAFETY_TEST_TIMEOUT_MS } from "./support/import-safety.js"; const poolConstructor = vi.hoisted(() => vi.fn()); @@ -7,16 +8,20 @@ vi.mock("pg", () => ({ Pool: poolConstructor })); describe("bootstrap-platform-owner CLI import safety", () => { const originalEnv = { ...process.env }; - it("does not read DATABASE_URL, connect to PostgreSQL, or run bootstrap merely by importing the module", async () => { - delete process.env.DATABASE_URL; + it( + "does not read DATABASE_URL, connect to PostgreSQL, or run bootstrap merely by importing the module", + async () => { + delete process.env.DATABASE_URL; - // Under Vitest, process.argv[1] is never this file's own path, so isDirectExecution() is - // false the same way it naturally is for apps/api/src/server.ts under its own test suite — - // no argv manipulation is needed to prove the guard holds. - await expect(import("../src/cli/bootstrap-platform-owner.js")).resolves.toBeDefined(); + // Under Vitest, process.argv[1] is never this file's own path, so isDirectExecution() is + // false the same way it naturally is for apps/api/src/server.ts under its own test suite — + // no argv manipulation is needed to prove the guard holds. + await expect(import("../src/cli/bootstrap-platform-owner.js")).resolves.toBeDefined(); - expect(poolConstructor).not.toHaveBeenCalled(); + expect(poolConstructor).not.toHaveBeenCalled(); - process.env = originalEnv; - }); + process.env = originalEnv; + }, + IMPORT_SAFETY_TEST_TIMEOUT_MS, + ); }); diff --git a/core/tests/cli-create-initial-owner.test.ts b/core/tests/cli-create-initial-owner.test.ts index ab8b3db..3c26a34 100644 --- a/core/tests/cli-create-initial-owner.test.ts +++ b/core/tests/cli-create-initial-owner.test.ts @@ -1,4 +1,5 @@ import { describe, expect, it, vi } from "vitest"; +import { IMPORT_SAFETY_TEST_TIMEOUT_MS } from "./support/import-safety.js"; const poolConstructor = vi.hoisted(() => vi.fn()); @@ -7,18 +8,22 @@ vi.mock("pg", () => ({ Pool: poolConstructor })); describe("create-initial-owner CLI import safety", () => { const originalEnv = { ...process.env }; - it("does not read DATABASE_URL, connect to PostgreSQL, or create anything merely by importing the module", async () => { - delete process.env.DATABASE_URL; + it( + "does not read DATABASE_URL, connect to PostgreSQL, or create anything merely by importing the module", + async () => { + delete process.env.DATABASE_URL; - // Under Vitest, process.argv[1] is never this file's own path, so isDirectExecution() is false - // exactly as it is for `bootstrap-platform-owner.ts` — no argv manipulation is needed to prove - // the guard holds. Importing the command must never prompt, connect, or write. - await expect(import("../src/cli/create-initial-owner.js")).resolves.toBeDefined(); + // Under Vitest, process.argv[1] is never this file's own path, so isDirectExecution() is false + // exactly as it is for `bootstrap-platform-owner.ts` — no argv manipulation is needed to prove + // the guard holds. Importing the command must never prompt, connect, or write. + await expect(import("../src/cli/create-initial-owner.js")).resolves.toBeDefined(); - expect(poolConstructor).not.toHaveBeenCalled(); + expect(poolConstructor).not.toHaveBeenCalled(); - process.env = originalEnv; - }); + process.env = originalEnv; + }, + IMPORT_SAFETY_TEST_TIMEOUT_MS, + ); it("reads no environment variable for the password", async () => { // The password is prompted for or read from standard input; it is deliberately never taken from diff --git a/core/tests/doctor-import.test.ts b/core/tests/doctor-import.test.ts index 47ebf72..0d6a20f 100644 --- a/core/tests/doctor-import.test.ts +++ b/core/tests/doctor-import.test.ts @@ -1,4 +1,5 @@ import { describe, expect, it, vi } from "vitest"; +import { IMPORT_SAFETY_TEST_TIMEOUT_MS } from "./support/import-safety.js"; const discoverModules = vi.fn(); const discoverModuleSchemas = vi.fn(); @@ -22,24 +23,28 @@ vi.mock("../src/migrations/index.js", async (importOriginal) => ({ })); describe("Doctor import safety", () => { - it("does not discover, connect, plan, or print when imported", async () => { - const log = vi.spyOn(console, "log").mockImplementation(() => undefined); - const error = vi.spyOn(console, "error").mockImplementation(() => undefined); + it( + "does not discover, connect, plan, or print when imported", + async () => { + const log = vi.spyOn(console, "log").mockImplementation(() => undefined); + const error = vi.spyOn(console, "error").mockImplementation(() => undefined); - await import("../src/diagnostics/index.js"); - await import("../src/cli/doctor.js"); - await import("../src/index.js"); + await import("../src/diagnostics/index.js"); + await import("../src/cli/doctor.js"); + await import("../src/index.js"); - expect(discoverModules).not.toHaveBeenCalled(); - expect(discoverModuleSchemas).not.toHaveBeenCalled(); - expect(discoverModuleMigrations).not.toHaveBeenCalled(); - expect(startModuleRuntime).not.toHaveBeenCalled(); - expect(createPostgresMigrationDatabase).not.toHaveBeenCalled(); - expect(planModuleMigrations).not.toHaveBeenCalled(); - expect(log).not.toHaveBeenCalled(); - expect(error).not.toHaveBeenCalled(); + expect(discoverModules).not.toHaveBeenCalled(); + expect(discoverModuleSchemas).not.toHaveBeenCalled(); + expect(discoverModuleMigrations).not.toHaveBeenCalled(); + expect(startModuleRuntime).not.toHaveBeenCalled(); + expect(createPostgresMigrationDatabase).not.toHaveBeenCalled(); + expect(planModuleMigrations).not.toHaveBeenCalled(); + expect(log).not.toHaveBeenCalled(); + expect(error).not.toHaveBeenCalled(); - log.mockRestore(); - error.mockRestore(); - }); + log.mockRestore(); + error.mockRestore(); + }, + IMPORT_SAFETY_TEST_TIMEOUT_MS, + ); }); diff --git a/core/tests/module-http-import.test.ts b/core/tests/module-http-import.test.ts index 1c425e9..d45ce85 100644 --- a/core/tests/module-http-import.test.ts +++ b/core/tests/module-http-import.test.ts @@ -1,4 +1,5 @@ import { describe, expect, it, vi } from "vitest"; +import { IMPORT_SAFETY_TEST_TIMEOUT_MS } from "./support/import-safety.js"; const readdir = vi.fn(); const readFile = vi.fn(); @@ -14,13 +15,17 @@ vi.mock("node:fs/promises", async (importOriginal) => ({ })); describe("module HTTP contract and runtime import safety", () => { - it("performs no filesystem I/O merely by importing the module runtime or the module-http contract", async () => { - await import("../src/modules/index.js"); - await import("@ekklesia/contracts"); + it( + "performs no filesystem I/O merely by importing the module runtime or the module-http contract", + async () => { + await import("../src/modules/index.js"); + await import("@ekklesia/contracts"); - expect(readdir).not.toHaveBeenCalled(); - expect(readFile).not.toHaveBeenCalled(); - expect(realpath).not.toHaveBeenCalled(); - expect(stat).not.toHaveBeenCalled(); - }); + expect(readdir).not.toHaveBeenCalled(); + expect(readFile).not.toHaveBeenCalled(); + expect(realpath).not.toHaveBeenCalled(); + expect(stat).not.toHaveBeenCalled(); + }, + IMPORT_SAFETY_TEST_TIMEOUT_MS, + ); }); diff --git a/core/tests/support/import-safety.ts b/core/tests/support/import-safety.ts new file mode 100644 index 0000000..6b266c3 --- /dev/null +++ b/core/tests/support/import-safety.ts @@ -0,0 +1,19 @@ +/** + * The per-test timeout for Core's import-safety tests, and only those. + * + * Each of these tests proves that importing a module does nothing (no I/O, no connection, no output) + * by performing a cold dynamic `import()` of a large Core source graph inside the test body. Vitest + * transforms and evaluates that whole graph on demand, and that work is CPU-bound. An ordinary test + * file's own top-level imports run before its tests start and fall under no per-test timeout; these + * deliberately do not, so the cost lands inside Vitest's default 5-second test timeout. + * + * Alone they take about 0.7–1.2 s. During a full `pnpm verify`, Turbo runs every package's suite at + * once and the CPU is saturated. Reproduced locally (six cores, Core's suite alongside five other + * packages' suites), the same tests took 3.0–5.4 s, and so timed out intermittently. + * + * 30 seconds is about five times the slowest time measured under load, which leaves room for slower + * shared CI runners. It matches the repository's integration-test timeout. It is still bounded, so a + * module that genuinely hangs while being imported, for example on a network call, still fails. The + * assertions themselves are unchanged: these tests fail on what an import does, never on speed. + */ +export const IMPORT_SAFETY_TEST_TIMEOUT_MS = 30_000; -- 2.54.0