Merge pull request 'test: give Core's import-safety tests a timeout that holds under full verify load' (#75) from test/import-safety-timeouts into main

Reviewed-on: https://git.riverofglorymn.org/River_of_Glory_Church/Ekklesia/pulls/75
Reviewed-by: The_River <[email protected]>

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 () => {
+  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,
+  );
 });

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 () => {
+  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 () => {
     // The password is prompted for or read from standard input; it is deliberately never taken from

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 () => {
+  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,
+  );
 });

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 () => {
+  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,
+  );
 });

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;