Skip to content

Commit

Permalink
Changes to how debug options are passed into the experimental version…
Browse files Browse the repository at this point in the history
… of PTVSD (debugger) (#1195)

* 🔨 change how debug options are passed to new PTVSD
* 📝 news entry
* Fixes #1168
  • Loading branch information
DonJayamanne authored Mar 26, 2018
1 parent bbf346e commit 3626de2
Show file tree
Hide file tree
Showing 11 changed files with 47 additions and 48 deletions.
1 change: 1 addition & 0 deletions news/3 Code Health/1168.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Changes to how debug options are passed into the experimental version of PTVSD (debugger).
4 changes: 4 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -901,6 +901,10 @@
"items": {
"type": "string",
"enum": [
"RedirectOutput",
"DebugStdLib",
"DjangoDebugging",
"FlaskDebugging",
"Sudo",
"Pyramid"
]
Expand Down
20 changes: 12 additions & 8 deletions src/client/debugger/Common/Contracts.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,13 +29,17 @@ export enum DebugFlags {
IgnoreCommandBursts = 1
}

export class DebugOptions {
public static get WaitOnAbnormalExit(): string { return 'WaitOnAbnormalExit'; }
public static get WaitOnNormalExit(): string { return 'WaitOnNormalExit'; }
public static get RedirectOutput(): string { return 'RedirectOutput'; }
public static get DjangoDebugging(): string { return 'DjangoDebugging'; }
public static get DebugStdLib(): string { return 'DebugStdLib'; }
public static get BreakOnSystemExitZero(): string { return 'BreakOnSystemExitZero'; }
export enum DebugOptions {
WaitOnAbnormalExit = 'WaitOnAbnormalExit',
WaitOnNormalExit = 'WaitOnNormalExit',
RedirectOutput = 'RedirectOutput',
DjangoDebugging = 'DjangoDebugging',
FlaskDebugging = 'FlaskDebugging',
DebugStdLib = 'DebugStdLib',
BreakOnSystemExitZero = 'BreakOnSystemExitZero',
Sudo = 'Sudo',
Pyramid = 'Pyramid',
FixFilePathCase = 'FixFilePathCase'
}

export interface ExceptionHandling {
Expand All @@ -57,7 +61,7 @@ export interface LaunchRequestArguments extends DebugProtocol.LaunchRequestArgum
args: string[];
applicationType?: string;
cwd?: string;
debugOptions?: string[];
debugOptions?: DebugOptions[];
env?: Object;
envFile: string;
exceptionHandling?: ExceptionHandling;
Expand Down
4 changes: 2 additions & 2 deletions src/client/debugger/DebugClients/LocalDebugClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import { PathUtils } from '../../common/platform/pathUtils';
import { CurrentProcess } from '../../common/process/currentProcess';
import { EnvironmentVariablesService } from '../../common/variables/environment';
import { IServiceContainer } from '../../ioc/types';
import { IDebugServer, IPythonProcess, LaunchRequestArguments } from '../Common/Contracts';
import { DebugOptions, IDebugServer, IPythonProcess, LaunchRequestArguments } from '../Common/Contracts';
import { IS_WINDOWS } from '../Common/Utils';
import { BaseDebugServer } from '../DebugServers/BaseDebugServer';
import { LocalDebugServer } from '../DebugServers/LocalDebugServer';
Expand Down Expand Up @@ -163,7 +163,7 @@ export class LocalDebugClient extends DebugClient<LaunchRequestArguments> {
}
// tslint:disable-next-line:member-ordering
protected buildLauncherArguments(): string[] {
const vsDebugOptions = ['RedirectOutput'];
const vsDebugOptions = [DebugOptions.RedirectOutput];
if (Array.isArray(this.args.debugOptions)) {
this.args.debugOptions.filter(opt => VALID_DEBUG_OPTIONS.indexOf(opt) >= 0)
.forEach(item => vsDebugOptions.push(item));
Expand Down
9 changes: 4 additions & 5 deletions src/client/debugger/configProviders/baseProvider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,11 @@ import { PythonLanguage } from '../../common/constants';
import { IFileSystem, IPlatformService } from '../../common/platform/types';
import { IConfigurationService } from '../../common/types';
import { IServiceContainer } from '../../ioc/types';
import { DebuggerType, LaunchRequestArguments } from '../Common/Contracts';
import { DebuggerType, DebugOptions, LaunchRequestArguments } from '../Common/Contracts';

// tslint:disable:no-invalid-template-strings

export type PythonDebugConfiguration = DebugConfiguration & LaunchRequestArguments;
export type PTVSDDebugConfiguration = PythonDebugConfiguration & { redirectOutput: boolean; fixFilePathCase: boolean };

@injectable()
export abstract class BaseConfigurationProvider implements DebugConfigurationProvider {
Expand Down Expand Up @@ -62,10 +61,10 @@ export abstract class BaseConfigurationProvider implements DebugConfigurationPro
debugConfiguration.debugOptions = [];
}
// Always redirect output.
if (debugConfiguration.debugOptions.indexOf('RedirectOutput') === -1) {
debugConfiguration.debugOptions.push('RedirectOutput');
if (debugConfiguration.debugOptions.indexOf(DebugOptions.RedirectOutput) === -1) {
debugConfiguration.debugOptions.push(DebugOptions.RedirectOutput);
}
if (debugConfiguration.debugOptions.indexOf('Pyramid') >= 0) {
if (debugConfiguration.debugOptions.indexOf(DebugOptions.Pyramid) >= 0) {
const platformService = this.serviceContainer.get<IPlatformService>(IPlatformService);
const fs = this.serviceContainer.get<IFileSystem>(IFileSystem);
const pserve = platformService.isWindows ? 'pserve.exe' : 'pserve';
Expand Down
10 changes: 6 additions & 4 deletions src/client/debugger/configProviders/pythonV2Provider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,8 @@ import { inject, injectable } from 'inversify';
import { Uri } from 'vscode';
import { IPlatformService } from '../../common/platform/types';
import { IServiceContainer } from '../../ioc/types';
import { BaseConfigurationProvider, PTVSDDebugConfiguration, PythonDebugConfiguration } from './baseProvider';
import { DebugOptions } from '../Common/Contracts';
import { BaseConfigurationProvider, PythonDebugConfiguration } from './baseProvider';

@injectable()
export class PythonV2DebugConfigurationProvider extends BaseConfigurationProvider {
Expand All @@ -20,8 +21,9 @@ export class PythonV2DebugConfigurationProvider extends BaseConfigurationProvide
debugConfiguration.stopOnEntry = false;

// Add PTVSD specific flags.
const ptvsdDebugConfigurationFlags = debugConfiguration as PTVSDDebugConfiguration;
ptvsdDebugConfigurationFlags.redirectOutput = Array.isArray(debugConfiguration.debugOptions) && debugConfiguration.debugOptions.indexOf('RedirectOutput') >= 0;
ptvsdDebugConfigurationFlags.fixFilePathCase = this.serviceContainer.get<IPlatformService>(IPlatformService).isWindows;
if (this.serviceContainer.get<IPlatformService>(IPlatformService).isWindows) {
debugConfiguration.debugOptions = Array.isArray(debugConfiguration.debugOptions) ? debugConfiguration.debugOptions : [];
debugConfiguration.debugOptions.push(DebugOptions.FixFilePathCase);
}
}
}
3 changes: 2 additions & 1 deletion src/client/unittests/common/debugLauncher.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { Uri } from 'vscode';
import { IDebugService, IWorkspaceService } from '../../common/application/types';
import { EXTENSION_ROOT_DIR } from '../../common/constants';
import { IConfigurationService } from '../../common/types';
import { DebugOptions } from '../../debugger/Common/Contracts';
import { IServiceContainer } from '../../ioc/types';
import { ITestDebugLauncher, LaunchOptions, TestProvider } from './types';

Expand Down Expand Up @@ -40,7 +41,7 @@ export class DebugLauncher implements ITestDebugLauncher {
cwd,
args: debugArgs,
console: 'none',
debugOptions: ['RedirectOutput']
debugOptions: [DebugOptions.RedirectOutput]
}).then(() => void (0));
}
private fixArgs(args: string[], testProvider: TestProvider, useExperimentalDebugger: boolean): string[] {
Expand Down
29 changes: 8 additions & 21 deletions src/test/debugger/configProvider/provider.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import { PythonLanguage } from '../../../client/common/constants';
import { IFileSystem, IPlatformService } from '../../../client/common/platform/types';
import { IConfigurationService, IPythonSettings } from '../../../client/common/types';
import { PythonDebugConfigurationProvider, PythonV2DebugConfigurationProvider } from '../../../client/debugger';
import { DebugOptions } from '../../../client/debugger/Common/Contracts';
import { IServiceContainer } from '../../../client/ioc/types';

[
Expand Down Expand Up @@ -260,24 +261,8 @@ import { IServiceContainer } from '../../../client/ioc/types';

expect(debugConfig).to.have.property('stopOnEntry', false);
expect(debugConfig).to.have.property('debugOptions');
expect((debugConfig as any).debugOptions).to.be.deep.equal(['RedirectOutput']);
});
test('Test redirection of output', async () => {
if (provider.debugType === 'python') {
return;
}
const pythonPath = `PythonPath_${new Date().toString()}`;
const workspaceFolder = createMoqWorkspaceFolder(__dirname);
const pythonFile = 'xyz.py';
setupIoc(pythonPath);
setupActiveEditor(pythonFile, PythonLanguage.language);

const debugConfig = await debugProvider.resolveDebugConfiguration!(workspaceFolder, { debugOptions: ['RedirectOutput'] } as any);

expect(debugConfig).to.have.property('redirectOutput');
expect((debugConfig as any).redirectOutput).to.be.equal(true, 'invalid value');
expect((debugConfig as any).debugOptions).to.be.deep.equal([DebugOptions.RedirectOutput]);
});

async function testFixFilePathCase(isWindows: boolean, isMac: boolean, isLinux: boolean) {
const pythonPath = `PythonPath_${new Date().toString()}`;
const workspaceFolder = createMoqWorkspaceFolder(__dirname);
Expand All @@ -286,9 +271,11 @@ import { IServiceContainer } from '../../../client/ioc/types';
setupActiveEditor(pythonFile, PythonLanguage.language);

const debugConfig = await debugProvider.resolveDebugConfiguration!(workspaceFolder, {} as DebugConfiguration);

expect(debugConfig).to.have.property('fixFilePathCase');
expect((debugConfig as any).fixFilePathCase).to.be.equal(isWindows, 'invalid value (true only for windows)');
if (isWindows) {
expect(debugConfig).to.have.property('debugOptions').contains(DebugOptions.FixFilePathCase);
} else {
expect(debugConfig).to.have.property('debugOptions').not.contains(DebugOptions.FixFilePathCase);
}
}
test('Test fixFilePathCase for Windows', async () => {
if (provider.debugType === 'python') {
Expand Down Expand Up @@ -318,7 +305,7 @@ import { IServiceContainer } from '../../../client/ioc/types';
setupIoc(pythonPath, isWindows, isMac, isLinux);
setupActiveEditor(pythonFile, PythonLanguage.language);

const options = addPyramidDebugOption ? { debugOptions: ['Pyramid'] } : {};
const options = addPyramidDebugOption ? { debugOptions: [DebugOptions.Pyramid] } : {};
fileSystem.setup(fs => fs.fileExistsSync(TypeMoq.It.isValue(pythonPath))).returns(() => pythonPathExists);

const debugConfig = await debugProvider.resolveDebugConfiguration!(workspaceFolder, options as any as DebugConfiguration);
Expand Down
4 changes: 2 additions & 2 deletions src/test/debugger/misc.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import { noop } from '../../client/common/core.utils';
import { IS_WINDOWS } from '../../client/common/platform/constants';
import { FileSystem } from '../../client/common/platform/fileSystem';
import { PlatformService } from '../../client/common/platform/platformService';
import { LaunchRequestArguments } from '../../client/debugger/Common/Contracts';
import { DebugOptions, LaunchRequestArguments } from '../../client/debugger/Common/Contracts';
import { sleep } from '../common';
import { IS_MULTI_ROOT_TEST, TEST_DEBUGGER } from '../initialize';
import { DEBUGGER_TIMEOUT } from './common/constants';
Expand Down Expand Up @@ -78,7 +78,7 @@ let testCounter = 0;
program: path.join(debugFilesPath, pythonFile),
cwd: debugFilesPath,
stopOnEntry,
debugOptions: ['RedirectOutput'],
debugOptions: [DebugOptions.RedirectOutput],
pythonPath: 'python',
args: [],
env,
Expand Down
4 changes: 2 additions & 2 deletions src/test/debugger/portAndHost.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import * as net from 'net';
import * as path from 'path';
import { DebugClient } from 'vscode-debugadapter-testsupport';
import { noop } from '../../client/common/core.utils';
import { LaunchRequestArguments } from '../../client/debugger/Common/Contracts';
import { DebugOptions, LaunchRequestArguments } from '../../client/debugger/Common/Contracts';
import { IS_MULTI_ROOT_TEST, TEST_DEBUGGER } from '../initialize';
import { DEBUGGER_TIMEOUT } from './common/constants';

Expand Down Expand Up @@ -49,7 +49,7 @@ const EXPERIMENTAL_DEBUG_ADAPTER = path.join(__dirname, '..', '..', 'client', 'd
program: path.join(debugFilesPath, pythonFile),
cwd: debugFilesPath,
stopOnEntry,
debugOptions: ['RedirectOutput'],
debugOptions: [DebugOptions.RedirectOutput],
pythonPath: 'python',
args: [],
envFile: '',
Expand Down
7 changes: 4 additions & 3 deletions src/test/unittests/common/debugLauncher.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import { IDebugService, IWorkspaceService } from '../../../client/common/applica
import { EXTENSION_ROOT_DIR } from '../../../client/common/constants';
import '../../../client/common/extensions';
import { IConfigurationService, IPythonSettings, IUnitTestSettings } from '../../../client/common/types';
import { DebugOptions } from '../../../client/debugger/Common/Contracts';
import { IServiceContainer } from '../../../client/ioc/types';
import { DebugLauncher } from '../../../client/unittests/common/debugLauncher';
import { TestProvider } from '../../../client/unittests/common/types';
Expand Down Expand Up @@ -47,7 +48,7 @@ suite('Unit Tests - Debug Launcher', () => {
});
function setupDebugManager(workspaceFolder: WorkspaceFolder, name: string, type: string,
request: string, program: string, cwd: string,
args: string[], console, debugOptions: string[],
args: string[], console, debugOptions: DebugOptions[],
testProvider: TestProvider, useExperimentalDebugger: boolean) {

const debugArgs = testProvider === 'unittest' && useExperimentalDebugger ? args.filter(item => item !== '--debug') : args;
Expand Down Expand Up @@ -96,7 +97,7 @@ suite('Unit Tests - Debug Launcher', () => {
const args = ['/one/two/three/testfile.py'];
const cwd = workspaceFolders[0].uri.fsPath;
const program = testLaunchScript;
setupDebugManager(workspaceFolders[0], 'Debug Unit Test', debuggerType, 'launch', program, cwd, args, 'none', ['RedirectOutput'], testProvider, useExperimentalDebugger);
setupDebugManager(workspaceFolders[0], 'Debug Unit Test', debuggerType, 'launch', program, cwd, args, 'none', [DebugOptions.RedirectOutput], testProvider, useExperimentalDebugger);

debugLauncher.launchDebugger({ cwd, args, testProvider }).ignoreErrors();
debugService.verifyAll();
Expand All @@ -111,7 +112,7 @@ suite('Unit Tests - Debug Launcher', () => {
const args = ['/one/two/three/testfile.py', '--debug', '1'];
const cwd = workspaceFolders[0].uri.fsPath;
const program = testLaunchScript;
setupDebugManager(workspaceFolders[0], 'Debug Unit Test', debuggerType, 'launch', program, cwd, args, 'none', ['RedirectOutput'], testProvider, useExperimentalDebugger);
setupDebugManager(workspaceFolders[0], 'Debug Unit Test', debuggerType, 'launch', program, cwd, args, 'none', [DebugOptions.RedirectOutput], testProvider, useExperimentalDebugger);

debugLauncher.launchDebugger({ cwd, args, testProvider }).ignoreErrors();
debugService.verifyAll();
Expand Down

0 comments on commit 3626de2

Please sign in to comment.