test(integration): say so when the database loses its schema (#154)
When the integration run lost its schema partway through, 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, and hours went into chasing the assertions instead. The cause is still open and needs runner-side evidence this cannot reach: whether the Postgres service container is being recreated mid-run, which would come back with an empty data directory. This is the half that can be fixed from here — whatever the cause, the next occurrence reads as "the database lost its schema" on the first line, names which tables are gone, and says that nothing in the suite drops tables so the database was replaced underneath the run. globalSetup asserts once after migrating, which establishes the fact the rest of the run depends on. Without it, a run that never had a schema and one that lost it midway are indistinguishable from the failures they produce. resetDb checks only when its TRUNCATE fails, rather than on every reset. It runs in a beforeEach several hundred times a suite, and an extra round trip each time to guard against a rare event would be paying continuously for it. When the schema is fine, an unrelated failure is passed through untouched rather than dressed up as a schema problem. The diagnostic is tested against a database that has actually lost its schema, not reasoned about. An earlier attempt dropped the schema before the run and proved nothing — globalSetup re-migrates, so it repaired itself and every test passed. The suite drops and rebuilds around each case and restores in afterAll; the full integration suite was then run twice to confirm the restore holds for everything ordered after it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
Claude Opus 5
@@ -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();
|
||||
});
|
||||
});
|
||||
@@ -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();
|
||||
}
|
||||
|
||||
@@ -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> {
|
||||
// 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
|
||||
|
||||
Reference in New Issue
Block a user