Ensure both python and pythonPath are not set when resolving conf…

…ig. (microsoft#20781) Fixes microsoft#20655
karthiknadig · Mar 6, 2023 · e9d302d · e9d302d
1 parent 5d1c5fb
commit e9d302d
Show file tree

Hide file tree

Showing 2 changed files with 139 additions and 11 deletions.
diff --git a/src/client/debugger/extension/configuration/resolvers/base.ts b/src/client/debugger/extension/configuration/resolvers/base.ts
@@ -120,16 +120,39 @@ export abstract class BaseConfigurationResolver<T extends DebugConfiguration>
                 undefined,
             );
         }
-        if (debugConfiguration.python === '${command:python.interpreterPath}' || !debugConfiguration.python) {
+
+        if (debugConfiguration.python === '${command:python.interpreterPath}') {
+            this.pythonPathSource = PythonPathSource.settingsJson;
+            const interpreterPath =
+                (await this.interpreterService.getActiveInterpreter(workspaceFolder))?.path ??
+                this.configurationService.getSettings(workspaceFolder).pythonPath;
+            debugConfiguration.python = interpreterPath;
+        } else if (debugConfiguration.python === undefined) {
             this.pythonPathSource = PythonPathSource.settingsJson;
+            debugConfiguration.python = debugConfiguration.pythonPath;
         } else {
             this.pythonPathSource = PythonPathSource.launchJson;
+            debugConfiguration.python = resolveVariables(
+                debugConfiguration.python ?? debugConfiguration.pythonPath,
+                workspaceFolder?.fsPath,
+                undefined,
+            );
+        }
+
+        if (
+            debugConfiguration.debugAdapterPython === '${command:python.interpreterPath}' ||
+            debugConfiguration.debugAdapterPython === undefined
+        ) {
+            debugConfiguration.debugAdapterPython = debugConfiguration.pythonPath ?? debugConfiguration.python;
+        }
+        if (
+            debugConfiguration.debugLauncherPython === '${command:python.interpreterPath}' ||
+            debugConfiguration.debugLauncherPython === undefined
+        ) {
+            debugConfiguration.debugLauncherPython = debugConfiguration.pythonPath ?? debugConfiguration.python;
         }
-        debugConfiguration.python = resolveVariables(
-            debugConfiguration.python ? debugConfiguration.python : undefined,
-            workspaceFolder?.fsPath,
-            undefined,
-        );
+
+        delete debugConfiguration.pythonPath;
     }
 
     protected static debugOption(debugOptions: DebugOptions[], debugOption: DebugOptions): void {

diff --git a/src/test/debugger/extension/configuration/resolvers/base.unit.test.ts b/src/test/debugger/extension/configuration/resolvers/base.unit.test.ts
@@ -158,7 +158,7 @@ suite('Debugging - Config Resolver', () => {
     test('Do nothing if debug configuration is undefined', async () => {
         await resolver.resolveAndUpdatePythonPath(undefined, (undefined as unknown) as LaunchRequestArguments);
     });
-    test('pythonPath in debug config must point to pythonPath in settings if pythonPath in config is not set', async () => {
+    test('python in debug config must point to pythonPath in settings if pythonPath in config is not set', async () => {
         const config = {};
         const pythonPath = path.join('1', '2', '3');
 
@@ -168,11 +168,11 @@ suite('Debugging - Config Resolver', () => {
 
         await resolver.resolveAndUpdatePythonPath(undefined, config as LaunchRequestArguments);
 
-        expect(config).to.have.property('pythonPath', pythonPath);
+        expect(config).to.have.property('python', pythonPath);
     });
-    test('pythonPath in debug config must point to pythonPath in settings  if pythonPath in config is ${command:python.interpreterPath}', async () => {
+    test('python in debug config must point to pythonPath in settings if pythonPath in config is ${command:python.interpreterPath}', async () => {
         const config = {
-            pythonPath: '${command:python.interpreterPath}',
+            python: '${command:python.interpreterPath}',
         };
         const pythonPath = path.join('1', '2', '3');
 
@@ -182,8 +182,113 @@ suite('Debugging - Config Resolver', () => {
 
         await resolver.resolveAndUpdatePythonPath(undefined, config as LaunchRequestArguments);
 
-        expect(config.pythonPath).to.equal(pythonPath);
+        expect(config.python).to.equal(pythonPath);
     });
+
+    test('config should only contain python and not pythonPath after resolving', async () => {
+        const config = { pythonPath: '${command:python.interpreterPath}', python: '${command:python.interpreterPath}' };
+        const pythonPath = path.join('1', '2', '3');
+
+        when(interpreterService.getActiveInterpreter(anything())).thenResolve({
+            path: pythonPath,
+        } as PythonEnvironment);
+
+        await resolver.resolveAndUpdatePythonPath(undefined, config as LaunchRequestArguments);
+        expect(config).to.not.have.property('pythonPath');
+        expect(config).to.have.property('python', pythonPath);
+    });
+
+    test('config should convert pythonPath to python, only if python is not set', async () => {
+        const config = { pythonPath: '${command:python.interpreterPath}', python: undefined };
+        const pythonPath = path.join('1', '2', '3');
+
+        when(interpreterService.getActiveInterpreter(anything())).thenResolve({
+            path: pythonPath,
+        } as PythonEnvironment);
+
+        await resolver.resolveAndUpdatePythonPath(undefined, config as LaunchRequestArguments);
+        expect(config).to.not.have.property('pythonPath');
+        expect(config).to.have.property('python', pythonPath);
+    });
+
+    test('config should not change python if python is different than pythonPath', async () => {
+        const expected = path.join('1', '2', '4');
+        const config = { pythonPath: '${command:python.interpreterPath}', python: expected };
+        const pythonPath = path.join('1', '2', '3');
+
+        when(interpreterService.getActiveInterpreter(anything())).thenResolve({
+            path: pythonPath,
+        } as PythonEnvironment);
+
+        await resolver.resolveAndUpdatePythonPath(undefined, config as LaunchRequestArguments);
+        expect(config).to.not.have.property('pythonPath');
+        expect(config).to.have.property('python', expected);
+    });
+
+    test('config should get python from interpreter service is nothing is set', async () => {
+        const config = {};
+        const pythonPath = path.join('1', '2', '3');
+
+        when(interpreterService.getActiveInterpreter(anything())).thenResolve({
+            path: pythonPath,
+        } as PythonEnvironment);
+
+        await resolver.resolveAndUpdatePythonPath(undefined, config as LaunchRequestArguments);
+        expect(config).to.not.have.property('pythonPath');
+        expect(config).to.have.property('python', pythonPath);
+    });
+
+    test('config should contain debugAdapterPython and debugLauncherPython', async () => {
+        const config = {};
+        const pythonPath = path.join('1', '2', '3');
+
+        when(interpreterService.getActiveInterpreter(anything())).thenResolve({
+            path: pythonPath,
+        } as PythonEnvironment);
+
+        await resolver.resolveAndUpdatePythonPath(undefined, config as LaunchRequestArguments);
+        expect(config).to.not.have.property('pythonPath');
+        expect(config).to.have.property('python', pythonPath);
+        expect(config).to.have.property('debugAdapterPython', pythonPath);
+        expect(config).to.have.property('debugLauncherPython', pythonPath);
+    });
+
+    test('config should not change debugAdapterPython and debugLauncherPython if already set', async () => {
+        const debugAdapterPythonPath = path.join('1', '2', '4');
+        const debugLauncherPythonPath = path.join('1', '2', '5');
+
+        const config = { debugAdapterPython: debugAdapterPythonPath, debugLauncherPython: debugLauncherPythonPath };
+        const pythonPath = path.join('1', '2', '3');
+
+        when(interpreterService.getActiveInterpreter(anything())).thenResolve({
+            path: pythonPath,
+        } as PythonEnvironment);
+
+        await resolver.resolveAndUpdatePythonPath(undefined, config as LaunchRequestArguments);
+        expect(config).to.not.have.property('pythonPath');
+        expect(config).to.have.property('python', pythonPath);
+        expect(config).to.have.property('debugAdapterPython', debugAdapterPythonPath);
+        expect(config).to.have.property('debugLauncherPython', debugLauncherPythonPath);
+    });
+
+    test('config should not resolve debugAdapterPython and debugLauncherPython', async () => {
+        const config = {
+            debugAdapterPython: '${command:python.interpreterPath}',
+            debugLauncherPython: '${command:python.interpreterPath}',
+        };
+        const pythonPath = path.join('1', '2', '3');
+
+        when(interpreterService.getActiveInterpreter(anything())).thenResolve({
+            path: pythonPath,
+        } as PythonEnvironment);
+
+        await resolver.resolveAndUpdatePythonPath(undefined, config as LaunchRequestArguments);
+        expect(config).to.not.have.property('pythonPath');
+        expect(config).to.have.property('python', pythonPath);
+        expect(config).to.have.property('debugAdapterPython', pythonPath);
+        expect(config).to.have.property('debugLauncherPython', pythonPath);
+    });
+
     const localHostTestMatrix: Record<string, boolean> = {
         localhost: true,
         '127.0.0.1': true,