Skip to content

Fix runTests to support paths with spaces on Windows#325

Closed
rzhao271 with Copilot wants to merge 3 commits into
mainfrom
copilot/fix-runtests-support-spaces
Closed

Fix runTests to support paths with spaces on Windows#325
rzhao271 with Copilot wants to merge 3 commits into
mainfrom
copilot/fix-runtests-support-spaces

Conversation

Copilot AI commented Mar 12, 2026

Copy link
Copy Markdown
Contributor

When shell: true is used on Windows, Node.js concatenates all arguments into a single command string passed to cmd.exe. This causes path arguments containing spaces (e.g. --extensionTestsPath=C:\Users\John Doe\test) to be split at the space, silently breaking the test run.

Changes

  • lib/runTest.tsinnerRunTests: Removed shell: true and the accompanying "${executable}" quoting. The VS Code binary is a real .exe, not a .cmd script, so it can be spawned directly. With shell: false (the default), Node.js/libuv passes each arg as a discrete entry to CreateProcessW, correctly preserving spaces in both the executable path and argument values.
// Before
const shell = process.platform === 'win32';
const cmd = cp.spawn(shell ? `"${executable}"` : executable, args, { env: fullEnv, shell });

// After
const cmd = cp.spawn(executable, args, { env: fullEnv });
  • lib/runTest.test.mts (new): Adds a unit test that mocks child_process.spawn and calls runTests with executable and path arguments containing spaces. The test asserts that the executable is passed verbatim (no quote-wrapping), shell is falsy, and --extensionTestsPath / --extensionDevelopmentPath arguments preserve the spaces correctly.

Copilot AI and others added 2 commits March 12, 2026 17:55
Co-authored-by: rzhao271 <7199958+rzhao271@users.noreply.github.com>
@rzhao271

rzhao271 commented Apr 6, 2026

Copy link
Copy Markdown
Contributor

@copilot Add a test case.

Copilot AI commented Apr 6, 2026

Copy link
Copy Markdown
Contributor Author

@copilot Add a test case.

Added lib/runTest.test.mts in d63bb94. The test mocks child_process.spawn and calls runTests with executable and path arguments that all contain spaces, then asserts:

  • the executable is passed as-is (not quote-wrapped)
  • shell is falsy
  • --extensionTestsPath and --extensionDevelopmentPath args contain the raw space-bearing paths

Copilot AI requested a review from rzhao271 April 6, 2026 19:49
Comment thread lib/runTest.ts
): Promise<number> {
const fullEnv = Object.assign({}, process.env, testRunnerEnv);
const shell = process.platform === 'win32';
const cmd = cp.spawn(shell ? `"${executable}"` : executable, args, { env: fullEnv, shell });

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this not regress #268?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants