add CI workflow #683
add CI workflow #683
Conversation
|
@moreorover is attempting to deploy a commit to the Better T Stack Team on Vercel. A member of the Team first needs to authorize it. |
|
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 updates the CLI test suite by removing the config-package test file and related helper utilities, modifying test case expectations across multiple integration and deployment tests, configuring test coverage in bunfig.toml, and adding Git user setup to the CI workflow. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing touches
🧪 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 |
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/cli/test/database-orm.test.ts (1)
167-186: Fix contradictory test configuration and assertion.The test is configured with
expectError: true(line 182) but then callsexpectSuccess(result)(line 185). This is logically inconsistent and will cause the test to fail.Based on the test name "should work with auth but no database", it appears the intent is to verify this configuration succeeds. Therefore, remove the
expectError: trueflag.Apply this diff:
it("should work with auth but no database (non-convex backend)", async () => { const result = await runTRPCTest({ projectName: "auth-no-db", database: "none", orm: "none", auth: "better-auth", backend: "hono", runtime: "bun", frontend: ["tanstack-router"], api: "trpc", addons: ["none"], examples: ["none"], dbSetup: "none", webDeploy: "none", serverDeploy: "none", - expectError: true, }); expectSuccess(result); });
🧹 Nitpick comments (2)
.github/workflows/test.yaml (1)
15-18: Consider pinning Bun version for CI determinism.Using
bun-version: latestcould lead to unpredictable CI behavior if a new Bun release introduces breaking changes or unexpected behavior. Consider pinning to a specific version (e.g.,1.1.0) and updating it deliberately.CLAUDE.md (1)
117-126: Consider adding language identifiers to code blocks.The code blocks at lines 117 and 172 are missing language specifiers, which helps with syntax highlighting and is a markdown best practice.
Apply this diff:
-``` +```text User Input (CLI flags or prompts) → Flag Processing (validation.ts) → Compatibility Validation (utils/compatibility-rules.ts)And at line 172:
-``` +```text templates/ ├── frontend/ # React, Next, Nuxt, Svelte, Solid, React Native
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (23)
.github/workflows/test.yaml(1 hunks).idea/.gitignore(1 hunks).idea/biome.xml(1 hunks).idea/create-better-t-stack.iml(1 hunks).idea/git_toolbox_prj.xml(1 hunks).idea/inspectionProfiles/Project_Default.xml(1 hunks).idea/jsLibraryMappings.xml(1 hunks).idea/misc.xml(1 hunks).idea/modules.xml(1 hunks).idea/vcs.xml(1 hunks)CLAUDE.md(1 hunks)apps/cli/test/addons.test.ts(1 hunks)apps/cli/test/api.test.ts(2 hunks)apps/cli/test/auth.test.ts(2 hunks)apps/cli/test/backend-runtime.test.ts(2 hunks)apps/cli/test/basic-configurations.test.ts(1 hunks)apps/cli/test/database-orm.test.ts(3 hunks)apps/cli/test/database-setup.test.ts(2 hunks)apps/cli/test/deployment.test.ts(7 hunks)apps/cli/test/examples.test.ts(2 hunks)apps/cli/test/frontend.test.ts(1 hunks)apps/cli/test/index.test.ts(1 hunks)apps/cli/test/integration.test.ts(4 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/convex_rules.mdc)
**/*.{ts,tsx}: Use Id from './_generated/dataModel' to type document ids (e.g., Id<'users'>)
Ensure Record key/value types align with validators (e.g., v.record(v.id('users'), v.string()) => Record<Id<'users'>, string>)
Be strict with types for document ids; prefer Id<'table'> over string
Use 'as const' for string literals in discriminated unions
When using Array and Record types, declare with explicit generic types (e.g., const arr: Array = ...)
**/*.{ts,tsx}: Use TypeScript type aliases instead of interface declarations.
Do not use explicit return types in TypeScript.
Files:
apps/cli/test/index.test.tsapps/cli/test/frontend.test.tsapps/cli/test/auth.test.tsapps/cli/test/basic-configurations.test.tsapps/cli/test/backend-runtime.test.tsapps/cli/test/integration.test.tsapps/cli/test/deployment.test.tsapps/cli/test/examples.test.tsapps/cli/test/database-setup.test.tsapps/cli/test/api.test.tsapps/cli/test/database-orm.test.tsapps/cli/test/addons.test.ts
**/*.{js,jsx,ts,tsx,mjs,cjs}
📄 CodeRabbit inference engine (.cursor/rules/use-bun-instead-of-node-vite-npm-pnpm.mdc)
**/*.{js,jsx,ts,tsx,mjs,cjs}: Do not use dotenv; Bun auto-loads .env
UseBun.serve()for HTTP/WebSockets; do not useexpress
Usebun:sqlitefor SQLite; do not usebetter-sqlite3
UseBun.redisfor Redis; do not useioredis
UseBun.sqlfor Postgres; do not usepgorpostgres.js
Use built-inWebSocket; do not usews
PreferBun.fileovernode:fsreadFile/writeFile
UseBun.$instead ofexecafor shelling out
Files:
apps/cli/test/index.test.tsapps/cli/test/frontend.test.tsapps/cli/test/auth.test.tsapps/cli/test/basic-configurations.test.tsapps/cli/test/backend-runtime.test.tsapps/cli/test/integration.test.tsapps/cli/test/deployment.test.tsapps/cli/test/examples.test.tsapps/cli/test/database-setup.test.tsapps/cli/test/api.test.tsapps/cli/test/database-orm.test.tsapps/cli/test/addons.test.ts
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/better-t-stack-repo.mdc)
Define functions using the standard function declaration syntax, not arrow functions.
Files:
apps/cli/test/index.test.tsapps/cli/test/frontend.test.tsapps/cli/test/auth.test.tsapps/cli/test/basic-configurations.test.tsapps/cli/test/backend-runtime.test.tsapps/cli/test/integration.test.tsapps/cli/test/deployment.test.tsapps/cli/test/examples.test.tsapps/cli/test/database-setup.test.tsapps/cli/test/api.test.tsapps/cli/test/database-orm.test.tsapps/cli/test/addons.test.ts
**/.github/workflows/**/*.@(yml|yaml)
📄 CodeRabbit inference engine (.cursor/rules/use-bun-instead-of-node-vite-npm-pnpm.mdc)
Use
bun run <script>instead ofnpm run,yarn run, orpnpm runin CI workflows
Files:
.github/workflows/test.yaml
🧠 Learnings (4)
📚 Learning: 2025-08-24T18:01:11.544Z
Learnt from: CR
Repo: AmanVarshney01/create-better-t-stack PR: 0
File: .cursor/rules/use-bun-instead-of-node-vite-npm-pnpm.mdc:0-0
Timestamp: 2025-08-24T18:01:11.544Z
Learning: Applies to {**/package.json,**/@(jest|vitest).config.@(js|ts|mjs|cjs)} : Use `bun test` to run tests
Applied to files:
apps/cli/test/index.test.tsapps/cli/test/frontend.test.tsapps/cli/test/auth.test.tsapps/cli/test/basic-configurations.test.tsapps/cli/test/backend-runtime.test.tsapps/cli/test/integration.test.tsapps/cli/test/deployment.test.tsapps/cli/test/examples.test.tsapps/cli/test/database-setup.test.tsapps/cli/test/api.test.tsapps/cli/test/addons.test.ts.github/workflows/test.yaml
📚 Learning: 2025-08-24T18:01:11.544Z
Learnt from: CR
Repo: AmanVarshney01/create-better-t-stack PR: 0
File: .cursor/rules/use-bun-instead-of-node-vite-npm-pnpm.mdc:0-0
Timestamp: 2025-08-24T18:01:11.544Z
Learning: Applies to {**/package.json,**/@(jest|vitest).config.@(js|ts|mjs|cjs)} : Use `bun test` instead of `jest` or `vitest`
Applied to files:
apps/cli/test/index.test.tsapps/cli/test/frontend.test.tsapps/cli/test/auth.test.tsapps/cli/test/basic-configurations.test.tsapps/cli/test/backend-runtime.test.tsapps/cli/test/integration.test.tsapps/cli/test/deployment.test.tsapps/cli/test/examples.test.tsapps/cli/test/database-setup.test.tsapps/cli/test/api.test.tsapps/cli/test/addons.test.ts
📚 Learning: 2025-08-24T18:01:11.544Z
Learnt from: CR
Repo: AmanVarshney01/create-better-t-stack PR: 0
File: .cursor/rules/use-bun-instead-of-node-vite-npm-pnpm.mdc:0-0
Timestamp: 2025-08-24T18:01:11.544Z
Learning: Applies to **/package.json : In package.json scripts, prefer running files with `bun <file>` instead of `node <file>` or `ts-node <file>`
Applied to files:
CLAUDE.md
📚 Learning: 2025-08-24T18:01:11.544Z
Learnt from: CR
Repo: AmanVarshney01/create-better-t-stack PR: 0
File: .cursor/rules/use-bun-instead-of-node-vite-npm-pnpm.mdc:0-0
Timestamp: 2025-08-24T18:01:11.544Z
Learning: Default to using Bun instead of Node.js
Applied to files:
apps/cli/test/deployment.test.ts
🧬 Code graph analysis (12)
apps/cli/test/index.test.ts (1)
apps/cli/test/test-utils.ts (1)
cleanupSmokeDirectory(31-38)
apps/cli/test/frontend.test.ts (1)
apps/cli/test/test-utils.ts (1)
cleanupSmokeDirectory(31-38)
apps/cli/test/auth.test.ts (1)
apps/cli/test/test-utils.ts (2)
cleanupSmokeDirectory(31-38)expectError(174-179)
apps/cli/test/basic-configurations.test.ts (1)
apps/cli/test/test-utils.ts (1)
cleanupSmokeDirectory(31-38)
apps/cli/test/backend-runtime.test.ts (1)
apps/cli/test/test-utils.ts (1)
cleanupSmokeDirectory(31-38)
apps/cli/test/integration.test.ts (1)
apps/cli/test/test-utils.ts (1)
cleanupSmokeDirectory(31-38)
apps/cli/test/deployment.test.ts (1)
apps/cli/test/test-utils.ts (1)
cleanupSmokeDirectory(31-38)
apps/cli/test/examples.test.ts (1)
apps/cli/test/test-utils.ts (1)
cleanupSmokeDirectory(31-38)
apps/cli/test/database-setup.test.ts (1)
apps/cli/test/test-utils.ts (1)
cleanupSmokeDirectory(31-38)
apps/cli/test/api.test.ts (1)
apps/cli/test/test-utils.ts (1)
cleanupSmokeDirectory(31-38)
apps/cli/test/database-orm.test.ts (1)
apps/cli/test/test-utils.ts (2)
cleanupSmokeDirectory(31-38)expectSuccess(162-172)
apps/cli/test/addons.test.ts (1)
apps/cli/test/test-utils.ts (1)
cleanupSmokeDirectory(31-38)
🪛 LanguageTool
CLAUDE.md
[style] ~212-~212: ‘with success’ might be wordy. Consider a shorter alternative.
Context: ...that: - Returns structured InitResult with success status, project paths, and timing - Alw...
(EN_WORDINESS_PREMIUM_WITH_SUCCESS)
🪛 markdownlint-cli2 (0.18.1)
CLAUDE.md
117-117: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
172-172: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🔇 Additional comments (26)
.idea/.gitignore (1)
1-5: Standard IDE configuration..idea/vcs.xml (1)
1-6: Standard IDE configuration..idea/misc.xml (1)
1-4: Standard IDE configuration..idea/jsLibraryMappings.xml (1)
1-6: Standard IDE configuration..idea/inspectionProfiles/Project_Default.xml (1)
1-6: Standard IDE configuration..idea/biome.xml (1)
1-9: Standard IDE configuration..idea/git_toolbox_prj.xml (1)
1-15: Standard IDE configuration..github/workflows/test.yaml (1)
1-29: GitHub Actions workflow follows guidelines and implements PR objectives.The workflow correctly:
- Triggers on pull requests to main
- Uses
bun installandbun testcommands (per coding guidelines and learnings)- Names the job "test" as required for branch protection rules
- Properly configures environment variables for telemetry and PostHog integration
- Runs tests in the correct working directory (apps/cli)
.idea/modules.xml (1)
1-8: LGTM!Standard IntelliJ IDEA module configuration file with correct structure.
.idea/create-better-t-stack.iml (1)
1-12: LGTM!Standard IntelliJ IDEA module configuration with appropriate folder exclusions.
apps/cli/test/api.test.ts (1)
21-27: LGTM!Cleanup lifecycle hooks are correctly implemented with proper async/await handling.
apps/cli/test/frontend.test.ts (1)
11-17: LGTM!Cleanup lifecycle hooks are correctly implemented with proper async/await handling.
apps/cli/test/basic-configurations.test.ts (1)
11-17: LGTM!Cleanup lifecycle hooks are correctly implemented with proper async/await handling.
apps/cli/test/index.test.ts (1)
9-18: LGTM!Cleanup lifecycle hooks are correctly implemented with proper async/await handling, consistent with the pattern across all test files.
apps/cli/test/database-setup.test.ts (1)
12-18: No race condition risk—tests execute sequentially.Verification confirms that tests run sequentially with
vitest run(no parallel flag set). Each project gets its own subdirectory within.smoke/based on projectName, so duplicate project names across test files (ai-solid-fail, trpc-nuxt-fail, web-deploy-no-frontend-fail) do not cause conflicts. The cleanup pattern is correctly implemented and safe.apps/cli/test/database-orm.test.ts (1)
12-18: LGTM! Proper test isolation with cleanup hooks.The addition of
beforeAllandafterAllhooks to clean up the smoke directory ensures tests run in a clean state and don't leave artifacts behind.apps/cli/test/examples.test.ts (1)
12-18: LGTM! Consistent test cleanup pattern.The lifecycle hooks ensure proper cleanup of the smoke directory before and after test execution.
apps/cli/test/backend-runtime.test.ts (2)
12-18: LGTM! Test isolation with cleanup hooks.Consistent with the cleanup pattern applied across the test suite.
474-477: LGTM! More informative error message.The updated error message now explicitly mentions TanStack Start as a supported frontend option alongside Next.js, improving developer experience.
apps/cli/test/auth.test.ts (2)
13-19: LGTM! Proper test cleanup.The lifecycle hooks ensure a clean test environment.
107-110: LGTM! More specific error message.The updated error message provides better context by explaining that the 'todo' example requires a database when a backend is present, rather than the generic "Authentication requires a database" message.
apps/cli/test/integration.test.ts (2)
12-18: LGTM! Test isolation established.Consistent cleanup pattern applied across the test suite.
202-203: LGTM! Deployment configuration updates.The changes to deployment settings and runtime configuration align with the test scenarios being validated.
Also applies to: 235-235
apps/cli/test/addons.test.ts (1)
12-18: LGTM! Clean test environment setup.The lifecycle hooks properly manage the smoke directory cleanup.
apps/cli/test/deployment.test.ts (2)
13-19: LGTM! Proper test cleanup.Consistent with the cleanup pattern applied throughout the test suite.
164-167: LGTM! Conditional runtime configuration for deployment targets.The runtime is now correctly set based on the deployment target:
workersruntime forwrangleroralchemyserver deploymentsbunruntime for other scenariosThis aligns with the deployment constraints where Cloudflare Workers and Alchemy require the workers runtime.
Also applies to: 251-253, 519-521
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.github/workflows/test.yaml (1)
20-23: Consider removing the pnpm setup step if not needed.The workflow uses
bunexclusively for dependency installation and test execution. The pnpm setup step may be redundant unless it's required for workspace resolution or other toolchain requirements in your monorepo. Verify and remove if unnecessary.To verify, check:
- Does
bun installhandle your workspace/monorepo structure without pnpm setup?- Is pnpm needed elsewhere in the workflow or for lockfile compatibility?
If both answers are "no", remove lines 20-23.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/test.yaml(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/.github/workflows/**/*.@(yml|yaml)
📄 CodeRabbit inference engine (.cursor/rules/use-bun-instead-of-node-vite-npm-pnpm.mdc)
Use
bun run <script>instead ofnpm run,yarn run, orpnpm runin CI workflows
Files:
.github/workflows/test.yaml
🧠 Learnings (2)
📚 Learning: 2025-08-24T18:01:11.544Z
Learnt from: CR
Repo: AmanVarshney01/create-better-t-stack PR: 0
File: .cursor/rules/use-bun-instead-of-node-vite-npm-pnpm.mdc:0-0
Timestamp: 2025-08-24T18:01:11.544Z
Learning: Applies to **/.github/workflows/**/*.@(yml|yaml) : Use `bun run <script>` instead of `npm run`, `yarn run`, or `pnpm run` in CI workflows
Applied to files:
.github/workflows/test.yaml
📚 Learning: 2025-08-24T18:01:11.544Z
Learnt from: CR
Repo: AmanVarshney01/create-better-t-stack PR: 0
File: .cursor/rules/use-bun-instead-of-node-vite-npm-pnpm.mdc:0-0
Timestamp: 2025-08-24T18:01:11.544Z
Learning: Applies to {**/package.json,**/@(jest|vitest).config.@(js|ts|mjs|cjs)} : Use `bun test` to run tests
Applied to files:
.github/workflows/test.yaml
⏰ 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: test
🔇 Additional comments (1)
.github/workflows/test.yaml (1)
1-34: Workflow structure and tooling are correct.The workflow correctly uses
bun install --frozen-lockfile(line 26) andbun test(line 33) as specified in the coding guidelines. Checkout, bun setup, dependency installation, and test execution are all configured appropriately. Environment variables for telemetry and analytics are properly sourced from GitHub Secrets.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/test.yaml(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/.github/workflows/**/*.@(yml|yaml)
📄 CodeRabbit inference engine (.cursor/rules/use-bun-instead-of-node-vite-npm-pnpm.mdc)
Use
bun run <script>instead ofnpm run,yarn run, orpnpm runin CI workflows
Files:
.github/workflows/test.yaml
🧠 Learnings (1)
📚 Learning: 2025-08-24T18:01:11.544Z
Learnt from: CR
Repo: AmanVarshney01/create-better-t-stack PR: 0
File: .cursor/rules/use-bun-instead-of-node-vite-npm-pnpm.mdc:0-0
Timestamp: 2025-08-24T18:01:11.544Z
Learning: Applies to **/.github/workflows/**/*.@(yml|yaml) : Use `bun run <script>` instead of `npm run`, `yarn run`, or `pnpm run` in CI workflows
Applied to files:
.github/workflows/test.yaml
⏰ 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: test
🔇 Additional comments (1)
.github/workflows/test.yaml (1)
1-40: LGTM! Workflow structure is sound.The CI pipeline appropriately checks out code, configures git (needed for test operations), sets up Bun and pnpm, installs dependencies with frozen lockfile for reproducibility, and runs tests in the target directory. Environment variables are properly sourced from secrets.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
bunfig.toml (1)
4-6: Configuration is valid but consider using array format for better CI reporting.The
coverageReporteroption in bunfig.toml can enable persistent code coverage reports, and the"lcov"format is correct. However, the coverageReporter accepts both string and array formats, with array format allowing multiple reporters like ["text", "lcov"].For CI environments, the array format is more valuable: you get both terminal output (for immediate feedback) and an lcov.info file (for integration with coverage tools and dashboards).
Apply this diff to enable both console and file-based coverage reporting:
[test] coverage = true -coverageReporter = "lcov" +coverageReporter = ["text", "lcov"]
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.github/workflows/test.yaml(1 hunks).gitignore(1 hunks)bunfig.toml(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- .gitignore
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/test.yaml
⏰ 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: test
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
bunfig.toml (1)
4-6: Consider adding coverage thresholds (optional).For future iterations, consider adding coverage threshold configurations (e.g., lines, statements, functions, branches) to enforce minimum coverage requirements and prevent regressions. This would help maintain test quality as the codebase grows.
[test] coverage = true coverageReporter = ["text", "lcov"] # Optional: add in a future update # coverageThreshold = { lines = 80, statements = 80, functions = 80, branches = 75 }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
bunfig.toml(1 hunks)
🔇 Additional comments (1)
bunfig.toml (1)
4-6: Coverage reporting configuration looks good.The new
[test]section enables coverage reporting with both "text" and "lcov" reporters, which is well-suited for CI: "text" provides readable output in logs, and "lcov" integrates with coverage tools. This aligns well with the enhanced test infrastructure and CI workflow being added.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
apps/cli/test/addons.test.ts(1 hunks)apps/cli/test/api.test.ts(2 hunks)apps/cli/test/auth.test.ts(2 hunks)apps/cli/test/deployment.test.ts(7 hunks)apps/cli/test/frontend.test.ts(1 hunks)apps/cli/test/integration.test.ts(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- apps/cli/test/frontend.test.ts
- apps/cli/test/addons.test.ts
- apps/cli/test/api.test.ts
- apps/cli/test/auth.test.ts
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/convex_rules.mdc)
**/*.{ts,tsx}: Use Id from './_generated/dataModel' to type document ids (e.g., Id<'users'>)
Ensure Record key/value types align with validators (e.g., v.record(v.id('users'), v.string()) => Record<Id<'users'>, string>)
Be strict with types for document ids; prefer Id<'table'> over string
Use 'as const' for string literals in discriminated unions
When using Array and Record types, declare with explicit generic types (e.g., const arr: Array = ...)
Files:
apps/cli/test/deployment.test.tsapps/cli/test/integration.test.ts
**/*.{js,jsx,ts,tsx,mjs,cjs}
📄 CodeRabbit inference engine (.cursor/rules/use-bun-instead-of-node-vite-npm-pnpm.mdc)
**/*.{js,jsx,ts,tsx,mjs,cjs}: Do not use dotenv; Bun auto-loads .env
UseBun.serve()for HTTP/WebSockets; do not useexpress
Usebun:sqlitefor SQLite; do not usebetter-sqlite3
UseBun.redisfor Redis; do not useioredis
UseBun.sqlfor Postgres; do not usepgorpostgres.js
Use built-inWebSocket; do not usews
PreferBun.fileovernode:fsreadFile/writeFile
UseBun.$instead ofexecafor shelling out
Files:
apps/cli/test/deployment.test.tsapps/cli/test/integration.test.ts
🧠 Learnings (4)
📓 Common learnings
Learnt from: CR
Repo: AmanVarshney01/create-better-t-stack PR: 0
File: .cursor/rules/use-bun-instead-of-node-vite-npm-pnpm.mdc:0-0
Timestamp: 2025-08-24T18:01:11.544Z
Learning: Applies to {**/package.json,**/@(jest|vitest).config.@(js|ts|mjs|cjs)} : Use `bun test` instead of `jest` or `vitest`
Learnt from: CR
Repo: AmanVarshney01/create-better-t-stack PR: 0
File: .cursor/rules/use-bun-instead-of-node-vite-npm-pnpm.mdc:0-0
Timestamp: 2025-08-24T18:01:11.544Z
Learning: Applies to {**/package.json,**/@(jest|vitest).config.@(js|ts|mjs|cjs)} : Use `bun test` to run tests
📚 Learning: 2025-08-24T18:01:11.544Z
Learnt from: CR
Repo: AmanVarshney01/create-better-t-stack PR: 0
File: .cursor/rules/use-bun-instead-of-node-vite-npm-pnpm.mdc:0-0
Timestamp: 2025-08-24T18:01:11.544Z
Learning: Applies to {**/package.json,**/@(jest|vitest).config.@(js|ts|mjs|cjs)} : Use `bun test` to run tests
Applied to files:
apps/cli/test/deployment.test.tsapps/cli/test/integration.test.ts
📚 Learning: 2025-08-24T18:01:11.544Z
Learnt from: CR
Repo: AmanVarshney01/create-better-t-stack PR: 0
File: .cursor/rules/use-bun-instead-of-node-vite-npm-pnpm.mdc:0-0
Timestamp: 2025-08-24T18:01:11.544Z
Learning: Applies to {**/package.json,**/@(jest|vitest).config.@(js|ts|mjs|cjs)} : Use `bun test` instead of `jest` or `vitest`
Applied to files:
apps/cli/test/deployment.test.tsapps/cli/test/integration.test.ts
📚 Learning: 2025-08-24T18:01:11.544Z
Learnt from: CR
Repo: AmanVarshney01/create-better-t-stack PR: 0
File: .cursor/rules/use-bun-instead-of-node-vite-npm-pnpm.mdc:0-0
Timestamp: 2025-08-24T18:01:11.544Z
Learning: Default to using Bun instead of Node.js
Applied to files:
apps/cli/test/deployment.test.ts
🧬 Code graph analysis (2)
apps/cli/test/deployment.test.ts (1)
apps/cli/test/test-utils.ts (1)
cleanupSmokeDirectory(31-38)
apps/cli/test/integration.test.ts (1)
apps/cli/test/test-utils.ts (1)
cleanupSmokeDirectory(31-38)
⏰ 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: test
🔇 Additional comments (4)
apps/cli/test/integration.test.ts (4)
1-1: LGTM! Cleanup hooks improve test isolation.The suite-level cleanup hooks prevent test artifact buildup and ensure a clean state before and after test runs.
Also applies to: 4-4, 12-18
480-480: Runtime change consistent with Wrangler deployment.Similar to line 235, the runtime changed from
buntoworkers, aligning with thewranglerdeployment targets (lines 489-490). This ensures the maximum complexity test uses a realistic configuration.
235-235: Runtime change from "bun" to "workers" is correct and required.Validation rules confirm that Wrangler server deployment requires
"workers"runtime. The test configuration is valid: it uses "hono" backend (the only supported backend for workers), SQLite database, "drizzle" ORM, and Wrangler deployment—all validated combinations in the CLI.
202-203: Deployment configuration is correct: "none" is the appropriate value for Express backend.The Express backend does not have alchemy deployment setup support. Alchemy deployment is configured only for frontend frameworks (Next.js, Nuxt, React Router, Solid, Svelte, TanStack Router, TanStack Start), as evidenced by the lack of an
alchemy-express-setup.tsfile inapps/cli/src/helpers/deployment/alchemy/. Setting bothwebDeployandserverDeployto"none"for the Express + Node + MongoDB + Mongoose stack correctly reflects the CLI's actual constraints.
|
So this fixes all the tests?? 👀 @moreorover |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
This is the final PR Bugbot will review for you during this billing cycle
Your free Bugbot reviews will reset on December 7
Details
You are on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
|
Do note that i plan to remove wrangler from web deploy and server deploy so we dont need to write tests for it |
Thats good. For now these test cases can stay, once these options are removed we will remove tests together. This does not only fix the tests but also run a test pipeline on every PR to make sure that the changes do not break anything going forward. I have noticed that you tend to commit straight into main branch, with this PR going forward it is greatly suggested to open a new PR making any changes into main branch this will run tests and at least somewhat make sure that stuff is working. It will also show coverage on source code after the tests are executed, suggest registering for https://github.com/marketplace/actions/report-lcov so that the coverage report data is more readable and gives more insights compared to just pipeline report. There is still a lot to improve with tests, but really need to think of how can we best test generated projects by CLI. |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/cli/test/test-utils.ts (1)
320-395: PreferBun.$overexecafor shell commands.The function uses
execato run package manager and turbo commands, but the project coding guidelines specify usingBun.$instead ofexecafor shelling out.As per coding guidelines.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/cli/test/test-utils.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/convex_rules.mdc)
**/*.{ts,tsx}: Use Id from './_generated/dataModel' to type document ids (e.g., Id<'users'>)
Ensure Record key/value types align with validators (e.g., v.record(v.id('users'), v.string()) => Record<Id<'users'>, string>)
Be strict with types for document ids; prefer Id<'table'> over string
Use 'as const' for string literals in discriminated unions
When using Array and Record types, declare with explicit generic types (e.g., const arr: Array = ...)
Files:
apps/cli/test/test-utils.ts
**/*.{js,jsx,ts,tsx,mjs,cjs}
📄 CodeRabbit inference engine (.cursor/rules/use-bun-instead-of-node-vite-npm-pnpm.mdc)
**/*.{js,jsx,ts,tsx,mjs,cjs}: Do not use dotenv; Bun auto-loads .env
UseBun.serve()for HTTP/WebSockets; do not useexpress
Usebun:sqlitefor SQLite; do not usebetter-sqlite3
UseBun.redisfor Redis; do not useioredis
UseBun.sqlfor Postgres; do not usepgorpostgres.js
Use built-inWebSocket; do not usews
PreferBun.fileovernode:fsreadFile/writeFile
UseBun.$instead ofexecafor shelling out
Files:
apps/cli/test/test-utils.ts
🧠 Learnings (3)
📓 Common learnings
Learnt from: CR
Repo: AmanVarshney01/create-better-t-stack PR: 0
File: .cursor/rules/use-bun-instead-of-node-vite-npm-pnpm.mdc:0-0
Timestamp: 2025-08-24T18:01:11.544Z
Learning: Applies to {**/package.json,**/@(jest|vitest).config.@(js|ts|mjs|cjs)} : Use `bun test` instead of `jest` or `vitest`
Learnt from: CR
Repo: AmanVarshney01/create-better-t-stack PR: 0
File: .cursor/rules/use-bun-instead-of-node-vite-npm-pnpm.mdc:0-0
Timestamp: 2025-08-24T18:01:11.544Z
Learning: Applies to {**/package.json,**/@(jest|vitest).config.@(js|ts|mjs|cjs)} : Use `bun test` to run tests
📚 Learning: 2025-08-24T18:01:11.544Z
Learnt from: CR
Repo: AmanVarshney01/create-better-t-stack PR: 0
File: .cursor/rules/use-bun-instead-of-node-vite-npm-pnpm.mdc:0-0
Timestamp: 2025-08-24T18:01:11.544Z
Learning: Applies to {**/package.json,**/@(jest|vitest).config.@(js|ts|mjs|cjs)} : Use `bun test` instead of `jest` or `vitest`
Applied to files:
apps/cli/test/test-utils.ts
📚 Learning: 2025-08-24T18:01:11.544Z
Learnt from: CR
Repo: AmanVarshney01/create-better-t-stack PR: 0
File: .cursor/rules/use-bun-instead-of-node-vite-npm-pnpm.mdc:0-0
Timestamp: 2025-08-24T18:01:11.544Z
Learning: Applies to {**/package.json,**/@(jest|vitest).config.@(js|ts|mjs|cjs)} : Use `bun test` to run tests
Applied to files:
apps/cli/test/test-utils.ts
⏰ 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: test
|
@AmanVarshney01 something is wrong the the config package tests, so removed those, I do not have this week or next to have a look at it. But the rest is good to be merged in. |
# Conflicts: # apps/cli/test/config-package.test.ts # apps/cli/test/deployment.test.ts
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
apps/cli/test/deployment.test.ts (2)
13-19: Already flagged: Race condition on shared cleanup.This concern about parallel test suites racing on the shared
.smokedirectory has already been raised in a previous review.
515-517: Already flagged: Logic error with OR operator.This concern about the incorrect OR operator (should be AND, or restructured logic) that causes
backend === "none"to incorrectly setruntime = "workers"has already been raised in a previous review.
🧹 Nitpick comments (3)
apps/cli/test/deployment.test.ts (1)
252-254: Remove redundant serverDeploy assignment.Line 254 reassigns
config.serverDeploy = "alchemy"but it's already set to"alchemy"on line 236. The assignment is harmless but unnecessary.Apply this diff to remove the redundant line:
} else if (backend === "hono") { config.runtime = "workers"; - config.serverDeploy = "alchemy"; } else {apps/cli/test/auth.test.ts (2)
13-19: Suite-level smoke directory cleanup is reasonable; check parallel test interactionUsing
beforeAll/afterAllto callcleanupSmokeDirectoryaround the"Authentication Configurations"suite is a good way to bound filesystem state. One thing to double-check:cleanupSmokeDirectory(apps/cli/test/test-utils.ts:30-37) deletes a shared.smokedirectory atprocess.cwd(). If Vitest is running test files in parallel workers and all suites share that same directory, there’s a risk that one suite’safterAllcould remove.smokewhile another suite is still using it, causing intermittent failures.If your Vitest config does enable parallel execution across files, consider either:
- Using per-suite (or per-worker) subdirectories under
.smoke, or- Centralizing cleanup in a global setup/teardown that coordinates access, or
- Running these CLI smoke tests in a single-threaded pool.
Not urgent, but worth verifying so this cleanup doesn’t become a new source of flakiness.
Can you confirm whether Vitest is currently running these suites in parallel workers and, if so, whether
.smokeis shared across them?
107-110: Updated error expectation aligns with the constraints; consider reducing brittlenessThe new expected message for the “better-auth + no database (non-convex)” failure matches the rule encoded in this file (todo example requires a DB when a non-Convex backend is selected), and using
expectError’stoContaincheck keeps it as a substring match rather than a full-string equality.To keep the test resilient to minor wording tweaks, you might optionally narrow the assertion to the most stable part of the message (for example, the second sentence about
--examples todo+database is 'none') if the first sentence is more likely to change.Please verify that the CLI currently emits this exact substring so the test doesn’t fail due to a copy or punctuation mismatch.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
apps/cli/test/api.test.ts(2 hunks)apps/cli/test/auth.test.ts(2 hunks)apps/cli/test/backend-runtime.test.ts(2 hunks)apps/cli/test/database-setup.test.ts(2 hunks)apps/cli/test/deployment.test.ts(7 hunks)apps/cli/test/frontend.test.ts(1 hunks)apps/cli/test/integration.test.ts(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- apps/cli/test/integration.test.ts
- apps/cli/test/backend-runtime.test.ts
- apps/cli/test/frontend.test.ts
- apps/cli/test/database-setup.test.ts
- apps/cli/test/api.test.ts
🧰 Additional context used
🧬 Code graph analysis (2)
apps/cli/test/auth.test.ts (1)
apps/cli/test/test-utils.ts (2)
cleanupSmokeDirectory(31-38)expectError(177-182)
apps/cli/test/deployment.test.ts (1)
apps/cli/test/test-utils.ts (1)
cleanupSmokeDirectory(31-38)
⏰ 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: test
🔇 Additional comments (2)
apps/cli/test/deployment.test.ts (1)
164-167: LGTM: Runtime correctly set for alchemy deployments.The runtime updates correctly enforce that alchemy deployments use Cloudflare Workers runtime:
- Line 164-167: Conditional logic for server deploy test
- Line 340: Combined web+server alchemy deployment
- Line 403: Server-only alchemy deployment
Also applies to: 340-340, 403-403
apps/cli/test/auth.test.ts (1)
1-10: Imports for lifecycle hooks and test utils look consistentThe added imports (
beforeAll,afterAllfromvitestandcleanupSmokeDirectoryfrom./test-utils) match their usages below and are type-safe; no issues here.Please confirm
bun testinapps/clipasses in your CI and local runs with these imports in place.
|
@AmanVarshney01, I patched up the tests, it runs trough successfully now, you may merge this in, also please have a look at https://about.codecov.io/ to get insights on coverage |
|
Thanks buddy @moreorover Sorry for the late review 🙏 |
# Conflicts: # apps/cli/test/addons.test.ts # apps/cli/test/api.test.ts # apps/cli/test/auth.test.ts # apps/cli/test/backend-runtime.test.ts # apps/cli/test/basic-configurations.test.ts # apps/cli/test/config-package.test.ts # apps/cli/test/database-orm.test.ts # apps/cli/test/database-setup.test.ts # apps/cli/test/deployment.test.ts # apps/cli/test/examples.test.ts # apps/cli/test/frontend.test.ts # apps/cli/test/index.test.ts # apps/cli/test/integration.test.ts # apps/cli/test/test-utils.ts
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
apps/cli/test/auth.test.ts (1)
80-99: Updated error message expectation is consistent; optional robustness tweakThe new, more specific error text matches the scenario and works fine with
expectError’s.toContaincheck. If you want to make the test a bit less brittle to future wording tweaks, you could assert on a shorter stable substring (e.g., just the “'todo' example requires a database…” part) instead of the full sentence, but the current form is also acceptable.apps/cli/test/deployment.test.ts (1)
447-505: All Deployment Options logic preserves Workers/serverDeploy invariantsDefaulting
runtimeto"workers"for these deploy-option permutations and then switching to"bun"only when bothwebDeployandserverDeployare"none"ensures there is noruntime: "workers"+serverDeploy: "none"combination slipping through, while still exercising the supported Workers + server deploy paths.If you want to make the intent even clearer, you could derive
runtimefromserverDeployin one place (e.g., a small helper) instead of seeding with"workers"and overriding for the(none, none)case, but the current logic is functionally sound.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
apps/cli/test/auth.test.ts(1 hunks)apps/cli/test/backend-runtime.test.ts(1 hunks)apps/cli/test/database-orm.test.ts(2 hunks)apps/cli/test/deployment.test.ts(6 hunks)apps/cli/test/integration.test.ts(4 hunks)apps/cli/test/test-utils.ts(0 hunks)
💤 Files with no reviewable changes (1)
- apps/cli/test/test-utils.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- apps/cli/test/backend-runtime.test.ts
- apps/cli/test/integration.test.ts
- apps/cli/test/database-orm.test.ts
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/better-t-stack-repo.mdc)
Define functions using the standard function declaration syntax, not arrow functions
Files:
apps/cli/test/deployment.test.tsapps/cli/test/auth.test.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/better-t-stack-repo.mdc)
**/*.{ts,tsx}: Use TypeScript type aliases instead of interface declarations
Do not use explicit return types
Files:
apps/cli/test/deployment.test.tsapps/cli/test/auth.test.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/use-bun-instead-of-node-vite-npm-pnpm.mdc)
**/*.{ts,tsx,js,jsx}: Usebun <file>instead ofnode <file>orts-node <file>for running TypeScript/JavaScript files
Bun automatically loads .env files, so don't use the dotenv package
UseBun.serve()which supports WebSockets, HTTPS, and routes instead ofexpress
Usebun:sqlitemodule for SQLite instead ofbetter-sqlite3
UseBun.redisfor Redis instead ofioredis
UseBun.sqlfor Postgres instead ofpgorpostgres.js
Use built-inWebSocketinstead of thewspackage
PreferBun.fileovernode:fsreadFile/writeFile methods
UseBun.$template literal syntax instead ofexecafor shell command execution
Import .css files directly in TypeScript/JavaScript files; Bun's CSS bundler will handle bundling
Run server withbun --hot <file>to enable hot reloading during development
Files:
apps/cli/test/deployment.test.tsapps/cli/test/auth.test.ts
**/*.test.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/use-bun-instead-of-node-vite-npm-pnpm.mdc)
**/*.test.{ts,tsx,js,jsx}: Usebun testinstead ofjestorvitestfor running tests
Usebun:testmodule withtestandexpectfunctions for writing tests
Files:
apps/cli/test/deployment.test.tsapps/cli/test/auth.test.ts
**/*.{ts,tsx,js,jsx,css}
📄 CodeRabbit inference engine (.cursor/rules/use-bun-instead-of-node-vite-npm-pnpm.mdc)
Use
bun build <file>instead ofwebpackoresbuildfor bundling TypeScript, JavaScript, and CSS files
Files:
apps/cli/test/deployment.test.tsapps/cli/test/auth.test.ts
**/*.{html,tsx,ts,jsx,js}
📄 CodeRabbit inference engine (.cursor/rules/use-bun-instead-of-node-vite-npm-pnpm.mdc)
Use HTML imports with
Bun.serve()for frontend instead of Vite
Files:
apps/cli/test/deployment.test.tsapps/cli/test/auth.test.ts
🧠 Learnings (2)
📚 Learning: 2025-12-03T07:48:26.410Z
Learnt from: CR
Repo: AmanVarshney01/create-better-t-stack PR: 0
File: .cursor/rules/use-bun-instead-of-node-vite-npm-pnpm.mdc:0-0
Timestamp: 2025-12-03T07:48:26.410Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Use `bun:test` module with `test` and `expect` functions for writing tests
Applied to files:
apps/cli/test/deployment.test.ts
📚 Learning: 2025-12-03T07:48:14.703Z
Learnt from: CR
Repo: AmanVarshney01/create-better-t-stack PR: 0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-12-03T07:48:14.703Z
Learning: Applies to convex/**/*.ts : Never use ctx.db inside of an action; actions don't have access to the database
Applied to files:
apps/cli/test/auth.test.ts
🧬 Code graph analysis (1)
apps/cli/test/auth.test.ts (1)
apps/cli/test/test-utils.ts (1)
expectError(174-179)
⏰ 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: test
🔇 Additional comments (4)
apps/cli/test/deployment.test.ts (4)
145-168: Server deploy runtime mapping matches constraintsTying
runtimetoserverDeployhere (alchemy→workers, otherwisebun) correctly encodes the Workers/server-deploy constraint and keeps these “valid config” cases aligned with what the CLI should allow.
217-248: Per-backend runtime/serverDeploy wiring looks consistentIn the “server deploy + all compatible backends” test, explicitly using
workers+alchemyforhonoand falling back tobun+nonefor the other backends cleanly encodes the idea that onlyhonoparticipates in server deployment while the others remain local. This matches the surrounding constraints and avoids invalid workers+no-server combinations.
317-337: Combined web+server deploy correctly forces Workers runtimeFor the web+server combo (
webDeploy: "alchemy", serverDeploy: "alchemy"), explicitly settingruntime: "workers"aligns with the rest of the suite’s Workers/serverDeploy expectations and keeps this scenario valid.
381-399: Server-deploy-only scenario correctly uses Workers runtimeIn the “server deploy only” case (
serverDeploy: "alchemy", webDeploy: "none"), usingruntime: "workers"is consistent with the Workers/serverDeploy coupling tested elsewhere, so this configuration should stay valid as those rules evolve.
|
@AmanVarshney01 updated test cases again |
2 participants
closes #684
Changes
CI/CD
.github/workflows/test.yaml) that runs on every PR tomainbun testinapps/clito ensure all tests pass before mergingImportant: Branch Protection
To maintain code quality and prevent broken builds from reaching
main, please:main. Create a PR for every change instead.mainbranch:maintest(the CI job name)This ensures that the test suite runs successfully before any code is merged into
main, catching issues early and maintaining a stable codebase.Testing
Run tests locally: