Skip to content

Commit 97db51c

Browse files
karthiknadigkarthiknadig
committed
Ensure both python and pythonPath are not set when resolving config.
1 parent a5005f6 commit 97db51c

File tree

2 files changed

+139
-11
lines changed
  • src
    • client/debugger/extension/configuration/resolvers
    • test/debugger/extension/configuration/resolvers

2 files changed

+139
-11
lines changed

‎src/client/debugger/extension/configuration/resolvers/base.ts‎

Lines changed: 29 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -120,16 +120,39 @@ export abstract class BaseConfigurationResolver<T extends DebugConfiguration>
120120
undefined,
121121
);
122122
}
123-
if (debugConfiguration.python === '${command:python.interpreterPath}' || !debugConfiguration.python) {
123+
124+
if (debugConfiguration.python === '${command:python.interpreterPath}') {
125+
this.pythonPathSource = PythonPathSource.settingsJson;
126+
const interpreterPath =
127+
(await this.interpreterService.getActiveInterpreter(workspaceFolder))?.path ??
128+
this.configurationService.getSettings(workspaceFolder).pythonPath;
129+
debugConfiguration.python = interpreterPath;
130+
} else if (debugConfiguration.python === undefined) {
124131
this.pythonPathSource = PythonPathSource.settingsJson;
132+
debugConfiguration.python = debugConfiguration.pythonPath;
125133
} else {
126134
this.pythonPathSource = PythonPathSource.launchJson;
135+
debugConfiguration.python = resolveVariables(
136+
debugConfiguration.python ?? debugConfiguration.pythonPath,
137+
workspaceFolder?.fsPath,
138+
undefined,
139+
);
140+
}
141+
142+
if (
143+
debugConfiguration.debugAdapterPython === '${command:python.interpreterPath}' ||
144+
debugConfiguration.debugAdapterPython === undefined
145+
) {
146+
debugConfiguration.debugAdapterPython = debugConfiguration.python;
147+
}
148+
if (
149+
debugConfiguration.debugLauncherPython === '${command:python.interpreterPath}' ||
150+
debugConfiguration.debugLauncherPython === undefined
151+
) {
152+
debugConfiguration.debugLauncherPython = debugConfiguration.python;
127153
}
128-
debugConfiguration.python = resolveVariables(
129-
debugConfiguration.python ? debugConfiguration.python : undefined,
130-
workspaceFolder?.fsPath,
131-
undefined,
132-
);
154+
155+
delete debugConfiguration.pythonPath;
133156
}
134157

