Skip to content

GetRemoteLauncherCommand should wrap path to ptvsd_launcher.py in quotes #5585

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
karrtikr merged 4 commits into from
May 10, 2019
Merged

GetRemoteLauncherCommand should wrap path to ptvsd_launcher.py in quotes #5585

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
18dfdc9
Corrected bug and added tests
May 8, 2019
94a322e
News entry
May 8, 2019
2371d41
Updated tests
May 8, 2019
4b626c8
Code review
May 10, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions news/2 Fixes/4966.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
getRemoteLauncherCommand should wrap path to ptvsd_launcher.py in quotes
12 changes: 8 additions & 4 deletions src/client/debugger/debugAdapter/DebugClients/launcherProvider.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,28 +5,32 @@

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

import { optional } from 'inversify';
import * as path from 'path';
import { EXTENSION_ROOT_DIR } from '../../../common/constants';
import { IDebugLauncherScriptProvider, IRemoteDebugLauncherScriptProvider, LocalDebugOptions, RemoteDebugOptions } from '../types';

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

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

export class RemoteDebuggerLauncherScriptProvider implements IRemoteDebugLauncherScriptProvider {
constructor(@optional() private script: string = pathToScript) { }
public getLauncherArgs(options: RemoteDebugOptions): string[] {
const waitArgs = options.waitUntilDebuggerAttaches ? ['--wait'] : [];
return [script, '--default', '--host', options.host, '--port', options.port.toString()].concat(waitArgs);
return [this.script.fileToCommandArgument(), '--default', '--host', options.host, '--port', options.port.toString()].concat(waitArgs);
}
}
77 changes: 48 additions & 29 deletions src/test/debugger/debugAdapter/debugClients/launcherProvider.unit.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,38 +11,57 @@ import { DebuggerLauncherScriptProvider, NoDebugLauncherScriptProvider, RemoteDe

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

// tslint:disable-next-line:max-func-body-length
suite('Debugger - Launcher Script Provider', () => {
test('Ensure launcher script exists', async () => {
expect(await fs.pathExists(expectedPath)).to.be.deep.equal(true, 'Debugger launcher script does not exist');
});
test('Test debug launcher args', async () => {
const args = new DebuggerLauncherScriptProvider().getLauncherArgs({ host: 'something', port: 1234 });
const expectedArgs = [expectedPath, '--default', '--client', '--host', 'something', '--port', '1234'];
expect(args).to.be.deep.equal(expectedArgs);
});
test('Test non-debug launcher args', async () => {
const args = new NoDebugLauncherScriptProvider().getLauncherArgs({ host: 'something', port: 1234 });
const expectedArgs = [expectedPath, '--default', '--nodebug', '--client', '--host', 'something', '--port', '1234'];
expect(args).to.be.deep.equal(expectedArgs);
});
test('Test debug launcher args and custom ptvsd', async () => {
const args = new DebuggerLauncherScriptProvider().getLauncherArgs({ host: 'something', port: 1234, customDebugger: true });
const expectedArgs = [expectedPath, '--custom', '--client', '--host', 'something', '--port', '1234'];
expect(args).to.be.deep.equal(expectedArgs);
});
test('Test non-debug launcher args and custom ptvsd', async () => {
const args = new NoDebugLauncherScriptProvider().getLauncherArgs({ host: 'something', port: 1234, customDebugger: true });
const expectedArgs = [expectedPath, '--custom', '--nodebug', '--client', '--host', 'something', '--port', '1234'];
expect(args).to.be.deep.equal(expectedArgs);
});
test('Test remote debug launcher args (and do not wait for debugger to attach)', async () => {
const args = new RemoteDebuggerLauncherScriptProvider().getLauncherArgs({ host: 'something', port: 1234, waitUntilDebuggerAttaches: false });
const expectedArgs = [expectedPath, '--default', '--host', 'something', '--port', '1234'];
expect(args).to.be.deep.equal(expectedArgs);
});
test('Test remote debug launcher args (and wait for debugger to attach)', async () => {
const args = new RemoteDebuggerLauncherScriptProvider().getLauncherArgs({ host: 'something', port: 1234, waitUntilDebuggerAttaches: true });
const expectedArgs = [expectedPath, '--default', '--host', 'something', '--port', '1234', '--wait'];
expect(args).to.be.deep.equal(expectedArgs);
const testsForLaunchProvider =
[
{
testName: 'When path to ptvsd launcher does not contains spaces',
path: path.join('path', 'to', 'ptvsd_launcher'),
expectedPath: 'path/to/ptvsd_launcher'
},
{
testName: 'When path to ptvsd launcher contains spaces',
path: path.join('path', 'to', 'ptvsd_launcher', 'with spaces'),
expectedPath: '"path/to/ptvsd_launcher/with spaces"'
}
];

testsForLaunchProvider.forEach(testParams => {
suite(testParams.testName, async () => {
test('Test debug launcher args', async () => {
const args = new DebuggerLauncherScriptProvider(testParams.path).getLauncherArgs({ host: 'something', port: 1234 });
const expectedArgs = [testParams.expectedPath, '--default', '--client', '--host', 'something', '--port', '1234'];
expect(args).to.be.deep.equal(expectedArgs);
});
test('Test non-debug launcher args', async () => {
const args = new NoDebugLauncherScriptProvider(testParams.path).getLauncherArgs({ host: 'something', port: 1234 });
const expectedArgs = [testParams.expectedPath, '--default', '--nodebug', '--client', '--host', 'something', '--port', '1234'];
expect(args).to.be.deep.equal(expectedArgs);
});
test('Test debug launcher args and custom ptvsd', async () => {
const args = new DebuggerLauncherScriptProvider(testParams.path).getLauncherArgs({ host: 'something', port: 1234, customDebugger: true });
const expectedArgs = [testParams.expectedPath, '--custom', '--client', '--host', 'something', '--port', '1234'];
expect(args).to.be.deep.equal(expectedArgs);
});
test('Test non-debug launcher args and custom ptvsd', async () => {
const args = new NoDebugLauncherScriptProvider(testParams.path).getLauncherArgs({ host: 'something', port: 1234, customDebugger: true });
const expectedArgs = [testParams.expectedPath, '--custom', '--nodebug', '--client', '--host', 'something', '--port', '1234'];
expect(args).to.be.deep.equal(expectedArgs);
});
test('Test remote debug launcher args (and do not wait for debugger to attach)', async () => {
const args = new RemoteDebuggerLauncherScriptProvider(testParams.path).getLauncherArgs({ host: 'something', port: 1234, waitUntilDebuggerAttaches: false });
const expectedArgs = [testParams.expectedPath, '--default', '--host', 'something', '--port', '1234'];
expect(args).to.be.deep.equal(expectedArgs);
});
test('Test remote debug launcher args (and wait for debugger to attach)', async () => {
const args = new RemoteDebuggerLauncherScriptProvider(testParams.path).getLauncherArgs({ host: 'something', port: 1234, waitUntilDebuggerAttaches: true });
const expectedArgs = [testParams.expectedPath, '--default', '--host', 'something', '--port', '1234', '--wait'];
expect(args).to.be.deep.equal(expectedArgs);
});
});
});
});
3 changes: 1 addition & 2 deletions src/test/extension.unit.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,10 @@
// tslint:disable:no-any

import { expect } from 'chai';
import * as path from 'path';
import { buildApi } from '../client/api';
import { EXTENSION_ROOT_DIR } from '../client/common/constants';

const expectedPath = path.join(EXTENSION_ROOT_DIR, 'pythonFiles', 'ptvsd_launcher.py');
const expectedPath = `${EXTENSION_ROOT_DIR.fileToCommandArgument()}/pythonFiles/ptvsd_launcher.py`;
Comment thread
karrtikr marked this conversation as resolved.

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