fix(test): replay full migration chain in source-registry realinfra test
tests/p0/source-registry-postgres.realinfra.test.ts only applied sql/002_source_registry.sql before running assertions. sql/007_ingestion_plane.sql later added opt_sources.owner_id, and src/repositories/postgres-source-repository.ts's INSERT has named that column ever since -- but nothing caught the drift because SOURCE_REGISTRY_TEST_DATABASE_URL is never set in CI (.gitea/workflows/ci.yml has no job that sets it), so this suite has silently skipped in every CI run. Verified against a real disposable Postgres 16 container: before this fix, all 7 tests failed with `column "owner_id" of relation "opt_sources" does not exist`; after replaying every sql/*.sql file in order (same pattern already used by tests/analytics/analytics-domain-postgres.realinfra.test.ts), all 7 pass. Found while auditing 0pt's Analytics/BI layer (MetricFlow/Cube/ClickHouse/ Trino/DuckDB) and standing up real Postgres/ClickHouse containers to verify those claims for real; this bug is adjacent (source registry, not analytics) but was directly encountered and is a small, safe, test-only fix. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
@@ -24,10 +24,24 @@
|
||||
* prove the rows are really there, not an artifact of the service's own
|
||||
* in-process bookkeeping.
|
||||
*
|
||||
* beforeAll applies sql/002_source_registry.sql itself (every statement is
|
||||
* `CREATE TABLE/INDEX IF NOT EXISTS`, so this is idempotent and safe to run
|
||||
* against an already-migrated database too) -- this test does not require
|
||||
* the operator to have pre-run migrations.
|
||||
* beforeAll applies every sql/NNN_*.sql file in order (same restore-drill
|
||||
* pattern as tests/analytics/analytics-domain-postgres.realinfra.test.ts),
|
||||
* not just sql/002_source_registry.sql -- this test does not require the
|
||||
* operator to have pre-run migrations.
|
||||
*
|
||||
* CORRECTED (this pass, 2026-09-14): it used to apply ONLY
|
||||
* sql/002_source_registry.sql. That was enough the day this file was
|
||||
* written, but sql/007_ingestion_plane.sql later added `opt_sources.owner_id`
|
||||
* (`ALTER TABLE opt_sources ADD COLUMN IF NOT EXISTS owner_id TEXT`), and
|
||||
* src/repositories/postgres-source-repository.ts's INSERT has named that
|
||||
* column ever since. Because this env var is never set in CI (confirmed by
|
||||
* grep over .gitea/workflows/ci.yml — no job references
|
||||
* SOURCE_REGISTRY_TEST_DATABASE_URL), the drift was never caught: every
|
||||
* assertion below failed with `column "owner_id" of relation "opt_sources"
|
||||
* does not exist` the moment it was actually run against a real, freshly
|
||||
* migrated-to-002-only Postgres. The fix is to stop hand-picking one
|
||||
* migration file and replay the whole chain, exactly like the analytics
|
||||
* suite already does.
|
||||
*/
|
||||
import { afterAll, beforeAll, describe, expect, it } from "vitest";
|
||||
import fs from "node:fs";
|
||||
@@ -56,9 +70,23 @@ beforeAll(async () => {
|
||||
adminSql`SELECT 1`,
|
||||
new Promise((_, reject) => setTimeout(() => reject(new Error("connect timeout")), 5000)),
|
||||
]);
|
||||
const migrationPath = path.join(process.cwd(), "sql", "002_source_registry.sql");
|
||||
const migration = fs.readFileSync(migrationPath, "utf8");
|
||||
await adminSql.unsafe(migration);
|
||||
// Replay every migration in order, not just 002 -- see the file header
|
||||
// "CORRECTED" note. Tolerate an individual file failing (mirrors
|
||||
// analytics-domain-postgres.realinfra.test.ts: sql/001 needs the
|
||||
// `vector` extension, which a stock postgres:16-alpine does not carry)
|
||||
// as long as the files this test actually depends on (002, 007) apply.
|
||||
const dir = path.join(process.cwd(), "sql");
|
||||
for (const file of fs
|
||||
.readdirSync(dir)
|
||||
.filter((f) => f.endsWith(".sql"))
|
||||
.sort()) {
|
||||
const body = fs.readFileSync(path.join(dir, file), "utf8");
|
||||
try {
|
||||
await adminSql.unsafe(body);
|
||||
} catch (err) {
|
||||
console.warn(`[realinfra] ${file} did not apply cleanly: ${String(err).slice(0, 200)}`);
|
||||
}
|
||||
}
|
||||
reachable = true;
|
||||
} catch (err) {
|
||||
reachable = false;
|
||||
|
||||
Reference in New Issue
Block a user