-
Notifications
You must be signed in to change notification settings - Fork 69
@W-21221947: Refactor tests to stub env vars instead of mocking the Config module #217
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
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
Refactors unit tests to stub environment variables directly (via Vitest) instead of mocking the Config module, centralizing common env setup in shared test utilities.
Changes:
- Replace
vi.mock('../../config.js')patterns across multiple test suites withvi.stubEnv(...)and a sharedstubDefaultEnvVars()helper. - Add
stubDefaultEnvVars()tosrc/testShared.tsand use it fromsrc/testSetup.tsand updated tests. - Update
src/config.test.tsto usevi.stubEnv/vi.unstubAllEnvsrather than manualprocess.envmutation.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| src/tools/workbooks/getWorkbook.test.ts | Switch config mocking to env stubbing via stubDefaultEnvVars() and per-test vi.stubEnv. |
| src/tools/views/getViewImage.test.ts | Switch config mocking to env stubbing via stubDefaultEnvVars() and per-test vi.stubEnv. |
| src/tools/views/getViewData.test.ts | Switch config mocking to env stubbing via stubDefaultEnvVars() and per-test vi.stubEnv. |
| src/tools/queryDatasource/queryDatasource.test.ts | Replace config mocking with env stubbing for datasource credentials and bounded-context filtering. |
| src/tools/queryDatasource/datasourceCredentials.test.ts | Replace config mocking with DATASOURCE_CREDENTIALS env stubbing. |
| src/tools/pulse/generateMetricValueInsightBundle/generatePulseMetricValueInsightBundleTool.test.ts | Replace config mocking with env stubbing for bounded-context datasource filtering. |
| src/tools/pulse/generateInsightBrief/generatePulseInsightBriefTool.test.ts | Replace config mocking with env stubbing for bounded-context datasource filtering. |
| src/tools/getDatasourceMetadata/getDatasourceMetadata.test.ts | Replace config mocking with env stubbing for feature flags and bounded-context datasource filtering. |
| src/testShared.ts | Introduce stubDefaultEnvVars() helper for shared env defaults. |
| src/testSetup.ts | Use stubDefaultEnvVars() instead of duplicating env stubs inline. |
| src/telemetry/init.test.ts | Replace config mocking with TELEMETRY_PROVIDER / TELEMETRY_PROVIDER_CONFIG env stubbing. |
| src/config.test.ts | Refactor env setup/teardown to use vi.unstubAllEnvs + vi.stubEnv rather than rewriting process.env. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
I initially made these changes as I was working on #216 but pulled them out to isolate them and make the other PR smaller.