Skip to content

Remove APIs related to ptvsd paths #11757

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
merged 1 commit into from
May 12, 2020
Merged

Remove APIs related to ptvsd paths #11757

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
30 changes: 4 additions & 26 deletions src/client/api.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,9 @@
'use strict';

import { isTestExecution } from './common/constants';
import { DebugAdapterNewPtvsd } from './common/experimentGroups';
import { traceError } from './common/logger';
import { IConfigurationService, IExperimentsManager, Resource } from './common/types';
import {
getDebugpyLauncherArgs,
getDebugpyPackagePath,
getPtvsdLauncherScriptArgs
} from './debugger/extension/adapter/remoteLaunchers';
import { IConfigurationService, Resource } from './common/types';
import { getDebugpyLauncherArgs, getDebugpyPackagePath } from './debugger/extension/adapter/remoteLaunchers';
import { IServiceContainer, IServiceManager } from './ioc/types';

/*
Expand Down Expand Up @@ -72,7 +67,6 @@ export function buildApi(
serviceManager: IServiceManager,
serviceContainer: IServiceContainer
): IExtensionApi {
const experimentsManager = serviceContainer.get<IExperimentsManager>(IExperimentsManager);
const configurationService = serviceContainer.get<IConfigurationService>(IConfigurationService);
const api = {
// 'ready' will propagate the exception, but we must log it here first.
Expand All @@ -86,30 +80,14 @@ export function buildApi(
port: number,
waitUntilDebuggerAttaches: boolean = true
): Promise<string[]> {
const useNewDADebugger = experimentsManager.inExperiment(DebugAdapterNewPtvsd.experiment);

if (useNewDADebugger) {
return getDebugpyLauncherArgs({
host,
port,
waitUntilDebuggerAttaches
});
}

return getPtvsdLauncherScriptArgs({
return getDebugpyLauncherArgs({
host,
port,
waitUntilDebuggerAttaches
});
},
async getDebuggerPackagePath(): Promise<string | undefined> {
const useNewDADebugger = experimentsManager.inExperiment(DebugAdapterNewPtvsd.experiment);

if (useNewDADebugger) {
return getDebugpyPackagePath();
}

return undefined;
return getDebugpyPackagePath();
}
},
settings: {
Expand Down
14 changes: 0 additions & 14 deletions src/client/debugger/extension/adapter/remoteLaunchers.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import { EXTENSION_ROOT_DIR } from '../../../common/constants';
import '../../../common/extensions';

const pathToPythonLibDir = path.join(EXTENSION_ROOT_DIR, 'pythonFiles', 'lib', 'python');
const pathToScript = path.join(EXTENSION_ROOT_DIR, 'pythonFiles', 'ptvsd_launcher.py');
const pathToDebugger = path.join(pathToPythonLibDir, 'debugpy');

export type RemoteDebugOptions = {
Expand All @@ -17,19 +16,6 @@ export type RemoteDebugOptions = {
waitUntilDebuggerAttaches: boolean;
};

export function getPtvsdLauncherScriptArgs(options: RemoteDebugOptions, script: string = pathToScript): string[] {
const waitArgs = options.waitUntilDebuggerAttaches ? ['--wait'] : [];
return [
script.fileToCommandArgument(),
'--default',
'--host',
options.host,
'--port',
options.port.toString(),
...waitArgs
];
}

export function getDebugpyLauncherArgs(options: RemoteDebugOptions, debuggerPath: string = pathToDebugger) {
const waitArgs = options.waitUntilDebuggerAttaches ? ['--wait-for-client'] : [];
return [debuggerPath.fileToCommandArgument(), '--listen', `${options.host}:${options.port}`, ...waitArgs];
Expand Down
84 changes: 12 additions & 72 deletions src/test/api.functional.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,40 +7,33 @@

import { assert, expect } from 'chai';
import * as path from 'path';
import { anyString, instance, mock, when } from 'ts-mockito';
import { instance, mock, when } from 'ts-mockito';
import { Uri } from 'vscode';
import { buildApi } from '../client/api';
import { ConfigurationService } from '../client/common/configuration/service';
import { EXTENSION_ROOT_DIR } from '../client/common/constants';
import { ExperimentsManager } from '../client/common/experiments';
import { IConfigurationService, IExperimentsManager } from '../client/common/types';
import { IConfigurationService } from '../client/common/types';
import { ServiceContainer } from '../client/ioc/container';
import { ServiceManager } from '../client/ioc/serviceManager';
import { IServiceContainer, IServiceManager } from '../client/ioc/types';

suite('Extension API', () => {
const expectedLauncherPath = path
.join(EXTENSION_ROOT_DIR, 'pythonFiles', 'ptvsd_launcher.py')
.fileToCommandArgument();
const debuggerPath = path.join(EXTENSION_ROOT_DIR, 'pythonFiles', 'lib', 'python', 'debugpy');
const ptvsdHost = 'somehost';
const ptvsdPort = 12345;
const debuggerHost = 'somehost';
const debuggerPort = 12345;

let serviceContainer: IServiceContainer;
let serviceManager: IServiceManager;
let experimentsManager: IExperimentsManager;
let configurationService: IConfigurationService;

setup(() => {
serviceContainer = mock(ServiceContainer);
serviceManager = mock(ServiceManager);
experimentsManager = mock(ExperimentsManager);
configurationService = mock(ConfigurationService);

when(serviceContainer.get<IConfigurationService>(IConfigurationService)).thenReturn(
instance(configurationService)
);
when(serviceContainer.get<IExperimentsManager>(IExperimentsManager)).thenReturn(instance(experimentsManager));
});

test('Execution command settings API returns expected array if interpreter is set', async () => {
Expand Down Expand Up @@ -69,97 +62,44 @@ suite('Extension API', () => {
expect(interpreterPath).to.equal(undefined, '');
});

test('Test debug launcher args (no-wait and not in experiment)', async () => {
test('Test debug launcher args (no-wait)', async () => {
const waitForAttach = false;
when(experimentsManager.inExperiment(anyString())).thenReturn(false);

const args = await buildApi(
Promise.resolve(),
instance(serviceManager),
instance(serviceContainer)
).debug.getRemoteLauncherCommand(ptvsdHost, ptvsdPort, waitForAttach);
const expectedArgs = [expectedLauncherPath, '--default', '--host', ptvsdHost, '--port', ptvsdPort.toString()];
).debug.getRemoteLauncherCommand(debuggerHost, debuggerPort, waitForAttach);
const expectedArgs = [debuggerPath.fileToCommandArgument(), '--listen', `${debuggerHost}:${debuggerPort}`];

expect(args).to.be.deep.equal(expectedArgs);
});

test('Test debug launcher args (no-wait and in experiment)', async () => {
const waitForAttach = false;
when(experimentsManager.inExperiment(anyString())).thenReturn(true);

const args = await buildApi(
Promise.resolve(),
instance(serviceManager),
instance(serviceContainer)
).debug.getRemoteLauncherCommand(ptvsdHost, ptvsdPort, waitForAttach);
const expectedArgs = [debuggerPath.fileToCommandArgument(), '--listen', `${ptvsdHost}:${ptvsdPort}`];

expect(args).to.be.deep.equal(expectedArgs);
});

test('Test debug launcher args (wait and not in experiment)', async () => {
const waitForAttach = true;
when(experimentsManager.inExperiment(anyString())).thenReturn(false);

const args = await buildApi(
Promise.resolve(),
instance(serviceManager),
instance(serviceContainer)
).debug.getRemoteLauncherCommand(ptvsdHost, ptvsdPort, waitForAttach);
const expectedArgs = [
expectedLauncherPath,
'--default',
'--host',
ptvsdHost,
'--port',
ptvsdPort.toString(),
'--wait'
];

expect(args).to.be.deep.equal(expectedArgs);
});

test('Test debug launcher args (wait and in experiment)', async () => {
test('Test debug launcher args (wait)', async () => {
const waitForAttach = true;
when(experimentsManager.inExperiment(anyString())).thenReturn(true);

const args = await buildApi(
Promise.resolve(),
instance(serviceManager),
instance(serviceContainer)
).debug.getRemoteLauncherCommand(ptvsdHost, ptvsdPort, waitForAttach);
).debug.getRemoteLauncherCommand(debuggerHost, debuggerPort, waitForAttach);
const expectedArgs = [
debuggerPath.fileToCommandArgument(),
'--listen',
`${ptvsdHost}:${ptvsdPort}`,
`${debuggerHost}:${debuggerPort}`,
'--wait-for-client'
];

expect(args).to.be.deep.equal(expectedArgs);
});

test('Test debugger package path when not in experiment', async () => {
when(experimentsManager.inExperiment(anyString())).thenReturn(false);

const pkgPath = await buildApi(
Promise.resolve(),
instance(serviceManager),
instance(serviceContainer)
).debug.getDebuggerPackagePath();

assert.isUndefined(pkgPath);
});

test('Test debugger package path when in experiment', async () => {
when(experimentsManager.inExperiment(anyString())).thenReturn(true);

test('Test debugger package path', async () => {
const pkgPath = await buildApi(
Promise.resolve(),
instance(serviceManager),
instance(serviceContainer)
).debug.getDebuggerPackagePath();

const expected = path.join(EXTENSION_ROOT_DIR, 'pythonFiles', 'lib', 'python', 'debugpy');
assert.equal(pkgPath, expected);
assert.equal(pkgPath, debuggerPath);
});
});
50 changes: 0 additions & 50 deletions src/test/debugger/extension/adapter/remoteLaunchers.unit.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,56 +9,6 @@ import { EXTENSION_ROOT_DIR } from '../../../../client/common/constants';
import '../../../../client/common/extensions';
import * as launchers from '../../../../client/debugger/extension/adapter/remoteLaunchers';

suite('External ptvsd Debugger Launcher', () => {
[
{
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"'
}
].forEach((testParams) => {
suite(testParams.testName, async () => {
test('Test remote debug launcher args (and do not wait for debugger to attach)', async () => {
const args = launchers.getPtvsdLauncherScriptArgs(
{
host: 'something',
port: 1234,
waitUntilDebuggerAttaches: false
},
testParams.path
);
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 = launchers.getPtvsdLauncherScriptArgs(
{
host: 'something',
port: 1234,
waitUntilDebuggerAttaches: true
},
testParams.path
);
const expectedArgs = [
testParams.expectedPath,
'--default',
'--host',
'something',
'--port',
'1234',
'--wait'
];
expect(args).to.be.deep.equal(expectedArgs);
});
});
});
});

suite('External debugpy Debugger Launcher', () => {
[
{
Expand Down