Skip to content

Commit 5fa77fe

Browse files
wenytang-msCopilot
andcommitted
test: add unit tests for noConfigScripts PATH append helper
Extract the PATH append-value computation into a vscode-free src/pathUtil.ts module so it can be unit-tested in plain Node mocha. Add test/pathUtil.test.ts covering: - Correct separator on Windows (;), Linux (:), and macOS (:). - The appended value always starts with a path separator (regression guard for #1637). - The scripts directory remains a standalone PATH entry when the user's PATH last entry has no trailing separator (the exact scenario reported in #1637, e.g. C:\Program Files\jreleaser\\). - A trailing separator on the user's PATH only produces a harmless empty entry, never glues another entry onto our scripts dir. - The scripts directory is preserved verbatim at the end of the value. All 9 tests pass under plain mocha; the file is also picked up by the existing Electron-based test runner via the **/**.test.js glob. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent bd3bc78 commit 5fa77fe

3 files changed

Lines changed: 133 additions & 8 deletions

File tree

src/noConfigDebugInit.ts

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import * as vscode from 'vscode';
88

99
import { sendInfo, sendError } from "vscode-extension-telemetry-wrapper";
1010
import { getJavaHome } from "./utility";
11+
import { buildNoConfigPathAppendValue } from "./pathUtil";
1112

1213
/**
1314
* Registers the configuration-less debugging setup for the extension.
@@ -91,14 +92,7 @@ export async function registerNoConfigDebug(
9192
}
9293

9394
const noConfigScriptsDir = path.join(extPath, 'bundled', 'scripts', 'noConfigScripts');
94-
const pathSeparator = process.platform === 'win32' ? ';' : ':';
95-
96-
// EnvironmentVariableCollection.append() does literal string concatenation and does
97-
// not insert a separator. Always prepend one so we never glue our directory onto the
98-
// last entry of the user's PATH. We cannot rely on process.env.PATH ending with a
99-
// separator, since the terminal's PATH may differ from the extension host's PATH.
100-
// A trailing empty PATH entry (when PATH already ends with the separator) is harmless.
101-
collection.append('PATH', `${pathSeparator}${noConfigScriptsDir}`);
95+
collection.append('PATH', buildNoConfigPathAppendValue(noConfigScriptsDir));
10296

10397
// create file system watcher for the debuggerAdapterEndpointFolder for when the communication port is written
10498
const fileSystemWatcher = vscode.workspace.createFileSystemWatcher(

src/pathUtil.ts

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
// Copyright (c) Microsoft Corporation. All rights reserved.
2+
// Licensed under the MIT license.
3+
4+
/**
5+
* Builds the value to append to PATH for the noConfigScripts directory.
6+
*
7+
* `vscode.EnvironmentVariableCollection.append()` performs literal string
8+
* concatenation and does NOT insert a path separator. We always prepend one so
9+
* we never glue our directory onto the last entry of the user's PATH (e.g.
10+
* `...;C:\Program Files\jreleaser\c:\Users\...\noConfigScripts`).
11+
*
12+
* We cannot rely on `process.env.PATH` ending with a separator: the integrated
13+
* terminal's PATH may differ from the extension host's PATH (it can be modified
14+
* by `terminal.integrated.env.*` or by other extensions' env-var collections).
15+
*
16+
* A leading separator is always safe: if the resolved PATH already ends with
17+
* one, the resulting empty PATH entry is harmless on both Windows and POSIX
18+
* shells.
19+
*
20+
* This module has no `vscode` import so it can be unit-tested in plain Node.
21+
*
22+
* @param scriptsDir absolute path to the noConfigScripts directory
23+
* @param platform the target platform; defaults to `process.platform`. Made
24+
* injectable so unit tests can exercise both Windows and
25+
* POSIX behavior on a single host.
26+
*/
27+
export function buildNoConfigPathAppendValue(
28+
scriptsDir: string,
29+
platform: NodeJS.Platform = process.platform,
30+
): string {
31+
const pathSeparator = platform === 'win32' ? ';' : ':';
32+
return `${pathSeparator}${scriptsDir}`;
33+
}

test/pathUtil.test.ts

Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,98 @@
1+
// Copyright (c) Microsoft Corporation. All rights reserved.
2+
// Licensed under the MIT license.
3+
4+
import * as assert from "assert";
5+
6+
import { buildNoConfigPathAppendValue } from "../src/pathUtil";
7+
8+
// Regression tests for issue #1637: the extension was appending its
9+
// noConfigScripts directory to PATH without a separator on some terminal
10+
// PATH configurations, gluing it onto the last entry of the user's PATH.
11+
suite("buildNoConfigPathAppendValue", () => {
12+
13+
const winDir = "C:\\Users\\me\\.vscode\\extensions\\vscjava.vscode-java-debug-0.59.0\\bundled\\scripts\\noConfigScripts";
14+
const posixDir = "/home/me/.vscode/extensions/vscjava.vscode-java-debug-0.59.0/bundled/scripts/noConfigScripts";
15+
16+
test("uses ';' as separator on Windows", () => {
17+
const result = buildNoConfigPathAppendValue(winDir, "win32");
18+
assert.strictEqual(result, `;${winDir}`);
19+
});
20+
21+
test("uses ':' as separator on Linux", () => {
22+
const result = buildNoConfigPathAppendValue(posixDir, "linux");
23+
assert.strictEqual(result, `:${posixDir}`);
24+
});
25+
26+
test("uses ':' as separator on macOS", () => {
27+
const result = buildNoConfigPathAppendValue(posixDir, "darwin");
28+
assert.strictEqual(result, `:${posixDir}`);
29+
});
30+
31+
test("always starts with a path separator (Windows)", () => {
32+
const result = buildNoConfigPathAppendValue(winDir, "win32");
33+
assert.ok(result.startsWith(";"), `expected leading ';', got: ${result}`);
34+
});
35+
36+
test("always starts with a path separator (POSIX)", () => {
37+
const result = buildNoConfigPathAppendValue(posixDir, "linux");
38+
assert.ok(result.startsWith(":"), `expected leading ':', got: ${result}`);
39+
});
40+
41+
test("never collapses scriptsDir into the previous PATH entry on Windows", () => {
42+
// Simulates the exact scenario from issue #1637: a user PATH whose
43+
// last entry has no trailing separator. After append, the script dir
44+
// must not be glued onto 'jreleaser\'.
45+
const userPath = "C:\\foo;C:\\Program Files\\jreleaser\\";
46+
const finalPath = userPath + buildNoConfigPathAppendValue(winDir, "win32");
47+
48+
const entries = finalPath.split(";");
49+
assert.ok(
50+
entries.includes("C:\\Program Files\\jreleaser\\"),
51+
`expected 'jreleaser\\' to remain a standalone PATH entry, got entries: ${JSON.stringify(entries)}`,
52+
);
53+
assert.ok(
54+
entries.includes(winDir),
55+
`expected scripts dir to be a standalone PATH entry, got entries: ${JSON.stringify(entries)}`,
56+
);
57+
});
58+
59+
test("never collapses scriptsDir into the previous PATH entry on POSIX", () => {
60+
const userPath = "/usr/bin:/opt/jreleaser/bin";
61+
const finalPath = userPath + buildNoConfigPathAppendValue(posixDir, "linux");
62+
63+
const entries = finalPath.split(":");
64+
assert.ok(
65+
entries.includes("/opt/jreleaser/bin"),
66+
`expected '/opt/jreleaser/bin' to remain a standalone PATH entry, got entries: ${JSON.stringify(entries)}`,
67+
);
68+
assert.ok(
69+
entries.includes(posixDir),
70+
`expected scripts dir to be a standalone PATH entry, got entries: ${JSON.stringify(entries)}`,
71+
);
72+
});
73+
74+
test("yields only an empty (harmless) entry when the user's PATH already ends with a separator", () => {
75+
// If the resolved terminal PATH already ends with ';', append produces
76+
// ';;'. The empty middle entry is ignored by Windows and (in this
77+
// position) effectively a no-op on POSIX shells.
78+
const userPath = "C:\\foo;C:\\bar;";
79+
const finalPath = userPath + buildNoConfigPathAppendValue(winDir, "win32");
80+
81+
const entries = finalPath.split(";");
82+
// The scripts dir must still be a standalone, valid entry.
83+
assert.ok(
84+
entries.includes(winDir),
85+
`expected scripts dir to remain standalone, got entries: ${JSON.stringify(entries)}`,
86+
);
87+
// No real entry should be merged with our scripts dir.
88+
assert.ok(
89+
!entries.some((e) => e !== winDir && e.endsWith(winDir)),
90+
`no entry should be glued to scripts dir, got entries: ${JSON.stringify(entries)}`,
91+
);
92+
});
93+
94+
test("scriptsDir appears unchanged at the end of the appended value", () => {
95+
const result = buildNoConfigPathAppendValue(winDir, "win32");
96+
assert.ok(result.endsWith(winDir), `expected value to end with scriptsDir, got: ${result}`);
97+
});
98+
});

0 commit comments

Comments
 (0)