Skip to content

No test ever executes the migration DDL os generate migration emits — both formats are pinned as TEXT only, so a scaffold that cannot run would read green #16279

Description

@os-litant

Observed while delivering #15521 (PR #16278). ⛔ Not a live defect: on the tree measured, both emitted formats DO run. This is a coverage gap, filed so it is not re-discovered.

What is missing

generateMigrationSql and generateMigrationTs are covered by three pin tests — generate-builtin-id-column.pin.test.ts, generate-field-type-vocabulary.pin.test.ts, generate-multiple-json-column.pin.test.ts. Every one of them asserts the emitted string. Nothing anywhere executes the result.

The two neighbouring tests that sound like they close this do not:

test what it actually covers
test/scaffold-emission-typechecks.test.ts GENERATOR_SCAFFOLD_TARGETS — the init/object/field scaffolds. It compiles those under the tsconfig they ship with. The migration output is not among them.
test/generate-scaffold-validates.test.ts [#14087] every os generatescaffold passesos validate`` — metadata scaffolds against the validator. DDL is not metadata and never reaches it.

So the failure mode nothing would catch: a generated migration that is syntactically fine as text and refused by the database, or a db.… call the emitted TypeScript makes that knex does not have. A text pin stays green through both. The emitted TypeScript is additionally typed db: any, so even compiling it would not recover the second half.

Evidence that the gap is real, and that today's output is fine

Delivering #15521 needed exactly this measurement and had to do it by hand, outside the test suite: the emitted migration was imported and its up(db) invoked against a live PostgreSQL 16.13 cluster, and the emitted SQL was executed verbatim, both read back from information_schema.columns. Both ran clean. That run lives in the PR's description, not in CI — nothing reproduces it.

Why it may be worth closing

The repo already has the ingredients: packages/cli/src/utils/schema-migrate.integration.test.ts and its siblings drive real databases, so an "emit, then execute, then read the catalog back" case has somewhere to live and a precedent to copy. That would also give the three existing pin tests a foundation — they currently agree with driver-sql on text, and #15521's whole cost was that matching text and matching catalog rows are two different claims.

Boundary

⛔ Deliberately not attached to #15521 or its PR — that card is ruled and scoped to two columns' nullability and default text, and this is a suite-shaped question about a different layer. Unassigned and unqueued, for triage.

Refs: #15521 · PR #16278 · #14087 (the scaffold-validates card).

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions