Conversation
- Replaced direct process.env assignments - Used vi.stubEnv for test environment - Added unstubEnvs: true to vitest config
|
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Rate limit exceeded@yamcodes has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 15 minutes and 1 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
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. WalkthroughTests across multiple packages switch from directly mutating Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
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 |
github-actions
bot
added
arkenv
commit: |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
- Fixed test setup and teardown
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/vite-plugin/src/index.test.ts (1)
50-65: Fix: alias "arkenv" during vite.build to avoid resolution errors in CI.Vite’s build runs outside Vitest’s mock layer; vi.mock("arkenv") won’t intercept module resolution. Alias "arkenv" to a local stub for the build.
Apply this diff to the vite.build config and add a stub:
await expect( vite.build({ configFile: false, root: config.root, plugins: [arkenvPlugin(config.envSchema)], logLevel: "error", + resolve: { + alias: { + // Prevent workspace/package resolution during tests + arkenv: join(__dirname, "__stubs__/arkenv.ts"), + }, + }, build: { lib: { entry: "index.ts", formats: ["es"], }, rollupOptions: { external: ["arkenv"], }, }, }), ).resolves.not.toThrow();Add this file at packages/vite-plugin/src/stubs/arkenv.ts:
// Minimal stub; adjust if you need to observe calls export const createEnv = (..._args: any[]) => {}; export default createEnv;Alternative: ensure packages/arkenv has valid exports (main/module/exports) and is built before tests; otherwise the build process can’t resolve it. See verification script attached in package.json comment.
🧹 Nitpick comments (2)
packages/arkenv/vitest.config.ts (1)
3-7: Consider mirroring env cleanup here for clarity.For explicitness in project-scoped runs, add test.unstubEnvs: true so behavior is clear even if run standalone.
export default defineProject({ - test: { - name: "arkenv", - }, + test: { + name: "arkenv", + unstubEnvs: true, + }, });packages/vite-plugin/src/index.test.ts (1)
27-31: Redundant env/mocks cleanup; simplify to one place.Given test.unstubEnvs: true at root, explicit vi.unstubAllEnvs() in both hooks is unnecessary. Also, afterEach(mockReset) already clears history; beforeEach(mockClear) can be dropped.
- beforeEach(() => { - // Clear environment variables and mock cleanup - vi.unstubAllEnvs(); - mockCreateEnv.mockClear(); - }); + beforeEach(() => { + mockCreateEnv.mockClear(); // or remove if you keep mockReset in afterEach + }); - afterEach(() => { - // Complete cleanup: restore environment and reset mocks - vi.unstubAllEnvs(); - mockCreateEnv.mockReset(); - }); + afterEach(() => { + mockCreateEnv.mockReset(); + });Also applies to: 33-37
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (8)
apps/www/lib/utils/github.test.ts(6 hunks)apps/www/vitest.config.ts(1 hunks)packages/arkenv/src/index.test.ts(4 hunks)packages/arkenv/vitest.config.ts(1 hunks)packages/vite-plugin/package.json(1 hunks)packages/vite-plugin/src/index.test.ts(3 hunks)packages/vite-plugin/vitest.config.ts(1 hunks)vitest.config.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.test.ts
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.test.ts: Place tests alongside source files and use the .test.ts suffix (e.g., create-env.test.ts, types.test.ts, errors.test.ts, utils.test.ts)
Use Vitest's describe/it structure in tests
Test both success and failure cases for environment validation and utilities
Mock process.env to cover different environment scenarios in tests
In tests, save and restore process.env in setup/teardown (beforeEach/afterEach) to avoid leakage between cases
Verify both runtime behavior and TypeScript types in tests
Files:
packages/vite-plugin/src/index.test.tspackages/arkenv/src/index.test.tsapps/www/lib/utils/github.test.ts
🧠 Learnings (4)
📓 Common learnings
Learnt from: CR
PR: yamcodes/arkenv#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-12T06:23:45.463Z
Learning: Applies to **/*.test.ts : Mock process.env to cover different environment scenarios in tests
Learnt from: CR
PR: yamcodes/arkenv#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-12T06:23:45.463Z
Learning: Applies to **/*.test.ts : In tests, save and restore process.env in setup/teardown (beforeEach/afterEach) to avoid leakage between cases
📚 Learning: 2025-09-12T06:23:45.463Z
Learnt from: CR
PR: yamcodes/arkenv#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-12T06:23:45.463Z
Learning: Applies to **/*.test.ts : In tests, save and restore process.env in setup/teardown (beforeEach/afterEach) to avoid leakage between cases
Applied to files:
packages/vite-plugin/src/index.test.tspackages/arkenv/src/index.test.tsapps/www/vitest.config.tsapps/www/lib/utils/github.test.ts
📚 Learning: 2025-09-12T06:23:45.463Z
Learnt from: CR
PR: yamcodes/arkenv#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-12T06:23:45.463Z
Learning: Applies to **/*.test.ts : Mock process.env to cover different environment scenarios in tests
Applied to files:
packages/vite-plugin/src/index.test.tspackages/arkenv/src/index.test.tsapps/www/vitest.config.tsapps/www/lib/utils/github.test.ts
📚 Learning: 2025-09-12T06:23:45.463Z
Learnt from: CR
PR: yamcodes/arkenv#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-12T06:23:45.463Z
Learning: Applies to **/*.test.ts : Test both success and failure cases for environment validation and utilities
Applied to files:
packages/arkenv/src/index.test.ts
🧬 Code graph analysis (2)
packages/vite-plugin/src/index.test.ts (2)
packages/vite-plugin/src/index.ts (1)
config(10-12)packages/vite-plugin/src/__fixtures__/basic/index.ts (1)
config(2-5)
apps/www/lib/utils/github.test.ts (1)
apps/www/components/page/edit-on-github.test.tsx (1)
process(47-51)
🪛 GitHub Actions: tests
packages/vite-plugin/src/index.test.ts
[error] 1-1: Failed to resolve entry for package "arkenv". The package may have incorrect main/module/exports specified in its package.json.
packages/vite-plugin/package.json
[error] 1-1: Test failed: Failed to resolve entry for package "arkenv". The package may have incorrect main/module/exports specified in its package.json. See details in test output and ensure the package entry points are correctly defined.
🔇 Additional comments (9)
apps/www/vitest.config.ts (1)
17-18: Good addition: automatic env cleanup enabled.unstubEnvs: true pairs well with vi.stubEnv usage and prevents cross-test leakage.
vitest.config.ts (1)
31-32: LGTM: repo-wide env isolation set.unstubEnvs: true aligns with the switch to vi.stubEnv and removes the need for manual env restoring.
packages/vite-plugin/src/index.test.ts (1)
43-47: Nice: env setup uses vi.stubEnv per key.This replaces direct mutations safely and pairs with unstubEnvs for isolation.
apps/www/lib/utils/github.test.ts (1)
31-31: Good migration to vi.stubEnv; confirm unset semantics for undefined.Switching to vi.stubEnv is correct and isolates env across tests. For the “not configured” cases, verify that vi.stubEnv(NAME, undefined) removes the var (it should on Vitest ≥1.4).
If unsure, add a quick assertion:
vi.stubEnv("NEXT_PUBLIC_GITHUB_URL", undefined); expect(process.env.NEXT_PUBLIC_GITHUB_URL).toBeUndefined();Also applies to: 43-48, 59-60, 120-136, 146-147
packages/arkenv/src/index.test.ts (5)
14-19: LGTM! Proper environment variable stubbing.The use of
vi.stubEnv(..., undefined)to clear environment variables before each test is the correct approach for Vitest's environment stubbing API. This ensures proper test isolation.Based on coding guidelines and learnings.
23-25: LGTM! Proper cleanup in afterEach.The combination of
vi.restoreAllMocks()andvi.unstubAllEnvs()ensures complete cleanup after each test, restoring both spies/mocks and environment variables to prevent test pollution.Based on coding guidelines and learnings.
44-44: LGTM! Correct environment variable stubbing.Using
vi.stubEnv()instead of directprocess.envassignment follows Vitest best practices and ensures proper test isolation with automatic cleanup viavi.unstubAllEnvs()in afterEach.Based on coding guidelines and learnings.
56-56: LGTM! Correct environment variable stubbing.Using
vi.stubEnv()instead of directprocess.envassignment follows Vitest best practices and ensures proper test isolation with automatic cleanup viavi.unstubAllEnvs()in afterEach.Based on coding guidelines and learnings.
84-84: LGTM! Correct environment variable stubbing.Using
vi.stubEnv()instead of directprocess.envassignment follows Vitest best practices and ensures proper test isolation with automatic cleanup viavi.unstubAllEnvs()in afterEach.Based on coding guidelines and learnings.
- Added alias to arkenv package - Enables easier testing
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
1 participant
Summary by CodeRabbit
Tests
Chores
Impact