From 82c595a7ab723755e141f03157d98fc5e707c634 Mon Sep 17 00:00:00 2001 From: Karthik Nadig Date: Wed, 1 Mar 2023 13:15:10 -0800 Subject: [PATCH] Ensure both `python` and `pythonPath` are not set when resolving config. --- .../extension/configuration/resolvers/base.ts | 35 +++++- .../configuration/resolvers/base.unit.test.ts | 115 +++++++++++++++++- 2 files changed, 139 insertions(+), 11 deletions(-) diff --git a/src/client/debugger/extension/configuration/resolvers/base.ts b/src/client/debugger/extension/configuration/resolvers/base.ts index 06b233eb1ad1..0dde8c34abf8 100644 --- 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 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 index b4478f7c3f0a..4da645bc34ac 100644 --- 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 = { localhost: true, '127.0.0.1': true,