Skip to content

fix(test): assert the renamed sandbox columns in the postgres placement test - #965

Merged
jayminwest merged 2 commits into
jayminwest:mainfrom
luantaraschi:fix/postgres-placement-columns
Aug 18, 2026
Merged

fix(test): assert the renamed sandbox columns in the postgres placement test#965
jayminwest merged 2 commits into
jayminwest:mainfrom
luantaraschi:fix/postgres-placement-columns

Conversation

@luantaraschi

Copy link
Copy Markdown
Collaborator

ci-postgres is red on main. The postgres arm of drop-placement-tables.test.ts asserts two columns that #963 renamed away.

#963 renamed them on both dialects:

  • src/db/migrations/0049_sad_mandroid.sql (sqlite): ALTER TABLE "runs" RENAME COLUMN "burrow_id" TO "sandbox_id", and the same for burrow_run_id.
  • src/db/migrations/postgres/0044_melted_the_executioner.sql: the same two renames.

The same commit updated the sqlite arm of this test to the new names, at lines 49 and 50. The postgres arm at lines 142 and 143 was left behind. This changes those two assertions and nothing else.

The replay case further up keeps burrow_id / burrow_run_id in its raw SQL on purpose, because it builds the pre-migration schema and then applies the drop. Only the header comment gained a line saying so, since the file now mentions both spellings.

Why it merged green

The postgres arm is test.skipIf(!isPostgresTestEnabled()), so it only runs under the ci-postgres workflow. The regular ci job skips it and so does a plain bun test.

Run #282 on warren/run_3gt7t2q3095d failed the same assertion at 06:03 on 17 August, and every ci-postgres run since has been red on the same line.

What I ran

bun test src/db/migrations/drop-placement-tables.test.ts gives 2 passing and 1 skipped. The skip is the postgres arm: I have no Postgres on this machine, so I could not exercise the assertion I am changing, and I would rather say that than imply otherwise. The static proof is migration 0044 above, and ci-postgres on this PR is the real measurement.

bun run typecheck and biome on the file are both clean.

…nt test

jayminwest#963 renamed runs.burrow_id to sandbox_id and runs.burrow_run_id to
sandbox_run_id on both dialects, in 0049_sad_mandroid.sql and in
postgres/0044_melted_the_executioner.sql. It updated the sqlite arm of
drop-placement-tables.test.ts to match and left the postgres arm on the
old names, so ci-postgres has been red since.

The postgres arm is skipIf(!isPostgresTestEnabled()), so the regular ci
job and a plain bun test both skip it, which is how it merged green.

The replay case keeps the old spellings in its raw SQL on purpose: it
builds the pre-migration schema before applying the drop.
@jayminwest
jayminwest merged commit abb6f6b into jayminwest:main Aug 18, 2026
5 checks passed
jayminwest added a commit that referenced this pull request Aug 18, 2026
…erge session)

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants