test(a2a-server): migrate process.env to vi.stubEnv() per GEMINI.md conventions#28487
test(a2a-server): migrate process.env to vi.stubEnv() per GEMINI.md conventions#28487kunalrawat425 wants to merge 1 commit into
Conversation
… conventions Direct process.env mutations can cause test leakage when tests run in the same process and a test fails before teardown runs. Migrate all direct assignments and deletes to vi.stubEnv() / vi.unstubAllEnvs() as documented in GEMINI.md testing conventions. Files changed: - src/commands/init.test.ts: beforeEach assignment → vi.stubEnv(); add afterEach vi.unstubAllEnvs() - src/http/app.test.ts: three inline delete/assignment mutations in /executeCommand tests → vi.stubEnv(); add vi.unstubAllEnvs() to the describe-block afterEach Fixes google-gemini#19826 Signed-off-by: Kunal Rawat <kunalrawat425@gmail.com>
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request standardizes environment variable handling in the a2a-server package tests. By migrating from direct process.env modifications to Vitest's built-in stubbing utilities, the changes improve test reliability and maintainability in accordance with the project's established testing conventions. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
|
📊 PR Size: size/S
|
There was a problem hiding this comment.
Code Review
This pull request refactors the test files init.test.ts and app.test.ts to use Vitest's environment stubbing utilities (vi.stubEnv and vi.unstubAllEnvs) instead of directly modifying process.env. The review feedback correctly points out that, according to the repository style guide, unsetting an environment variable should be done by passing an empty string '' to vi.stubEnv rather than undefined.
| vi.spyOn(commandRegistry, 'get').mockReturnValue(mockCommand); | ||
|
|
||
| delete process.env['CODER_AGENT_WORKSPACE_PATH']; | ||
| vi.stubEnv('CODER_AGENT_WORKSPACE_PATH', undefined); |
There was a problem hiding this comment.
According to the repository style guide, to "unset" an environment variable, you should use an empty string vi.stubEnv('NAME', '') instead of passing undefined.
| vi.stubEnv('CODER_AGENT_WORKSPACE_PATH', undefined); | |
| vi.stubEnv('CODER_AGENT_WORKSPACE_PATH', ''); |
References
- The Repository Style Guide states: 'To "unset" a variable, use an empty string
vi.stubEnv('NAME', '').' (link)
| vi.spyOn(commandRegistry, 'get').mockReturnValue(mockWorkspaceCommand); | ||
|
|
||
| delete process.env['CODER_AGENT_WORKSPACE_PATH']; | ||
| vi.stubEnv('CODER_AGENT_WORKSPACE_PATH', undefined); |
There was a problem hiding this comment.
According to the repository style guide, to "unset" an environment variable, you should use an empty string vi.stubEnv('NAME', '') instead of passing undefined.
| vi.stubEnv('CODER_AGENT_WORKSPACE_PATH', undefined); | |
| vi.stubEnv('CODER_AGENT_WORKSPACE_PATH', ''); |
References
- The Repository Style Guide states: 'To "unset" a variable, use an empty string
vi.stubEnv('NAME', '').' (link)
Summary
Replaces all direct
process.envmutations inpackages/a2a-server/tests withvi.stubEnv()/vi.unstubAllEnvs()as documented in GEMINI.md:Changes
src/commands/init.test.tsprocess.env['CODER_AGENT_WORKSPACE_PATH'] = …→vi.stubEnv(…); addafterEach(() => vi.unstubAllEnvs())src/http/app.test.tsdelete process.env[…]/process.env[…] = …→vi.stubEnv(…, undefined)/vi.stubEnv(…, value); addvi.unstubAllEnvs()to describe-blockafterEachTest plan
npx vitest run packages/a2a-server/src/commands/init.test.ts packages/a2a-server/src/http/app.test.ts— all 25 tests passFixes #19826