Skip to content

Commit 12b454f

Browse files
committed
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.
1 parent f650237 commit 12b454f

File tree

5 files changed

+152
-26
lines changed

5 files changed

+152
-26
lines changed

package.json

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,9 +18,10 @@
1818
"type": "commonjs",
1919
"main": "./dist/extension.js",
2020
"scripts": {
21-
"build": "concurrently -g -n webviews,extension \"pnpm build:webviews\" \"node esbuild.mjs\"",
21+
"build": "concurrently -g -n webviews,extension,compile-tests:integration \"pnpm build:webviews\" \"node esbuild.mjs\" \"pnpm compile-tests:integration\"",
2222
"build:production": "cross-env NODE_ENV=production pnpm build",
2323
"build:webviews": "pnpm -r --filter \"./packages/*\" --parallel build",
24+
"compile-tests:integration": "tsc -p test/integration --outDir out --noCheck",
2425
"format": "prettier --write --cache --cache-strategy content .",
2526
"format:check": "prettier --check --cache --cache-strategy content .",
2627
"lint": "eslint --cache --cache-strategy content .",
@@ -29,7 +30,7 @@
2930
"package:prerelease": "pnpm build:production && vsce package --pre-release --no-dependencies",
3031
"test": "cross-env CI=true ELECTRON_RUN_AS_NODE=1 electron node_modules/vitest/vitest.mjs",
3132
"test:extension": "cross-env ELECTRON_RUN_AS_NODE=1 electron node_modules/vitest/vitest.mjs --project extension",
32-
"test:integration": "tsc -p test/integration --outDir out --noCheck && node esbuild.mjs && vscode-test",
33+
"test:integration": "pnpm compile-tests:integration && node esbuild.mjs && vscode-test",
3334
"test:webview": "cross-env ELECTRON_RUN_AS_NODE=1 electron node_modules/vitest/vitest.mjs --project webview",
3435
"typecheck": "concurrently -g -n extension,tests,packages \"tsc --noEmit\" \"tsc --noEmit -p test\" \"pnpm typecheck:packages\"",
3536
"typecheck:packages": "pnpm -r --filter \"./packages/*\" --parallel typecheck",

test/unit/core/cliExec.test.ts

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,22 @@
11
import fs from "fs/promises";
22
import os from "os";
33
import path from "path";
4-
import { beforeAll, describe, expect, it } from "vitest";
5-
6-
import * as cliExec from "@/core/cliExec";
4+
import { beforeAll, describe, expect, it, vi } from "vitest";
75

86
import { MockConfigurationProvider } from "../../mocks/testHelpers";
97
import { isWindows, writeExecutable } from "../../utils/platform";
108

119
import type { CliEnv } from "@/core/cliExec";
1210

11+
// Shim execFile so .js test scripts are run through node cross-platform.
12+
vi.mock("node:child_process", async (importOriginal) => {
13+
const { shimExecFile } = await import("../../utils/platform");
14+
return shimExecFile(await importOriginal());
15+
});
16+
17+
// Import after mock so the module picks up the shimmed execFile.
18+
const cliExec = await import("@/core/cliExec");
19+
1320
describe("cliExec", () => {
1421
const tmp = path.join(os.tmpdir(), "vscode-coder-tests-cliExec");
1522
let echoArgsBin: string;

test/unit/remote/sshProcess.test.ts

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import find from "find-process";
22
import { vol } from "memfs";
33
import * as fsPromises from "node:fs/promises";
4+
import path from "node:path";
45
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
56

67
import {
@@ -313,8 +314,10 @@ describe("SshProcessMonitor", () => {
313314
});
314315
const logPath = await waitForEvent(monitor.onLogFilePathChange);
315316

316-
expect(logPath).toBe("/proxy-logs/999.log");
317-
expect(monitor.getLogFilePath()).toBe("/proxy-logs/999.log");
317+
expect(logPath).toBe(path.join("/proxy-logs", "999.log"));
318+
expect(monitor.getLogFilePath()).toBe(
319+
path.join("/proxy-logs", "999.log"),
320+
);
318321
});
319322

320323
it("finds log file with prefix pattern", async () => {
@@ -330,7 +333,7 @@ describe("SshProcessMonitor", () => {
330333
});
331334
const logPath = await waitForEvent(monitor.onLogFilePathChange);
332335

333-
expect(logPath).toBe("/proxy-logs/coder-ssh-999.log");
336+
expect(logPath).toBe(path.join("/proxy-logs", "coder-ssh-999.log"));
334337
});
335338

336339
it("returns undefined when no proxyLogDir set", async () => {
@@ -373,7 +376,7 @@ describe("SshProcessMonitor", () => {
373376
});
374377
const logPath = await waitForEvent(monitor.onLogFilePathChange);
375378

376-
expect(logPath).toBe("/proxy-logs/2024-01-03-999.log");
379+
expect(logPath).toBe(path.join("/proxy-logs", "2024-01-03-999.log"));
377380
});
378381

379382
it("sorts log files using localeCompare for consistent cross-platform ordering", async () => {
@@ -395,7 +398,7 @@ describe("SshProcessMonitor", () => {
395398

396399
// With localeCompare: ["a", "Z"] -> reversed -> "Z" first
397400
// With plain sort(): ["Z", "a"] -> reversed -> "a" first (WRONG)
398-
expect(logPath).toBe("/proxy-logs/Z-999.log");
401+
expect(logPath).toBe(path.join("/proxy-logs", "Z-999.log"));
399402
});
400403
});
401404

test/utils/platform.test.ts

Lines changed: 81 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,18 @@
1-
import { describe, expect, it } from "vitest";
1+
import * as cp from "node:child_process";
2+
import fs from "node:fs/promises";
3+
import os from "node:os";
4+
import path from "node:path";
5+
import { promisify } from "node:util";
6+
import { beforeAll, describe, expect, it } from "vitest";
27

38
import {
49
expectPathsEqual,
510
exitCommand,
11+
isWindows,
612
printCommand,
713
printEnvCommand,
8-
isWindows,
14+
shimExecFile,
15+
writeExecutable,
916
} from "./platform";
1017

1118
describe("platform utils", () => {
@@ -83,4 +90,76 @@ describe("platform utils", () => {
8390
},
8491
);
8592
});
93+
94+
describe("writeExecutable", () => {
95+
const tmp = path.join(os.tmpdir(), "vscode-coder-tests-platform");
96+
97+
beforeAll(async () => {
98+
await fs.rm(tmp, { recursive: true, force: true });
99+
await fs.mkdir(tmp, { recursive: true });
100+
});
101+
102+
it("writes a .js file and returns its path", async () => {
103+
const result = await writeExecutable(tmp, "test-script", "// hello");
104+
expect(result).toBe(path.join(tmp, "test-script.js"));
105+
expect(await fs.readFile(result, "utf-8")).toBe("// hello");
106+
});
107+
108+
it("overwrites existing files", async () => {
109+
await writeExecutable(tmp, "overwrite", "first");
110+
const result = await writeExecutable(tmp, "overwrite", "second");
111+
expect(await fs.readFile(result, "utf-8")).toBe("second");
112+
});
113+
});
114+
115+
describe("shimExecFile", () => {
116+
const tmp = path.join(os.tmpdir(), "vscode-coder-tests-shim");
117+
const mod = shimExecFile(cp);
118+
const shimmedExecFile = promisify(mod.execFile);
119+
120+
beforeAll(async () => {
121+
await fs.rm(tmp, { recursive: true, force: true });
122+
await fs.mkdir(tmp, { recursive: true });
123+
});
124+
125+
it("runs .js files through node", async () => {
126+
const script = await writeExecutable(
127+
tmp,
128+
"echo",
129+
'process.stdout.write("ok");',
130+
);
131+
const { stdout } = await shimmedExecFile(script);
132+
expect(stdout).toBe("ok");
133+
});
134+
135+
it("passes args through to the script", async () => {
136+
const script = await writeExecutable(
137+
tmp,
138+
"echo-args",
139+
"process.stdout.write(process.argv.slice(2).join(','));",
140+
);
141+
const { stdout } = await shimmedExecFile(script, ["a", "b", "c"]);
142+
expect(stdout).toBe("a,b,c");
143+
});
144+
145+
it("does not rewrite non-.js files", async () => {
146+
await expect(shimmedExecFile("/nonexistent/binary")).rejects.toThrow(
147+
"ENOENT",
148+
);
149+
});
150+
151+
it("preserves the callback form", async () => {
152+
const script = path.join(tmp, "echo.js");
153+
const stdout = await new Promise<string>((resolve, reject) => {
154+
mod.execFile(script, (err, out) =>
155+
err ? reject(new Error(err.message)) : resolve(out),
156+
);
157+
});
158+
expect(stdout).toBe("ok");
159+
});
160+
161+
it("does not touch spawn", () => {
162+
expect(mod.spawn).toBe(cp.spawn);
163+
});
164+
});
86165
});

