From 177348737426e22c8415151c0007a0a80d73178e Mon Sep 17 00:00:00 2001 From: Ehab Younes Date: Wed, 8 Apr 2026 16:16:23 +0300 Subject: [PATCH 1/2] fix: use cross-env for Windows-compatible env variables and fix vitest electron path Unix-style `VAR=value` syntax in npm scripts doesn't work on Windows. Added cross-env to build:production, test, test:extension, and test:webview scripts. Changed vitest.nodeExecutable to point to the actual electron binary instead of the .bin shim, which doesn't resolve on Windows (only electron.cmd exists there). --- .vscode/settings.json | 2 +- package.json | 11 ++++++----- pnpm-lock.yaml | 44 ++++++++++++++++++++++++++++++------------- 3 files changed, 38 insertions(+), 19 deletions(-) diff --git a/.vscode/settings.json b/.vscode/settings.json index 9dcd366b..e4473024 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -14,5 +14,5 @@ "vitest.nodeEnv": { "ELECTRON_RUN_AS_NODE": "1" }, - "vitest.nodeExecutable": "node_modules/.bin/electron" + "vitest.nodeExecutable": "node_modules/electron/dist/electron" } diff --git a/package.json b/package.json index abafdf0e..6034b8cf 100644 --- a/package.json +++ b/package.json @@ -19,7 +19,7 @@ "main": "./dist/extension.js", "scripts": { "build": "concurrently -g -n webviews,extension \"pnpm build:webviews\" \"node esbuild.mjs\"", - "build:production": "NODE_ENV=production pnpm build", + "build:production": "cross-env NODE_ENV=production pnpm build", "build:webviews": "pnpm -r --filter \"./packages/*\" --parallel build", "format": "prettier --write --cache --cache-strategy content .", "format:check": "prettier --check --cache --cache-strategy content .", @@ -27,10 +27,10 @@ "lint:fix": "pnpm lint --fix", "package": "pnpm build:production && vsce package --no-dependencies", "package:prerelease": "pnpm build:production && vsce package --pre-release --no-dependencies", - "test": "CI=true ELECTRON_RUN_AS_NODE=1 electron node_modules/vitest/vitest.mjs", - "test:extension": "ELECTRON_RUN_AS_NODE=1 electron node_modules/vitest/vitest.mjs --project extension", + "test": "cross-env CI=true ELECTRON_RUN_AS_NODE=1 electron node_modules/vitest/vitest.mjs", + "test:extension": "cross-env ELECTRON_RUN_AS_NODE=1 electron node_modules/vitest/vitest.mjs --project extension", "test:integration": "tsc -p test/integration --outDir out --noCheck && node esbuild.mjs && vscode-test", - "test:webview": "ELECTRON_RUN_AS_NODE=1 electron node_modules/vitest/vitest.mjs --project webview", + "test:webview": "cross-env ELECTRON_RUN_AS_NODE=1 electron node_modules/vitest/vitest.mjs --project webview", "typecheck": "concurrently -g -n extension,tests,packages \"tsc --noEmit\" \"tsc --noEmit -p test\" \"pnpm typecheck:packages\"", "typecheck:packages": "pnpm -r --filter \"./packages/*\" --parallel typecheck", "watch": "concurrently -g -n extension,webviews \"pnpm watch:extension\" \"pnpm watch:webviews\"", @@ -609,8 +609,9 @@ "bufferutil": "^4.1.0", "coder": "catalog:", "concurrently": "^9.2.1", + "cross-env": "^10.1.0", "dayjs": "^1.11.20", - "electron": "39.8.1", + "electron": "39.8.5", "esbuild": "^0.28.0", "eslint": "^10.2.0", "eslint-config-prettier": "^10.1.8", diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 366dbc50..df007601 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -188,16 +188,19 @@ importers: version: 4.1.0 coder: specifier: 'catalog:' - version: https://codeload.github.com/coder/coder/tar.gz/beb99c17de2864974620782e1b68c4a6f6fad135 + version: https://codeload.github.com/coder/coder/tar.gz/983819860f6c189e568934ecadbe546959a526f9 concurrently: specifier: ^9.2.1 version: 9.2.1 + cross-env: + specifier: ^10.1.0 + version: 10.1.0 dayjs: specifier: ^1.11.20 version: 1.11.20 electron: - specifier: 39.8.1 - version: 39.8.1 + specifier: 39.8.5 + version: 39.8.5 esbuild: specifier: ^0.28.0 version: 0.28.0 @@ -533,6 +536,9 @@ packages: '@emnapi/wasi-threads@1.2.1': resolution: {integrity: sha512-uTII7OYF+/Mes/MrcIOYp5yOtSMLBWSIoLPpcgwipoiKbli6k322tcoFsxoIIxPDqW01SQGAgko4EzZi2BNv2w==} + '@epic-web/invariant@1.0.0': + resolution: {integrity: sha512-lrTPqgvfFQtR/eY/qkIzp98OGdNJu0m5ji3q/nJI8v3SXkRKEnWiOxMmbvcSoAIzv/cGiuvRy57k4suKQSAdwA==} + '@esbuild/aix-ppc64@0.28.0': resolution: {integrity: sha512-lhRUCeuOyJQURhTxl4WkpFTjIsbDayJHih5kZC1giwE+MhIzAb7mEsQMqMf18rHLsrb5qI1tafG20mLxEWcWlA==} engines: {node: '>=18'} @@ -1762,8 +1768,8 @@ packages: engines: {node: '>=6.0.0'} hasBin: true - basic-ftp@5.2.0: - resolution: {integrity: sha512-VoMINM2rqJwJgfdHq6RiUudKt2BV+FY5ZFezP/ypmwayk68+NzzAQy4XXLlqsGD4MCzq3DrmNFD/uUmBJuGoXw==} + basic-ftp@5.2.1: + resolution: {integrity: sha512-0yaL8JdxTknKDILitVpfYfV2Ob6yb3udX/hK97M7I3jOeznBNxQPtVvTUtnhUkyHlxFWyr5Lvknmgzoc7jf+1Q==} engines: {node: '>=10.0.0'} bidi-js@1.0.3: @@ -1923,8 +1929,8 @@ packages: resolution: {integrity: sha512-gfrHV6ZPkquExvMh9IOkKsBzNDk6sDuZ6DdBGUBkvFnTCqCxzpuq48RySgP0AnaqQkw2zynOFj9yly6T1Q2G5Q==} engines: {node: '>=16'} - coder@https://codeload.github.com/coder/coder/tar.gz/beb99c17de2864974620782e1b68c4a6f6fad135: - resolution: {tarball: https://codeload.github.com/coder/coder/tar.gz/beb99c17de2864974620782e1b68c4a6f6fad135} + coder@https://codeload.github.com/coder/coder/tar.gz/983819860f6c189e568934ecadbe546959a526f9: + resolution: {tarball: https://codeload.github.com/coder/coder/tar.gz/983819860f6c189e568934ecadbe546959a526f9} version: 0.0.0 color-convert@2.0.1: @@ -1967,6 +1973,11 @@ packages: core-util-is@1.0.3: resolution: {integrity: sha512-ZQBvi1DcpJ4GDqanjucZ2Hj3wEO5pZDS89BWbkcrvdxksJorwUDDZamX9ldFkp9aw2lmBDLgkObEA4DWNJ9FYQ==} + cross-env@10.1.0: + resolution: {integrity: sha512-GsYosgnACZTADcmEyJctkJIoqAhHjttw7RsFrVoJNXbsWWqaq6Ym+7kZjq6mS45O0jij6vtiReppKQEtqWy6Dw==} + engines: {node: '>=20'} + hasBin: true + cross-spawn@7.0.6: resolution: {integrity: sha512-uV2QOWP2nWzsy2aMp8aRibhi9dlzF5Hgh5SHaB9OiTGEyDTiJJyx0uy51QXdyWbtAHNua4XJzUKca3OzKUd3vA==} engines: {node: '>= 8'} @@ -2128,8 +2139,8 @@ packages: electron-to-chromium@1.5.332: resolution: {integrity: sha512-7OOtytmh/rINMLwaFTbcMVvYXO3AUm029X0LcyfYk0B557RlPkdpTpnH9+htMlfu5dKwOmT0+Zs2Aw+lnn6TeQ==} - electron@39.8.1: - resolution: {integrity: sha512-8gHotB8Adb+f8d+xkHk6om32j4McOF7D6X8J2kwZoSNciAXHd84kqVwwr6mi/x43LAmdqkM13RXbPy3Jfq2wnQ==} + electron@39.8.5: + resolution: {integrity: sha512-q6+LiQIcTadSyvtPgLDQkCtVA9jQJXQVMrQcctfOJILh6OFMN+UJJLRkuUTy8CZDYeCIBn1ZycqsL1dAXugxZA==} engines: {node: '>= 12.20.55'} hasBin: true @@ -4544,6 +4555,8 @@ snapshots: tslib: 2.8.1 optional: true + '@epic-web/invariant@1.0.0': {} + '@esbuild/aix-ppc64@0.28.0': optional: true @@ -5841,7 +5854,7 @@ snapshots: baseline-browser-mapping@2.10.16: {} - basic-ftp@5.2.0: {} + basic-ftp@5.2.1: {} bidi-js@1.0.3: dependencies: @@ -6029,7 +6042,7 @@ snapshots: cockatiel@3.2.1: {} - coder@https://codeload.github.com/coder/coder/tar.gz/beb99c17de2864974620782e1b68c4a6f6fad135: {} + coder@https://codeload.github.com/coder/coder/tar.gz/983819860f6c189e568934ecadbe546959a526f9: {} color-convert@2.0.1: dependencies: @@ -6064,6 +6077,11 @@ snapshots: core-util-is@1.0.3: {} + cross-env@10.1.0: + dependencies: + '@epic-web/invariant': 1.0.0 + cross-spawn: 7.0.6 + cross-spawn@7.0.6: dependencies: path-key: 3.1.1 @@ -6216,7 +6234,7 @@ snapshots: electron-to-chromium@1.5.332: {} - electron@39.8.1: + electron@39.8.5: dependencies: '@electron/get': 2.0.3 '@types/node': 22.19.17 @@ -6690,7 +6708,7 @@ snapshots: get-uri@7.0.0: dependencies: - basic-ftp: 5.2.0 + basic-ftp: 5.2.1 data-uri-to-buffer: 7.0.0 debug: 4.4.3(supports-color@8.1.1) transitivePeerDependencies: From 0121302045e52781f0d54b287de5a67c8e2d6483 Mon Sep 17 00:00:00 2001 From: Ehab Younes Date: Wed, 8 Apr 2026 16:28:10 +0300 Subject: [PATCH 2/2] fix: make speedtest and sshProcess tests pass on Windows - speedtest tests: use process.execPath instead of .cmd wrapper since execFile cannot spawn .cmd files on Windows (EINVAL) - sshProcess tests: use path.join for expected log paths so separators match the OS (backslashes on Windows, forward slashes on Unix) - Build integration tests on project build. --- package.json | 5 +- test/unit/core/cliExec.test.ts | 13 +++-- test/unit/remote/sshProcess.test.ts | 13 +++-- test/utils/platform.test.ts | 83 ++++++++++++++++++++++++++++- test/utils/platform.ts | 64 +++++++++++++++++----- 5 files changed, 152 insertions(+), 26 deletions(-) diff --git a/package.json b/package.json index 6034b8cf..a43cac3d 100644 --- a/package.json +++ b/package.json @@ -18,9 +18,10 @@ "type": "commonjs", "main": "./dist/extension.js", "scripts": { - "build": "concurrently -g -n webviews,extension \"pnpm build:webviews\" \"node esbuild.mjs\"", + "build": "concurrently -g -n webviews,extension,compile-tests:integration \"pnpm build:webviews\" \"node esbuild.mjs\" \"pnpm compile-tests:integration\"", "build:production": "cross-env NODE_ENV=production pnpm build", "build:webviews": "pnpm -r --filter \"./packages/*\" --parallel build", + "compile-tests:integration": "tsc -p test/integration --outDir out --noCheck", "format": "prettier --write --cache --cache-strategy content .", "format:check": "prettier --check --cache --cache-strategy content .", "lint": "eslint --cache --cache-strategy content .", @@ -29,7 +30,7 @@ "package:prerelease": "pnpm build:production && vsce package --pre-release --no-dependencies", "test": "cross-env CI=true ELECTRON_RUN_AS_NODE=1 electron node_modules/vitest/vitest.mjs", "test:extension": "cross-env ELECTRON_RUN_AS_NODE=1 electron node_modules/vitest/vitest.mjs --project extension", - "test:integration": "tsc -p test/integration --outDir out --noCheck && node esbuild.mjs && vscode-test", + "test:integration": "pnpm compile-tests:integration && node esbuild.mjs && vscode-test", "test:webview": "cross-env ELECTRON_RUN_AS_NODE=1 electron node_modules/vitest/vitest.mjs --project webview", "typecheck": "concurrently -g -n extension,tests,packages \"tsc --noEmit\" \"tsc --noEmit -p test\" \"pnpm typecheck:packages\"", "typecheck:packages": "pnpm -r --filter \"./packages/*\" --parallel typecheck", diff --git a/test/unit/core/cliExec.test.ts b/test/unit/core/cliExec.test.ts index 0adc9839..402f23b7 100644 --- a/test/unit/core/cliExec.test.ts +++ b/test/unit/core/cliExec.test.ts @@ -1,15 +1,22 @@ import fs from "fs/promises"; import os from "os"; import path from "path"; -import { beforeAll, describe, expect, it } from "vitest"; - -import * as cliExec from "@/core/cliExec"; +import { beforeAll, describe, expect, it, vi } from "vitest"; import { MockConfigurationProvider } from "../../mocks/testHelpers"; import { isWindows, writeExecutable } from "../../utils/platform"; import type { CliEnv } from "@/core/cliExec"; +// Shim execFile so .js test scripts are run through node cross-platform. +vi.mock("node:child_process", async (importOriginal) => { + const { shimExecFile } = await import("../../utils/platform"); + return shimExecFile(await importOriginal()); +}); + +// Import after mock so the module picks up the shimmed execFile. +const cliExec = await import("@/core/cliExec"); + describe("cliExec", () => { const tmp = path.join(os.tmpdir(), "vscode-coder-tests-cliExec"); diff --git a/test/unit/remote/sshProcess.test.ts b/test/unit/remote/sshProcess.test.ts index 8b401fd0..b8726b33 100644 --- a/test/unit/remote/sshProcess.test.ts +++ b/test/unit/remote/sshProcess.test.ts @@ -1,6 +1,7 @@ import find from "find-process"; import { vol } from "memfs"; import * as fsPromises from "node:fs/promises"; +import path from "node:path"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { @@ -313,8 +314,10 @@ describe("SshProcessMonitor", () => { }); const logPath = await waitForEvent(monitor.onLogFilePathChange); - expect(logPath).toBe("/proxy-logs/999.log"); - expect(monitor.getLogFilePath()).toBe("/proxy-logs/999.log"); + expect(logPath).toBe(path.join("/proxy-logs", "999.log")); + expect(monitor.getLogFilePath()).toBe( + path.join("/proxy-logs", "999.log"), + ); }); it("finds log file with prefix pattern", async () => { @@ -330,7 +333,7 @@ describe("SshProcessMonitor", () => { }); const logPath = await waitForEvent(monitor.onLogFilePathChange); - expect(logPath).toBe("/proxy-logs/coder-ssh-999.log"); + expect(logPath).toBe(path.join("/proxy-logs", "coder-ssh-999.log")); }); it("returns undefined when no proxyLogDir set", async () => { @@ -373,7 +376,7 @@ describe("SshProcessMonitor", () => { }); const logPath = await waitForEvent(monitor.onLogFilePathChange); - expect(logPath).toBe("/proxy-logs/2024-01-03-999.log"); + expect(logPath).toBe(path.join("/proxy-logs", "2024-01-03-999.log")); }); it("sorts log files using localeCompare for consistent cross-platform ordering", async () => { @@ -395,7 +398,7 @@ describe("SshProcessMonitor", () => { // With localeCompare: ["a", "Z"] -> reversed -> "Z" first // With plain sort(): ["Z", "a"] -> reversed -> "a" first (WRONG) - expect(logPath).toBe("/proxy-logs/Z-999.log"); + expect(logPath).toBe(path.join("/proxy-logs", "Z-999.log")); }); }); diff --git a/test/utils/platform.test.ts b/test/utils/platform.test.ts index c04820d6..090522fe 100644 --- a/test/utils/platform.test.ts +++ b/test/utils/platform.test.ts @@ -1,11 +1,18 @@ -import { describe, expect, it } from "vitest"; +import * as cp from "node:child_process"; +import fs from "node:fs/promises"; +import os from "node:os"; +import path from "node:path"; +import { promisify } from "node:util"; +import { beforeAll, describe, expect, it } from "vitest"; import { expectPathsEqual, exitCommand, + isWindows, printCommand, printEnvCommand, - isWindows, + shimExecFile, + writeExecutable, } from "./platform"; describe("platform utils", () => { @@ -83,4 +90,76 @@ describe("platform utils", () => { }, ); }); + + describe("writeExecutable", () => { + const tmp = path.join(os.tmpdir(), "vscode-coder-tests-platform"); + + beforeAll(async () => { + await fs.rm(tmp, { recursive: true, force: true }); + await fs.mkdir(tmp, { recursive: true }); + }); + + it("writes a .js file and returns its path", async () => { + const result = await writeExecutable(tmp, "test-script", "// hello"); + expect(result).toBe(path.join(tmp, "test-script.js")); + expect(await fs.readFile(result, "utf-8")).toBe("// hello"); + }); + + it("overwrites existing files", async () => { + await writeExecutable(tmp, "overwrite", "first"); + const result = await writeExecutable(tmp, "overwrite", "second"); + expect(await fs.readFile(result, "utf-8")).toBe("second"); + }); + }); + + describe("shimExecFile", () => { + const tmp = path.join(os.tmpdir(), "vscode-coder-tests-shim"); + const mod = shimExecFile(cp); + const shimmedExecFile = promisify(mod.execFile); + + beforeAll(async () => { + await fs.rm(tmp, { recursive: true, force: true }); + await fs.mkdir(tmp, { recursive: true }); + }); + + it("runs .js files through node", async () => { + const script = await writeExecutable( + tmp, + "echo", + 'process.stdout.write("ok");', + ); + const { stdout } = await shimmedExecFile(script); + expect(stdout).toBe("ok"); + }); + + it("passes args through to the script", async () => { + const script = await writeExecutable( + tmp, + "echo-args", + "process.stdout.write(process.argv.slice(2).join(','));", + ); + const { stdout } = await shimmedExecFile(script, ["a", "b", "c"]); + expect(stdout).toBe("a,b,c"); + }); + + it("does not rewrite non-.js files", async () => { + await expect(shimmedExecFile("/nonexistent/binary")).rejects.toThrow( + "ENOENT", + ); + }); + + it("preserves the callback form", async () => { + const script = path.join(tmp, "echo.js"); + const stdout = await new Promise((resolve, reject) => { + mod.execFile(script, (err, out) => + err ? reject(new Error(err.message)) : resolve(out), + ); + }); + expect(stdout).toBe("ok"); + }); + + it("does not touch spawn", () => { + expect(mod.spawn).toBe(cp.spawn); + }); + }); }); diff --git a/test/utils/platform.ts b/test/utils/platform.ts index 9c72caf7..704f08dc 100644 --- a/test/utils/platform.ts +++ b/test/utils/platform.ts @@ -3,6 +3,8 @@ import os from "node:os"; import path from "node:path"; import { expect } from "vitest"; +import type { ChildProcess } from "node:child_process"; + export function isWindows(): boolean { return os.platform() === "win32"; } @@ -39,27 +41,61 @@ export function printEnvCommand(key: string, varName: string): string { } /** - * Write a cross-platform executable that runs the given JS code. - * On Unix creates a shebang script; on Windows creates a .cmd wrapper. - * Returns the path to the executable. + * Write a JS file that can be executed cross-platform. + * Tests that use `execFile` on the returned path should apply + * {@link shimExecFile} so `.js` files are run through `process.execPath`. */ export async function writeExecutable( dir: string, name: string, code: string, ): Promise { - if (isWindows()) { - const jsPath = path.join(dir, `${name}.js`); - const cmdPath = path.join(dir, `${name}.cmd`); - await fs.writeFile(jsPath, code); - await fs.writeFile(cmdPath, `@node "${jsPath}" %*\r\n`); - return cmdPath; - } + const jsPath = path.join(dir, `${name}.js`); + await fs.writeFile(jsPath, code); + return jsPath; +} + +/** + * If `file` is a `.js` path, prepend it into the args array and swap the + * binary to `process.execPath` so `execFile` works on every platform + * (Windows cannot `execFile` script wrappers). + */ +function prepend(file: string, rest: unknown[]): [string, ...unknown[]] { + if (!file.endsWith(".js")) return [file, ...rest]; + const hasArgs = Array.isArray(rest[0]); + const cliArgs = hasArgs ? (rest[0] as string[]) : []; + const remaining = hasArgs ? rest.slice(1) : rest; + return [process.execPath, [file, ...cliArgs], ...remaining]; +} + +/** + * Shim `child_process.execFile` so `.js` files are launched through node. + * Use with `vi.mock`: + * + * ```ts + * vi.mock("node:child_process", async (importOriginal) => { + * const { shimExecFile } = await import("../../utils/platform"); + * return shimExecFile(await importOriginal()); + * }); + * ``` + */ +type ChildProcessModule = typeof import("node:child_process"); +type Callable = (...args: unknown[]) => unknown; + +export function shimExecFile(mod: ChildProcessModule): ChildProcessModule { + const { execFile: original, ...rest } = mod; + + const execFile = (file: string, ...args: unknown[]): ChildProcess => + (original as unknown as Callable)(...prepend(file, args)) as ChildProcess; + + const sym = Symbol.for("nodejs.util.promisify.custom"); + const originalCustom = (original as unknown as Record)[sym]; + (execFile as unknown as Record)[sym] = ( + file: string, + ...args: unknown[] + ): unknown => originalCustom(...prepend(file, args)); - const binPath = path.join(dir, name); - await fs.writeFile(binPath, `#!/usr/bin/env node\n${code}`); - await fs.chmod(binPath, "755"); - return binPath; + return { ...rest, execFile } as ChildProcessModule; } export function expectPathsEqual(actual: string, expected: string) {