test: migrate integration tests and vscode extension tests (Part 3)#10725
test: migrate integration tests and vscode extension tests (Part 3)#10725joehan wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Code Review
This pull request cleans up unused node-fetch imports, adds a --debug flag to local hosting tests, and updates VS Code integration tests to use a temporary user data directory and mock authentication via nock. Feedback on these changes highlights a cross-platform compatibility issue with hardcoding /tmp on Windows, and recommends properly saving and restoring process.env.FIREBASE_TOKEN in tests to prevent test pollution.
| const extensionTestsPath = path.resolve(__dirname, "./suite/index"); | ||
|
|
||
| // Download VS Code, unzip it and run the integration test | ||
| const tmpUserData = path.join("/tmp", `vsc-ud-${Math.random().toString(36).substring(2, 7)}`); |
There was a problem hiding this comment.
Hardcoding "/tmp" as the temporary directory path will fail on Windows platforms, as /tmp is not a standard directory there. Use os.tmpdir() to resolve the temporary directory in a cross-platform manner.
| const tmpUserData = path.join("/tmp", `vsc-ud-${Math.random().toString(36).substring(2, 7)}`); | |
| const tmpUserData = path.join(require("os").tmpdir(), `vsc-ud-${Math.random().toString(36).substring(2, 7)}`); |
| authStub = stub(auth, "getAccessToken").resolves({ | ||
| access_token: "an_access_token", | ||
| }); | ||
| process.env.FIREBASE_TOKEN = "mock_refresh_token"; |
There was a problem hiding this comment.
Modifying process.env.FIREBASE_TOKEN directly without restoring its original value can cause side effects or failures in other tests running in the same process (especially in CI environments where a real token might be set). Consider saving the original value and restoring it in teardown.
(globalThis as any).originalFirebaseToken = process.env.FIREBASE_TOKEN;
process.env.FIREBASE_TOKEN = "mock_refresh_token";| }); | ||
|
|
||
| teardown(() => { | ||
| delete process.env.FIREBASE_TOKEN; |
There was a problem hiding this comment.
Restore the original FIREBASE_TOKEN instead of unconditionally deleting it, to avoid breaking other tests that might run in the same process.
if ((globalThis as any).originalFirebaseToken === undefined) {
delete process.env.FIREBASE_TOKEN;
} else {
process.env.FIREBASE_TOKEN = (globalThis as any).originalFirebaseToken;
}fd88b59 to
7b512c3
Compare
846f7b1 to
0624e98
Compare
Description
This is Part 3 of the native fetch migration. It contains the integration and VS Code extension tests.
requireAuthWrapper(false)in the VS Code extension test runner (firebase-vscode/src/test/runTest.ts) to initialize the CLI auth state and prevent unhandled token refresh rejections.scripts/hosting-tests/, etc.).This PR is stacked on top of: