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. WalkthroughAdds suite-level setup/teardown invoking cleanupSmokeDirectory across many CLI tests, removes several test helper exports and a large config-package test file, adjusts multiple test expectations and deployment/runtime parameters, enables test coverage in bunfig, and adds a Git user setup step to the CI test workflow. Changes
Sequence Diagram(s)sequenceDiagram
participant CI as GitHub Actions (Test)
participant Runner as Job Runner
participant Repo as Repository
participant Vitest as Vitest Test Runner
participant Utils as test-utils.cleanupSmokeDirectory
rect rgba(243,244,246,0.9)
CI->>Runner: checkout + setup Bun & pnpm
Runner->>Repo: checkout code
end
rect rgba(238,242,255,0.9)
CI->>Runner: Setup Git user (new step)
Runner->>Runner: git config user.name & user.email
end
rect rgba(230,255,237,0.9)
Runner->>Vitest: run tests (apps/cli)
Vitest->>Utils: beforeAll -> cleanupSmokeDirectory()
Utils-->>Vitest: cleanup done
Vitest->>Vitest: execute test suites
Vitest->>Utils: afterAll -> cleanupSmokeDirectory()
Utils-->>Vitest: cleanup done
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (2 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used🧬 Code graph analysis (1)apps/cli/test/deployment.test.ts (1)
⏰ 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)
🔇 Additional comments (2)
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.
apps/cli/test/deployment.test.ts
Outdated
| config.runtime = "bun"; | ||
| } else if (backend === "hono") { | ||
| config.runtime = "workers"; | ||
| config.serverDeploy = "wrangler"; |
There was a problem hiding this comment.
Bug: Incomplete Server Deployment Test Coverage
The test claims to verify server deployment with all compatible backends, but serverDeploy is only set for hono. For express, fastify, and elysia backends, serverDeploy remains undefined, so these backends aren't actually testing server deployment functionality despite the test name indicating they should.
|
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 |
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: