test(bun): basic e2e tests #457
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. WalkthroughExports PostgreSQL test config, adds Bun-based end-to-end tests and generated schema/type artifacts, updates generated payload types to accept ClientOptions, adjusts workspace catalog and test orchestration to use turbo, and adds CI step to install Bun. (50 words) Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 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 ignored due to path filters (1)
📒 Files selected for processing (3)
🧰 Additional context used🧠 Learnings (8)📓 Common learnings📚 Learning: 2025-11-26T01:55:04.540ZApplied to files:
📚 Learning: 2025-11-26T01:55:04.540ZApplied to files:
📚 Learning: 2025-11-26T01:55:04.540ZApplied to files:
📚 Learning: 2025-11-26T01:55:04.540ZApplied to files:
📚 Learning: 2025-11-26T01:55:04.540ZApplied to files:
📚 Learning: 2025-11-26T01:55:04.540ZApplied to files:
📚 Learning: 2025-11-26T01:55:04.540ZApplied to files:
🧬 Code graph analysis (1)tests/bun/e2e.test.ts (5)
⏰ 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 Code Review: Bun E2E TestsReviewing PR that adds Bun e2e testing infrastructure to ZenStack v3. Review Tasks
Review SummaryThis PR adds comprehensive Bun e2e testing infrastructure to ZenStack v3. Overall, the implementation is well-structured and follows good practices. Here's my detailed analysis: ✅ Positive AspectsTest Structure & Organization
Code Quality
Integration
🔍 Areas for ImprovementCode Quality Issues
Test Coverage Gaps
Configuration Issues
🚨 Security Considerations
📈 Performance Considerations
🔧 RecommendationsHigh Priority
Medium Priority
Low Priority
🎯 ConclusionThis is a high-quality addition that follows ZenStack's patterns and provides valuable testing infrastructure for Bun runtime compatibility. The implementation is solid with only minor improvements needed. The PR successfully:
Recommendation: Approve with minor fixes - address the type casting and error handling issues before merging. |
There was a problem hiding this comment.
Pull request overview
This PR adds basic end-to-end tests for Bun runtime support, expanding the project's test coverage to ensure ZenStack works correctly with Bun. The changes include infrastructure updates to support Bun testing, new test files, schema generation scripts, and updates to generated type definitions.
Key Changes:
- Added new Bun e2e test suite with SQLite and PostgreSQL support
- Exported TEST_PG_URL and TEST_PG_CONFIG from testtools for better reusability
- Updated turbo.json to disable test caching and pass through TEST_DB_PROVIDER environment variable
- Updated catalog versions and cleaned up unused dependencies
Reviewed changes
Copilot reviewed 18 out of 19 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/bun/e2e.test.ts | New Bun e2e test file testing basic CRUD and policy enforcement across sqlite and postgres |
| tests/bun/package.json | Package configuration for Bun test suite with dependencies and test scripts |
| tests/bun/tsconfig.json | TypeScript configuration for Bun tests with bun-types |
| tests/bun/scripts/generate.ts | Schema generation script using bunx zen generate |
| tests/bun/schemas/*.ts | Auto-generated schema files for Bun tests |
| tests/bun/schemas/schema.zmodel | ZModel schema definition with User/Post models and access policies |
| packages/testtools/src/client.ts | Exported TEST_PG_CONFIG and TEST_PG_URL for reusability across test suites |
| turbo.json | Disabled test caching and added TEST_DB_PROVIDER to global pass-through env vars |
| pnpm-workspace.yaml | Reorganized and updated catalog (alphabetized, updated pg version) |
| tests/e2e/package.json | Removed unused @types/uuid dependency |
| tests/regression/package.json | Added @types/node dependency |
| tests/regression/tsconfig.json | Added "node" to types array |
| tests/regression/test/issue-*/input.ts | Updated generated type signatures with ClientOptions parameter |
| package.json | Changed test script from vitest to turbo for consistent test orchestration |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.
|
Claude finished @ymc9's task —— View job Code Review: Bun E2E TestsReviewing PR that adds Bun e2e testing infrastructure to ZenStack v3. Review Tasks
Review SummaryThis PR adds comprehensive Bun e2e testing infrastructure to ZenStack v3. Overall, the implementation is well-structured and follows good practices. Here's my detailed analysis: ✅ Positive AspectsTest Structure & Organization
Code Quality
Integration
🔍 Areas for ImprovementCode Quality Issues
Test Coverage Gaps
Configuration Issues
🚨 Security Considerations
📈 Performance Considerations
🔧 RecommendationsHigh Priority
Medium Priority
Low Priority
🎯 ConclusionThis is a high-quality addition that follows ZenStack's patterns and provides valuable testing infrastructure for Bun runtime compatibility. The implementation is solid with only minor improvements needed. The PR successfully:
Recommendation: Approve with minor fixes - address the type casting and error handling issues before merging. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (6)
tests/bun/scripts/generate.ts (2)
16-18: Remove unnecessaryasynckeyword.The
generatefunction uses synchronousexecSync, making theasynckeyword misleading. Also consider adding error handling for generation failures.-async function generate(schemaPath: string) { - execSync('bunx --bun zen generate', { cwd: path.dirname(schemaPath) }); +function generate(schemaPath: string) { + execSync('bunx --bun zen generate', { cwd: path.dirname(schemaPath), stdio: 'inherit' }); }Adding
stdio: 'inherit'would surface any generation errors to the console.
8-14: Consider simplifying the main function.Since
generatedoesn't need to be async, the main function can also be simplified.-async function main() { +function main() { const zmodelFiles = [...glob.sync(path.resolve(dir, '../schemas/*.zmodel'))]; for (const file of zmodelFiles) { console.log(`Generating TS schema for: ${file}`); - await generate(file); + generate(file); } }packages/testtools/src/client.ts (1)
28-36: Consider URL-encoding credentials in TEST_PG_URL.If
TEST_PG_PASSWORDcontains special characters (e.g.,@,:,/,#), the URL will be malformed. Consider encoding user/password components.-export const TEST_PG_URL = `postgres://${TEST_PG_CONFIG.user}:${TEST_PG_CONFIG.password}@${TEST_PG_CONFIG.host}:${TEST_PG_CONFIG.port}`; +export const TEST_PG_URL = `postgres://${encodeURIComponent(TEST_PG_CONFIG.user)}:${encodeURIComponent(TEST_PG_CONFIG.password)}@${TEST_PG_CONFIG.host}:${TEST_PG_CONFIG.port}`;tests/bun/package.json (1)
22-27: Consider removing unused@zenstackhq/vitest-configdependency.Since this package uses
bun test(line 10) rather than Vitest, the@zenstackhq/vitest-configdev dependency appears unused. Consider removing it to keep dependencies minimal."devDependencies": { "@types/pg": "^8.15.6", "@zenstackhq/typescript-config": "workspace:*", - "@zenstackhq/vitest-config": "workspace:*", "bun-types": "^1.3.3" }tests/bun/e2e.test.ts (1)
84-111: Consider explicit Postgres DB/Pool lifecycle management to avoid flaky e2e runsThe Postgres branch dynamically drops/creates databases and instantiates a new
PoolpercreateClientcall, but the pool isn’t explicitly cleaned up or tied to a teardown, and the DB names are hard-coded. In more parallel or long-running e2e scenarios this can cause:
- lingering connections keeping the process alive
- transient failures if other workers reuse the same DB names
Consider:
- tracking created
Poolinstances and closing them in a test teardown (e.g., a file-level cleanup hook), and/or- deriving unique DB names per worker/run (e.g., suffixing with a random or process-specific identifier) to avoid cross-run collisions.
This is not a blocker but would make the Bun e2e suite more robust.
tests/bun/schemas/schema.zmodel (1)
1-45: Schema and policies match the Bun e2e expectationsThe
User/Postmodels and policy rules (@@deny('all', auth() == null), author-based full access, and published-only read) line up with what the Bun e2e test asserts for anon / author / other user counts, so the model is consistent with the test behavior. Since this is the authoritative ZModel source, just ensurezenstack generateis rerun whenever this file changes so the TS artifacts undertests/bun/schemasstay in sync.
📜 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 (17)
package.json(1 hunks)packages/testtools/src/client.ts(2 hunks)pnpm-workspace.yaml(1 hunks)tests/bun/e2e.test.ts(1 hunks)tests/bun/package.json(1 hunks)tests/bun/schemas/input.ts(1 hunks)tests/bun/schemas/models.ts(1 hunks)tests/bun/schemas/schema.ts(1 hunks)tests/bun/schemas/schema.zmodel(1 hunks)tests/bun/scripts/generate.ts(1 hunks)tests/bun/tsconfig.json(1 hunks)tests/e2e/package.json(0 hunks)tests/regression/package.json(1 hunks)tests/regression/test/issue-204/input.ts(2 hunks)tests/regression/test/issue-422/input.ts(4 hunks)tests/regression/tsconfig.json(1 hunks)turbo.json(1 hunks)
💤 Files with no reviewable changes (1)
- tests/e2e/package.json
🧰 Additional context used
📓 Path-based instructions (1)
**/*.zmodel
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.zmodel: Always runzenstack generateafter modifying ZModel schemas
ZModel schema files should define database structure and policies that compile to TypeScript viazenstack generate
Files:
tests/bun/schemas/schema.zmodel
🧠 Learnings (12)
📓 Common learnings
Learnt from: CR
Repo: zenstackhq/zenstack-v3 PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T01:55:04.540Z
Learning: Applies to tests/e2e/**/*.{ts,tsx} : E2E tests should validate real-world schema compatibility with established projects
📚 Learning: 2025-11-26T01:55:04.540Z
Learnt from: CR
Repo: zenstackhq/zenstack-v3 PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T01:55:04.540Z
Learning: Use Turbo for build orchestration and run `pnpm build`, `pnpm watch`, `pnpm lint`, and `pnpm test` for development tasks
Applied to files:
turbo.jsonpackage.json
📚 Learning: 2025-11-26T01:55:04.540Z
Learnt from: CR
Repo: zenstackhq/zenstack-v3 PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T01:55:04.540Z
Learning: Applies to tests/**/type*.{ts,tsx} : Ensure TypeScript inference and type coverage are validated through type coverage tests
Applied to files:
tests/bun/tsconfig.jsontests/bun/schemas/models.tstests/bun/e2e.test.tstests/regression/tsconfig.jsontests/regression/package.json
📚 Learning: 2025-11-26T01:55:04.540Z
Learnt from: CR
Repo: zenstackhq/zenstack-v3 PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T01:55:04.540Z
Learning: Applies to tests/e2e/**/*.{ts,tsx} : E2E tests should validate real-world schema compatibility with established projects
Applied to files:
tests/bun/tsconfig.jsontests/bun/package.jsontests/bun/schemas/models.tstests/bun/e2e.test.tstests/bun/scripts/generate.tstests/regression/tsconfig.jsontests/regression/package.jsontests/bun/schemas/schema.tstests/bun/schemas/input.ts
📚 Learning: 2025-11-26T01:55:04.540Z
Learnt from: CR
Repo: zenstackhq/zenstack-v3 PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T01:55:04.540Z
Learning: Applies to **/*.zmodel : ZModel schema files should define database structure and policies that compile to TypeScript via `zenstack generate`
Applied to files:
tests/bun/tsconfig.jsontests/bun/schemas/models.tstests/bun/scripts/generate.tstests/regression/tsconfig.jsontests/regression/package.jsontests/bun/schemas/schema.zmodeltests/bun/schemas/schema.tstests/bun/schemas/input.ts
📚 Learning: 2025-11-26T01:55:04.540Z
Learnt from: CR
Repo: zenstackhq/zenstack-v3 PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T01:55:04.540Z
Learning: Applies to packages/zenstackhq/cli/**/*.test.{ts,tsx} : CLI package tests should focus on action-specific tests for each command
Applied to files:
tests/bun/package.jsontests/regression/tsconfig.jsontests/regression/package.json
📚 Learning: 2025-11-26T01:55:04.540Z
Learnt from: CR
Repo: zenstackhq/zenstack-v3 PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T01:55:04.540Z
Learning: Use `pnpm` with workspaces for package management, pinned to version `pnpm10.12.1`
Applied to files:
pnpm-workspace.yaml
📚 Learning: 2025-11-26T01:55:04.540Z
Learnt from: CR
Repo: zenstackhq/zenstack-v3 PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T01:55:04.540Z
Learning: Applies to packages/zenstackhq/orm/**/*.{ts,tsx} : Use Kysely as the query builder interface for low-level database queries, avoiding raw SQL when possible
Applied to files:
pnpm-workspace.yamltests/bun/schemas/models.tstests/regression/test/issue-204/input.tstests/bun/schemas/schema.tstests/regression/test/issue-422/input.tstests/bun/schemas/input.ts
📚 Learning: 2025-11-26T01:55:04.540Z
Learnt from: CR
Repo: zenstackhq/zenstack-v3 PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T01:55:04.540Z
Learning: Applies to packages/zenstackhq/orm/**/*.{ts,tsx} : Implement plugin hooks at ORM, Kysely, and entity mutation levels for query interception and customization
Applied to files:
pnpm-workspace.yamltests/bun/schemas/models.tstests/regression/test/issue-204/input.tstests/regression/package.jsontests/bun/schemas/schema.tstests/regression/test/issue-422/input.tstests/bun/schemas/input.ts
📚 Learning: 2025-10-21T16:09:31.218Z
Learnt from: ymc9
Repo: zenstackhq/zenstack-v3 PR: 319
File: packages/runtime/src/client/executor/zenstack-query-executor.ts:63-72
Timestamp: 2025-10-21T16:09:31.218Z
Learning: In ZenStack, TypeDefs can be inherited by models. When a TypeDef contains fields with `map` attributes, those mapped field names need to be processed by the QueryNameMapper since they become part of the inheriting model's schema. Therefore, when checking if a schema has mapped names (e.g., in `schemaHasMappedNames`), both `schema.models` and `schema.typeDefs` must be inspected for `@map` and `map` attributes.
Applied to files:
tests/bun/schemas/models.tstests/bun/schemas/schema.ts
📚 Learning: 2025-11-26T01:55:04.540Z
Learnt from: CR
Repo: zenstackhq/zenstack-v3 PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T01:55:04.540Z
Learning: Applies to packages/zenstackhq/orm/**/*.test.{ts,tsx} : ORM package tests should include comprehensive client API tests and policy tests
Applied to files:
tests/regression/test/issue-204/input.tstests/bun/e2e.test.tstests/regression/package.jsontests/bun/schemas/schema.tstests/regression/test/issue-422/input.tstests/bun/schemas/input.ts
📚 Learning: 2025-11-26T01:55:04.540Z
Learnt from: CR
Repo: zenstackhq/zenstack-v3 PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T01:55:04.540Z
Learning: Applies to **/*.zmodel : Always run `zenstack generate` after modifying ZModel schemas
Applied to files:
tests/bun/scripts/generate.ts
🧬 Code graph analysis (5)
packages/testtools/src/client.ts (2)
packages/orm/src/client/crud/dialects/postgresql.ts (1)
provider(33-35)packages/orm/src/client/crud/dialects/sqlite.ts (1)
provider(27-29)
tests/bun/e2e.test.ts (5)
packages/plugins/policy/src/plugin.ts (1)
PolicyPlugin(6-29)packages/common-helpers/src/clone.ts (1)
clone(6-24)tests/bun/schemas/schema.ts (1)
schema(176-176)packages/testtools/src/client.ts (1)
TEST_PG_URL(35-35)packages/orm/src/dialects/postgres.ts (1)
PostgresDialect(1-1)
tests/bun/schemas/schema.ts (2)
packages/schema/src/expression-utils.ts (1)
ExpressionUtils(19-123)packages/schema/src/schema.ts (1)
SchemaDef(11-19)
tests/regression/test/issue-422/input.ts (1)
tests/bun/schemas/input.ts (1)
UserGetPayload(30-30)
tests/bun/schemas/input.ts (1)
tests/regression/test/issue-422/input.ts (20)
UserFindManyArgs(31-31)UserFindUniqueArgs(32-32)UserFindFirstArgs(33-33)UserCreateArgs(34-34)UserCreateManyArgs(35-35)UserCreateManyAndReturnArgs(36-36)UserUpdateArgs(37-37)UserUpdateManyArgs(38-38)UserUpdateManyAndReturnArgs(39-39)UserUpsertArgs(40-40)UserDeleteArgs(41-41)UserDeleteManyArgs(42-42)UserCountArgs(43-43)UserAggregateArgs(44-44)UserGroupByArgs(45-45)UserWhereInput(46-46)UserSelect(47-47)UserInclude(48-48)UserOmit(49-49)UserGetPayload(50-50)
⏰ 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). (4)
- GitHub Check: Agent
- GitHub Check: build-test (20.x, postgresql)
- GitHub Check: build-test (20.x, sqlite)
- GitHub Check: claude-review
🔇 Additional comments (16)
tests/bun/tsconfig.json (1)
1-8: LGTM!The TypeScript configuration is well-suited for Bun-based tests:
noEmit: trueis appropriate since tests don't need to emit JS, andbun-typescorrectly provides Bun-specific type definitions.tests/regression/package.json (1)
20-21: LGTM!Adding
@types/nodevia catalog reference correctly pairs with the tsconfig update and follows the monorepo's dependency management pattern.tests/regression/tsconfig.json (1)
5-5: LGTM!Adding
"node"to the types array correctly enables Node.js type definitions, complementing the@types/nodedevDependency.packages/testtools/src/client.ts (1)
109-109: LGTM!Using the centralized
TEST_PG_URLconstant simplifies the URL construction and ensures consistency.tests/regression/test/issue-204/input.ts (1)
9-10: Auto-generated file updated correctly.The new
ClientOptionsgeneric parameter with a default value maintains backward compatibility while enabling client option customization in payload types.tests/regression/test/issue-422/input.ts (2)
9-10: Auto-generated file updated consistently.The
ClientOptionsimport and updated payload type signatures are consistent with other generated files in the PR.
30-30: Payload type signatures follow consistent pattern.
SessionGetPayload,UserGetPayload, andProfileGetPayloadall correctly include the newOptionsgeneric parameter with appropriate defaults, matching the pattern intests/bun/schemas/input.tsandtests/regression/test/issue-204/input.ts.pnpm-workspace.yaml (1)
29-30: Fix Zod v4 breaking changes before deploying ^4.0.0.Zod 4.0 has multiple breaking changes that will break the current codebase:
zod/v3 import path removed (
packages/orm/src/client/crud/validator/utils.ts):import { ZodIssueCode } from 'zod/v3'will fail. In Zod 4, usez.ZodIssueCodedirectly..strict() and .passthrough() methods removed (
packages/server/src/api/rest/index.tslines 210, 223, 225): Replace.passthrough()withz.looseObject()and.strict()withz.strictObject()where applicable.ZodError API changes: Error handling in
packages/orm/src/utils/zod-utils.tsandpackages/server/src/api/utils.tsmay be affected by ZodError format/flatten() removal in favor of treeifyError().Run the automated codemod (
npx zod-v3-to-v4) and update imports and schema definitions accordingly before upgrading.⛔ Skipped due to learnings
Learnt from: CR Repo: zenstackhq/zenstack-v3 PR: 0 File: CLAUDE.md:0-0 Timestamp: 2025-11-26T01:55:04.540Z Learning: Applies to **/*.zmodel : ZModel schema files should define database structure and policies that compile to TypeScript via `zenstack generate`Learnt from: CR Repo: zenstackhq/zenstack-v3 PR: 0 File: CLAUDE.md:0-0 Timestamp: 2025-11-26T01:55:04.540Z Learning: Applies to packages/zenstackhq/orm/**/*.test.{ts,tsx} : ORM package tests should include comprehensive client API tests and policy testsLearnt from: CR Repo: zenstackhq/zenstack-v3 PR: 0 File: CLAUDE.md:0-0 Timestamp: 2025-11-26T01:55:04.540Z Learning: Applies to packages/zenstackhq/orm/**/*.{ts,tsx} : Implement plugin hooks at ORM, Kysely, and entity mutation levels for query interception and customizationLearnt from: CR Repo: zenstackhq/zenstack-v3 PR: 0 File: CLAUDE.md:0-0 Timestamp: 2025-11-26T01:55:04.540Z Learning: Applies to tests/e2e/**/*.{ts,tsx} : E2E tests should validate real-world schema compatibility with established projectspackage.json (1)
11-11: LGTM!The switch from
vitest runtoturbo run testaligns with the project's Turbo-based orchestration strategy. This enables proper dependency handling (tests depend on build) and consistent test execution across all workspaces.turbo.json (2)
21-22: LGTM!The
globalPassThroughEnvconfiguration correctly allowsTEST_DB_PROVIDERto be passed through to test tasks, enabling database provider selection during test execution.
17-20: ThedependsOn: ["build"]configuration is correct.Test packages depend on @zenstackhq workspace packages (@cli, @Orm, @language, @Schema, @plugin-policy, @SDK, @testtools). However, using
"build"(without caret) is intentional and correct because thebuildtask itself hasdependsOn: ["^build"]. This creates the proper dependency chain: dependencies are built first (via^build), then the current package is built (viabuild), then tests run. Adding"^build"directly to the test task would be redundant since the required builds are already ensured through thebuildtask's dependencies.The
cache: falsesetting is appropriate for tests to ensure consistent execution.tests/bun/package.json (1)
1-11: LGTM!The package configuration is well-structured for Bun-based e2e tests. The build script properly sequences generation and type-checking, and the test script correctly uses Bun's native test runner.
tests/bun/schemas/models.ts (1)
1-14: LGTM!This is a correctly structured generated file exporting model types and enum values. The pattern of dual-exporting
Roleas both a const (runtime) and type (compile-time) is idiomatic TypeScript for enum-like values.tests/bun/schemas/schema.ts (1)
1-177: LGTM!This generated schema file correctly defines the ZenStack ORM schema with:
- User and Post models with proper relations and access control policies
- CommonFields type definition for shared fields
- Role enum with ADMIN and USER values
- Proper use of
as const satisfies SchemaDeffor type safety while preserving literal typesThe schema structure aligns with the
SchemaDeftype definition and usesExpressionUtilscorrectly for policy expressions.tests/bun/e2e.test.ts (1)
13-42: Good coverage and alignment with policies across sqlite/postgresThe two tests exercise both providers with realistic
User/Postflows and validate the policy behavior you defined in the schema (anon → 0 posts, author → 2 posts, other user → 1 published post). Cloning the schema per provider and pushing it before use keeps cross-provider state isolated. This looks solid for a first Bun e2e pass.Also applies to: 44-81
tests/bun/schemas/input.ts (1)
1-50: Generated input types look consistent with existing patternsThe User/Post arg and payload aliases mirror the existing regression input patterns (including the
ClientOptions-awareGetPayloadsignatures) and the file is clearly generator-owned. No issues here as long as it continues to be maintained via the ZenStack CLI rather than manual edits.
|
Claude finished @ymc9's task —— View job Code Review: Bun E2E TestsReviewing PR that adds Bun e2e testing infrastructure to ZenStack v3. Review Tasks
Review SummaryThis PR adds comprehensive Bun e2e testing infrastructure to ZenStack v3. Overall, the implementation is well-structured and follows good practices. Here's my detailed analysis: ✅ Positive AspectsTest Structure & Organization
Code Quality
Integration
🔍 Areas for ImprovementCode Quality Issues
Test Coverage Gaps
Configuration Issues
🚨 Security Considerations
📈 Performance Considerations
🔧 RecommendationsHigh Priority
Medium Priority
Low Priority
🎯 ConclusionThis is a high-quality addition that follows ZenStack's patterns and provides valuable testing infrastructure for Bun runtime compatibility. The implementation is solid with only minor improvements needed. The PR successfully:
Recommendation: Approve with minor fixes - address the type casting and error handling issues before merging. |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.github/workflows/build-test.yml (1)
54-57: Pin a specific Bun version instead of "latest" for reproducible CI builds.Using
bun-version: latestmeans each CI run may use a different version, introducing non-determinism and making it harder to debug failures tied to version changes. Follow the same pinning strategy as the Node.js version (line 48-52).- name: Use Bun uses: oven-sh/setup-bun@v2 with: - bun-version: latest + bun-version: 1.x # or specify a concrete version like 1.2.3If you prefer to always test against the latest Bun, document this as an intentional choice in a comment.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/build-test.yml(1 hunks)tests/bun/e2e.test.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/bun/e2e.test.ts
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: CR
Repo: zenstackhq/zenstack-v3 PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T01:55:04.540Z
Learning: Applies to tests/e2e/**/*.{ts,tsx} : E2E tests should validate real-world schema compatibility with established projects
Learnt from: CR
Repo: zenstackhq/zenstack-v3 PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T01:55:04.540Z
Learning: Applies to packages/zenstackhq/orm/**/*.test.{ts,tsx} : ORM package tests should include comprehensive client API tests and policy tests
📚 Learning: 2025-11-26T01:55:04.540Z
Learnt from: CR
Repo: zenstackhq/zenstack-v3 PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T01:55:04.540Z
Learning: Use Turbo for build orchestration and run `pnpm build`, `pnpm watch`, `pnpm lint`, and `pnpm test` for development tasks
Applied to files:
.github/workflows/build-test.yml
⏰ 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). (2)
- GitHub Check: build-test (20.x, postgresql)
- GitHub Check: build-test (20.x, sqlite)
🔇 Additional comments (1)
.github/workflows/build-test.yml (1)
54-85: No action needed — Bun e2e tests are already included in the test orchestration.The
tests/bundirectory is part of the pnpm workspace and has atestscript configured. Whenpnpm run testexecutesturbo run test, Turbo automatically discovers and runs the test scripts in all workspace packages, including thebun testcommand fromtests/bun/package.json. The CI workflow is correctly set up to run Bun e2e tests as part of the standard test step.
|
Claude finished @ymc9's task —— View job Code Review: Bun E2E TestsReviewing PR that adds Bun e2e testing infrastructure to ZenStack v3. Review Tasks
Review SummaryThis PR adds comprehensive Bun e2e testing infrastructure to ZenStack v3. Overall, the implementation is well-structured and follows good practices. Here's my detailed analysis: ✅ Positive AspectsTest Structure & Organization
Code Quality
Integration
🔍 Areas for ImprovementCode Quality Issues
Test Coverage Gaps
Configuration Issues
🚨 Security Considerations
📈 Performance Considerations
🔧 RecommendationsHigh Priority
Medium Priority
Low Priority
🎯 ConclusionThis is a high-quality addition that follows ZenStack's patterns and provides valuable testing infrastructure for Bun runtime compatibility. The implementation is solid with only minor improvements needed. The PR successfully:
Recommendation: Approve with minor fixes - address the type casting and error handling issues before merging. |
Summary by CodeRabbit
New Features
Tests
Improvements
Chores
✏️ Tip: You can customize this high-level summary in your review settings.