Skip to content

Commit

Permalink
Ensure both python and pythonPath are not set when resolving conf…
Browse files Browse the repository at this point in the history 
…ig. (#20781)

Fixes #20655
  • Loading branch information
@karthiknadig
karthiknadig authored Mar 1, 2023
1 parent a5005f6 commit be75eb2
Show file tree
Hide file tree
Showing 2 changed files with 139 additions and 11 deletions.
35 changes: 29 additions & 6 deletions src/client/debugger/extension/configuration/resolvers/base.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
115 changes: 110 additions & 5 deletions src/test/debugger/extension/configuration/resolvers/base.unit.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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');

Expand All @@ -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');

Expand All @@ -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,
Expand Down

0 comments on commit be75eb2

Please sign in to comment.