Skip to content

Commit 377e6b0

Browse files
Copiloteleanorjboyd
andcommitted
Fix double semicolon issue in PATH environment variable
Co-authored-by: eleanorjboyd <[email protected]>
1 parent 5dbd890 commit 377e6b0

File tree

2 files changed

+104
-2
lines changed

2 files changed

+104
-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: 97 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,13 @@ 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+
// The value should be the scripts directory, with or without separator
75+
// depending on whether the current PATH ends with a separator
76+
const pathSeparator = process.platform === 'win32' ? ';' : ':';
77+
const currentPath = process.env.PATH || '';
78+
const needsSeparator = currentPath.length > 0 && !currentPath.endsWith(pathSeparator);
79+
const expectedValue = needsSeparator ? `${pathSeparator}${noConfigScriptsDir}` : noConfigScriptsDir;
80+
assert(value === expectedValue);
7581
}
7682
})
7783
.returns(envVarCollectionAppendStub);
@@ -88,6 +94,96 @@ suite('setup for no-config debug scenario', function () {
8894
sinon.assert.calledOnce(envVarCollectionAppendStub);
8995
});
9096

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

0 commit comments

Comments
 (0)