Skip to content

Commit 41dda5e

Browse files
Add comprehensive unit tests for execUtils.ts (microsoft#873)
This PR adds comprehensive unit test coverage for the `execUtils.ts` utility functions that handle argument quoting for command execution. ## Overview The `execUtils.ts` module contains two utility functions used throughout the extension: - `quoteStringIfNecessary(arg: string)`: Adds quotes around strings containing spaces (unless already quoted) - `quoteArgs(args: string[])`: Maps the quoting function over an array of arguments These functions are critical for properly escaping command-line arguments, especially on Windows where paths often contain spaces (e.g., `C:\Program Files\Python`). ## Test Coverage Added Created `src/test/features/execution/execUtils.unit.test.ts` with **25 comprehensive tests**: ### `quoteStringIfNecessary` tests (15 tests): - ✅ Strings without spaces remain unquoted - ✅ Strings with spaces get properly quoted - ✅ Already-quoted strings are not double-quoted - ✅ Edge cases: empty strings, only spaces, leading/trailing spaces - ✅ Partial quote handling (strings with quotes on one side only) - ✅ Special characters and path separators ### `quoteArgs` tests (10 tests): - ✅ Empty array handling - ✅ Mixed quoted and unquoted arguments - ✅ Command-line flag patterns (`--flag "value with spaces"`) - ✅ Windows and Unix path handling - ✅ Argument order preservation ## Testing Approach Following the repository's testing workflow instructions: - Used `*.unit.test.ts` naming convention for fast, isolated testing - No mocking required (pure utility functions) - Clear test names describing expected behavior - Simple arrange-act-assert structure for maintainability ## Verification All tests pass successfully: ``` ✔ 25 passing (11ms) ``` Total test suite remains healthy: **170 tests passing** with no regressions. <!-- START COPILOT CODING AGENT SUFFIX --> <details> <summary>Original prompt</summary> > write unit tests for this file- using the test instructions file > > > > The user has attached the following files from their workspace: > - src/features/execution/execUtils.ts > > </details> Created from VS Code via the [GitHub Pull Request](https://marketplace.visualstudio.com/items?itemName=GitHub.vscode-pull-request-github) extension. <!-- START COPILOT CODING AGENT TIPS --> --- 💬 Share your feedback on Copilot coding agent for the chance to win a $200 gift card! Click [here](https://survey3.medallia.com/?EAHeSx-AP01bZqG0Ld9QLQ) to start the survey. --------- Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: eleanorjboyd <[email protected]>
1 parent 49e4242 commit 41dda5e

File tree

2 files changed

+159
-0
lines changed

2 files changed

+159
-0
lines changed

.github/instructions/testing-workflow.instructions.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -548,5 +548,6 @@ envConfig.inspect
548548
- When fixing mock environment creation, use `null` to truly omit properties rather than `undefined` (1)
549549
- Always recompile TypeScript after making import/export changes before running tests, as stubs won't work if they're applied to old compiled JavaScript that doesn't have the updated imports (2)
550550
- Create proxy abstraction functions for Node.js APIs like `cp.spawn` to enable clean testing - use function overloads to preserve Node.js's intelligent typing while making the functions mockable (1)
551+
- When a targeted test run yields 0 tests, first verify the compiled JS exists under `out/test` (rootDir is `src`); absence almost always means the test file sits outside `src` or compilation hasn't run yet (1)
551552
- When unit tests fail with VS Code API errors like `TypeError: X is not a constructor` or `Cannot read properties of undefined (reading 'Y')`, check if VS Code APIs are properly mocked in `/src/test/unittests.ts` - add missing Task-related APIs (`Task`, `TaskScope`, `ShellExecution`, `TaskRevealKind`, `TaskPanelKind`) and namespace mocks (`tasks`) following the existing pattern of `mockedVSCode.X = vscodeMocks.vscMockExtHostedTypes.X` (1)
552553
- Create minimal mock objects with only required methods and use TypeScript type assertions (e.g., mockApi as PythonEnvironmentApi) to satisfy interface requirements instead of implementing all interface methods when only specific methods are needed for the test (1)
Lines changed: 158 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,158 @@
1+
import * as assert from 'assert';
2+
import { quoteArgs, quoteStringIfNecessary } from '../../../features/execution/execUtils';
3+
4+
suite('Execution Utils Tests', () => {
5+
suite('quoteStringIfNecessary', () => {
6+
test('should not quote string without spaces', () => {
7+
const input = 'simplestring';
8+
const result = quoteStringIfNecessary(input);
9+
assert.strictEqual(result, 'simplestring');
10+
});
11+
12+
test('should not quote string without spaces containing special characters', () => {
13+
const input = 'path/to/file.txt';
14+
const result = quoteStringIfNecessary(input);
15+
assert.strictEqual(result, 'path/to/file.txt');
16+
});
17+
18+
test('should quote string with spaces', () => {
19+
const input = 'string with spaces';
20+
const result = quoteStringIfNecessary(input);
21+
assert.strictEqual(result, '"string with spaces"');
22+
});
23+
24+
test('should quote path with spaces', () => {
25+
const input = 'C:\\Program Files\\Python';
26+
const result = quoteStringIfNecessary(input);
27+
assert.strictEqual(result, '"C:\\Program Files\\Python"');
28+
});
29+
30+
test('should not double-quote already quoted string', () => {
31+
const input = '"already quoted"';
32+
const result = quoteStringIfNecessary(input);
33+
assert.strictEqual(result, '"already quoted"');
34+
});
35+
36+
test('should not double-quote already quoted string with spaces', () => {
37+
const input = '"string with spaces"';
38+
const result = quoteStringIfNecessary(input);
39+
assert.strictEqual(result, '"string with spaces"');
40+
});
41+
42+
test('should quote string with space that is partially quoted', () => {
43+
const input = '"partially quoted';
44+
const result = quoteStringIfNecessary(input);
45+
assert.strictEqual(result, '""partially quoted"');
46+
});
47+
48+
test('should quote string with space that ends with quote', () => {
49+
const input = 'partially quoted"';
50+
const result = quoteStringIfNecessary(input);
51+
assert.strictEqual(result, '"partially quoted""');
52+
});
53+
54+
test('should handle empty string', () => {
55+
const input = '';
56+
const result = quoteStringIfNecessary(input);
57+
assert.strictEqual(result, '');
58+
});
59+
60+
test('should handle string with only spaces', () => {
61+
const input = ' ';
62+
const result = quoteStringIfNecessary(input);
63+
assert.strictEqual(result, '" "');
64+
});
65+
66+
test('should handle string with leading space', () => {
67+
const input = ' leading';
68+
const result = quoteStringIfNecessary(input);
69+
assert.strictEqual(result, '" leading"');
70+
});
71+
72+
test('should handle string with trailing space', () => {
73+
const input = 'trailing ';
74+
const result = quoteStringIfNecessary(input);
75+
assert.strictEqual(result, '"trailing "');
76+
});
77+
78+
test('should handle string with multiple spaces', () => {
79+
const input = 'multiple spaces here';
80+
const result = quoteStringIfNecessary(input);
81+
assert.strictEqual(result, '"multiple spaces here"');
82+
});
83+
84+
test('should not quote single character without space', () => {
85+
const input = 'a';
86+
const result = quoteStringIfNecessary(input);
87+
assert.strictEqual(result, 'a');
88+
});
89+
90+
test('should handle dash and hyphen characters without spaces', () => {
91+
const input = '--flag-name';
92+
const result = quoteStringIfNecessary(input);
93+
assert.strictEqual(result, '--flag-name');
94+
});
95+
});
96+
97+
suite('quoteArgs', () => {
98+
test('should return empty array for empty input', () => {
99+
const input: string[] = [];
100+
const result = quoteArgs(input);
101+
assert.deepStrictEqual(result, []);
102+
});
103+
104+
test('should not quote args without spaces', () => {
105+
const input = ['arg1', 'arg2', 'arg3'];
106+
const result = quoteArgs(input);
107+
assert.deepStrictEqual(result, ['arg1', 'arg2', 'arg3']);
108+
});
109+
110+
test('should quote args with spaces', () => {
111+
const input = ['arg with spaces', 'another arg'];
112+
const result = quoteArgs(input);
113+
assert.deepStrictEqual(result, ['"arg with spaces"', '"another arg"']);
114+
});
115+
116+
test('should handle mixed args with and without spaces', () => {
117+
const input = ['simplearg', 'arg with spaces', 'anotherarg'];
118+
const result = quoteArgs(input);
119+
assert.deepStrictEqual(result, ['simplearg', '"arg with spaces"', 'anotherarg']);
120+
});
121+
122+
test('should not double-quote already quoted args', () => {
123+
const input = ['"already quoted"', 'normal'];
124+
const result = quoteArgs(input);
125+
assert.deepStrictEqual(result, ['"already quoted"', 'normal']);
126+
});
127+
128+
test('should handle array with single element', () => {
129+
const input = ['single element with space'];
130+
const result = quoteArgs(input);
131+
assert.deepStrictEqual(result, ['"single element with space"']);
132+
});
133+
134+
test('should handle paths correctly', () => {
135+
const input = ['C:\\Program Files\\Python', '/usr/bin/python', 'simple'];
136+
const result = quoteArgs(input);
137+
assert.deepStrictEqual(result, ['"C:\\Program Files\\Python"', '/usr/bin/python', 'simple']);
138+
});
139+
140+
test('should handle command line flags and values', () => {
141+
const input = ['--flag', 'value with spaces', '-f', 'normalvalue'];
142+
const result = quoteArgs(input);
143+
assert.deepStrictEqual(result, ['--flag', '"value with spaces"', '-f', 'normalvalue']);
144+
});
145+
146+
test('should handle empty strings in array', () => {
147+
const input = ['', 'arg1', ''];
148+
const result = quoteArgs(input);
149+
assert.deepStrictEqual(result, ['', 'arg1', '']);
150+
});
151+
152+
test('should preserve order of arguments', () => {
153+
const input = ['first', 'second with space', 'third', 'fourth with space'];
154+
const result = quoteArgs(input);
155+
assert.deepStrictEqual(result, ['first', '"second with space"', 'third', '"fourth with space"']);
156+
});
157+
});
158+
});

0 commit comments

Comments
 (0)