test: give Core's import-safety tests a timeout that holds under full verify load
CI / verify (pull_request) Successful in 1m5s
CI / infrastructure-smoke (pull_request) Successful in 37s
CI / migration-smoke (pull_request) Successful in 1m55s
CI / image-build (pull_request) Successful in 24s
CI / deploy-smoke (same-origin) (pull_request) Successful in 49s
CI / deploy-smoke (split) (pull_request) Successful in 1m31s
CI / verify (pull_request) Successful in 1m5s
CI / infrastructure-smoke (pull_request) Successful in 37s
CI / migration-smoke (pull_request) Successful in 1m55s
CI / image-build (pull_request) Successful in 24s
CI / deploy-smoke (same-origin) (pull_request) Successful in 49s
CI / deploy-smoke (split) (pull_request) Successful in 1m31s
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 <[email protected]>
This commit is contained in:
co-authored by
Claude Opus 5.5
parent
b1e3244c1d
commit
796ad4f16e
@@ -1,4 +1,5 @@
|
|||||||
import { describe, expect, it, vi } from "vitest";
|
import { describe, expect, it, vi } from "vitest";
|
||||||
|
import { IMPORT_SAFETY_TEST_TIMEOUT_MS } from "./support/import-safety.js";
|
||||||
|
|
||||||
const poolConstructor = vi.hoisted(() => vi.fn());
|
const poolConstructor = vi.hoisted(() => vi.fn());
|
||||||
|
|
||||||
@@ -7,16 +8,20 @@ vi.mock("pg", () => ({ Pool: poolConstructor }));
|
|||||||
describe("bootstrap-platform-owner CLI import safety", () => {
|
describe("bootstrap-platform-owner CLI import safety", () => {
|
||||||
const originalEnv = { ...process.env };
|
const originalEnv = { ...process.env };
|
||||||
|
|
||||||
it("does not read DATABASE_URL, connect to PostgreSQL, or run bootstrap merely by importing the module", async () => {
|
it(
|
||||||
delete process.env.DATABASE_URL;
|
"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
|
// 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 —
|
// 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.
|
// no argv manipulation is needed to prove the guard holds.
|
||||||
await expect(import("../src/cli/bootstrap-platform-owner.js")).resolves.toBeDefined();
|
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,
|
||||||
|
);
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -1,4 +1,5 @@
|
|||||||
import { describe, expect, it, vi } from "vitest";
|
import { describe, expect, it, vi } from "vitest";
|
||||||
|
import { IMPORT_SAFETY_TEST_TIMEOUT_MS } from "./support/import-safety.js";
|
||||||
|
|
||||||
const poolConstructor = vi.hoisted(() => vi.fn());
|
const poolConstructor = vi.hoisted(() => vi.fn());
|
||||||
|
|
||||||
@@ -7,18 +8,22 @@ vi.mock("pg", () => ({ Pool: poolConstructor }));
|
|||||||
describe("create-initial-owner CLI import safety", () => {
|
describe("create-initial-owner CLI import safety", () => {
|
||||||
const originalEnv = { ...process.env };
|
const originalEnv = { ...process.env };
|
||||||
|
|
||||||
it("does not read DATABASE_URL, connect to PostgreSQL, or create anything merely by importing the module", async () => {
|
it(
|
||||||
delete process.env.DATABASE_URL;
|
"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
|
// 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
|
// 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.
|
// the guard holds. Importing the command must never prompt, connect, or write.
|
||||||
await expect(import("../src/cli/create-initial-owner.js")).resolves.toBeDefined();
|
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 () => {
|
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
|
// The password is prompted for or read from standard input; it is deliberately never taken from
|
||||||
|
|||||||
@@ -1,4 +1,5 @@
|
|||||||
import { describe, expect, it, vi } from "vitest";
|
import { describe, expect, it, vi } from "vitest";
|
||||||
|
import { IMPORT_SAFETY_TEST_TIMEOUT_MS } from "./support/import-safety.js";
|
||||||
|
|
||||||
const discoverModules = vi.fn();
|
const discoverModules = vi.fn();
|
||||||
const discoverModuleSchemas = vi.fn();
|
const discoverModuleSchemas = vi.fn();
|
||||||
@@ -22,24 +23,28 @@ vi.mock("../src/migrations/index.js", async (importOriginal) => ({
|
|||||||
}));
|
}));
|
||||||
|
|
||||||
describe("Doctor import safety", () => {
|
describe("Doctor import safety", () => {
|
||||||
it("does not discover, connect, plan, or print when imported", async () => {
|
it(
|
||||||
const log = vi.spyOn(console, "log").mockImplementation(() => undefined);
|
"does not discover, connect, plan, or print when imported",
|
||||||
const error = vi.spyOn(console, "error").mockImplementation(() => undefined);
|
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/diagnostics/index.js");
|
||||||
await import("../src/cli/doctor.js");
|
await import("../src/cli/doctor.js");
|
||||||
await import("../src/index.js");
|
await import("../src/index.js");
|
||||||
|
|
||||||
expect(discoverModules).not.toHaveBeenCalled();
|
expect(discoverModules).not.toHaveBeenCalled();
|
||||||
expect(discoverModuleSchemas).not.toHaveBeenCalled();
|
expect(discoverModuleSchemas).not.toHaveBeenCalled();
|
||||||
expect(discoverModuleMigrations).not.toHaveBeenCalled();
|
expect(discoverModuleMigrations).not.toHaveBeenCalled();
|
||||||
expect(startModuleRuntime).not.toHaveBeenCalled();
|
expect(startModuleRuntime).not.toHaveBeenCalled();
|
||||||
expect(createPostgresMigrationDatabase).not.toHaveBeenCalled();
|
expect(createPostgresMigrationDatabase).not.toHaveBeenCalled();
|
||||||
expect(planModuleMigrations).not.toHaveBeenCalled();
|
expect(planModuleMigrations).not.toHaveBeenCalled();
|
||||||
expect(log).not.toHaveBeenCalled();
|
expect(log).not.toHaveBeenCalled();
|
||||||
expect(error).not.toHaveBeenCalled();
|
expect(error).not.toHaveBeenCalled();
|
||||||
|
|
||||||
log.mockRestore();
|
log.mockRestore();
|
||||||
error.mockRestore();
|
error.mockRestore();
|
||||||
});
|
},
|
||||||
|
IMPORT_SAFETY_TEST_TIMEOUT_MS,
|
||||||
|
);
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -1,4 +1,5 @@
|
|||||||
import { describe, expect, it, vi } from "vitest";
|
import { describe, expect, it, vi } from "vitest";
|
||||||
|
import { IMPORT_SAFETY_TEST_TIMEOUT_MS } from "./support/import-safety.js";
|
||||||
|
|
||||||
const readdir = vi.fn();
|
const readdir = vi.fn();
|
||||||
const readFile = 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", () => {
|
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 () => {
|
it(
|
||||||
await import("../src/modules/index.js");
|
"performs no filesystem I/O merely by importing the module runtime or the module-http contract",
|
||||||
await import("@ekklesia/contracts");
|
async () => {
|
||||||
|
await import("../src/modules/index.js");
|
||||||
|
await import("@ekklesia/contracts");
|
||||||
|
|
||||||
expect(readdir).not.toHaveBeenCalled();
|
expect(readdir).not.toHaveBeenCalled();
|
||||||
expect(readFile).not.toHaveBeenCalled();
|
expect(readFile).not.toHaveBeenCalled();
|
||||||
expect(realpath).not.toHaveBeenCalled();
|
expect(realpath).not.toHaveBeenCalled();
|
||||||
expect(stat).not.toHaveBeenCalled();
|
expect(stat).not.toHaveBeenCalled();
|
||||||
});
|
},
|
||||||
|
IMPORT_SAFETY_TEST_TIMEOUT_MS,
|
||||||
|
);
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -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;
|
||||||
Reference in New Issue
Block a user