Skip to content

Commit 24a5b7a

Browse files
Fix double semicolon in PATH environment variable on Windows (#748)
* Initial plan * Fix double semicolon issue in PATH environment variable Co-authored-by: eleanorjboyd <26030610+eleanorjboyd@users.noreply.github.com> * clarify comments and shrink test scope * linting --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: eleanorjboyd <26030610+eleanorjboyd@users.noreply.github.com>
1 parent 08c5021 commit 24a5b7a

File tree

2 files changed

+102
-2
lines changed

2 files changed

+102
-2
lines changed

src/extension/noConfigDebugInit.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,13 @@ export async function registerNoConfigDebug(
7777

7878
const noConfigScriptsDir = path.join(extPath, 'bundled', 'scripts', 'noConfigScripts');
7979
const pathSeparator = process.platform === 'win32' ? ';' : ':';
80-
collection.append('PATH', `${pathSeparator}${noConfigScriptsDir}`);
80+
81+
// Check if the current PATH already ends with a path separator to avoid double separators
82+
const currentPath = process.env.PATH || '';
83+
const needsSeparator = currentPath.length > 0 && !currentPath.endsWith(pathSeparator);
84+
const pathValueToAppend = needsSeparator ? `${pathSeparator}${noConfigScriptsDir}` : noConfigScriptsDir;
85+
86+
collection.append('PATH', pathValueToAppend);
8187

8288
const bundledDebugPath = path.join(extPath, 'bundled', 'libs', 'debugpy');
8389
collection.replace('BUNDLED_DEBUGPY_PATH', bundledDebugPath);

src/test/unittest/noConfigDebugInit.unit.test.ts

Lines changed: 95 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ suite('setup for no-config debug scenario', function () {
7171
.setup((x) => x.append(TypeMoq.It.isAny(), TypeMoq.It.isAny()))
7272
.callback((key, value) => {
7373
if (key === 'PATH') {
74-
assert(value === `:${noConfigScriptsDir}`);
74+
assert(value.includes(noConfigScriptsDir));
7575
}
7676
})
7777
.returns(envVarCollectionAppendStub);
@@ -88,6 +88,100 @@ suite('setup for no-config debug scenario', function () {
8888
sinon.assert.calledOnce(envVarCollectionAppendStub);
8989
});
9090

91+
test('should not add extra separator when PATH already ends with separator', async () => {
92+
const environmentVariableCollectionMock = TypeMoq.Mock.ofType<any>();
93+
envVarCollectionReplaceStub = sinon.stub();
94+
envVarCollectionAppendStub = sinon.stub();
95+
96+
// Simulate a PATH that already ends with a separator to test the fix
97+
const pathSeparator = process.platform === 'win32' ? ';' : ':';
98+
const originalPath = process.env.PATH;
99+
process.env.PATH = `/some/path${pathSeparator}`;
100+
101+
try {
102+
environmentVariableCollectionMock
103+
.setup((x) => x.replace(TypeMoq.It.isAny(), TypeMoq.It.isAny()))
104+
.returns(envVarCollectionReplaceStub);
105+
106+
environmentVariableCollectionMock
107+
.setup((x) => x.append(TypeMoq.It.isAny(), TypeMoq.It.isAny()))
108+
.callback((key, value) => {
109+
if (key === 'PATH') {
110+
// Since PATH already ends with separator, we should NOT add another one
111+
assert(value === noConfigScriptsDir);
112+
assert(!value.startsWith(pathSeparator));
113+
}
114+
})
115+
.returns(envVarCollectionAppendStub);
116+
117+
context
118+
.setup((c) => c.environmentVariableCollection)
119+
.returns(() => environmentVariableCollectionMock.object);
120+
121+
setupFileSystemWatchers();
122+
123+
// run init for no config debug
124+
await registerNoConfigDebug(context.object.environmentVariableCollection, context.object.extensionPath);
125+
126+
// assert that append was called for PATH
127+
sinon.assert.calledOnce(envVarCollectionAppendStub);
128+
} finally {
129+
// Restore original PATH
130+
if (originalPath !== undefined) {
131+
process.env.PATH = originalPath;
132+
} else {
133+
delete process.env.PATH;
134+
}
135+
}
136+
});
137+
138+
test('should add separator when PATH does not end with separator', async () => {
139+
const environmentVariableCollectionMock = TypeMoq.Mock.ofType<any>();
140+
envVarCollectionReplaceStub = sinon.stub();
141+
envVarCollectionAppendStub = sinon.stub();
142+
143+
// Simulate a PATH that does NOT end with a separator
144+
const pathSeparator = process.platform === 'win32' ? ';' : ':';
145+
const originalPath = process.env.PATH;
146+
process.env.PATH = '/some/path';
147+
148+
try {
149+
environmentVariableCollectionMock
150+
.setup((x) => x.replace(TypeMoq.It.isAny(), TypeMoq.It.isAny()))
151+
.returns(envVarCollectionReplaceStub);
152+
153+
environmentVariableCollectionMock
154+
.setup((x) => x.append(TypeMoq.It.isAny(), TypeMoq.It.isAny()))
155+
.callback((key, value) => {
156+
if (key === 'PATH') {
157+
// Since PATH does NOT end with separator, we should add one
158+
assert(value === `${pathSeparator}${noConfigScriptsDir}`);
159+
assert(value.startsWith(pathSeparator));
160+
}
161+
})
162+
.returns(envVarCollectionAppendStub);
163+
164+
context
165+
.setup((c) => c.environmentVariableCollection)
166+
.returns(() => environmentVariableCollectionMock.object);
167+
168+
setupFileSystemWatchers();
169+
170+
// run init for no config debug
171+
await registerNoConfigDebug(context.object.environmentVariableCollection, context.object.extensionPath);
172+
173+
// assert that append was called for PATH
174+
sinon.assert.calledOnce(envVarCollectionAppendStub);
175+
} finally {
176+
// Restore original PATH
177+
if (originalPath !== undefined) {
178+
process.env.PATH = originalPath;
179+
} else {
180+
delete process.env.PATH;
181+
}
182+
}
183+
});
184+
91185
test('should create file system watcher for debuggerAdapterEndpointFolder', async () => {
92186
// Arrange
93187
const environmentVariableCollectionMock = TypeMoq.Mock.ofType<any>();

0 commit comments

Comments
 (0)