test(nextly): prove migrate:fresh discovery against a real postgres - #1745
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Warning Review limit reachedNext included review available in 3 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@codex review |
|
Codex Review: Didn't find any major issues. Can't wait for the next one! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Follows #1736, from a Codex P1 on its merged head. Test-only, so no changeset.
What the existing test could not see
The unit test asserts the rendered SQL contains
to_regclassandquote_ident.That is necessary and not sufficient: it stays green if the predicate is
inverted with
<>, or joined withORinstead ofAND. Both enumerate thewrong relations, and this is a command that drops what it enumerates.
It is the failure
AGENTS.mdnames — a property that returns green from thebroken implementation too, carrying the authority of having been checked. I
quoted that rule at someone else this morning and then shipped an instance of it.
The property that actually separates correct from broken is which rows come
back, and only a database can answer that.
The fixture, built so each wrong answer shows
search_pathistenant, public, and four tables sit in three schemas:shadowedtenant_onlypublic_onlyhidden_onlypublic_onlyis the row that fails acurrent_schema()implementation — theversion I wrote first, and the one this PR's predecessor corrected.
hidden_onlyis the row that fails an inverted or
OR-ed predicate.shadowedis the rowthat fails a query returning every copy rather than the resolved one.
A fourth case asserts
pg_classandpg_namespacenever appear:pg_catalogison every search path implicitly, so relation visibility on its own admits it.
Session pinning
One
pg.Clientheld for the file, becausegetDrizzle()wraps a Pool — aSET search_pathon a pooled connection binds to whichever client served it andthe reads may run on another. A first case asserts
SHOW search_pathreallycontains the tenant schema, so a reset session fails as a fixture problem rather
than looking like a defect in the predicate.
Verification
Locally: typechecks, lints, 11,901 unit tests pass, and the file is collected
by
vitest.integration.config.ts(4 tests, skipped for the documented missing-URLreason) rather than silently excluded.
from this machine. The Postgres CI leg is the oracle. The sibling probe added in
#1721/#1727 uses this same shape and has since passed on all three dialects, so
the harness itself is proven even though these cases are new.