ReviewOS

stacks/bun-query-builder

No execution-level test coverage for MySQL/Postgres (only SQL-text assertions)

#1038
Closed glennmichael123 opened this 2 months ago ·
2 months ago

Summary

Tests set dialect: 'mysql'/'postgres' only to assert generated SQL text (migrations, foreign-keys-inline, types-and-inference). No test ever executes CRUD/relations against a real MySQL/Postgres, so the entire DriverExecutor class (RETURNING insert, LAST_INSERT_ID() fallback, extractChanges/extractInsertId against real driver result shapes) and the dozens of config.dialect === 'postgres'/'mysql' runtime branches in client.ts are unverified.

This is the root cause that lets dialect bugs slip through (see the batch of placeholder/JSON/union/change-count issues filed alongside this).

Suggested fix

Add a containerized integration suite / CI matrix that actually runs CRUD + relations against pg + mysql. Even a smoke test of DriverExecutor.insert/run would catch result-shape regressions.

Severity: high (process gap). Found via internal code audit.

9 days ago

Addressed in 89e9bc6 for Postgres. Added test/integration.postgres.test.ts — real execution coverage against a live Postgres exercising the DriverExecutor + client.ts $n branches end-to-end: RETURNING inserts, UPDATE/DELETE affected-row counts (command tag), find/where/first, count + aggregates, hasMany eager-load, and the raw selectFrom builder. Skipped when no PG is reachable (seeder.e2e pattern); runs in a fresh subprocess so an earlier configureOrm() can't mask the network path.

Keeping this open-worthy note: MySQL execution coverage is still TODO (no MySQL available in this environment) — leaving that as the remaining gap if you want to track it separately.

Sign in to comment on this issue.