Merge pull request 'test(integration): say so when the database loses its schema (#154)' (#278) from fix/154-name-the-schema-loss into main

Reviewed-on: #278

backend/tests/integration/schemaLoss.integration.test.ts

@@ -0,0 +1,76 @@
+import { pool } from '../../src/db';
+import { assertSchemaPresent, migrate, resetDb, testPool, closeDb } from './setup/testDb';
+
+/**
+ * The diagnostic for #154, exercised against a database that has actually lost
+ * its schema.
+ *
+ * The failure this guards is not hypothetical and not cheap: the integration run
+ * lost its schema partway through, and it presented as 36 assertion errors about
+ * categories, price filters and favourite notifications. The real message —
+ * `relation "items" does not exist` — was further down the same log.
+ *
+ * This suite destroys and rebuilds the schema, so it restores in afterAll and
+ * `migrate()` is what puts it back: dropping the schema takes `pgmigrations`
+ * with it, so node-pg-migrate re-runs everything rather than believing it has
+ * already done so.
+ */
+async function dropEverything(): Promise<void> {
+  await testPool.query('DROP SCHEMA public CASCADE; CREATE SCHEMA public;');
+}
+
+afterAll(async () => {
+  // Put it back before anything else in the run touches it. Ordering matters:
+  // jest runs these in band, so a suite after this one would otherwise find
+  // nothing at all.
+  await migrate();
+  await pool.end();
+  await closeDb();
+});
+
+describe('when the database loses its schema', () => {
+  it('assertSchemaPresent says so, rather than letting the suite guess', async () => {
+    await dropEverything();
+
+    await expect(assertSchemaPresent('in a test')).rejects.toThrow(/has no schema \(in a test\)/);
+
+    await migrate();
+  });
+
+  it('names the tables that are missing', async () => {
+    await dropEverything();
+
+    // The count and a few names, not the whole list: the point is that a reader
+    // can tell at a glance this is a missing schema rather than a logic bug.
+    await expect(assertSchemaPresent('in a test')).rejects.toThrow(/Missing 18 of 18 tables/);
+    await expect(assertSchemaPresent('in a test')).rejects.toThrow(/items/);
+
+    await migrate();
+  });
+
+  it('points at the cause rather than leaving it to be rediscovered', async () => {
+    await dropEverything();
+
+    await expect(assertSchemaPresent('in a test')).rejects.toThrow(/TRUNCATE does not/);
+    await expect(assertSchemaPresent('in a test')).rejects.toThrow(/#154/);
+
+    await migrate();
+  });
+
+  // The path a real run takes: resetDb's TRUNCATE is what fails first, and it
+  // has to turn that into the schema message rather than passing on a bare
+  // "relation does not exist".
+  it('resetDb turns a failed truncate into the schema message', async () => {
+    await dropEverything();
+
+    await expect(resetDb()).rejects.toThrow(/has no schema \(while resetting between tests\)/);
+
+    await migrate();
+  });
+
+  // And it must not swallow unrelated failures behind a schema message.
+  it('says nothing about the schema when the schema is fine', async () => {
+    await expect(assertSchemaPresent('in a test')).resolves.toBeUndefined();
+    await expect(resetDb()).resolves.toBeUndefined();
+  });
+});

backend/tests/integration/setup/globalSetup.ts

@@ -25,7 +25,14 @@ async function waitForDb(retries = 20): Promise<void> {
 
 export default async function globalSetup(): Promise<void> {
   await waitForDb();
-  const { migrate, closeDb } = await import('./testDb');
+  const { migrate, closeDb, assertSchemaPresent } = await import('./testDb');
   await migrate();
+
+  // Cheap, once, and it establishes the fact the rest of the run depends on:
+  // the schema was here when we started. Without it, a run that never had a
+  // schema and a run that lost one midway are indistinguishable from the
+  // failures they produce. See #154.
+  await assertSchemaPresent('immediately after migrating');
+
   await closeDb();
 }

backend/tests/integration/setup/testDb.ts

@@ -26,13 +26,92 @@ export async function migrate(): Promise<void> {
   });
 }
 
+/**
+ * The tables `resetDb` truncates. Also, therefore, exactly the set whose absence
+ * makes this suite meaningless.
+ */
+const REQUIRED_TABLES = [
+  'admin_settings',
+  'cart_items',
+  'carts',
+  'categories',
+  'checkout_items',
+  'checkouts',
+  'customer_sessions',
+  'customer_tokens',
+  'customers',
+  'favorites',
+  'item_drafts',
+  'item_images',
+  'item_tags',
+  'items',
+  'orders',
+  'shipping_addresses',
+  'tags',
+  'upload_links'
+] as const;
+
+/** Which of them the database does not currently have. */
+async function missingTables(): Promise<string[]> {
+  const { rows } = await testPool.query<{ table_name: string }>(
+    `SELECT table_name FROM information_schema.tables
+     WHERE table_schema = 'public' AND table_type = 'BASE TABLE'`
+  );
+  const present = new Set(rows.map((row) => row.table_name));
+  return REQUIRED_TABLES.filter((name) => !present.has(name));
+}
+
+/**
+ * Says plainly when the schema has gone, rather than letting the suite report
+ * it as unrelated logic failures.
+ *
+ * In #154 the integration run lost its schema partway through — a Postgres
+ * service container recreated mid-run comes back with an empty data directory —
+ * and it presented as 36 assertion errors about categories, price filters and
+ * favourite notifications. The real message, `relation "items" does not exist`,
+ * was further down the same log. Hours went into chasing the assertions.
+ *
+ * The cause is still open and needs runner-side evidence. This is the half that
+ * can be fixed from here: whatever the cause, the next occurrence should read as
+ * "the database lost its schema" on the first line.
+ */
+export async function assertSchemaPresent(context: string): Promise<void> {
+  const missing = await missingTables();
+  if (missing.length === 0) return;
+
+  throw new Error(
+    `The test database has no schema (${context}).
+
+` +
+      `Missing ${missing.length} of ${REQUIRED_TABLES.length} tables: ${missing.join(', ')}.
+
+` +
+      `Migrations ran at the start of this run, so the schema existed and has since gone. ` +
+      `Nothing in this suite drops tables — TRUNCATE does not — so the database itself was ` +
+      `replaced or restarted underneath the run. On CI the likeliest cause is the Postgres ` +
+      `service container being recreated, which comes back with an empty data directory. ` +
+      `See #154.`
+  );
+}
+
 export async function resetDb(): Promise<void> {
-  await testPool.query(`
-    TRUNCATE TABLE item_drafts, upload_links, orders, checkout_items, checkouts,
-      shipping_addresses, cart_items, carts, customer_tokens, customer_sessions, favorites,
-      customers, item_tags, item_images, items, tags, categories
-    RESTART IDENTITY CASCADE
-  `);
+  // The schema check runs only when the truncate fails, not on every reset.
+  // resetDb runs in a beforeEach several hundred times a suite, and an extra
+  // round trip each time to guard against something that has happened once
+  // would be paying continuously for a rare event. A failure here is where the
+  // information is worth having.
+  try {
+    await testPool.query(`
+      TRUNCATE TABLE item_drafts, upload_links, orders, checkout_items, checkouts,
+        shipping_addresses, cart_items, carts, customer_tokens, customer_sessions, favorites,
+        customers, item_tags, item_images, items, tags, categories
+      RESTART IDENTITY CASCADE
+    `);
+  } catch (err) {
+    await assertSchemaPresent('while resetting between tests');
+    // The schema is there, so this is not #154 — let the original speak.
+    throw err;
+  }
 
   // admin_settings is not truncated — it holds the seeded cart_expiry_hours
   // default that other suites read. But the email template rows in it are test