refactor: move e2e orm tests to e2e project, fix bundle issues #285
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughThis PR externalizes and packages the policy plugin, centralizes shared utilities into runtime/query-utils and SDK, relocates policy error types into runtime, updates exports across packages, and migrates tests to use @zenstackhq/runtime and @zenstackhq/testtools. It also adjusts dependency specs to catalog-based resolution and streamlines build/test configurations. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (6)
🚧 Files skipped from review as they are similar to previous changes (1)
🧰 Additional context used📓 Path-based instructions (2){packages,samples,tests}/**📄 CodeRabbit inference engine (CLAUDE.md)
Files:
tests/e2e/**📄 CodeRabbit inference engine (CLAUDE.md)
Files:
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
🔇 Additional comments (4)
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 |
|
Claude finished @ymc9's task —— View job PR Review: Move e2e ORM Tests & Fix Bundle IssuesI've completed a comprehensive review of this refactoring PR. This is a substantial and well-executed reorganization with solid technical improvements. Review Tasks
✅ Code Quality & Best PracticesExcellent structural improvements:
Bundle configuration improvements in
✅ Test Coverage & OrganizationCoverage maintained and improved:
✅ Performance ConsiderationsPositive performance impacts:
|
There was a problem hiding this comment.
Pull Request Overview
Moves ORM end-to-end tests into a dedicated e2e test project and refactors runtime packaging to address bundle and import structure issues.
- Consolidates test utilities into @zenstackhq/testtools (adds policy + vitest helpers, centralizes client creation).
- Refactors runtime build: splits policy plugin into separate tsup build entry; moves policy errors/types into core errors, migrates query/kysely utils.
- Cleans up generated schema tweaks and standardizes imports across tests.
Reviewed Changes
Copilot reviewed 120 out of 154 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/regression/vitest.config.ts | Adds shared setup file for regression tests. |
| tests/regression/tsconfig.json | Adds @zenstackhq/testtools types. |
| tests/e2e/vitest.config.ts | Adds shared setup for e2e tests. |
| tests/e2e/tsconfig.json | Simplifies and adds testtools types. |
| tests/e2e/package.json | Introduces build/generation scripts, expands dependencies for e2e environment. |
| tests/e2e/orm/scripts/generate.ts | Removes manual runtime path rewriting post-generation. |
| tests/e2e/orm/schemas/** | Replaces relative runtime imports with package imports. |
| tests/e2e/orm/query-builder/query-builder.test.ts | Switches to centralized testtools client. |
| tests/e2e/orm/prisma-consistency/*.test.ts | Reorders/cleans imports; adjusts cli path depth. |
| tests/e2e/orm/prisma-consistency/test-utils.ts | Updates CLI path to new relative location. |
| tests/e2e/orm/policy/**/*.test.ts | Replaces local utils with testtools policy/test client utilities. |
| tests/e2e/orm/policy/utils.ts | Repoints imports to runtime + testtools. |
| tests/e2e/orm/plugin-infra/*.test.ts | Switches to runtime/testtools exports. |
| tests/e2e/orm/client-api/**/*.test.ts | Standardizes imports to runtime/testtools; removes dist references. |
| pnpm-workspace.yaml | Adds better-sqlite3 and pg to catalog for shared versioning. |
| packages/testtools/src/vitest-ext.ts | Repoints error imports to runtime central errors. |
| packages/testtools/src/index.ts | Exposes new client and vitest extensions. |
| packages/testtools/src/client.ts | Refactors to use runtime directly; adds createPolicyTestClient helper. |
| packages/testtools/package.json | Adds runtime/common deps, type export, build step copying types. |
| packages/runtime/tsup.config.ts | Splits build into two configs; externalizes plugin internals. |
| packages/runtime/tsconfig.json | Removes test sources from compilation scope. |
| packages/runtime/tsconfig.build.json | Removes separate build tsconfig (simplification). |
| packages/runtime/src/plugins/policy/**/*.ts | Refactors to use QueryUtils and relocates errors. |
| packages/runtime/src/client/query-utils.ts | Adds alias/field/model extraction utilities (absorbs former kysely-utils). |
| packages/runtime/src/client/kysely-utils.ts | Removed in favor of consolidated query-utils. |
| packages/runtime/src/client/index.ts | Re-exports QueryUtils and CRUD; reorganizes exports. |
| packages/runtime/src/client/executor/** | Updates imports for relocated utilities. |
| packages/runtime/src/client/errors.ts | Moves RejectedByPolicy* error definitions into core. |
| packages/runtime/src/client/crud/operations/*.ts | Adjusts imports to new error locations and plugin detection. |
| packages/runtime/src/client/crud/operations/base.ts | Changes policy plugin detection mechanism. |
| packages/runtime/package.json | Simplifies build/test scripts; adjusts peer dependency version specs. |
| packages/cli/package.json | Normalizes better-sqlite3 version to catalog reference. |
| (Many test files) | Standardizes import paths to new runtime and testtools structure; no functional logic changes. |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
|
Claude finished @ymc9's task —— View job PR Review: Move e2e ORM Tests & Fix Bundle IssuesI've completed a comprehensive review of this refactoring PR. This is a substantial and well-executed reorganization with solid technical improvements. Review Tasks
✅ Code Quality & Best PracticesExcellent structural improvements:
Bundle configuration improvements in
✅ Test Coverage & OrganizationCoverage maintained and improved:
✅ Performance ConsiderationsPositive performance impacts:
|
|
Claude finished @ymc9's task —— View job PR Review: Move e2e ORM Tests & Fix Bundle IssuesI've completed a comprehensive review of this refactoring PR. This is a substantial and well-executed reorganization with solid technical improvements. Review Tasks
✅ Code Quality & Best PracticesExcellent structural improvements:
Bundle configuration improvements in
✅ Test Coverage & OrganizationCoverage maintained and improved:
✅ Performance ConsiderationsPositive performance impacts:
|
|
Claude finished @ymc9's task —— View job Claude finished @ymc9's task —— View job PR Review: Move e2e ORM Tests & Fix Bundle IssuesI've completed a comprehensive review of this refactoring PR. This is a substantial and well-executed reorganization with solid technical improvements. Review Tasks
✅ Code Quality & Best PracticesExcellent structural improvements:
Bundle configuration improvements in
✅ Test Coverage & OrganizationCoverage maintained and improved:
✅ Performance ConsiderationsPositive performance impacts:
|
|
Claude finished @ymc9's task —— View job PR Review: Move e2e ORM Tests & Fix Bundle IssuesI've completed a comprehensive review of this refactoring PR. This is a substantial and well-executed reorganization with solid technical improvements. Review Tasks
✅ Code Quality & Best PracticesExcellent structural improvements:
Bundle configuration improvements in
✅ Test Coverage & OrganizationCoverage maintained and improved:
✅ Performance ConsiderationsPositive performance impacts:
|
|
Claude finished @ymc9's task —— View job PR Review: Move e2e ORM Tests & Fix Bundle IssuesI've completed a comprehensive review of this refactoring PR. This is a substantial and well-executed reorganization with solid technical improvements. Review Tasks
✅ Code Quality & Best PracticesExcellent structural improvements:
Bundle configuration improvements in
✅ Test Coverage & OrganizationCoverage maintained and improved:
✅ Performance ConsiderationsPositive performance impacts:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
tests/e2e/orm/policy/basic-schema-read.test.ts (2)
50-50: Add generic parameter for consistency and type safety.Line 26 correctly instantiates
PolicyPlugin<typeof schema>()with the generic parameter, but this instantiation omits it. For consistency and to ensure type safety across all tests, add the generic parameter here as well.Apply this diff:
- const anonClient = client.$use(new PolicyPlugin()); + const anonClient = client.$use(new PolicyPlugin<typeof schema>());
71-71: Add generic parameter for consistency and type safety.Line 26 correctly instantiates
PolicyPlugin<typeof schema>()with the generic parameter, but this instantiation omits it. For consistency and to ensure type safety across all tests, add the generic parameter here as well.Apply this diff:
- const anonClient = client.$use(new PolicyPlugin()); + const anonClient = client.$use(new PolicyPlugin<typeof schema>());tests/e2e/orm/policy/migrated/omit.test.ts (1)
51-51: Remove debug artifact.The
console.logstatement should be removed as it's a debug leftover and serves no purpose in the test.Apply this diff to remove the debug statement:
- console.log(found);
♻️ Duplicate comments (1)
packages/runtime/src/client/crud/operations/base.ts (1)
108-111: Constructor name comparison is brittle and breaks with minification.Relying on
constructor.name === 'PolicyPlugin'is fragile and will fail in production builds that minify code. This approach also breaks with subclassing or class renaming.Consider one of these robust alternatives:
Option 1: Symbol-based identification (preferred)
In the PolicyPlugin class:
export const IS_POLICY_PLUGIN = Symbol('isPolicyPlugin'); export class PolicyPlugin { [IS_POLICY_PLUGIN] = true; // ... }Then check:
- protected get hasPolicyEnabled() { - return this.options.plugins?.some((plugin) => plugin.constructor.name === 'PolicyPlugin'); - } + protected get hasPolicyEnabled() { + return this.options.plugins?.some((plugin) => IS_POLICY_PLUGIN in plugin); + }Option 2: Type guard pattern
function isPolicyPlugin(plugin: Plugin): plugin is PolicyPlugin { return 'isPolicyPlugin' in plugin && plugin.isPolicyPlugin === true; }Option 3: Import and use instanceof (if PolicyPlugin is accessible)
+import { PolicyPlugin } from '../path/to/policy-plugin'; + protected get hasPolicyEnabled() { - return this.options.plugins?.some((plugin) => plugin.constructor.name === 'PolicyPlugin'); + return this.options.plugins?.some((plugin) => plugin instanceof PolicyPlugin); }Based on learnings
🧹 Nitpick comments (1)
tests/e2e/orm/client-api/import.test.ts (1)
1-5: Consolidate duplicate imports from the same package.Both lines 1 and 5 import from
@zenstackhq/testtools. Merge them into a single import statement for better readability.Apply this diff:
-import { createTestProject, generateTsSchemaInPlace } from '@zenstackhq/testtools'; import fs from 'node:fs'; import path from 'node:path'; import { describe, expect, it } from 'vitest'; -import { createTestClient } from '@zenstackhq/testtools'; +import { createTestProject, generateTsSchemaInPlace, createTestClient } from '@zenstackhq/testtools';
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (107)
packages/cli/package.json(1 hunks)packages/plugins/policy/package.json(1 hunks)packages/plugins/policy/src/column-collector.ts(1 hunks)packages/plugins/policy/src/expression-evaluator.ts(1 hunks)packages/plugins/policy/src/expression-transformer.ts(12 hunks)packages/plugins/policy/src/functions.ts(3 hunks)packages/plugins/policy/src/index.ts(0 hunks)packages/plugins/policy/src/plugin.ts(1 hunks)packages/plugins/policy/src/policy-handler.ts(14 hunks)packages/plugins/policy/src/types.ts(1 hunks)packages/plugins/policy/src/utils.ts(1 hunks)packages/plugins/policy/tsconfig.json(0 hunks)packages/plugins/policy/tsup.config.ts(1 hunks)packages/plugins/policy/vitest.config.ts(1 hunks)packages/runtime/package.json(2 hunks)packages/runtime/src/client/crud/operations/base.ts(1 hunks)packages/runtime/src/client/crud/operations/create.ts(1 hunks)packages/runtime/src/client/crud/operations/delete.ts(1 hunks)packages/runtime/src/client/crud/operations/update.ts(1 hunks)packages/runtime/src/client/errors.ts(1 hunks)packages/runtime/src/client/executor/name-mapper.ts(1 hunks)packages/runtime/src/client/executor/zenstack-query-executor.ts(1 hunks)packages/runtime/src/client/index.ts(1 hunks)packages/runtime/src/client/kysely-utils.ts(0 hunks)packages/runtime/src/client/query-utils.ts(2 hunks)packages/runtime/src/plugins/policy/errors.ts(0 hunks)packages/runtime/src/schema/index.ts(1 hunks)packages/runtime/tsconfig.json(1 hunks)packages/runtime/tsup.config.ts(0 hunks)packages/runtime/vitest.config.ts(1 hunks)packages/sdk/src/expression-utils.ts(1 hunks)packages/sdk/src/index.ts(1 hunks)packages/testtools/package.json(2 hunks)packages/testtools/src/client.ts(3 hunks)packages/testtools/src/index.ts(1 hunks)packages/testtools/src/vitest-ext.ts(1 hunks)pnpm-workspace.yaml(1 hunks)tests/e2e/orm/client-api/aggregate.test.ts(1 hunks)tests/e2e/orm/client-api/compound-id.test.ts(1 hunks)tests/e2e/orm/client-api/computed-fields.test.ts(1 hunks)tests/e2e/orm/client-api/count.test.ts(1 hunks)tests/e2e/orm/client-api/create-many-and-return.test.ts(1 hunks)tests/e2e/orm/client-api/create-many.test.ts(1 hunks)tests/e2e/orm/client-api/create.test.ts(1 hunks)tests/e2e/orm/client-api/default-values.test.ts(1 hunks)tests/e2e/orm/client-api/delegate.test.ts(1 hunks)tests/e2e/orm/client-api/delete-many.test.ts(1 hunks)tests/e2e/orm/client-api/delete.test.ts(1 hunks)tests/e2e/orm/client-api/filter.test.ts(1 hunks)tests/e2e/orm/client-api/find.test.ts(1 hunks)tests/e2e/orm/client-api/group-by.test.ts(1 hunks)tests/e2e/orm/client-api/import.test.ts(1 hunks)tests/e2e/orm/client-api/mixin.test.ts(1 hunks)tests/e2e/orm/client-api/name-mapping.test.ts(1 hunks)tests/e2e/orm/client-api/raw-query.test.ts(1 hunks)tests/e2e/orm/client-api/relation/many-to-many.test.ts(1 hunks)tests/e2e/orm/client-api/relation/one-to-many.test.ts(1 hunks)tests/e2e/orm/client-api/relation/one-to-one.test.ts(1 hunks)tests/e2e/orm/client-api/relation/self-relation.test.ts(1 hunks)tests/e2e/orm/client-api/scalar-list.test.ts(1 hunks)tests/e2e/orm/client-api/transaction.test.ts(1 hunks)tests/e2e/orm/client-api/type-coverage.test.ts(1 hunks)tests/e2e/orm/client-api/typed-json-fields.test.ts(1 hunks)tests/e2e/orm/client-api/undefined-values.test.ts(1 hunks)tests/e2e/orm/client-api/update-many.test.ts(1 hunks)tests/e2e/orm/client-api/update.test.ts(1 hunks)tests/e2e/orm/client-api/upsert.test.ts(1 hunks)tests/e2e/orm/client-api/utils.ts(1 hunks)tests/e2e/orm/plugin-infra/entity-mutation-hooks.test.ts(1 hunks)tests/e2e/orm/plugin-infra/on-kysely-query.test.ts(1 hunks)tests/e2e/orm/plugin-infra/on-query-hooks.test.ts(1 hunks)tests/e2e/orm/policy/basic-schema-read.test.ts(2 hunks)tests/e2e/orm/policy/crud/create.test.ts(1 hunks)tests/e2e/orm/policy/crud/delete.test.ts(1 hunks)tests/e2e/orm/policy/crud/dumb-rules.test.ts(1 hunks)tests/e2e/orm/policy/crud/post-update.test.ts(1 hunks)tests/e2e/orm/policy/crud/read.test.ts(1 hunks)tests/e2e/orm/policy/crud/update.test.ts(1 hunks)tests/e2e/orm/policy/migrated/auth.test.ts(1 hunks)tests/e2e/orm/policy/migrated/client-extensions.test.ts(1 hunks)tests/e2e/orm/policy/migrated/connect-disconnect.test.ts(3 hunks)tests/e2e/orm/policy/migrated/create-many-and-return.test.ts(1 hunks)tests/e2e/orm/policy/migrated/cross-model-field-comparison.test.ts(1 hunks)tests/e2e/orm/policy/migrated/current-model.test.ts(2 hunks)tests/e2e/orm/policy/migrated/current-operation.test.ts(1 hunks)tests/e2e/orm/policy/migrated/deep-nested.test.ts(1 hunks)tests/e2e/orm/policy/migrated/empty-policy.test.ts(1 hunks)tests/e2e/orm/policy/migrated/field-comparison.test.ts(1 hunks)tests/e2e/orm/policy/migrated/multi-field-unique.test.ts(1 hunks)tests/e2e/orm/policy/migrated/multi-id-fields.test.ts(1 hunks)tests/e2e/orm/policy/migrated/nested-to-many.test.ts(1 hunks)tests/e2e/orm/policy/migrated/nested-to-one.test.ts(1 hunks)tests/e2e/orm/policy/migrated/omit.test.ts(1 hunks)tests/e2e/orm/policy/migrated/petstore-sample.test.ts(1 hunks)tests/e2e/orm/policy/migrated/query-reduction.test.ts(1 hunks)tests/e2e/orm/policy/migrated/relation-check.test.ts(1 hunks)tests/e2e/orm/policy/migrated/relation-many-to-many-filter.test.ts(1 hunks)tests/e2e/orm/policy/migrated/relation-one-to-many-filter.test.ts(1 hunks)tests/e2e/orm/policy/migrated/relation-one-to-one-filter.test.ts(1 hunks)tests/e2e/orm/policy/migrated/self-relation.test.ts(1 hunks)tests/e2e/orm/policy/migrated/todo-sample.test.ts(1 hunks)tests/e2e/orm/policy/migrated/toplevel-operations.test.ts(1 hunks)tests/e2e/orm/policy/migrated/unique-as-id.test.ts(1 hunks)tests/e2e/orm/policy/migrated/update-many-and-return.test.ts(1 hunks)tests/e2e/orm/policy/migrated/view.test.ts(1 hunks)tests/e2e/orm/policy/utils.ts(1 hunks)tests/e2e/orm/prisma-consistency/attributes.test.ts(1 hunks)
⛔ Files not processed due to max files limit (26)
- tests/e2e/orm/prisma-consistency/basic-models.test.ts
- tests/e2e/orm/prisma-consistency/compound-ids.test.ts
- tests/e2e/orm/prisma-consistency/enums.test.ts
- tests/e2e/orm/prisma-consistency/field-types.test.ts
- tests/e2e/orm/prisma-consistency/relation-validation.test.ts
- tests/e2e/orm/prisma-consistency/relations-many-to-many.test.ts
- tests/e2e/orm/prisma-consistency/relations-one-to-one.test.ts
- tests/e2e/orm/prisma-consistency/relations-self.test.ts
- tests/e2e/orm/prisma-consistency/test-utils.ts
- tests/e2e/orm/prisma-consistency/unique-constraints.test.ts
- tests/e2e/orm/query-builder/query-builder.test.ts
- tests/e2e/orm/schemas/basic/schema.ts
- tests/e2e/orm/schemas/delegate/schema.ts
- tests/e2e/orm/schemas/delegate/typecheck.ts
- tests/e2e/orm/schemas/name-mapping/schema.ts
- tests/e2e/orm/schemas/petstore/schema.ts
- tests/e2e/orm/schemas/todo/schema.ts
- tests/e2e/orm/schemas/typing/schema.ts
- tests/e2e/orm/schemas/typing/typecheck.ts
- tests/e2e/orm/scripts/generate.ts
- tests/e2e/package.json
- tests/e2e/tsconfig.json
- tests/e2e/vitest.config.ts
- tests/regression/tsconfig.json
- tests/regression/vitest.config.ts
- vitest.config.ts
💤 Files with no reviewable changes (5)
- packages/runtime/tsup.config.ts
- packages/runtime/src/plugins/policy/errors.ts
- packages/plugins/policy/src/index.ts
- packages/plugins/policy/tsconfig.json
- packages/runtime/src/client/kysely-utils.ts
🧰 Additional context used
📓 Path-based instructions (3)
{packages,samples,tests}/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place packages only under
packages/,samples/, ortests/
Files:
packages/runtime/src/client/executor/zenstack-query-executor.tspackages/testtools/src/vitest-ext.tstests/e2e/orm/client-api/compound-id.test.tspackages/runtime/src/client/crud/operations/base.tspackages/runtime/src/schema/index.tspackages/testtools/src/index.tstests/e2e/orm/policy/migrated/unique-as-id.test.tstests/e2e/orm/client-api/aggregate.test.tstests/e2e/orm/plugin-infra/entity-mutation-hooks.test.tstests/e2e/orm/client-api/filter.test.tspackages/runtime/vitest.config.tstests/e2e/orm/client-api/group-by.test.tspackages/plugins/policy/src/policy-handler.tstests/e2e/orm/policy/crud/post-update.test.tstests/e2e/orm/client-api/count.test.tstests/e2e/orm/client-api/find.test.tspackages/plugins/policy/package.jsontests/e2e/orm/policy/crud/delete.test.tstests/e2e/orm/policy/migrated/view.test.tstests/e2e/orm/client-api/create.test.tstests/e2e/orm/policy/migrated/current-operation.test.tspackages/plugins/policy/src/functions.tstests/e2e/orm/policy/migrated/connect-disconnect.test.tstests/e2e/orm/policy/migrated/todo-sample.test.tstests/e2e/orm/plugin-infra/on-query-hooks.test.tstests/e2e/orm/client-api/upsert.test.tstests/e2e/orm/policy/migrated/relation-one-to-one-filter.test.tspackages/sdk/src/index.tspackages/runtime/tsconfig.jsontests/e2e/orm/policy/crud/dumb-rules.test.tstests/e2e/orm/policy/migrated/petstore-sample.test.tstests/e2e/orm/client-api/transaction.test.tstests/e2e/orm/policy/crud/update.test.tspackages/runtime/src/client/crud/operations/delete.tstests/e2e/orm/policy/migrated/nested-to-one.test.tspackages/sdk/src/expression-utils.tstests/e2e/orm/plugin-infra/on-kysely-query.test.tstests/e2e/orm/policy/migrated/deep-nested.test.tstests/e2e/orm/policy/basic-schema-read.test.tstests/e2e/orm/policy/migrated/relation-check.test.tspackages/runtime/src/client/errors.tstests/e2e/orm/policy/migrated/update-many-and-return.test.tspackages/runtime/src/client/crud/operations/update.tstests/e2e/orm/policy/migrated/omit.test.tstests/e2e/orm/policy/migrated/current-model.test.tstests/e2e/orm/policy/migrated/multi-id-fields.test.tstests/e2e/orm/policy/migrated/multi-field-unique.test.tspackages/runtime/package.jsontests/e2e/orm/policy/migrated/empty-policy.test.tspackages/plugins/policy/src/expression-evaluator.tspackages/plugins/policy/vitest.config.tstests/e2e/orm/client-api/undefined-values.test.tstests/e2e/orm/client-api/scalar-list.test.tspackages/plugins/policy/tsup.config.tstests/e2e/orm/prisma-consistency/attributes.test.tstests/e2e/orm/policy/migrated/create-many-and-return.test.tstests/e2e/orm/client-api/delete-many.test.tstests/e2e/orm/client-api/type-coverage.test.tstests/e2e/orm/policy/migrated/self-relation.test.tstests/e2e/orm/client-api/mixin.test.tspackages/runtime/src/client/index.tstests/e2e/orm/policy/migrated/cross-model-field-comparison.test.tspackages/plugins/policy/src/types.tstests/e2e/orm/policy/migrated/field-comparison.test.tstests/e2e/orm/policy/crud/create.test.tstests/e2e/orm/client-api/relation/many-to-many.test.tspackages/runtime/src/client/executor/name-mapper.tstests/e2e/orm/policy/migrated/relation-one-to-many-filter.test.tstests/e2e/orm/client-api/computed-fields.test.tstests/e2e/orm/client-api/relation/one-to-many.test.tspackages/runtime/src/client/query-utils.tstests/e2e/orm/policy/migrated/auth.test.tstests/e2e/orm/policy/crud/read.test.tstests/e2e/orm/client-api/delegate.test.tstests/e2e/orm/policy/migrated/nested-to-many.test.tstests/e2e/orm/client-api/name-mapping.test.tstests/e2e/orm/policy/migrated/toplevel-operations.test.tstests/e2e/orm/client-api/update-many.test.tspackages/cli/package.jsontests/e2e/orm/policy/migrated/client-extensions.test.tstests/e2e/orm/client-api/raw-query.test.tstests/e2e/orm/policy/migrated/relation-many-to-many-filter.test.tstests/e2e/orm/client-api/delete.test.tspackages/runtime/src/client/crud/operations/create.tstests/e2e/orm/client-api/create-many.test.tspackages/plugins/policy/src/utils.tspackages/plugins/policy/src/column-collector.tstests/e2e/orm/client-api/update.test.tstests/e2e/orm/policy/utils.tstests/e2e/orm/client-api/import.test.tstests/e2e/orm/client-api/relation/one-to-one.test.tstests/e2e/orm/client-api/create-many-and-return.test.tstests/e2e/orm/client-api/typed-json-fields.test.tspackages/testtools/src/client.tstests/e2e/orm/client-api/relation/self-relation.test.tstests/e2e/orm/client-api/default-values.test.tspackages/plugins/policy/src/plugin.tspackages/plugins/policy/src/expression-transformer.tstests/e2e/orm/policy/migrated/query-reduction.test.tstests/e2e/orm/client-api/utils.tspackages/testtools/package.json
tests/e2e/**
📄 CodeRabbit inference engine (CLAUDE.md)
End-to-end tests must live under
tests/e2e/
Files:
tests/e2e/orm/client-api/compound-id.test.tstests/e2e/orm/policy/migrated/unique-as-id.test.tstests/e2e/orm/client-api/aggregate.test.tstests/e2e/orm/plugin-infra/entity-mutation-hooks.test.tstests/e2e/orm/client-api/filter.test.tstests/e2e/orm/client-api/group-by.test.tstests/e2e/orm/policy/crud/post-update.test.tstests/e2e/orm/client-api/count.test.tstests/e2e/orm/client-api/find.test.tstests/e2e/orm/policy/crud/delete.test.tstests/e2e/orm/policy/migrated/view.test.tstests/e2e/orm/client-api/create.test.tstests/e2e/orm/policy/migrated/current-operation.test.tstests/e2e/orm/policy/migrated/connect-disconnect.test.tstests/e2e/orm/policy/migrated/todo-sample.test.tstests/e2e/orm/plugin-infra/on-query-hooks.test.tstests/e2e/orm/client-api/upsert.test.tstests/e2e/orm/policy/migrated/relation-one-to-one-filter.test.tstests/e2e/orm/policy/crud/dumb-rules.test.tstests/e2e/orm/policy/migrated/petstore-sample.test.tstests/e2e/orm/client-api/transaction.test.tstests/e2e/orm/policy/crud/update.test.tstests/e2e/orm/policy/migrated/nested-to-one.test.tstests/e2e/orm/plugin-infra/on-kysely-query.test.tstests/e2e/orm/policy/migrated/deep-nested.test.tstests/e2e/orm/policy/basic-schema-read.test.tstests/e2e/orm/policy/migrated/relation-check.test.tstests/e2e/orm/policy/migrated/update-many-and-return.test.tstests/e2e/orm/policy/migrated/omit.test.tstests/e2e/orm/policy/migrated/current-model.test.tstests/e2e/orm/policy/migrated/multi-id-fields.test.tstests/e2e/orm/policy/migrated/multi-field-unique.test.tstests/e2e/orm/policy/migrated/empty-policy.test.tstests/e2e/orm/client-api/undefined-values.test.tstests/e2e/orm/client-api/scalar-list.test.tstests/e2e/orm/prisma-consistency/attributes.test.tstests/e2e/orm/policy/migrated/create-many-and-return.test.tstests/e2e/orm/client-api/delete-many.test.tstests/e2e/orm/client-api/type-coverage.test.tstests/e2e/orm/policy/migrated/self-relation.test.tstests/e2e/orm/client-api/mixin.test.tstests/e2e/orm/policy/migrated/cross-model-field-comparison.test.tstests/e2e/orm/policy/migrated/field-comparison.test.tstests/e2e/orm/policy/crud/create.test.tstests/e2e/orm/client-api/relation/many-to-many.test.tstests/e2e/orm/policy/migrated/relation-one-to-many-filter.test.tstests/e2e/orm/client-api/computed-fields.test.tstests/e2e/orm/client-api/relation/one-to-many.test.tstests/e2e/orm/policy/migrated/auth.test.tstests/e2e/orm/policy/crud/read.test.tstests/e2e/orm/client-api/delegate.test.tstests/e2e/orm/policy/migrated/nested-to-many.test.tstests/e2e/orm/client-api/name-mapping.test.tstests/e2e/orm/policy/migrated/toplevel-operations.test.tstests/e2e/orm/client-api/update-many.test.tstests/e2e/orm/policy/migrated/client-extensions.test.tstests/e2e/orm/client-api/raw-query.test.tstests/e2e/orm/policy/migrated/relation-many-to-many-filter.test.tstests/e2e/orm/client-api/delete.test.tstests/e2e/orm/client-api/create-many.test.tstests/e2e/orm/client-api/update.test.tstests/e2e/orm/policy/utils.tstests/e2e/orm/client-api/import.test.tstests/e2e/orm/client-api/relation/one-to-one.test.tstests/e2e/orm/client-api/create-many-and-return.test.tstests/e2e/orm/client-api/typed-json-fields.test.tstests/e2e/orm/client-api/relation/self-relation.test.tstests/e2e/orm/client-api/default-values.test.tstests/e2e/orm/policy/migrated/query-reduction.test.tstests/e2e/orm/client-api/utils.ts
pnpm-workspace.yaml
📄 CodeRabbit inference engine (CLAUDE.md)
Use pnpm workspaces for the monorepo
Files:
pnpm-workspace.yaml
🧬 Code graph analysis (3)
tests/e2e/orm/policy/basic-schema-read.test.ts (2)
packages/plugins/policy/src/plugin.ts (1)
PolicyPlugin(6-29)packages/plugins/policy/src/expression-transformer.ts (1)
schema(81-83)
packages/testtools/src/client.ts (5)
tests/e2e/orm/policy/utils.ts (1)
createPolicyTestClient(14-25)packages/sdk/src/schema/schema.ts (1)
SchemaDef(10-18)packages/plugins/policy/src/expression-transformer.ts (1)
schema(81-83)packages/runtime/src/client/contract.ts (1)
ClientContract(50-159)packages/plugins/policy/src/plugin.ts (1)
PolicyPlugin(6-29)
packages/plugins/policy/src/expression-transformer.ts (2)
packages/common-helpers/src/tiny-invariant.ts (1)
invariant(4-14)packages/runtime/src/schema/expression.ts (1)
ExpressionUtils(16-120)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: claude-review
🔇 Additional comments (97)
tests/e2e/orm/client-api/transaction.test.ts (1)
2-2: LGTM! Clean migration to external packages.The import updates correctly externalize dependencies to
@zenstackhq/runtimeand@zenstackhq/testtools, aligning with the PR's objective to refactor the runtime tests. The type-only import forClientContractfollows TypeScript best practices.Also applies to: 4-4
tests/e2e/orm/policy/migrated/cross-model-field-comparison.test.ts (1)
2-2: LGTM! Import updated to use published testtools package.The change from a local relative import to
@zenstackhq/testtoolsaligns with the PR's objective to centralize test utilities into published packages. ThecreatePolicyTestClienthelper is now correctly sourced from the scoped package.tests/e2e/orm/prisma-consistency/attributes.test.ts (2)
1-1: LGTM!Removing the unused
expectimport from Vitest is appropriate since the test file usesexpectValidationFailureandexpectValidationSuccesswrappers from test-utils instead of directly calling Vitest'sexpect.
2-8: LGTM!The reordering of imports from
./test-utilsis a cosmetic change that maintains all necessary symbols. All imported utilities are properly used throughout the test file.tests/e2e/orm/client-api/create-many.test.ts (1)
2-4: LGTM! Import refactor aligns with package restructuring.The migration from local relative imports to scoped
@zenstackhq/runtimeand@zenstackhq/testtoolspackages is correct. The type-only import forClientContractand runtime import forcreateTestClientare appropriately used.tests/e2e/orm/policy/migrated/deep-nested.test.ts (1)
2-2: LGTM!The import update correctly migrates from a local utility module to the published
@zenstackhq/testtoolspackage. ThecreatePolicyTestClienthelper is confirmed to be exported from the testtools package, and the usage pattern remains unchanged throughout the test file.Based on learnings.
tests/e2e/orm/client-api/create-many-and-return.test.ts (1)
2-4: LGTM! Import refactor aligns with published packages.The import changes correctly switch from local paths to the published
@zenstackhq/runtimeand@zenstackhq/testtoolspackages, consistent with the PR's objective to consolidate runtime/testtools exports. The type-only import forClientContractis appropriate.packages/plugins/policy/vitest.config.ts (1)
1-4: LGTM – Vitest config correctly extends shared base with no overrides.tests/e2e/orm/client-api/relation/self-relation.test.ts (1)
2-2: LGTM! Import source updated to@zenstackhq/testtoolsand the dependency is declared in package.json.tests/e2e/orm/policy/basic-schema-read.test.ts (2)
1-3: LGTM! Imports correctly updated to use external packages.The imports have been successfully refactored to use the external @zenstackhq packages (
plugin-policy,runtime,testtools) instead of local paths, which aligns with the PR's objective of centralizing and modularizing the codebase.
26-26: LGTM! Correct usage of generic parameter.The PolicyPlugin is correctly instantiated with the generic parameter
<typeof schema>, ensuring type safety as per the refactored plugin system.tests/e2e/orm/policy/crud/dumb-rules.test.ts (1)
2-2: LGTM — external test client import
The@zenstackhq/testtoolspackage exportscreatePolicyTestClientwith the expected overloads and no breaking changes were found.tests/e2e/orm/policy/migrated/relation-one-to-one-filter.test.ts (1)
2-2: LGTM —@zenstackhq/testtoolsis declared in package.json.tests/e2e/orm/policy/migrated/nested-to-many.test.ts (1)
2-2: Confirm consistent import source
No remaining local imports ofcreatePolicyTestClientintests/e2e/orm/policy/migrated/; all tests now import from@zenstackhq/testtools.tests/e2e/orm/client-api/computed-fields.test.ts (1)
1-2: Approve import path updates:@zenstackhq/runtime/helpersre-exports Kysely’ssql, and@zenstackhq/testtoolsexportscreateTestClientwith the expected API.tests/e2e/orm/client-api/relation/one-to-one.test.ts (1)
1-1: LGTM – external test utilities migration complete
All local../../utilsimports have been removed andcreateTestClientis correctly exported by@zenstackhq/testtools.tests/e2e/orm/policy/migrated/todo-sample.test.ts (1)
2-4: Import refactoring verified
Imports from@zenstackhq/runtimeand@zenstackhq/testtoolsresolve correctly, dependencies are declared intests/e2e/package.json, and the exported symbols (ClientContract,createPolicyTestClient) are present.tests/e2e/orm/policy/migrated/unique-as-id.test.ts (1)
2-2: Verified createPolicyTestClient export from @zenstackhq/testtools. Import path is correct and aligns with PR objectives.tests/e2e/orm/client-api/compound-id.test.ts (1)
2-2: LGTM — VerifiedcreateTestClientis exported with compatible signatures from@zenstackhq/testtools.tests/e2e/orm/client-api/relation/many-to-many.test.ts (1)
2-2: LGTM. The import change aligns with the refactor and@zenstackhq/testtoolsis already declared as a workspace dependency in tests/e2e/package.json.tests/e2e/orm/client-api/filter.test.ts (1)
2-2: LGTM – verify package exports
EnsureClientContractis exported by@zenstackhq/runtimeandcreateTestClientby@zenstackhq/testtools, and that both packages are declared in package.json.tests/e2e/orm/client-api/name-mapping.test.ts (2)
5-5: VerifiedcreateTestClientexport and signature: it’s publicly exported with parameters(schema, options?, schemaFile?), matching the test usage.
3-3: Confirm imports from @zenstackhq/runtime and @zenstackhq/testtools Both ClientContract and createTestClient are publicly exported; no changes required.tests/e2e/orm/client-api/group-by.test.ts (1)
2-4: LGTM — imports verified.
ClientContract and createTestClient exports exist, dependencies declared, and no legacy relative imports remain.tests/e2e/orm/client-api/relation/one-to-many.test.ts (2)
4-85: LGTM!The test implementation is well-structured with proper cleanup, clear test cases, and appropriate assertions. Both unnamed and named one-to-many relation scenarios are covered effectively.
2-2: Approve centralized test utility import
createTestClient is exported by @zenstackhq/testtools with a compatible signature.tests/e2e/orm/policy/migrated/empty-policy.test.ts (2)
5-105: LGTM! Test logic remains unchanged.The test implementations are correct and consistent. The only change is the import source, which aligns with the PR's objective to centralize test utilities. Pending verification that
createPolicyTestClientis properly exported from@zenstackhq/testtoolswith a compatible API.
2-2: VerifycreatePolicyTestClientexport and dependency
- Confirm
@zenstackhq/testtoolsis listed under dependencies in your rootpackage.json.- Ensure
createPolicyTestClientis exported by that package and its API matches the previous local implementation.tests/e2e/orm/client-api/typed-json-fields.test.ts (1)
2-2: LGTM: external test utility importedThe change aligns with the PR’s goal and the dependency is declared.
- Verify that
createTestClientis exported by@zenstackhq/testtoolswith a compatible signature.- Once all tests switch off local utils, remove
tests/e2e/orm/client-api/utils.tsto clean up dead code.tests/e2e/orm/client-api/scalar-list.test.ts (2)
2-2: LGTM! Centralizing test utilities.The migration from a local relative import to
@zenstackhq/testtoolsaligns with the PR's goal to standardize test tooling and improves maintainability.
17-19: All createTestClient imports migrated to @zenstackhq/testtools
No remaining relative imports found in tests/e2e.packages/runtime/vitest.config.ts (1)
4-4: Confirm test setup removal is safe.
- Verify any required initialization (mocks, globals, env vars) has been moved into the shared/base config.
- Run the tests in
packages/runtimeto ensure they pass without the explicittest.setupFiles.tests/e2e/orm/client-api/default-values.test.ts (2)
78-108: LGTM! Comprehensive default value testing.The test implementation correctly:
- Validates multiple ID format generators (uuid, uuid7, cuid, cuid2, nanoid, ulid)
- Tests datetime default (
now())- Verifies boolean defaults
- Covers mixed scenarios where some fields use defaults and others are explicitly set
The test logic is sound and the assertions are appropriate.
2-3: VerifySchemaDefexport in@zenstackhq/runtime/schema
Confirm that theSchemaDeftype is present and exported in the published package’sschema.d.ts, as build artifacts aren’t available locally.tests/e2e/orm/client-api/update-many.test.ts (2)
4-4: Verify createTestClient exportClient-api tests now import
createTestClientfrom@zenstackhq/testtools; confirm this function is exported by the package.
2-2: Verified ClientContract export: theClientContract<…>type is defined and exported inpackages/runtime/src/client/contract.ts, so importing it from@zenstackhq/runtimeis correct.tests/e2e/orm/policy/migrated/relation-one-to-many-filter.test.ts (1)
2-2: LGTM! Import migration to external test utilities.The import path update from local
../utilsto@zenstackhq/testtoolscorrectly implements the PR's objective to standardize test utilities across the codebase.tests/e2e/orm/policy/migrated/create-many-and-return.test.ts (1)
2-2: LGTM! Consistent import migration.Import correctly updated to use the external
@zenstackhq/testtoolspackage.tests/e2e/orm/policy/migrated/auth.test.ts (1)
2-2: LGTM! Import path correctly updated.tests/e2e/orm/policy/migrated/self-relation.test.ts (1)
2-2: LGTM! Import refactored to external package.tests/e2e/orm/policy/migrated/view.test.ts (1)
2-2: LGTM! Import migration complete.tests/e2e/orm/policy/migrated/query-reduction.test.ts (1)
2-2: LGTM! Import correctly updated.tests/e2e/orm/policy/migrated/relation-many-to-many-filter.test.ts (1)
2-2: LGTM! Import path updated to external package.pnpm-workspace.yaml (1)
17-18: Approve database driver additions
better-sqlite3 and pg are used in multiple workspace packages (e2e tests, runtime, policy plugin, blog sample, testtools, CLI), so adding them to the pnpm workspace catalog ensures consistent versions.tests/e2e/orm/policy/migrated/multi-id-fields.test.ts (1)
2-2: Import aligns with shared test helpersUsing the scoped package keeps these e2e tests on the same helper implementation as the rest of the suite.
packages/testtools/src/vitest-ext.ts (1)
1-1: Good move pulling errors from runtimeImporting the canonical runtime error classes avoids duplicated definitions and keeps the
instanceofchecks accurate.tests/e2e/orm/policy/migrated/field-comparison.test.ts (1)
2-2: Consistent helper importNice to see this test pulling
createPolicyTestClientfrom the shared package now.tests/e2e/orm/policy/migrated/relation-check.test.ts (1)
2-2: Unified helper sourceThis keeps the relation-check tests on the same packaged helper as the rest of the e2e suite.
packages/plugins/policy/src/expression-evaluator.ts (1)
4-13: Runtime schema import looks rightPointing these expression types at
@zenstackhq/runtime/schemaensures the plugin tracks the new canonical schema definitions.packages/plugins/policy/src/column-collector.ts (1)
2-2: SDK dependency verified Thepackages/plugins/policy/package.jsonalready declares@zenstackhq/sdkunder dependencies.tests/e2e/orm/policy/migrated/client-extensions.test.ts (1)
1-2: External exports verifiedBoth
definePluginandcreatePolicyTestClientare exported by their respective packages; imports are correct.tests/e2e/orm/client-api/raw-query.test.ts (1)
2-4: Verified external package exports:ClientContractandcreateTestClientare both exported by@zenstackhq/runtimeand@zenstackhq/testtools, respectively.packages/cli/package.json (1)
56-56: Catalog entry verified for better-sqlite3
better-sqlite3is defined inpnpm-workspace.yamlunder the catalog section.packages/sdk/src/index.ts (1)
3-4: New public exports verified
Bothexpression-utilsanddefault-operation-node-visitorexist, export public classes (ExpressionVisitor,DefaultOperationNodeVisitor), and introduce no circular dependencies.packages/runtime/src/client/index.ts (1)
2-11: Verify removal of directsqlexport and new API surface
QueryUtilsdoesn’t re-exportsql; importingsqlfrom@zenstackhq/runtimewill now break—confirm this is intended or re-addexport { sql } from 'kysely';.- Broader re-exports (
contract,options,plugin) and new exports (getCrudDialect,BaseCrudDialect) are present.tests/e2e/orm/policy/migrated/toplevel-operations.test.ts (1)
1-1: Verified export of createPolicyTestClient. The function is exported inclient.ts, re-exported insrc/index.ts, and included inpackage.jsonexports.packages/runtime/package.json (2)
7-7: LGTM! Build script appropriately simplified.The removal of test-related commands from the build script aligns with the PR objective of moving e2e ORM tests to the e2e project. The simplified script now focuses solely on type-checking and bundling.
77-79: LGTM! Catalog-based dependency resolution.Using catalog references for peer dependencies (better-sqlite3 and pg) improves consistency with workspace-wide dependency management and simplifies version maintenance.
packages/runtime/src/client/query-utils.ts (2)
2-11: LGTM! Import expansion for new utilities.The expanded Kysely imports provide the necessary types for the new node inspection helpers added below.
380-410: LGTM! Well-designed node inspection utilities.The three new helpers (
stripAlias,extractModelName,extractFieldName) provide clean abstractions for Kysely OperationNode inspection:
stripAlias: Properly handles both aliased and non-aliased nodesextractModelName: Correctly extracts table identifiers after stripping aliasesextractFieldName: Handles multiple node types (ReferenceNode, ColumnNode)These utilities centralize logic previously scattered across multiple files, improving maintainability.
packages/runtime/src/client/executor/zenstack-query-executor.ts (1)
30-30: LGTM! Import source updated.The import source change from
kysely-utilstoquery-utilsaligns with the centralization of Kysely node inspection utilities into the runtime's query-utils module.packages/runtime/src/client/executor/name-mapper.ts (1)
20-20: LGTM! Import consolidation.The consolidated import from
query-utilscentralizes all Kysely node inspection utilities into a single source, improving code organization.packages/testtools/src/client.ts (2)
4-6: LGTM! External package imports.The import updates align with the PR's goal of consuming published ZenStack packages (
@zenstackhq/plugin-policy,@zenstackhq/runtime) instead of local source references.
169-188: LGTM! Well-designed helper with proper plugin merging.The
createPolicyTestClienthelper provides a clean API for creating policy-enabled test clients. The implementation correctly:
- Provides three overloads for type safety
- Delegates to
createTestClientfor consistency- Merges user-supplied plugins with
PolicyPlugin(Line 185:plugins: [...(options?.plugins ?? []), new PolicyPlugin()]) rather than overwriting themThis addresses the past review feedback about plugin merging.
tests/e2e/orm/policy/migrated/petstore-sample.test.ts (1)
2-2: LGTM! External test utility import.The import source change from a local utility module to
@zenstackhq/testtoolsaligns with the PR's objective of consuming published test utilities.tests/e2e/orm/client-api/delegate.test.ts (1)
3-5: LGTM! External package imports.The import updates to use
@zenstackhq/runtimeand@zenstackhq/testtoolsalign with the PR's migration from local source references to published packages.packages/plugins/policy/package.json (2)
18-33: LGTM! Dual module exports properly configured.The export configuration correctly provides both ESM (
import) and CJS (require) entry points with appropriate type definitions, ensuring compatibility with different module systems.
34-42: LGTM! Well-structured dependency declarations.Dependencies are appropriately scoped:
- Internal ZenStack packages use
workspace:*references- External packages use
catalog:for centralized version management- Kysely correctly declared as a peer dependency (runtime requirement provided by consumer)
tests/e2e/orm/client-api/type-coverage.test.ts (1)
3-3: LGTM!The import migration from local test utilities to
@zenstackhq/testtoolsis clean and aligns with the PR's objective to externalize test helpers.tests/e2e/orm/policy/crud/read.test.ts (1)
2-2: LGTM!The import source update from local utilities to
@zenstackhq/testtoolsis consistent with the broader refactoring to externalize test tooling.packages/plugins/policy/src/plugin.ts (1)
1-2: LGTM!The migration of runtime type imports from relative paths to external package imports (
@zenstackhq/runtimeand@zenstackhq/runtime/schema) correctly reflects the policy plugin's new package structure consuming runtime as a published dependency.packages/runtime/src/client/crud/operations/delete.ts (1)
4-4: LGTM!Consolidating policy error imports into the runtime errors module (
../../errors) correctly centralizes error definitions and removes the dependency on the plugin-specific error path.tests/e2e/orm/policy/crud/delete.test.ts (1)
2-2: LGTM!Consistent migration of
createPolicyTestClientimport to@zenstackhq/testtoolsaligns with the test utility externalization strategy.tests/e2e/orm/client-api/utils.ts (1)
1-1: LGTM!The
ClientContracttype import correctly migrates from local source to the external@zenstackhq/runtimepackage, maintaining consistency with the runtime externalization effort.packages/runtime/src/client/crud/operations/create.ts (1)
4-4: LGTM!The policy error import consolidation into
../../errorsis consistent with the same refactoring applied indelete.ts, correctly centralizing error definitions within the runtime module.tests/e2e/orm/policy/crud/create.test.ts (1)
2-2: LGTM!The final test file correctly migrates to
@zenstackhq/testtools, completing the consistent pattern of test utility externalization across all reviewed test files.tests/e2e/orm/policy/migrated/multi-field-unique.test.ts (1)
2-3: LGTM! Clean import migration to external packages.The imports have been correctly updated to use the externalized
@zenstackhq/runtimeand@zenstackhq/testtoolspackages, aligning with the broader repository refactoring effort.tests/e2e/orm/plugin-infra/on-query-hooks.test.ts (1)
1-2: LGTM! Import sources correctly updated.The migration to external packages is correct, and the addition of
definePluginimport aligns with its usage in the test suite.tests/e2e/orm/policy/migrated/connect-disconnect.test.ts (3)
2-2: LGTM! Import migration completed.The import source has been correctly updated to use the external
@zenstackhq/testtoolspackage.
305-320: Verify that the re-enabled test passes.This test was previously skipped and has now been enabled. Ensure that it passes consistently in CI and handles the implicit many-to-many policy enforcement correctly.
352-379: Verify that the re-enabled test passes.This test was previously skipped and has now been enabled. Ensure that it passes consistently in CI and correctly validates explicit many-to-many join table policy enforcement.
tests/e2e/orm/plugin-infra/entity-mutation-hooks.test.ts (1)
1-2: LGTM! Import sources correctly updated.The migration to external
@zenstackhq/runtimeand@zenstackhq/testtoolspackages is correct and consistent with the repository-wide refactoring.tests/e2e/orm/client-api/count.test.ts (1)
2-2: LGTM! Import sources correctly updated.The imports have been properly migrated to the external packages, maintaining type safety and test functionality.
Also applies to: 4-4
tests/e2e/orm/policy/utils.ts (1)
1-4: LGTM! Import sources correctly updated.The migration to external packages is correct, including the new
@zenstackhq/plugin-policypackage for thePolicyPluginclass. The utility function correctly wraps the test client creation with policy plugin support.packages/runtime/src/client/crud/operations/update.ts (1)
4-4: LGTM! Policy error types correctly relocated.The import path has been updated to reflect the consolidation of policy error types into the runtime errors module (
../../errors). This aligns with the broader refactoring effort to centralize error definitions.tests/e2e/orm/client-api/delete.test.ts (1)
2-2: LGTM! Import sources correctly updated.The migration to external packages is correct and consistent with the repository-wide refactoring effort.
Also applies to: 4-4
tests/e2e/orm/client-api/update.test.ts (1)
2-4: LGTM! Import sources updated correctly.The imports now source from external packages (
@zenstackhq/runtimeand@zenstackhq/testtools) instead of local paths, aligning with the PR's goal to externalize test utilities and runtime types.tests/e2e/orm/policy/migrated/current-operation.test.ts (1)
2-2: LGTM! Import source updated correctly.The import of
createPolicyTestClientnow sources from@zenstackhq/testtoolsinstead of a local relative path, aligning with the PR's externalization strategy.tests/e2e/orm/policy/crud/update.test.ts (1)
2-2: LGTM! Import source updated correctly.The import of
createPolicyTestClientnow sources from@zenstackhq/testtools, consistent with the broader migration to external test utilities.packages/plugins/policy/src/utils.ts (1)
1-2: LGTM! Import sources updated correctly.The imports now source from external runtime packages (
@zenstackhq/runtimeand@zenstackhq/runtime/schema) instead of local relative paths, aligning with the refactor to externalize runtime types and utilities.tests/e2e/orm/client-api/upsert.test.ts (1)
2-4: LGTM! Import sources updated correctly.The imports now source from external packages (
@zenstackhq/runtimeand@zenstackhq/testtools) instead of local paths, consistent with the PR's externalization strategy.tests/e2e/orm/client-api/undefined-values.test.ts (1)
2-4: LGTM! Import sources updated correctly.The imports now source from external packages (
@zenstackhq/runtimeand@zenstackhq/testtools) instead of local paths, aligning with the repository-wide migration.tests/e2e/orm/client-api/find.test.ts (1)
2-5: LGTM! Import sources updated correctly.The imports now source from external packages:
ClientContractfrom@zenstackhq/runtime- Error types (
InputValidationError,NotFoundError) from@zenstackhq/runtimecreateTestClientfrom@zenstackhq/testtoolsThis aligns with the PR's goal to externalize runtime types, error classes, and test utilities.
tests/e2e/orm/policy/migrated/current-model.test.ts (2)
2-2: LGTM! Import source updated correctly.The import of
createPolicyTestClientnow sources from@zenstackhq/testtools, consistent with the externalization strategy.
110-129: LGTM! Test enabled and schema syntax updated.The test is no longer skipped, and the schema syntax has been updated to reflect language changes:
abstract model→typeextends→withThis suggests the feature is now supported and the test can run successfully.
tests/e2e/orm/client-api/aggregate.test.ts (1)
1-2: Public runtime/testtools imports look solid.Switching to the published
ClientContractandcreateTestClientkeeps the tests aligned with the packaged surface. No gaps spotted.packages/runtime/src/client/errors.ts (1)
42-71: LGTM: Well-structured policy error types.The enum and error class provide a clean, type-safe API for representing policy rejections. The JSDoc comments are thorough, the default values are sensible (NO_ACCESS as default reason, conditional message construction), and the design allows both runtime and plugin code to use a centralized error surface.
|
Claude finished @ymc9's task —— View job PR Review: Move e2e ORM Tests & Fix Bundle IssuesI've completed a comprehensive review of this refactoring PR. This is a substantial and well-executed reorganization with solid technical improvements. Review Tasks
✅ Code Quality & Best PracticesExcellent structural improvements:
Bundle configuration improvements in
✅ Test Coverage & OrganizationCoverage maintained and improved:
✅ Performance ConsiderationsPositive performance impacts:
|
|
Claude finished @ymc9's task —— View job PR Review: Move e2e ORM Tests & Fix Bundle IssuesI've completed a comprehensive review of this refactoring PR. This is a substantial and well-executed reorganization with solid technical improvements. Review Tasks
✅ Code Quality & Best PracticesExcellent structural improvements:
Bundle configuration improvements in
✅ Test Coverage & OrganizationCoverage maintained and improved:
✅ Performance ConsiderationsPositive performance impacts:
|
Summary by CodeRabbit
New Features
Refactor
Chores