From 3626de2247dfd20804060a64f94f2151014f23eb Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Mon, 26 Mar 2018 11:40:51 -0700 Subject: [PATCH] Changes to how debug options are passed into the experimental version of PTVSD (debugger) (#1195) * :hammer: change how debug options are passed to new PTVSD * :memo: news entry * Fixes #1168 --- news/3 Code Health/1168.md | 1 + package.json | 4 +++ src/client/debugger/Common/Contracts.ts | 20 ++++++++----- .../debugger/DebugClients/LocalDebugClient.ts | 4 +-- .../debugger/configProviders/baseProvider.ts | 9 +++--- .../configProviders/pythonV2Provider.ts | 10 ++++--- src/client/unittests/common/debugLauncher.ts | 3 +- .../debugger/configProvider/provider.test.ts | 29 +++++-------------- src/test/debugger/misc.test.ts | 4 +-- src/test/debugger/portAndHost.test.ts | 4 +-- .../unittests/common/debugLauncher.test.ts | 7 +++-- 11 files changed, 47 insertions(+), 48 deletions(-) create mode 100644 news/3 Code Health/1168.md diff --git a/news/3 Code Health/1168.md b/news/3 Code Health/1168.md new file mode 100644 index 000000000000..191f91bdd741 --- /dev/null +++ b/news/3 Code Health/1168.md @@ -0,0 +1 @@ +Changes to how debug options are passed into the experimental version of PTVSD (debugger). diff --git a/package.json b/package.json index b996bfd75f93..62ca582bcf96 100644 --- a/package.json +++ b/package.json @@ -901,6 +901,10 @@ "items": { "type": "string", "enum": [ + "RedirectOutput", + "DebugStdLib", + "DjangoDebugging", + "FlaskDebugging", "Sudo", "Pyramid" ] diff --git a/src/client/debugger/Common/Contracts.ts b/src/client/debugger/Common/Contracts.ts index e8cce80fe552..0d82722dc6ca 100644 --- a/src/client/debugger/Common/Contracts.ts +++ b/src/client/debugger/Common/Contracts.ts @@ -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 { @@ -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; diff --git a/src/client/debugger/DebugClients/LocalDebugClient.ts b/src/client/debugger/DebugClients/LocalDebugClient.ts index d376f18a34aa..fd366af2412f 100644 --- a/src/client/debugger/DebugClients/LocalDebugClient.ts +++ b/src/client/debugger/DebugClients/LocalDebugClient.ts @@ -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'; @@ -163,7 +163,7 @@ export class LocalDebugClient extends DebugClient { } // 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)); diff --git a/src/client/debugger/configProviders/baseProvider.ts b/src/client/debugger/configProviders/baseProvider.ts index fef1a3c377da..f7786eca9b70 100644 --- a/src/client/debugger/configProviders/baseProvider.ts +++ b/src/client/debugger/configProviders/baseProvider.ts @@ -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 { @@ -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); const fs = this.serviceContainer.get(IFileSystem); const pserve = platformService.isWindows ? 'pserve.exe' : 'pserve'; diff --git a/src/client/debugger/configProviders/pythonV2Provider.ts b/src/client/debugger/configProviders/pythonV2Provider.ts index 78ad13af10a0..f9d124d95e0e 100644 --- a/src/client/debugger/configProviders/pythonV2Provider.ts +++ b/src/client/debugger/configProviders/pythonV2Provider.ts @@ -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 { @@ -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).isWindows; + if (this.serviceContainer.get(IPlatformService).isWindows) { + debugConfiguration.debugOptions = Array.isArray(debugConfiguration.debugOptions) ? debugConfiguration.debugOptions : []; + debugConfiguration.debugOptions.push(DebugOptions.FixFilePathCase); + } } } diff --git a/src/client/unittests/common/debugLauncher.ts b/src/client/unittests/common/debugLauncher.ts index d27dd8af96d7..12cc2f59cb83 100644 --- a/src/client/unittests/common/debugLauncher.ts +++ b/src/client/unittests/common/debugLauncher.ts @@ -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'; @@ -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[] { diff --git a/src/test/debugger/configProvider/provider.test.ts b/src/test/debugger/configProvider/provider.test.ts index 050631d18d01..ebe4a0ffbdfe 100644 --- a/src/test/debugger/configProvider/provider.test.ts +++ b/src/test/debugger/configProvider/provider.test.ts @@ -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'; [ @@ -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); @@ -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') { @@ -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); diff --git a/src/test/debugger/misc.test.ts b/src/test/debugger/misc.test.ts index aa26b79acba4..90fc5669447f 100644 --- a/src/test/debugger/misc.test.ts +++ b/src/test/debugger/misc.test.ts @@ -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'; @@ -78,7 +78,7 @@ let testCounter = 0; program: path.join(debugFilesPath, pythonFile), cwd: debugFilesPath, stopOnEntry, - debugOptions: ['RedirectOutput'], + debugOptions: [DebugOptions.RedirectOutput], pythonPath: 'python', args: [], env, diff --git a/src/test/debugger/portAndHost.test.ts b/src/test/debugger/portAndHost.test.ts index edfd2a2a158e..6d2d79491bbf 100644 --- a/src/test/debugger/portAndHost.test.ts +++ b/src/test/debugger/portAndHost.test.ts @@ -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'; @@ -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: '', diff --git a/src/test/unittests/common/debugLauncher.test.ts b/src/test/unittests/common/debugLauncher.test.ts index 25e76223049b..82a6299e4b83 100644 --- a/src/test/unittests/common/debugLauncher.test.ts +++ b/src/test/unittests/common/debugLauncher.test.ts @@ -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'; @@ -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; @@ -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(); @@ -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();