test/utils/platform.ts

Lines changed: 50 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@ import os from "node:os";
33
import path from "node:path";
44
import { expect } from "vitest";
55

6+
import type { ChildProcess } from "node:child_process";
7+
68
export function isWindows(): boolean {
79
return os.platform() === "win32";
810
}
@@ -39,27 +41,61 @@ export function printEnvCommand(key: string, varName: string): string {
3941
}
4042

4143
/**
42-
* Write a cross-platform executable that runs the given JS code.
43-
* On Unix creates a shebang script; on Windows creates a .cmd wrapper.
44-
* Returns the path to the executable.
44+
* Write a JS file that can be executed cross-platform.
45+
* Tests that use `execFile` on the returned path should apply
46+
* {@link shimExecFile} so `.js` files are run through `process.execPath`.
4547
*/
4648
export async function writeExecutable(
4749
dir: string,
4850
name: string,
4951
code: string,
5052
): Promise<string> {
51-
if (isWindows()) {
52-
const jsPath = path.join(dir, `${name}.js`);
53-
const cmdPath = path.join(dir, `${name}.cmd`);
54-
await fs.writeFile(jsPath, code);
55-
await fs.writeFile(cmdPath, `@node "${jsPath}" %*\r\n`);
56-
return cmdPath;
57-
}
53+
const jsPath = path.join(dir, `${name}.js`);
54+
await fs.writeFile(jsPath, code);
55+
return jsPath;
56+
}
57+
58+
/**
59+
* If `file` is a `.js` path, prepend it into the args array and swap the
60+
* binary to `process.execPath` so `execFile` works on every platform
61+
* (Windows cannot `execFile` script wrappers).
62+
*/
63+
function prepend(file: string, rest: unknown[]): [string, ...unknown[]] {
64+
if (!file.endsWith(".js")) return [file, ...rest];
65+
const hasArgs = Array.isArray(rest[0]);
66+
const cliArgs = hasArgs ? (rest[0] as string[]) : [];
67+
const remaining = hasArgs ? rest.slice(1) : rest;
68+
return [process.execPath, [file, ...cliArgs], ...remaining];
69+
}
70+
71+
/**
72+
* Shim `child_process.execFile` so `.js` files are launched through node.
73+
* Use with `vi.mock`:
74+
*
75+
* ```ts
76+
* vi.mock("node:child_process", async (importOriginal) => {
77+
* const { shimExecFile } = await import("../../utils/platform");
78+
* return shimExecFile(await importOriginal());
79+
* });
80+
* ```
81+
*/
82+
type ChildProcessModule = typeof import("node:child_process");
83+
type Callable = (...args: unknown[]) => unknown;
84+
85+
export function shimExecFile(mod: ChildProcessModule): ChildProcessModule {
86+
const { execFile: original, ...rest } = mod;
87+
88+
const execFile = (file: string, ...args: unknown[]): ChildProcess =>
89+
(original as unknown as Callable)(...prepend(file, args)) as ChildProcess;
90+
91+
const sym = Symbol.for("nodejs.util.promisify.custom");
92+
const originalCustom = (original as unknown as Record<symbol, Callable>)[sym];
93+
(execFile as unknown as Record<symbol, unknown>)[sym] = (
94+
file: string,
95+
...args: unknown[]
96+
): unknown => originalCustom(...prepend(file, args));
5897

59-
const binPath = path.join(dir, name);
60-
await fs.writeFile(binPath, `#!/usr/bin/env node\n${code}`);
61-
await fs.chmod(binPath, "755");
62-
return binPath;
98+
return { ...rest, execFile } as ChildProcessModule;
6399
}
64100

65101
export function expectPathsEqual(actual: string, expected: string) {

0 commit comments

Comments
 (0)