chore: update telemetry test not to fail without key VSCODE-517 #1169
Conversation
tculig
force-pushed
the
VSCODE-517-update-telemetry-test
branch
from
f920a50 to
b8e0dac
Compare
tculig
force-pushed
the
VSCODE-517-update-telemetry-test
branch
from
6c9c3d7 to
949e13b
Compare
tculig
force-pushed
the
VSCODE-517-update-telemetry-test
branch
from
949e13b to
f512f0c
Compare
himanshusinghs
left a comment
himanshusinghs
left a comment
There was a problem hiding this comment.
Was wondering if we could simply get away with providing the test-segment-key through the npm test script?
Additionally we might still want to have a test to confirm that the final build contains the segment key.
| // eslint-disable-next-line @typescript-eslint/no-var-requires | ||
| const Module = require('module'); | ||
| const originalRequire = Module.prototype.require; | ||
| Module.prototype.require = function (id: string, ...args: any[]): any { | ||
| if (id === '../../../../constants') { | ||
| return { segmentKey: process.env.SEGMENT_KEY }; | ||
| } | ||
| return originalRequire.apply(this, [id, ...args]); | ||
| }; | ||
|
|
||
| // Store the original require for restoration | ||
| originalRequireFunction = originalRequire; |
There was a problem hiding this comment.
This seems super heavy-handed - the require(segmentKeyFileLocation) call is used as a shorthand to reading a json file. Rather than mocking it, we could do something like:
const readSegmentKeyFromDisk = (): string => {
if (process.env.CI) {
return require('../../../../constants').segmentKey;
}
// "mock" the on-disk segment key
return process.env.SEGMENT_KEY;
}| ); | ||
|
|
||
| // Mock readSegmentKey to return a fake key for local testing | ||
| sandbox.replace( |
There was a problem hiding this comment.
This should only be done for non-CI runs, right?
package.json
Outdated
| "watch:extension": "npm run compile:extension -- -watch", | ||
| "watch:extension-bundles": "webpack --mode development --watch", | ||
| "pretest": "npm run compile", | ||
| "pretest": "cross-env SEGMENT_KEY=test-segment-key npm run compile", |
There was a problem hiding this comment.
do we need to set this in both places?
There was a problem hiding this comment.
the env variable gets baked into the config file at compile time, and there is a unit tests which checks the env values against the config file value, so the answer is yes, we do need it in both places. Alternatively, we can just skip the test that makes that check.
Description
https://jira.mongodb.org/browse/VSCODE-517
Motivation and Context
Open Questions
Dependents
Types of changes