Skip to content

Commit cc4364b

Browse files
author
Kartik Raj
authored
GetRemoteLauncherCommand should wrap path to ptvsd_launcher.py in quotes (microsoft#5585)
* Corrected bug and added tests * News entry * Updated tests * Code review
1 parent e8a53f2 commit cc4364b

4 files changed

Lines changed: 58 additions & 35 deletions

File tree

‎news/2 Fixes/4966.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
getRemoteLauncherCommand should wrap path to ptvsd_launcher.py in quotes

‎src/client/debugger/debugAdapter/DebugClients/launcherProvider.ts‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,28 +5,32 @@
55

66
// tslint:disable:max-classes-per-file
77

8+
import { optional } from 'inversify';
89
import * as path from 'path';
910
import { EXTENSION_ROOT_DIR } from '../../../common/constants';
1011
import { IDebugLauncherScriptProvider, IRemoteDebugLauncherScriptProvider, LocalDebugOptions, RemoteDebugOptions } from '../types';
1112

12-
const script = path.join(EXTENSION_ROOT_DIR, 'pythonFiles', 'ptvsd_launcher.py');
13+
const pathToScript = path.join(EXTENSION_ROOT_DIR, 'pythonFiles', 'ptvsd_launcher.py');
1314
export class NoDebugLauncherScriptProvider implements IDebugLauncherScriptProvider<LocalDebugOptions> {
15+
constructor(@optional() private script: string = pathToScript) { }
1416
public getLauncherArgs(options: LocalDebugOptions): string[] {
1517
const customDebugger = options.customDebugger ? '--custom' : '--default';
16-
return [script, customDebugger, '--nodebug', '--client', '--host', options.host, '--port', options.port.toString()];
18+
return [this.script.fileToCommandArgument(), customDebugger, '--nodebug', '--client', '--host', options.host, '--port', options.port.toString()];
1719
}
1820
}
1921

2022
export class DebuggerLauncherScriptProvider implements IDebugLauncherScriptProvider<LocalDebugOptions> {
23+
constructor(@optional() private script: string = pathToScript) { }
2124
public getLauncherArgs(options: LocalDebugOptions): string[] {
2225
const customDebugger = options.customDebugger ? '--custom' : '--default';
23-
return [script, customDebugger, '--client', '--host', options.host, '--port', options.port.toString()];
26+
return [this.script.fileToCommandArgument(), customDebugger, '--client', '--host', options.host, '--port', options.port.toString()];
2427
}
2528
}
2629

2730
export class RemoteDebuggerLauncherScriptProvider implements IRemoteDebugLauncherScriptProvider {
31+
constructor(@optional() private script: string = pathToScript) { }
2832
public getLauncherArgs(options: RemoteDebugOptions): string[] {
2933
const waitArgs = options.waitUntilDebuggerAttaches ? ['--wait'] : [];
30-
return [script, '--default', '--host', options.host, '--port', options.port.toString()].concat(waitArgs);
34+
return [this.script.fileToCommandArgument(), '--default', '--host', options.host, '--port', options.port.toString()].concat(waitArgs);
3135
}
3236
}

‎src/test/debugger/debugAdapter/debugClients/launcherProvider.unit.test.ts‎

Lines changed: 48 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -11,38 +11,57 @@ import { DebuggerLauncherScriptProvider, NoDebugLauncherScriptProvider, RemoteDe
1111

1212
const expectedPath = path.join(EXTENSION_ROOT_DIR, 'pythonFiles', 'ptvsd_launcher.py');
1313

14+
// tslint:disable-next-line:max-func-body-length
1415
suite('Debugger - Launcher Script Provider', () => {
1516
test('Ensure launcher script exists', async () => {
1617
expect(await fs.pathExists(expectedPath)).to.be.deep.equal(true, 'Debugger launcher script does not exist');
1718
});
18-
test('Test debug launcher args', async () => {
19-
const args = new DebuggerLauncherScriptProvider().getLauncherArgs({ host: 'something', port: 1234 });
20-
const expectedArgs = [expectedPath, '--default', '--client', '--host', 'something', '--port', '1234'];
21-
expect(args).to.be.deep.equal(expectedArgs);
22-
});
23-
test('Test non-debug launcher args', async () => {
24-
const args = new NoDebugLauncherScriptProvider().getLauncherArgs({ host: 'something', port: 1234 });
25-
const expectedArgs = [expectedPath, '--default', '--nodebug', '--client', '--host', 'something', '--port', '1234'];
26-
expect(args).to.be.deep.equal(expectedArgs);
27-
});
28-
test('Test debug launcher args and custom ptvsd', async () => {
29-
const args = new DebuggerLauncherScriptProvider().getLauncherArgs({ host: 'something', port: 1234, customDebugger: true });
30-
const expectedArgs = [expectedPath, '--custom', '--client', '--host', 'something', '--port', '1234'];
31-
expect(args).to.be.deep.equal(expectedArgs);
32-
});
33-
test('Test non-debug launcher args and custom ptvsd', async () => {
34-
const args = new NoDebugLauncherScriptProvider().getLauncherArgs({ host: 'something', port: 1234, customDebugger: true });
35-
const expectedArgs = [expectedPath, '--custom', '--nodebug', '--client', '--host', 'something', '--port', '1234'];
36-
expect(args).to.be.deep.equal(expectedArgs);
37-
});
38-
test('Test remote debug launcher args (and do not wait for debugger to attach)', async () => {
39-
const args = new RemoteDebuggerLauncherScriptProvider().getLauncherArgs({ host: 'something', port: 1234, waitUntilDebuggerAttaches: false });
40-
const expectedArgs = [expectedPath, '--default', '--host', 'something', '--port', '1234'];
41-
expect(args).to.be.deep.equal(expectedArgs);
42-
});
43-
test('Test remote debug launcher args (and wait for debugger to attach)', async () => {
44-
const args = new RemoteDebuggerLauncherScriptProvider().getLauncherArgs({ host: 'something', port: 1234, waitUntilDebuggerAttaches: true });
45-
const expectedArgs = [expectedPath, '--default', '--host', 'something', '--port', '1234', '--wait'];
46-
expect(args).to.be.deep.equal(expectedArgs);
19+
const testsForLaunchProvider =
20+
[
21+
{
22+
testName: 'When path to ptvsd launcher does not contains spaces',
23+
path: path.join('path', 'to', 'ptvsd_launcher'),
24+
expectedPath: 'path/to/ptvsd_launcher'
25+
},
26+
{
27+
testName: 'When path to ptvsd launcher contains spaces',
28+
path: path.join('path', 'to', 'ptvsd_launcher', 'with spaces'),
29+
expectedPath: '"path/to/ptvsd_launcher/with spaces"'
30+
}
31+
];
32+
33+
testsForLaunchProvider.forEach(testParams => {
34+
suite(testParams.testName, async () => {
35+
test('Test debug launcher args', async () => {
36+
const args = new DebuggerLauncherScriptProvider(testParams.path).getLauncherArgs({ host: 'something', port: 1234 });
37+
const expectedArgs = [testParams.expectedPath, '--default', '--client', '--host', 'something', '--port', '1234'];
38+
expect(args).to.be.deep.equal(expectedArgs);
39+
});
40+
test('Test non-debug launcher args', async () => {
41+
const args = new NoDebugLauncherScriptProvider(testParams.path).getLauncherArgs({ host: 'something', port: 1234 });
42+
const expectedArgs = [testParams.expectedPath, '--default', '--nodebug', '--client', '--host', 'something', '--port', '1234'];
43+
expect(args).to.be.deep.equal(expectedArgs);
44+
});
45+
test('Test debug launcher args and custom ptvsd', async () => {
46+
const args = new DebuggerLauncherScriptProvider(testParams.path).getLauncherArgs({ host: 'something', port: 1234, customDebugger: true });
47+
const expectedArgs = [testParams.expectedPath, '--custom', '--client', '--host', 'something', '--port', '1234'];
48+
expect(args).to.be.deep.equal(expectedArgs);
49+
});
50+
test('Test non-debug launcher args and custom ptvsd', async () => {
51+
const args = new NoDebugLauncherScriptProvider(testParams.path).getLauncherArgs({ host: 'something', port: 1234, customDebugger: true });
52+
const expectedArgs = [testParams.expectedPath, '--custom', '--nodebug', '--client', '--host', 'something', '--port', '1234'];
53+
expect(args).to.be.deep.equal(expectedArgs);
54+
});
55+
test('Test remote debug launcher args (and do not wait for debugger to attach)', async () => {
56+
const args = new RemoteDebuggerLauncherScriptProvider(testParams.path).getLauncherArgs({ host: 'something', port: 1234, waitUntilDebuggerAttaches: false });
57+
const expectedArgs = [testParams.expectedPath, '--default', '--host', 'something', '--port', '1234'];
58+
expect(args).to.be.deep.equal(expectedArgs);
59+
});
60+
test('Test remote debug launcher args (and wait for debugger to attach)', async () => {
61+
const args = new RemoteDebuggerLauncherScriptProvider(testParams.path).getLauncherArgs({ host: 'something', port: 1234, waitUntilDebuggerAttaches: true });
62+
const expectedArgs = [testParams.expectedPath, '--default', '--host', 'something', '--port', '1234', '--wait'];
63+
expect(args).to.be.deep.equal(expectedArgs);
64+
});
65+
});
4766
});
4867
});

‎src/test/extension.unit.test.ts‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,11 +6,10 @@
66
// tslint:disable:no-any
77

88
import { expect } from 'chai';
9-
import * as path from 'path';
109
import { buildApi } from '../client/api';
1110
import { EXTENSION_ROOT_DIR } from '../client/common/constants';
1211

13-
const expectedPath = path.join(EXTENSION_ROOT_DIR, 'pythonFiles', 'ptvsd_launcher.py');
12+
const expectedPath = `${EXTENSION_ROOT_DIR.fileToCommandArgument()}/pythonFiles/ptvsd_launcher.py`;
1413

1514
suite('Extension API Debugger', () => {
1615
test('Test debug launcher args (no-wait)', async () => {

0 commit comments

Comments
 (0)