Port upstream Prisma query/write/data-type behaviours as multi-target ORM tests#995
Open
SevInf wants to merge 4 commits into
Open
Port upstream Prisma query/write/data-type behaviours as multi-target ORM tests#995SevInf wants to merge 4 commits into
SevInf wants to merge 4 commits into
Conversation
A PSL-authored fixture (DataRow plus ParamRow/BytesRow/BigParent/BigChild) covering every parameterized and key-typed Postgres column the data-type ports need, and a shared datatypes-helpers harness. Wired into the integration `emit` script so `fixtures:check` regenerates and gates it. Signed-off-by: Serhii Tatarintsev <tatarintsev@prisma.io>
Ports concrete behaviours from the upstream Prisma client functional suite and engine connector-test-kit into result-level sql-orm-client integration tests: filters, combinators, relation predicates, ordering/pagination, aggregation/groupBy, distinct, writes, and data-type round-trips/filters. Self-relation `some` (#980) and int8 string reads (#983) are asserted correctly and marked `it.fails` against the filed bugs. Signed-off-by: Serhii Tatarintsev <tatarintsev@prisma.io>
Ports the SQLite-only upstream regression (some { ... in [...] } fed into a
count must not fan out and duplicate the parent) against SQLite via the e2e
ORM harness, asserting count 1.
Signed-off-by: Serhii Tatarintsev <tatarintsev@prisma.io>
Ports two MongoDB-only upstream behaviours against Mongo: a string value containing a literal `$` is treated as data (filter + update), and an OR[NOT eq, eq] filter evaluates to the expected result set. Signed-off-by: Serhii Tatarintsev <tatarintsev@prisma.io>
Contributor
📝 WalkthroughWalkthroughChangesThe PR adds broad ported regression coverage for SQL-ORM datatype handling, filters, relations, ordering, aggregation, writes, and projections, plus MongoDB and SQLite regression tests. It also adds a PostgreSQL datatype contract fixture, runtime helpers, and contract emission wiring. Ported ORM coverage
Estimated code review effort: 5 (Critical) | ~120 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
size-limit report 📦
|
@prisma-next/extension-author-tools
@prisma-next/mongo-runtime
@prisma-next/family-mongo
@prisma-next/sql-runtime
@prisma-next/family-sql
@prisma-next/extension-arktype-json
@prisma-next/middleware-cache
@prisma-next/mongo
@prisma-next/extension-paradedb
@prisma-next/extension-pgvector
@prisma-next/extension-postgis
@prisma-next/postgres
@prisma-next/sql-orm-client
@prisma-next/sqlite
@prisma-next/extension-supabase
@prisma-next/target-mongo
@prisma-next/adapter-mongo
@prisma-next/driver-mongo
@prisma-next/contract
@prisma-next/utils
@prisma-next/config
@prisma-next/errors
@prisma-next/framework-components
@prisma-next/operations
@prisma-next/ts-render
@prisma-next/contract-authoring
@prisma-next/ids
@prisma-next/psl-parser
@prisma-next/psl-printer
@prisma-next/cli
@prisma-next/cli-telemetry
@prisma-next/config-loader
@prisma-next/emitter
@prisma-next/language-server
@prisma-next/migration-tools
prisma-next
@prisma-next/vite-plugin-contract-emit
@prisma-next/mongo-codec
@prisma-next/mongo-contract
@prisma-next/mongo-value
@prisma-next/mongo-contract-psl
@prisma-next/mongo-contract-ts
@prisma-next/mongo-emitter
@prisma-next/mongo-schema-ir
@prisma-next/mongo-query-ast
@prisma-next/mongo-orm
@prisma-next/mongo-query-builder
@prisma-next/mongo-lowering
@prisma-next/mongo-wire
@prisma-next/sql-contract
@prisma-next/sql-errors
@prisma-next/sql-operations
@prisma-next/sql-schema-ir
@prisma-next/sql-contract-psl
@prisma-next/sql-contract-ts
@prisma-next/sql-contract-emitter
@prisma-next/sql-lane-query-builder
@prisma-next/sql-relational-core
@prisma-next/sql-builder
@prisma-next/target-postgres
@prisma-next/target-sqlite
@prisma-next/adapter-postgres
@prisma-next/adapter-sqlite
@prisma-next/driver-postgres
@prisma-next/driver-sqlite
commit: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
test/integration/test/mongo/ported-regressions.test.ts (1)
26-26: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valuePrefer separate
expect()assertions overtoMatchObject()for multiple fields.Based on learnings, test files should prefer separate
expect()assertions for each field instead of combining checks withtoMatchObject(). This yields clearer, more actionable failure messages that pinpoint exactly which field failed.
test/integration/test/mongo/ported-regressions.test.ts#L26-L26: Replaceexpect(results[0]).toMatchObject(...)with separate.toBe()checks forresults[0].nameandresults[0].email.test/integration/test/mongo/ported-regressions.test.ts#L41-L41: Replaceexpect(updated).toMatchObject(...)with separate.toBe()checks forupdated.nameandupdated.email.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/integration/test/mongo/ported-regressions.test.ts` at line 26, Replace the combined toMatchObject assertions in test/integration/test/mongo/ported-regressions.test.ts at lines 26-26 and 41-41 with separate toBe assertions: check results[0].name and results[0].email independently at the first site, and updated.name and updated.email independently at the second site.Source: Learnings
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@test/integration/package.json`:
- Line 7: Update the emit:check git diff scope to include
test/sql-orm-client/fixtures/datatypes-psl/generated alongside the existing
generated fixture paths. Keep the emit command and other diff checks unchanged
so datatype fixture output is validated for stale generated JSON and types.
In `@test/integration/test/sql-orm-client/ported-regressions.test.ts`:
- Around line 113-139: Update the entry 109 test to use one shared full-length
blindCast<Char<36>> constant for the UserRole.roleId fixture values. Reuse that
constant in seedUserRoles, the cursor passed to cursor(), and the expected rows,
replacing the short roleId literals while preserving the existing ordering and
results.
---
Nitpick comments:
In `@test/integration/test/mongo/ported-regressions.test.ts`:
- Line 26: Replace the combined toMatchObject assertions in
test/integration/test/mongo/ported-regressions.test.ts at lines 26-26 and 41-41
with separate toBe assertions: check results[0].name and results[0].email
independently at the first site, and updated.name and updated.email
independently at the second site.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: ffc02073-e556-4dd8-b52e-a764c378e12e
⛔ Files ignored due to path filters (2)
test/integration/test/sql-orm-client/fixtures/datatypes-psl/generated/contract.d.tsis excluded by!**/generated/**test/integration/test/sql-orm-client/fixtures/datatypes-psl/generated/contract.jsonis excluded by!**/generated/**
📒 Files selected for processing (14)
test/e2e/framework/test/sqlite/ported-regressions.test.tstest/integration/package.jsontest/integration/test/mongo/ported-regressions.test.tstest/integration/test/sql-orm-client/datatypes-helpers.tstest/integration/test/sql-orm-client/fixtures/datatypes-psl/contract.prismatest/integration/test/sql-orm-client/fixtures/datatypes-psl/prisma-next.config.tstest/integration/test/sql-orm-client/ported-datatypes-params.test.tstest/integration/test/sql-orm-client/ported-datatypes-regressions.test.tstest/integration/test/sql-orm-client/ported-datatypes.test.tstest/integration/test/sql-orm-client/ported-filters-relations.test.tstest/integration/test/sql-orm-client/ported-filters-scalar.test.tstest/integration/test/sql-orm-client/ported-ordering-aggregation.test.tstest/integration/test/sql-orm-client/ported-regressions.test.tstest/integration/test/sql-orm-client/ported-writes-simple.test.ts
| "type": "module", | ||
| "scripts": { | ||
| "emit": "node ../../packages/1-framework/3-tooling/cli/dist/cli.js contract emit --config test/fixtures/prisma-next.config.ts && node ../../packages/1-framework/3-tooling/cli/dist/cli.js contract emit --config test/namespaced-accessors/fixtures/prisma-next.config.ts && node ../../packages/1-framework/3-tooling/cli/dist/cli.js contract emit --config test/sql-orm-client/fixtures/scalar-lists/prisma-next.config.ts && (node ../../packages/1-framework/3-tooling/cli/dist/cli.js contract emit --config test/mongo/fixtures/prisma-next.config.ts || true) && pnpm emit:authoring", | ||
| "emit": "node ../../packages/1-framework/3-tooling/cli/dist/cli.js contract emit --config test/fixtures/prisma-next.config.ts && node ../../packages/1-framework/3-tooling/cli/dist/cli.js contract emit --config test/namespaced-accessors/fixtures/prisma-next.config.ts && node ../../packages/1-framework/3-tooling/cli/dist/cli.js contract emit --config test/sql-orm-client/fixtures/scalar-lists/prisma-next.config.ts && node ../../packages/1-framework/3-tooling/cli/dist/cli.js contract emit --config test/sql-orm-client/fixtures/datatypes-psl/prisma-next.config.ts && (node ../../packages/1-framework/3-tooling/cli/dist/cli.js contract emit --config test/mongo/fixtures/prisma-next.config.ts || true) && pnpm emit:authoring", |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Include the datatype fixture output in emit:check.
This emitter updates test/sql-orm-client/fixtures/datatypes-psl/generated, but the scoped git diff --exit-code on Line 9 does not check that path. Contract source changes can therefore leave stale JSON/types undetected.
Proposed fix
- "emit:check": "pnpm emit && git diff --exit-code test/fixtures/contract.json test/fixtures/contract.d.ts test/authoring/parity test/authoring/side-by-side",
+ "emit:check": "pnpm emit && git diff --exit-code test/fixtures/contract.json test/fixtures/contract.d.ts test/sql-orm-client/fixtures/datatypes-psl/generated test/authoring/parity test/authoring/side-by-side",🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@test/integration/package.json` at line 7, Update the emit:check git diff
scope to include test/sql-orm-client/fixtures/datatypes-psl/generated alongside
the existing generated fixture paths. Keep the emit command and other diff
checks unchanged so datatype fixture output is validated for stale generated
JSON and types.
Comment on lines
+113
to
+139
| it( | ||
| 'entry 109: composite multi-field cursor with parameterised values', | ||
| async () => { | ||
| await withCollectionRuntime(async (runtime) => { | ||
| const userRoles = createUserRolesCollection(runtime); | ||
|
|
||
| await seedUserRoles(runtime, [ | ||
| { userId: 1, roleId: 'a', level: 10 }, | ||
| { userId: 1, roleId: 'b', level: 20 }, | ||
| { userId: 1, roleId: 'c', level: 30 }, | ||
| { userId: 2, roleId: 'a', level: 40 }, | ||
| ]); | ||
|
|
||
| // Composite (userId, roleId) cursor: the boundary tuple is bound as | ||
| // parameters, and the seek advances past (1, 'a') within user 1. | ||
| const rows = await userRoles | ||
| .where((r) => r.userId.eq(1)) | ||
| .orderBy([(r) => r.userId.asc(), (r) => r.roleId.asc()]) | ||
| .cursor({ userId: 1, roleId: 'a' }) | ||
| .skip(0) | ||
| .take(5) | ||
| .all(); | ||
|
|
||
| expect(rows).toEqual([ | ||
| { userId: 1, roleId: 'b', level: 20 }, | ||
| { userId: 1, roleId: 'c', level: 30 }, | ||
| ]); |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
ast-grep outline test/integration/test/sql-orm-client/runtime-helpers.ts \
--match 'SeedUserRole|seedUserRoles' --view expanded
rg -n -C3 '\bUserRole\b|\broleId\b' \
test/integration/test/sql-orm-client/helpers.ts \
test/integration/test/sql-orm-client/runtime-helpers.ts \
test/integration/test/sql-orm-client/ported-regressions.test.ts \
test/integration/test/sql-orm-client/ported-writes-simple.test.tsRepository: prisma/prisma-next
Length of output: 11806
Brand the UserRole.roleId fixtures — use a single full-length blindCast<Char<36>> constant for these seeds, cursor inputs, and expectations; the short literals don’t match the UserRole.roleId contract used elsewhere in the test suite.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@test/integration/test/sql-orm-client/ported-regressions.test.ts` around lines
113 - 139, Update the entry 109 test to use one shared full-length
blindCast<Char<36>> constant for the UserRole.roleId fixture values. Reuse that
constant in seedUserRoles, the cursor passed to cursor(), and the expected rows,
replacing the short roleId literals while preserving the existing ordering and
results.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Linked issue
n/a — no Linear ticket; this is test-coverage work.
This PR pins tests against real bugs surfaced during the work: #980 (self-referential relation predicate returns
[]), #981 (@mapcolumn names with spaces emit invalidcontract.d.ts), and #983 (int8/bigint reads return a string). #982 was opened during the work and then closed as invalid.At a glance
The bulk of the change is result-level ORM tests that pin upstream Prisma behaviours against a live database. For example, the empty-
inedge case (empty list matches zero rows, not invalid SQL):Before this PR the
sql-orm-clientsuite asserted operator→AST shapes at the unit level, but had almost no result-levelwhere/write coverage against a real database.Decision
This PR ports concrete, individually-cited behaviours from the upstream Prisma client functional suite and the engine connector-test-kit into prisma-next's own test suites, each against the database the upstream test actually targets:
It also adds a small enabling change: a PSL-authored data-types fixture (
fixtures/datatypes-psl) covering the parameterized and key-typed Postgres columns the data-type ports need (numeric/char via named types, bytea PK, a BigInt-keyed relation), wired into the integrationemitscript sofixtures:checkregenerates and gates it.Where prisma-next is genuinely buggy, the ported test asserts the correct upstream behaviour and is marked
it.failsagainst the filed issue — it flips green the moment the bug is fixed. No test works around a bug by asserting wrong output.How it fits together
DataRowplusParamRow/BytesRow/BigParent/BigChild) and a shareddatatypes-helpers.tsthat builds the runtime, creates the tables, and hands back typed collections. Parameterized native types (numeric(20,8),char(12)) are declared as PSL named types — the supported path for supplyingtypeParams.ported-*.test.tsfiles, all driving the existing PGlite integration harness with whole-shapetoEqualassertions.some { … in [...] }fed into acount()must not fan out and double-count the parent; asserted against a real SQLite database.$-containing string value is treated as data (filter + update), and anOR[ NOT eq, eq ]filter resolves correctly.some(sql-orm-client: relation predicate (some/every/none) returns wrong results on self-referential relations #980) and every int8 read (postgres: int8/bigint reads return a string but the codec declares number (should be bigint for both output type and runtime value) #983) are asserted correctly and quarantined underit.failswith a comment pointing at the issue.What lands in this PR
datatypes-pslfixture (+ generated contract), the shared helper, and theemitwiringported-*.test.tsfiles (124 cases: 117 pass, 7it.fails)Notes for the reviewer
it.failsare deliberate, not skips. Six int8 tests (postgres: int8/bigint reads return a string but the codec declares number (should be bigint for both output type and runtime value) #983) and one self-relation test (sql-orm-client: relation predicate (some/every/none) returns wrong results on self-referential relations #980) assert the correct result and are expected to fail until the bug lands; each carries a comment with the issue link and the "remove.failswhen fixed" instruction.numeric/charcolumns are declared via atypes { … }block; the inline@db.Numeric(...)field-attribute form is (correctly) rejected at authoring, so the fixture uses the named-type path.SELECT NULL(no raw-select surface on the ORM client today).mongodb-memory-servercan't provision a binary on NixOS; the Mongo tests run in CI, and locally viaMONGOMS_SYSTEM_BINARY=<nix mongod>.Testing performed
pnpm --filter @prisma-next/integration-tests exec vitest run test/sql-orm-client/ported-*.test.ts— 124 cases: 117 passed, 7 expected-fail.pnpm --filter @prisma-next/e2e-tests exec vitest run test/sqlite/ported-regressions.test.ts— 1 passed.MONGOMS_SYSTEM_BINARY=<nix mongod> pnpm --filter @prisma-next/integration-tests exec vitest run test/mongo/ported-regressions.test.ts— 3 passed.pnpm build(Turbo) and the pre-commit biome/lint hooks on every commit.Skill update
n/a — tests, fixture, and emit-wiring only; no user-facing surface (CLI, public API, config, error codes) changes.
Follow-ups
it.failstests and switch their assertions to the fixed shapes once #980 and #983 land (int8 →bigint, self-relationsome).Alternatives considered
$-operator handling) prove nothing on Postgres. Each is ported against the database it targets.it.failsagainst the tracked issue instead.mn-pslprecedent and exercising the PSL authoring path (named types) directly.Checklist
git commit -s) per the DCO.TML-NNNN: <sentence-case title>form.n/a — internal only).Summary by CodeRabbit