135158
protected static debugOption(debugOptions: DebugOptions[], debugOption: DebugOptions): void {

‎src/test/debugger/extension/configuration/resolvers/base.unit.test.ts‎

Lines changed: 110 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -158,7 +158,7 @@ suite('Debugging - Config Resolver', () => {
158158
test('Do nothing if debug configuration is undefined', async () => {
159159
await resolver.resolveAndUpdatePythonPath(undefined, (undefined as unknown) as LaunchRequestArguments);
160160
});
161-
test('pythonPath in debug config must point to pythonPath in settings if pythonPath in config is not set', async () => {
161+
test('python in debug config must point to pythonPath in settings if pythonPath in config is not set', async () => {
162162
const config = {};
163163
const pythonPath = path.join('1', '2', '3');
164164

@@ -168,11 +168,11 @@ suite('Debugging - Config Resolver', () => {
168168

169169
await resolver.resolveAndUpdatePythonPath(undefined, config as LaunchRequestArguments);
170170

171-
expect(config).to.have.property('pythonPath', pythonPath);
171+
expect(config).to.have.property('python', pythonPath);
172172
});
173-
test('pythonPath in debug config must point to pythonPath in settings if pythonPath in config is ${command:python.interpreterPath}', async () => {
173+
test('python in debug config must point to pythonPath in settings if pythonPath in config is ${command:python.interpreterPath}', async () => {
174174
const config = {
175-
pythonPath: '${command:python.interpreterPath}',
175+
python: '${command:python.interpreterPath}',
176176
};
177177
const pythonPath = path.join('1', '2', '3');
178178

@@ -182,8 +182,113 @@ suite('Debugging - Config Resolver', () => {
182182

183183
await resolver.resolveAndUpdatePythonPath(undefined, config as LaunchRequestArguments);
184184

185-
expect(config.pythonPath).to.equal(pythonPath);
185+
expect(config.python).to.equal(pythonPath);
186186
});
187+
188+
test('config should only contain python and not pythonPath after resolving', async () => {
189+
const config = { pythonPath: '${command:python.interpreterPath}', python: '${command:python.interpreterPath}' };
190+
const pythonPath = path.join('1', '2', '3');
191+
192+
when(interpreterService.getActiveInterpreter(anything())).thenResolve({
193+
path: pythonPath,
194+
} as PythonEnvironment);
195+
196+
await resolver.resolveAndUpdatePythonPath(undefined, config as LaunchRequestArguments);
197+
expect(config).to.not.have.property('pythonPath');
198+
expect(config).to.have.property('python', pythonPath);
199+
});
200+
201+
test('config should convert pythonPath to python, only if python is not set', async () => {
202+
const config = { pythonPath: '${command:python.interpreterPath}', python: undefined };
203+
const pythonPath = path.join('1', '2', '3');
204+
205+
when(interpreterService.getActiveInterpreter(anything())).thenResolve({
206+
path: pythonPath,
207+
} as PythonEnvironment);
208+
209+
await resolver.resolveAndUpdatePythonPath(undefined, config as LaunchRequestArguments);
210+
expect(config).to.not.have.property('pythonPath');
211+
expect(config).to.have.property('python', pythonPath);
212+
});
213+
214+
test('config should not change python if python is different than pythonPath', async () => {
215+
const expected = path.join('1', '2', '4');
216+
const config = { pythonPath: '${command:python.interpreterPath}', python: expected };
217+
const pythonPath = path.join('1', '2', '3');
218+
219+
when(interpreterService.getActiveInterpreter(anything())).thenResolve({
220+
path: pythonPath,
221+
} as PythonEnvironment);
222+
223+
await resolver.resolveAndUpdatePythonPath(undefined, config as LaunchRequestArguments);
224+
expect(config).to.not.have.property('pythonPath');
225+
expect(config).to.have.property('python', expected);
226+
});
227+
228+
test('config should get python from interpreter service is nothing is set', async () => {
229+
const config = {};
230+
const pythonPath = path.join('1', '2', '3');
231+
232+
when(interpreterService.getActiveInterpreter(anything())).thenResolve({
233+
path: pythonPath,
234+
} as PythonEnvironment);
235+
236+
await resolver.resolveAndUpdatePythonPath(undefined, config as LaunchRequestArguments);
237+
expect(config).to.not.have.property('pythonPath');
238+
expect(config).to.have.property('python', pythonPath);
239+
});
240+
241+
test('config should contain debugAdapterPython and debugLauncherPython', async () => {
242+
const config = {};
243+
const pythonPath = path.join('1', '2', '3');
244+
245+
when(interpreterService.getActiveInterpreter(anything())).thenResolve({
246+
path: pythonPath,
247+
} as PythonEnvironment);
248+
249+
await resolver.resolveAndUpdatePythonPath(undefined, config as LaunchRequestArguments);
250+
expect(config).to.not.have.property('pythonPath');
251+
expect(config).to.have.property('python', pythonPath);
252+
expect(config).to.have.property('debugAdapterPython', pythonPath);
253+
expect(config).to.have.property('debugLauncherPython', pythonPath);
254+
});
255+
256+
test('config should not change debugAdapterPython and debugLauncherPython if already set', async () => {
257+
const debugAdapterPythonPath = path.join('1', '2', '4');
258+
const debugLauncherPythonPath = path.join('1', '2', '5');
259+
260+
const config = { debugAdapterPython: debugAdapterPythonPath, debugLauncherPython: debugLauncherPythonPath };
261+
const pythonPath = path.join('1', '2', '3');
262+
263+
when(interpreterService.getActiveInterpreter(anything())).thenResolve({
264+
path: pythonPath,
265+
} as PythonEnvironment);
266+
267+
await resolver.resolveAndUpdatePythonPath(undefined, config as LaunchRequestArguments);
268+
expect(config).to.not.have.property('pythonPath');
269+
expect(config).to.have.property('python', pythonPath);
270+
expect(config).to.have.property('debugAdapterPython', debugAdapterPythonPath);
271+
expect(config).to.have.property('debugLauncherPython', debugLauncherPythonPath);
272+
});
273+
274+
test('config should not resolve debugAdapterPython and debugLauncherPython', async () => {
275+
const config = {
276+
debugAdapterPython: '${command:python.interpreterPath}',
277+
debugLauncherPython: '${command:python.interpreterPath}',
278+
};
279+
const pythonPath = path.join('1', '2', '3');
280+
281+
when(interpreterService.getActiveInterpreter(anything())).thenResolve({
282+
path: pythonPath,
283+
} as PythonEnvironment);
284+
285+
await resolver.resolveAndUpdatePythonPath(undefined, config as LaunchRequestArguments);
286+
expect(config).to.not.have.property('pythonPath');
287+
expect(config).to.have.property('python', pythonPath);
288+
expect(config).to.have.property('debugAdapterPython', pythonPath);
289+
expect(config).to.have.property('debugLauncherPython', pythonPath);
290+
});
291+
187292
const localHostTestMatrix: Record<string, boolean> = {
188293
localhost: true,
189294
'127.0.0.1': true,

0 commit comments

Comments
 (0)