From 5dbd890e195ad81afaaebf1b302aac2a172c2e87 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 9 Jul 2025 16:43:37 +0000 Subject: [PATCH 1/4] Initial plan From 377e6b089c6a35698f0f41b9643bba01bcdd3717 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 9 Jul 2025 16:51:54 +0000 Subject: [PATCH 2/4] Fix double semicolon issue in PATH environment variable Co-authored-by: eleanorjboyd <26030610+eleanorjboyd@users.noreply.github.com> --- src/extension/noConfigDebugInit.ts | 8 +- .../unittest/noConfigDebugInit.unit.test.ts | 98 ++++++++++++++++++- 2 files changed, 104 insertions(+), 2 deletions(-) diff --git a/src/extension/noConfigDebugInit.ts b/src/extension/noConfigDebugInit.ts index 428c478..0a2a423 100644 --- a/src/extension/noConfigDebugInit.ts +++ b/src/extension/noConfigDebugInit.ts @@ -77,7 +77,13 @@ export async function registerNoConfigDebug( const noConfigScriptsDir = path.join(extPath, 'bundled', 'scripts', 'noConfigScripts'); const pathSeparator = process.platform === 'win32' ? ';' : ':'; - collection.append('PATH', `${pathSeparator}${noConfigScriptsDir}`); + + // Check if the current PATH already ends with a path separator to avoid double separators + const currentPath = process.env.PATH || ''; + const needsSeparator = currentPath.length > 0 && !currentPath.endsWith(pathSeparator); + const pathValueToAppend = needsSeparator ? `${pathSeparator}${noConfigScriptsDir}` : noConfigScriptsDir; + + collection.append('PATH', pathValueToAppend); const bundledDebugPath = path.join(extPath, 'bundled', 'libs', 'debugpy'); collection.replace('BUNDLED_DEBUGPY_PATH', bundledDebugPath); diff --git a/src/test/unittest/noConfigDebugInit.unit.test.ts b/src/test/unittest/noConfigDebugInit.unit.test.ts index a4f9c79..16d682e 100644 --- a/src/test/unittest/noConfigDebugInit.unit.test.ts +++ b/src/test/unittest/noConfigDebugInit.unit.test.ts @@ -71,7 +71,13 @@ suite('setup for no-config debug scenario', function () { .setup((x) => x.append(TypeMoq.It.isAny(), TypeMoq.It.isAny())) .callback((key, value) => { if (key === 'PATH') { - assert(value === `:${noConfigScriptsDir}`); + // The value should be the scripts directory, with or without separator + // depending on whether the current PATH ends with a separator + const pathSeparator = process.platform === 'win32' ? ';' : ':'; + const currentPath = process.env.PATH || ''; + const needsSeparator = currentPath.length > 0 && !currentPath.endsWith(pathSeparator); + const expectedValue = needsSeparator ? `${pathSeparator}${noConfigScriptsDir}` : noConfigScriptsDir; + assert(value === expectedValue); } }) .returns(envVarCollectionAppendStub); @@ -88,6 +94,96 @@ suite('setup for no-config debug scenario', function () { sinon.assert.calledOnce(envVarCollectionAppendStub); }); + test('should not add extra separator when PATH already ends with separator', async () => { + const environmentVariableCollectionMock = TypeMoq.Mock.ofType(); + envVarCollectionReplaceStub = sinon.stub(); + envVarCollectionAppendStub = sinon.stub(); + + // Simulate a PATH that already ends with a separator to test the fix + const pathSeparator = process.platform === 'win32' ? ';' : ':'; + const originalPath = process.env.PATH; + process.env.PATH = `/some/path${pathSeparator}`; + + try { + environmentVariableCollectionMock + .setup((x) => x.replace(TypeMoq.It.isAny(), TypeMoq.It.isAny())) + .returns(envVarCollectionReplaceStub); + + environmentVariableCollectionMock + .setup((x) => x.append(TypeMoq.It.isAny(), TypeMoq.It.isAny())) + .callback((key, value) => { + if (key === 'PATH') { + // When PATH already ends with separator, we should NOT add another one + assert(value === noConfigScriptsDir); + assert(!value.startsWith(pathSeparator)); + } + }) + .returns(envVarCollectionAppendStub); + + context.setup((c) => c.environmentVariableCollection).returns(() => environmentVariableCollectionMock.object); + + setupFileSystemWatchers(); + + // run init for no config debug + await registerNoConfigDebug(context.object.environmentVariableCollection, context.object.extensionPath); + + // assert that append was called for PATH + sinon.assert.calledOnce(envVarCollectionAppendStub); + } finally { + // Restore original PATH + if (originalPath !== undefined) { + process.env.PATH = originalPath; + } else { + delete process.env.PATH; + } + } + }); + + test('should add separator when PATH does not end with separator', async () => { + const environmentVariableCollectionMock = TypeMoq.Mock.ofType(); + envVarCollectionReplaceStub = sinon.stub(); + envVarCollectionAppendStub = sinon.stub(); + + // Simulate a PATH that does NOT end with a separator + const pathSeparator = process.platform === 'win32' ? ';' : ':'; + const originalPath = process.env.PATH; + process.env.PATH = '/some/path'; + + try { + environmentVariableCollectionMock + .setup((x) => x.replace(TypeMoq.It.isAny(), TypeMoq.It.isAny())) + .returns(envVarCollectionReplaceStub); + + environmentVariableCollectionMock + .setup((x) => x.append(TypeMoq.It.isAny(), TypeMoq.It.isAny())) + .callback((key, value) => { + if (key === 'PATH') { + // When PATH does not end with separator, we should add one + assert(value === `${pathSeparator}${noConfigScriptsDir}`); + assert(value.startsWith(pathSeparator)); + } + }) + .returns(envVarCollectionAppendStub); + + context.setup((c) => c.environmentVariableCollection).returns(() => environmentVariableCollectionMock.object); + + setupFileSystemWatchers(); + + // run init for no config debug + await registerNoConfigDebug(context.object.environmentVariableCollection, context.object.extensionPath); + + // assert that append was called for PATH + sinon.assert.calledOnce(envVarCollectionAppendStub); + } finally { + // Restore original PATH + if (originalPath !== undefined) { + process.env.PATH = originalPath; + } else { + delete process.env.PATH; + } + } + }); + test('should create file system watcher for debuggerAdapterEndpointFolder', async () => { // Arrange const environmentVariableCollectionMock = TypeMoq.Mock.ofType(); From da41f5fabfff8c08e3ff3fac130e81095833f047 Mon Sep 17 00:00:00 2001 From: eleanorjboyd <26030610+eleanorjboyd@users.noreply.github.com> Date: Wed, 9 Jul 2025 10:06:54 -0700 Subject: [PATCH 3/4] clarify comments and shrink test scope --- .../unittest/noConfigDebugInit.unit.test.ts | 24 +++++++++---------- 1 file changed, 11 insertions(+), 13 deletions(-) diff --git a/src/test/unittest/noConfigDebugInit.unit.test.ts b/src/test/unittest/noConfigDebugInit.unit.test.ts index 16d682e..3cf6e40 100644 --- a/src/test/unittest/noConfigDebugInit.unit.test.ts +++ b/src/test/unittest/noConfigDebugInit.unit.test.ts @@ -71,13 +71,7 @@ suite('setup for no-config debug scenario', function () { .setup((x) => x.append(TypeMoq.It.isAny(), TypeMoq.It.isAny())) .callback((key, value) => { if (key === 'PATH') { - // The value should be the scripts directory, with or without separator - // depending on whether the current PATH ends with a separator - const pathSeparator = process.platform === 'win32' ? ';' : ':'; - const currentPath = process.env.PATH || ''; - const needsSeparator = currentPath.length > 0 && !currentPath.endsWith(pathSeparator); - const expectedValue = needsSeparator ? `${pathSeparator}${noConfigScriptsDir}` : noConfigScriptsDir; - assert(value === expectedValue); + assert(value.includes(noConfigScriptsDir)); } }) .returns(envVarCollectionAppendStub); @@ -108,19 +102,21 @@ suite('setup for no-config debug scenario', function () { environmentVariableCollectionMock .setup((x) => x.replace(TypeMoq.It.isAny(), TypeMoq.It.isAny())) .returns(envVarCollectionReplaceStub); - + environmentVariableCollectionMock .setup((x) => x.append(TypeMoq.It.isAny(), TypeMoq.It.isAny())) .callback((key, value) => { if (key === 'PATH') { - // When PATH already ends with separator, we should NOT add another one + // Since PATH already ends with separator, we should NOT add another one assert(value === noConfigScriptsDir); assert(!value.startsWith(pathSeparator)); } }) .returns(envVarCollectionAppendStub); - context.setup((c) => c.environmentVariableCollection).returns(() => environmentVariableCollectionMock.object); + context + .setup((c) => c.environmentVariableCollection) + .returns(() => environmentVariableCollectionMock.object); setupFileSystemWatchers(); @@ -153,19 +149,21 @@ suite('setup for no-config debug scenario', function () { environmentVariableCollectionMock .setup((x) => x.replace(TypeMoq.It.isAny(), TypeMoq.It.isAny())) .returns(envVarCollectionReplaceStub); - + environmentVariableCollectionMock .setup((x) => x.append(TypeMoq.It.isAny(), TypeMoq.It.isAny())) .callback((key, value) => { if (key === 'PATH') { - // When PATH does not end with separator, we should add one + // Since PATH does NOT end with separator, we should add one assert(value === `${pathSeparator}${noConfigScriptsDir}`); assert(value.startsWith(pathSeparator)); } }) .returns(envVarCollectionAppendStub); - context.setup((c) => c.environmentVariableCollection).returns(() => environmentVariableCollectionMock.object); + context + .setup((c) => c.environmentVariableCollection) + .returns(() => environmentVariableCollectionMock.object); setupFileSystemWatchers(); From 31e41487c8f9a2ad16745db524d70fa1ddb68e3d Mon Sep 17 00:00:00 2001 From: eleanorjboyd <26030610+eleanorjboyd@users.noreply.github.com> Date: Wed, 9 Jul 2025 10:08:13 -0700 Subject: [PATCH 4/4] linting --- src/extension/noConfigDebugInit.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/extension/noConfigDebugInit.ts b/src/extension/noConfigDebugInit.ts index 0a2a423..23ed379 100644 --- a/src/extension/noConfigDebugInit.ts +++ b/src/extension/noConfigDebugInit.ts @@ -77,12 +77,12 @@ export async function registerNoConfigDebug( const noConfigScriptsDir = path.join(extPath, 'bundled', 'scripts', 'noConfigScripts'); const pathSeparator = process.platform === 'win32' ? ';' : ':'; - + // Check if the current PATH already ends with a path separator to avoid double separators const currentPath = process.env.PATH || ''; const needsSeparator = currentPath.length > 0 && !currentPath.endsWith(pathSeparator); const pathValueToAppend = needsSeparator ? `${pathSeparator}${noConfigScriptsDir}` : noConfigScriptsDir; - + collection.append('PATH', pathValueToAppend); const bundledDebugPath = path.join(extPath, 'bundled', 'libs', 'debugpy');