-
Notifications
You must be signed in to change notification settings - Fork 34
Fix env variable caching and add testing arch for environment variable functionality #1044
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR fixes environment variable caching issues (#936) by introducing proper tracking of injected environment variables per workspace. The implementation ensures that variables removed or commented out in .env files are properly cleaned up from the terminal environment, and that only tracked variables managed by this injector are removed (preserving variables set by other managers like BASH_ENV).
Key changes:
- Added
trackedEnvVarsMap to track which environment variables are set per workspace - Refactored injection logic to compare previous and current state, deleting variables that are no longer present
- Updated disposal logic to only clean up tracked variables instead of clearing all environment variables
- Comprehensive test suite with both unit tests and integration tests using fixture files
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| src/features/terminal/terminalEnvVarInjector.ts | Added tracking mechanism for environment variables per workspace, refactored injection and cleanup logic to handle variable removal, and improved dispose method to preserve non-tracked variables |
| src/test/features/terminalEnvVarInjectorBasic.unit.test.ts | Comprehensive unit tests for core functionality including variable addition, updates, deletion, and tracking using test interface pattern for private method access |
| src/test/features/terminalEnvVarInjector.test.ts | Integration tests using real file fixtures to verify complete workflows including file modifications, deletions, configuration changes, and multi-workspace scenarios |
| src/test/fixtures/terminalEnvVarInjector/*.env | Test fixture files covering various scenarios: basic variables, empty files, commented variables, custom paths, unset values, and multi-workspace setups |
| .github/instructions/testing-workflow.instructions.md | Added learnings about fixture-based testing, private access patterns, path handling consistency, and VS Code file watcher limitations |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
fixes #936