fix(ai-commit-msg): clone env baseline to stop cumulative test mutations#291
fix(ai-commit-msg): clone env baseline to stop cumulative test mutations#291stephansama wants to merge 1 commit into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ 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. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
✨ 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 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the test suite in core/ai-commit-msg/test/ai.test.ts to prevent environment variable mutations from leaking between test cases by cloning process.env. The reviewer noted that reassigning process.env to a plain object can break Node.js-specific behaviors and recommended using Vitest's built-in vi.stubEnv() and vi.unstubAllEnvs() methods for a more robust implementation.
|
|
||
| afterEach(() => { | ||
| process.env = originalEnvironment; | ||
| process.env = { ...originalEnvironment }; |
There was a problem hiding this comment.
Reassigning process.env to a plain object (e.g., { ...originalEnvironment }) is generally discouraged in Node.js. It replaces the special internal environment object with a standard JavaScript object, which loses the built-in behavior of process.env (such as automatically converting all assigned values to strings). This can lead to subtle bugs if the code under test or other libraries expect the standard behavior.
For Vitest, the idiomatic and safer way to manage environment variables is using vi.stubEnv() in tests and vi.unstubAllEnvs() in afterEach. This avoids manual object manipulation and ensures the environment is correctly restored without side effects on the global process object.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
@stephansama/ai-commit-msg
@stephansama/alfred-kaomoji
@stephansama/astro-iconify-svgmap
@stephansama/auto-readme
@stephansama/catppuccin-jsonresume-theme
@stephansama/catppuccin-opml
@stephansama/catppuccin-rss
@stephansama/catppuccin-typedoc
@stephansama/catppuccin-xsl
@stephansama/eslint-config
create-stephansama-example
@stephansama/find-makefile-targets
@stephansama/github-env
@stephansama/multipublish
@stephansama/pnpm-hooks
@stephansama/prettier-plugin-handlebars
@stephansama/remark-asciinema
@stephansama/single-file
@stephansama/svelte-social-share-links
@stephansama/typed-env
@stephansama/typed-events
@stephansama/typed-nocodb-api
@stephansama/typed-templates
@stephansama/types-github-action-env
@stephansama/types-lhci
commit: |
1 participant
Closes STE-33
core/ai-commit-msg/test/ai.test.tscapturedconst originalEnvironment = process.envby reference, then individual tests diddelete process.env.Xto set up the "missing env" cases. The deletions mutated the same underlying object asoriginalEnvironment, so theafterEachrestoration was a no-op for any key that had been deleted. In CI environments where additional env vars are present (CI,GITHUB_ACTIONS, etc.), the cumulative deletions corrupted the baseline that later tests relied on.Fix: shallow-clone
process.envintooriginalEnvironmentonce, and re-clone on eachafterEachrestoration. Net diff: 6 lines.Acceptance criteria
pnpm exec vitest run --root core/ai-commit-msg→ 27/27 green)ai.test.ts(will confirm after merge)