From e326d9aef01a48a11567a8266daba3cf2abd4c16 Mon Sep 17 00:00:00 2001 From: Paula Camargo Date: Mon, 14 Nov 2022 12:27:08 -0800 Subject: [PATCH 1/4] Remove DI from config launch --- .../launch.json/launchJsonReader.ts | 52 ++++++------- .../launch.json/updaterService.ts | 11 +-- .../launch.json/updaterServiceHelper.ts | 13 ++-- .../debugger/extension/configuration/types.ts | 8 +- .../extension/configuration/utils/common.ts | 10 ++- .../debugger/extension/debugCommands.ts | 9 +-- .../debugger/extension/serviceRegistry.ts | 4 +- src/client/testing/common/debugLauncher.ts | 18 ++--- .../launch.json/updaterServer.unit.test.ts | 23 +----- .../updaterServerHelper.unit.test.ts | 78 ++++++++++--------- .../extension/debugCommands.unit.test.ts | 11 --- .../extension/serviceRegistry.unit.test.ts | 4 +- .../testing/common/debugLauncher.unit.test.ts | 11 +-- 13 files changed, 98 insertions(+), 154 deletions(-) diff --git a/src/client/debugger/extension/configuration/launch.json/launchJsonReader.ts b/src/client/debugger/extension/configuration/launch.json/launchJsonReader.ts index d600a37665b1..f8e0bb2143e6 100644 --- a/src/client/debugger/extension/configuration/launch.json/launchJsonReader.ts +++ b/src/client/debugger/extension/configuration/launch.json/launchJsonReader.ts @@ -2,44 +2,36 @@ // Licensed under the MIT License. import * as path from 'path'; +import * as fs from 'fs-extra'; import { parse } from 'jsonc-parser'; -import { inject, injectable } from 'inversify'; import { DebugConfiguration, Uri, WorkspaceFolder } from 'vscode'; -import { IFileSystem } from '../../../../common/platform/types'; -import { ILaunchJsonReader } from '../types'; -import { IWorkspaceService } from '../../../../common/application/types'; +import { getWorkspaceFolder } from '../utils/workspaceFolder'; -@injectable() -export class LaunchJsonReader implements ILaunchJsonReader { - constructor( - @inject(IFileSystem) private readonly fs: IFileSystem, - @inject(IWorkspaceService) private readonly workspaceService: IWorkspaceService, - ) {} +export async function getConfigurationsForWorkspace(workspace: WorkspaceFolder): Promise { + const filename = path.join(workspace.uri.fsPath, '.vscode', 'launch.json'); - public async getConfigurationsForWorkspace(workspace: WorkspaceFolder): Promise { - const filename = path.join(workspace.uri.fsPath, '.vscode', 'launch.json'); - - if (!(await this.fs.fileExists(filename))) { - return []; - } + if (!(await fs.pathExists(filename))) { + return []; + } - const text = await this.fs.readFile(filename); - const parsed = parse(text, [], { allowTrailingComma: true, disallowComments: false }); - if (!parsed.configurations || !Array.isArray(parsed.configurations)) { - throw Error('Missing field in launch.json: configurations'); - } - if (!parsed.version) { - throw Error('Missing field in launch.json: version'); - } - // We do not bother ensuring each item is a DebugConfiguration... - return parsed.configurations; + const text = await fs.readFile(filename, 'utf-8'); + const parsed = parse(text, [], { allowTrailingComma: true, disallowComments: false }); + if (!parsed.configurations || !Array.isArray(parsed.configurations)) { + throw Error('Missing field in launch.json: configurations'); + } + if (!parsed.version) { + throw Error('Missing field in launch.json: version'); } + // We do not bother ensuring each item is a DebugConfiguration... + return parsed.configurations; +} - public async getConfigurationsByUri(uri: Uri): Promise { - const workspace = this.workspaceService.getWorkspaceFolder(uri); +export async function getConfigurationsByUri(uri?: Uri): Promise { + if (uri) { + const workspace = getWorkspaceFolder(uri); if (workspace) { - return this.getConfigurationsForWorkspace(workspace); + return getConfigurationsForWorkspace(workspace); } - return []; } + return []; } diff --git a/src/client/debugger/extension/configuration/launch.json/updaterService.ts b/src/client/debugger/extension/configuration/launch.json/updaterService.ts index e0b2ebd19dde..33cd171a7998 100644 --- a/src/client/debugger/extension/configuration/launch.json/updaterService.ts +++ b/src/client/debugger/extension/configuration/launch.json/updaterService.ts @@ -5,7 +5,7 @@ import { inject, injectable } from 'inversify'; import { IExtensionSingleActivationService } from '../../../../activation/types'; -import { ICommandManager, IDocumentManager, IWorkspaceService } from '../../../../common/application/types'; +import { ICommandManager } from '../../../../common/application/types'; import { IDisposableRegistry } from '../../../../common/types'; import { IDebugConfigurationService } from '../../types'; import { LaunchJsonUpdaterServiceHelper } from './updaterServiceHelper'; @@ -17,18 +17,11 @@ export class LaunchJsonUpdaterService implements IExtensionSingleActivationServi constructor( @inject(ICommandManager) private readonly commandManager: ICommandManager, @inject(IDisposableRegistry) private readonly disposableRegistry: IDisposableRegistry, - @inject(IWorkspaceService) private readonly workspace: IWorkspaceService, - @inject(IDocumentManager) private readonly documentManager: IDocumentManager, @inject(IDebugConfigurationService) private readonly configurationProvider: IDebugConfigurationService, ) {} public async activate(): Promise { - const handler = new LaunchJsonUpdaterServiceHelper( - this.commandManager, - this.workspace, - this.documentManager, - this.configurationProvider, - ); + const handler = new LaunchJsonUpdaterServiceHelper(this.commandManager, this.configurationProvider); this.disposableRegistry.push( this.commandManager.registerCommand( 'python.SelectAndInsertDebugConfiguration', diff --git a/src/client/debugger/extension/configuration/launch.json/updaterServiceHelper.ts b/src/client/debugger/extension/configuration/launch.json/updaterServiceHelper.ts index 2de042ca11c4..9c7a797f8306 100644 --- a/src/client/debugger/extension/configuration/launch.json/updaterServiceHelper.ts +++ b/src/client/debugger/extension/configuration/launch.json/updaterServiceHelper.ts @@ -5,11 +5,13 @@ import { createScanner, parse, SyntaxKind } from 'jsonc-parser'; import { CancellationToken, DebugConfiguration, Position, Range, TextDocument, WorkspaceEdit } from 'vscode'; -import { ICommandManager, IDocumentManager, IWorkspaceService } from '../../../../common/application/types'; +import { ICommandManager } from '../../../../common/application/types'; import { noop } from '../../../../common/utils/misc'; import { captureTelemetry } from '../../../../telemetry'; import { EventName } from '../../../../telemetry/constants'; import { IDebugConfigurationService } from '../../types'; +import { applyEdit, getActiveTextEditor } from '../utils/common'; +import { getWorkspaceFolder } from '../utils/workspaceFolder'; type PositionOfCursor = 'InsideEmptyArray' | 'BeforeItem' | 'AfterItem'; type PositionOfComma = 'BeforeCursor'; @@ -17,8 +19,6 @@ type PositionOfComma = 'BeforeCursor'; export class LaunchJsonUpdaterServiceHelper { constructor( private readonly commandManager: ICommandManager, - private readonly workspace: IWorkspaceService, - private readonly documentManager: IDocumentManager, private readonly configurationProvider: IDebugConfigurationService, ) {} @@ -28,8 +28,9 @@ export class LaunchJsonUpdaterServiceHelper { position: Position, token: CancellationToken, ): Promise { - if (this.documentManager.activeTextEditor && this.documentManager.activeTextEditor.document === document) { - const folder = this.workspace.getWorkspaceFolder(document.uri); + const activeTextEditor = getActiveTextEditor(); + if (activeTextEditor && activeTextEditor.document === document) { + const folder = getWorkspaceFolder(document.uri); const configs = await this.configurationProvider.provideDebugConfigurations!(folder, token); if (!token.isCancellationRequested && Array.isArray(configs) && configs.length > 0) { @@ -66,7 +67,7 @@ export class LaunchJsonUpdaterServiceHelper { const formattedJson = LaunchJsonUpdaterServiceHelper.getTextForInsertion(config, cursorPosition, commaPosition); const workspaceEdit = new WorkspaceEdit(); workspaceEdit.insert(document.uri, position, formattedJson); - await this.documentManager.applyEdit(workspaceEdit); + await applyEdit(workspaceEdit); this.commandManager.executeCommand('editor.action.formatDocument').then(noop, noop); } diff --git a/src/client/debugger/extension/configuration/types.ts b/src/client/debugger/extension/configuration/types.ts index c888fc89ddec..eaebf6d435c4 100644 --- a/src/client/debugger/extension/configuration/types.ts +++ b/src/client/debugger/extension/configuration/types.ts @@ -3,7 +3,7 @@ 'use strict'; -import { CancellationToken, DebugConfiguration, Uri, WorkspaceFolder } from 'vscode'; +import { CancellationToken, DebugConfiguration, WorkspaceFolder } from 'vscode'; export const IDebugConfigurationResolver = Symbol('IDebugConfigurationResolver'); export interface IDebugConfigurationResolver { @@ -19,9 +19,3 @@ export interface IDebugConfigurationResolver { token?: CancellationToken, ): Promise; } - -export const ILaunchJsonReader = Symbol('ILaunchJsonReader'); -export interface ILaunchJsonReader { - getConfigurationsForWorkspace(workspace: WorkspaceFolder): Promise; - getConfigurationsByUri(uri?: Uri): Promise; -} diff --git a/src/client/debugger/extension/configuration/utils/common.ts b/src/client/debugger/extension/configuration/utils/common.ts index 36572419908f..6ded7a84aa5d 100644 --- a/src/client/debugger/extension/configuration/utils/common.ts +++ b/src/client/debugger/extension/configuration/utils/common.ts @@ -6,7 +6,7 @@ 'use strict'; -import { WorkspaceFolder, window, TextEditor } from 'vscode'; +import { WorkspaceFolder, window, TextEditor, WorkspaceEdit, workspace } from 'vscode'; import { getWorkspaceFolder } from './workspaceFolder'; /** @@ -26,9 +26,9 @@ export function resolveVariables( folder: WorkspaceFolder | undefined, ): string | undefined { if (value) { - const workspace = folder ? getWorkspaceFolder(folder.uri) : undefined; + const workspaceFolder = folder ? getWorkspaceFolder(folder.uri) : undefined; const variablesObject: { [key: string]: any } = {}; - variablesObject.workspaceFolder = workspace ? workspace.uri.fsPath : rootFolder; + variablesObject.workspaceFolder = workspaceFolder ? workspaceFolder.uri.fsPath : rootFolder; const regexp = /\$\{(.*?)\}/g; return value.replace(regexp, (match: string, name: string) => { @@ -46,3 +46,7 @@ export function getActiveTextEditor(): TextEditor | undefined { const { activeTextEditor } = window; return activeTextEditor; } + +export function applyEdit(edit: WorkspaceEdit): Thenable { + return workspace.applyEdit(edit); +} diff --git a/src/client/debugger/extension/debugCommands.ts b/src/client/debugger/extension/debugCommands.ts index 2980bde046f8..14a108d27793 100644 --- a/src/client/debugger/extension/debugCommands.ts +++ b/src/client/debugger/extension/debugCommands.ts @@ -10,10 +10,10 @@ import { Commands } from '../../common/constants'; import { IDisposableRegistry } from '../../common/types'; import { sendTelemetryEvent } from '../../telemetry'; import { EventName } from '../../telemetry/constants'; -import { ILaunchJsonReader } from './configuration/types'; import { DebugPurpose, LaunchRequestArguments } from '../types'; import { IInterpreterService } from '../../interpreter/contracts'; import { noop } from '../../common/utils/misc'; +import { getConfigurationsByUri } from './configuration/launch.json/launchJsonReader'; @injectable() export class DebugCommands implements IExtensionSingleActivationService { @@ -22,7 +22,6 @@ export class DebugCommands implements IExtensionSingleActivationService { constructor( @inject(ICommandManager) private readonly commandManager: ICommandManager, @inject(IDebugService) private readonly debugService: IDebugService, - @inject(ILaunchJsonReader) private readonly launchJsonReader: ILaunchJsonReader, @inject(IDisposableRegistry) private readonly disposables: IDisposableRegistry, @inject(IInterpreterService) private readonly interpreterService: IInterpreterService, ) {} @@ -36,15 +35,15 @@ export class DebugCommands implements IExtensionSingleActivationService { this.commandManager.executeCommand(Commands.TriggerEnvironmentSelection, file).then(noop, noop); return; } - const config = await this.getDebugConfiguration(file); + const config = await DebugCommands.getDebugConfiguration(file); this.debugService.startDebugging(undefined, config); }), ); return Promise.resolve(); } - private async getDebugConfiguration(uri?: Uri): Promise { - const configs = (await this.launchJsonReader.getConfigurationsByUri(uri)).filter((c) => c.request === 'launch'); + private static async getDebugConfiguration(uri?: Uri): Promise { + const configs = (await getConfigurationsByUri(uri)).filter((c) => c.request === 'launch'); for (const config of configs) { if ((config as LaunchRequestArguments).purpose?.includes(DebugPurpose.DebugInTerminal)) { if (!config.program && !config.module && !config.code) { diff --git a/src/client/debugger/extension/serviceRegistry.ts b/src/client/debugger/extension/serviceRegistry.ts index cbce444962ef..a8c5ae7bbfcc 100644 --- a/src/client/debugger/extension/serviceRegistry.ts +++ b/src/client/debugger/extension/serviceRegistry.ts @@ -16,12 +16,11 @@ import { PythonDebugConfigurationService } from './configuration/debugConfigurat import { DynamicPythonDebugConfigurationService } from './configuration/dynamicdebugConfigurationService'; import { LaunchJsonCompletionProvider } from './configuration/launch.json/completionProvider'; import { InterpreterPathCommand } from './configuration/launch.json/interpreterPathCommand'; -import { LaunchJsonReader } from './configuration/launch.json/launchJsonReader'; import { LaunchJsonUpdaterService } from './configuration/launch.json/updaterService'; import { AttachConfigurationResolver } from './configuration/resolvers/attach'; import { DebugEnvironmentVariablesHelper, IDebugEnvironmentVariablesService } from './configuration/resolvers/helper'; import { LaunchConfigurationResolver } from './configuration/resolvers/launch'; -import { IDebugConfigurationResolver, ILaunchJsonReader } from './configuration/types'; +import { IDebugConfigurationResolver } from './configuration/types'; import { DebugCommands } from './debugCommands'; import { ChildProcessAttachEventHandler } from './hooks/childProcessAttachHandler'; import { ChildProcessAttachService } from './hooks/childProcessAttachService'; @@ -89,5 +88,4 @@ export function registerTypes(serviceManager: IServiceManager): void { AttachProcessProviderFactory, ); serviceManager.addSingleton(IExtensionSingleActivationService, DebugCommands); - serviceManager.addSingleton(ILaunchJsonReader, LaunchJsonReader); } diff --git a/src/client/testing/common/debugLauncher.ts b/src/client/testing/common/debugLauncher.ts index 5c8bfd537f79..b166a7935962 100644 --- a/src/client/testing/common/debugLauncher.ts +++ b/src/client/testing/common/debugLauncher.ts @@ -1,35 +1,35 @@ import { inject, injectable, named } from 'inversify'; import * as path from 'path'; +import * as vscode from 'vscode'; import { DebugConfiguration, Uri, WorkspaceFolder } from 'vscode'; -import { IApplicationShell, IDebugService, IWorkspaceService } from '../../common/application/types'; +import { IApplicationShell, IDebugService } from '../../common/application/types'; import { EXTENSION_ROOT_DIR } from '../../common/constants'; import * as internalScripts from '../../common/process/internal/scripts'; import { IConfigurationService, IPythonSettings } from '../../common/types'; import { DebuggerTypeName } from '../../debugger/constants'; -import { IDebugConfigurationResolver, ILaunchJsonReader } from '../../debugger/extension/configuration/types'; +import { IDebugConfigurationResolver } from '../../debugger/extension/configuration/types'; import { DebugPurpose, LaunchRequestArguments } from '../../debugger/types'; import { IServiceContainer } from '../../ioc/types'; import { traceError } from '../../logging'; import { TestProvider } from '../types'; import { ITestDebugLauncher, LaunchOptions } from './types'; import * as nls from 'vscode-nls'; +import { getConfigurationsForWorkspace } from '../../debugger/extension/configuration/launch.json/launchJsonReader'; +import { getWorkspaceFolder } from '../../common/vscodeApis/workspaceApis'; const localize: nls.LocalizeFunc = nls.loadMessageBundle(); @injectable() export class DebugLauncher implements ITestDebugLauncher { private readonly configService: IConfigurationService; - private readonly workspaceService: IWorkspaceService; constructor( @inject(IServiceContainer) private serviceContainer: IServiceContainer, @inject(IDebugConfigurationResolver) @named('launch') private readonly launchResolver: IDebugConfigurationResolver, - @inject(ILaunchJsonReader) private readonly launchJsonReader: ILaunchJsonReader, ) { this.configService = this.serviceContainer.get(IConfigurationService); - this.workspaceService = this.serviceContainer.get(IWorkspaceService); } public async launchDebugger(options: LaunchOptions) { @@ -58,7 +58,7 @@ export class DebugLauncher implements ITestDebugLauncher { } public async readAllDebugConfigs(workspace: WorkspaceFolder): Promise { try { - const configs = await this.launchJsonReader.getConfigurationsForWorkspace(workspace); + const configs = await getConfigurationsForWorkspace(workspace); return configs; } catch (exc) { traceError('could not get debug config', exc); @@ -70,15 +70,15 @@ export class DebugLauncher implements ITestDebugLauncher { } } private resolveWorkspaceFolder(cwd: string): WorkspaceFolder { - const hasWorkspaceFolders = (this.workspaceService.workspaceFolders?.length || 0) > 0; + const hasWorkspaceFolders = (vscode.workspace.workspaceFolders?.length || 0) > 0; if (!hasWorkspaceFolders) { throw new Error('Please open a workspace'); } const cwdUri = cwd ? Uri.file(cwd) : undefined; - let workspaceFolder = this.workspaceService.getWorkspaceFolder(cwdUri); + let workspaceFolder = getWorkspaceFolder(cwdUri); if (!workspaceFolder) { - workspaceFolder = this.workspaceService.workspaceFolders![0]; + workspaceFolder = vscode.workspace.workspaceFolders![0]; } return workspaceFolder; } diff --git a/src/test/debugger/extension/configuration/launch.json/updaterServer.unit.test.ts b/src/test/debugger/extension/configuration/launch.json/updaterServer.unit.test.ts index 8a54697eb7d6..e61656783dcd 100644 --- a/src/test/debugger/extension/configuration/launch.json/updaterServer.unit.test.ts +++ b/src/test/debugger/extension/configuration/launch.json/updaterServer.unit.test.ts @@ -5,9 +5,7 @@ import { instance, mock, verify } from 'ts-mockito'; import { CommandManager } from '../../../../../client/common/application/commandManager'; -import { DocumentManager } from '../../../../../client/common/application/documentManager'; -import { ICommandManager, IDocumentManager, IWorkspaceService } from '../../../../../client/common/application/types'; -import { WorkspaceService } from '../../../../../client/common/application/workspace'; +import { ICommandManager } from '../../../../../client/common/application/types'; import { PythonDebugConfigurationService } from '../../../../../client/debugger/extension/configuration/debugConfigurationService'; import { LaunchJsonUpdaterService } from '../../../../../client/debugger/extension/configuration/launch.json/updaterService'; import { LaunchJsonUpdaterServiceHelper } from '../../../../../client/debugger/extension/configuration/launch.json/updaterServiceHelper'; @@ -16,29 +14,14 @@ import { IDebugConfigurationService } from '../../../../../client/debugger/exten suite('Debugging - launch.json Updater Service', () => { let helper: LaunchJsonUpdaterServiceHelper; let commandManager: ICommandManager; - let workspace: IWorkspaceService; - let documentManager: IDocumentManager; let debugConfigService: IDebugConfigurationService; setup(() => { commandManager = mock(CommandManager); - workspace = mock(WorkspaceService); - documentManager = mock(DocumentManager); debugConfigService = mock(PythonDebugConfigurationService); - helper = new LaunchJsonUpdaterServiceHelper( - instance(commandManager), - instance(workspace), - instance(documentManager), - instance(debugConfigService), - ); + helper = new LaunchJsonUpdaterServiceHelper(instance(commandManager), instance(debugConfigService)); }); test('Activation will register the required commands', async () => { - const service = new LaunchJsonUpdaterService( - instance(commandManager), - [], - instance(workspace), - instance(documentManager), - instance(debugConfigService), - ); + const service = new LaunchJsonUpdaterService(instance(commandManager), [], instance(debugConfigService)); await service.activate(); verify( commandManager.registerCommand( diff --git a/src/test/debugger/extension/configuration/launch.json/updaterServerHelper.unit.test.ts b/src/test/debugger/extension/configuration/launch.json/updaterServerHelper.unit.test.ts index ec64c2bbf2dc..a3fcd9cd7feb 100644 --- a/src/test/debugger/extension/configuration/launch.json/updaterServerHelper.unit.test.ts +++ b/src/test/debugger/extension/configuration/launch.json/updaterServerHelper.unit.test.ts @@ -18,9 +18,9 @@ import { Uri, } from 'vscode'; import { CommandManager } from '../../../../../client/common/application/commandManager'; -import { DocumentManager } from '../../../../../client/common/application/documentManager'; -import { ICommandManager, IDocumentManager, IWorkspaceService } from '../../../../../client/common/application/types'; -import { WorkspaceService } from '../../../../../client/common/application/workspace'; +import * as common from '../../../../../client/debugger/extension/configuration/utils/common'; +import { ICommandManager } from '../../../../../client/common/application/types'; +import * as workspaceFolder from '../../../../../client/debugger/extension/configuration/utils/workspaceFolder'; import { PythonDebugConfigurationService } from '../../../../../client/debugger/extension/configuration/debugConfigurationService'; import { LaunchJsonUpdaterServiceHelper } from '../../../../../client/debugger/extension/configuration/launch.json/updaterServiceHelper'; import { IDebugConfigurationService } from '../../../../../client/debugger/extension/types'; @@ -33,24 +33,25 @@ type LaunchJsonSchema = { suite('Debugging - launch.json Updater Service', () => { let helper: LaunchJsonUpdaterServiceHelper; let commandManager: ICommandManager; - let workspace: IWorkspaceService; - let documentManager: IDocumentManager; + let getWorkspaceFolderStub: sinon.SinonStub; + let getActiveTextEditorStub: sinon.SinonStub; + let applyEditStub: sinon.SinonStub; let debugConfigService: IDebugConfigurationService; + const sandbox = sinon.createSandbox(); setup(() => { commandManager = mock(CommandManager); - workspace = mock(WorkspaceService); - documentManager = mock(DocumentManager); + getWorkspaceFolderStub = sinon.stub(workspaceFolder, 'getWorkspaceFolder'); + getActiveTextEditorStub = sinon.stub(common, 'getActiveTextEditor'); + applyEditStub = sinon.stub(common, 'applyEdit'); debugConfigService = mock(PythonDebugConfigurationService); sandbox.stub(LaunchJsonUpdaterServiceHelper, 'isCommaImmediatelyBeforeCursor').returns(false); - helper = new LaunchJsonUpdaterServiceHelper( - instance(commandManager), - instance(workspace), - instance(documentManager), - instance(debugConfigService), - ); + helper = new LaunchJsonUpdaterServiceHelper(instance(commandManager), instance(debugConfigService)); + }); + teardown(() => { + sandbox.restore(); + sinon.restore(); }); - teardown(() => sandbox.restore()); test('Configuration Array is detected as being empty', async () => { const document = typemoq.Mock.ofType(); @@ -244,19 +245,19 @@ suite('Debugging - launch.json Updater Service', () => { const document = typemoq.Mock.ofType(); document.setup((doc) => doc.getText(typemoq.It.isAny())).returns(() => json); document.setup((doc) => doc.offsetAt(typemoq.It.isAny())).returns(() => json.indexOf('},') + 1); - when(documentManager.applyEdit(anything())).thenResolve(); + applyEditStub.returns(undefined); when(commandManager.executeCommand('editor.action.formatDocument')).thenResolve(); await helper.insertDebugConfiguration(document.object, new Position(0, 0), config); - verify(documentManager.applyEdit(anything())).once(); + assert(applyEditStub.calledOnce); verify(commandManager.executeCommand('editor.action.formatDocument')).once(); }); test('No changes to configuration if there is not active document', async () => { const document = typemoq.Mock.ofType(); const position = new Position(0, 0); const { token } = new CancellationTokenSource(); - when(documentManager.activeTextEditor).thenReturn(); + getActiveTextEditorStub.returns(undefined); let debugConfigInserted = false; helper.insertDebugConfiguration = async () => { debugConfigInserted = true; @@ -264,8 +265,8 @@ suite('Debugging - launch.json Updater Service', () => { await helper.selectAndInsertDebugConfig(document.object, position, token); - verify(documentManager.activeTextEditor).atLeast(1); - verify(workspace.getWorkspaceFolder(anything())).never(); + assert(getActiveTextEditorStub.calledOnce); + assert(getWorkspaceFolderStub.neverCalledWith(anything)); assert.strictEqual(debugConfigInserted, false); }); test('No changes to configuration if the active document is not same as the document passed in', async () => { @@ -277,7 +278,7 @@ suite('Debugging - launch.json Updater Service', () => { .setup((t) => t.document) .returns(() => ('x' as unknown) as TextDocument) .verifiable(typemoq.Times.atLeastOnce()); - when(documentManager.activeTextEditor).thenReturn(textEditor.object); + getActiveTextEditorStub.returns(textEditor.object); let debugConfigInserted = false; helper.insertDebugConfiguration = async () => { debugConfigInserted = true; @@ -285,9 +286,8 @@ suite('Debugging - launch.json Updater Service', () => { await helper.selectAndInsertDebugConfig(document.object, position, token); - verify(documentManager.activeTextEditor).atLeast(1); - verify(documentManager.activeTextEditor).atLeast(1); - verify(workspace.getWorkspaceFolder(anything())).never(); + assert(getActiveTextEditorStub.called); + getWorkspaceFolderStub.neverCalledWith(anything); textEditor.verifyAll(); assert.strictEqual(debugConfigInserted, false); }); @@ -309,8 +309,8 @@ suite('Debugging - launch.json Updater Service', () => { .setup((t) => t.document) .returns(() => document.object) .verifiable(typemoq.Times.atLeastOnce()); - when(documentManager.activeTextEditor).thenReturn(textEditor.object); - when(workspace.getWorkspaceFolder(docUri)).thenReturn(folder); + getActiveTextEditorStub.returns(textEditor.object); + getWorkspaceFolderStub.returns(folder); when(debugConfigService.provideDebugConfigurations!(folder, token)).thenResolve(([''] as unknown) as void); let debugConfigInserted = false; helper.insertDebugConfiguration = async () => { @@ -319,9 +319,9 @@ suite('Debugging - launch.json Updater Service', () => { await helper.selectAndInsertDebugConfig(document.object, position, token); - verify(documentManager.activeTextEditor).atLeast(1); - verify(documentManager.activeTextEditor).atLeast(1); - verify(workspace.getWorkspaceFolder(docUri)).atLeast(1); + assert(getActiveTextEditorStub.called); + assert(getWorkspaceFolderStub.called); + textEditor.verifyAll(); document.verifyAll(); assert.strictEqual(debugConfigInserted, false); @@ -343,8 +343,10 @@ suite('Debugging - launch.json Updater Service', () => { .setup((t) => t.document) .returns(() => document.object) .verifiable(typemoq.Times.atLeastOnce()); - when(documentManager.activeTextEditor).thenReturn(textEditor.object); - when(workspace.getWorkspaceFolder(docUri)).thenReturn(folder); + + getActiveTextEditorStub.returns(textEditor.object); + getWorkspaceFolderStub.returns(folder); + when(debugConfigService.provideDebugConfigurations!(folder, token)).thenResolve(([] as unknown) as void); let debugConfigInserted = false; helper.insertDebugConfiguration = async () => { @@ -353,9 +355,9 @@ suite('Debugging - launch.json Updater Service', () => { await helper.selectAndInsertDebugConfig(document.object, position, token); - verify(documentManager.activeTextEditor).atLeast(1); - verify(documentManager.activeTextEditor).atLeast(1); - verify(workspace.getWorkspaceFolder(docUri)).atLeast(1); + assert(getActiveTextEditorStub.called); + assert(getWorkspaceFolderStub.withArgs(docUri).called); + textEditor.verifyAll(); document.verifyAll(); assert.strictEqual(debugConfigInserted, false); @@ -377,8 +379,8 @@ suite('Debugging - launch.json Updater Service', () => { .setup((t) => t.document) .returns(() => document.object) .verifiable(typemoq.Times.atLeastOnce()); - when(documentManager.activeTextEditor).thenReturn(textEditor.object); - when(workspace.getWorkspaceFolder(docUri)).thenReturn(folder); + getActiveTextEditorStub.returns(textEditor.object); + getWorkspaceFolderStub.withArgs(docUri).returns(folder); when(debugConfigService.provideDebugConfigurations!(folder, token)).thenResolve(([ 'config', ] as unknown) as void); @@ -389,9 +391,9 @@ suite('Debugging - launch.json Updater Service', () => { await helper.selectAndInsertDebugConfig(document.object, position, token); - verify(documentManager.activeTextEditor).atLeast(1); - verify(documentManager.activeTextEditor).atLeast(1); - verify(workspace.getWorkspaceFolder(docUri)).atLeast(1); + assert(getActiveTextEditorStub.called); + assert(getActiveTextEditorStub.called); + assert(getWorkspaceFolderStub.calledOnceWithExactly(docUri)); textEditor.verifyAll(); document.verifyAll(); assert.strictEqual(debugConfigInserted, true); diff --git a/src/test/debugger/extension/debugCommands.unit.test.ts b/src/test/debugger/extension/debugCommands.unit.test.ts index 8432f9356561..3c023f3f1450 100644 --- a/src/test/debugger/extension/debugCommands.unit.test.ts +++ b/src/test/debugger/extension/debugCommands.unit.test.ts @@ -12,7 +12,6 @@ import { IDisposableRegistry } from '../../../client/common/types'; import { DebugCommands } from '../../../client/debugger/extension/debugCommands'; import { EXTENSION_ROOT_DIR_FOR_TESTS } from '../../constants'; import * as telemetry from '../../../client/telemetry'; -import { ILaunchJsonReader } from '../../../client/debugger/extension/configuration/types'; import { IInterpreterService } from '../../../client/interpreter/contracts'; import { PythonEnvironment } from '../../../client/pythonEnvironments/info'; @@ -20,7 +19,6 @@ suite('Debugging - commands', () => { let commandManager: typemoq.IMock; let debugService: typemoq.IMock; let disposables: typemoq.IMock; - let launchJsonReader: typemoq.IMock; let interpreterService: typemoq.IMock; let debugCommands: IExtensionSingleActivationService; @@ -30,12 +28,6 @@ suite('Debugging - commands', () => { .setup((c) => c.executeCommand(typemoq.It.isAny(), typemoq.It.isAny())) .returns(() => Promise.resolve()); debugService = typemoq.Mock.ofType(); - launchJsonReader = typemoq.Mock.ofType(); - launchJsonReader - .setup((l) => l.getConfigurationsByUri(typemoq.It.isAny())) - .returns(() => Promise.resolve([])) - .verifiable(typemoq.Times.once()); - disposables = typemoq.Mock.ofType(); interpreterService = typemoq.Mock.ofType(); interpreterService @@ -61,7 +53,6 @@ suite('Debugging - commands', () => { debugCommands = new DebugCommands( commandManager.object, debugService.object, - launchJsonReader.object, disposables.object, interpreterService.object, ); @@ -83,7 +74,6 @@ suite('Debugging - commands', () => { debugCommands = new DebugCommands( commandManager.object, debugService.object, - launchJsonReader.object, disposables.object, interpreterService.object, ); @@ -92,6 +82,5 @@ suite('Debugging - commands', () => { await callback(Uri.file(path.join(EXTENSION_ROOT_DIR_FOR_TESTS, 'test.py'))); commandManager.verifyAll(); debugService.verifyAll(); - launchJsonReader.verifyAll(); }); }); diff --git a/src/test/debugger/extension/serviceRegistry.unit.test.ts b/src/test/debugger/extension/serviceRegistry.unit.test.ts index 73390b28c627..43d81bbe1385 100644 --- a/src/test/debugger/extension/serviceRegistry.unit.test.ts +++ b/src/test/debugger/extension/serviceRegistry.unit.test.ts @@ -14,11 +14,10 @@ import { IAttachProcessProviderFactory } from '../../../client/debugger/extensio import { PythonDebugConfigurationService } from '../../../client/debugger/extension/configuration/debugConfigurationService'; import { LaunchJsonCompletionProvider } from '../../../client/debugger/extension/configuration/launch.json/completionProvider'; import { InterpreterPathCommand } from '../../../client/debugger/extension/configuration/launch.json/interpreterPathCommand'; -import { LaunchJsonReader } from '../../../client/debugger/extension/configuration/launch.json/launchJsonReader'; import { LaunchJsonUpdaterService } from '../../../client/debugger/extension/configuration/launch.json/updaterService'; import { AttachConfigurationResolver } from '../../../client/debugger/extension/configuration/resolvers/attach'; import { LaunchConfigurationResolver } from '../../../client/debugger/extension/configuration/resolvers/launch'; -import { IDebugConfigurationResolver, ILaunchJsonReader } from '../../../client/debugger/extension/configuration/types'; +import { IDebugConfigurationResolver } from '../../../client/debugger/extension/configuration/types'; import { DebugCommands } from '../../../client/debugger/extension/debugCommands'; import { ChildProcessAttachEventHandler } from '../../../client/debugger/extension/hooks/childProcessAttachHandler'; import { ChildProcessAttachService } from '../../../client/debugger/extension/hooks/childProcessAttachService'; @@ -141,6 +140,5 @@ suite('Debugging - Service Registry', () => { DebugCommands, ), ).once(); - verify(serviceManager.addSingleton(ILaunchJsonReader, LaunchJsonReader)).once(); }); }); diff --git a/src/test/testing/common/debugLauncher.unit.test.ts b/src/test/testing/common/debugLauncher.unit.test.ts index 44ff5478922b..9a6778aaf75d 100644 --- a/src/test/testing/common/debugLauncher.unit.test.ts +++ b/src/test/testing/common/debugLauncher.unit.test.ts @@ -15,10 +15,8 @@ import '../../../client/common/extensions'; import { IFileSystem, IPlatformService } from '../../../client/common/platform/types'; import { IConfigurationService, IPythonSettings } from '../../../client/common/types'; import { DebuggerTypeName } from '../../../client/debugger/constants'; -import { LaunchJsonReader } from '../../../client/debugger/extension/configuration/launch.json/launchJsonReader'; import { IDebugEnvironmentVariablesService } from '../../../client/debugger/extension/configuration/resolvers/helper'; import { LaunchConfigurationResolver } from '../../../client/debugger/extension/configuration/resolvers/launch'; -import { ILaunchJsonReader } from '../../../client/debugger/extension/configuration/types'; import { DebugOptions } from '../../../client/debugger/types'; import { IInterpreterService } from '../../../client/interpreter/contracts'; import { IServiceContainer } from '../../../client/ioc/types'; @@ -41,7 +39,6 @@ suite('Unit Tests - Debug Launcher', () => { let filesystem: TypeMoq.IMock; let settings: TypeMoq.IMock; let debugEnvHelper: TypeMoq.IMock; - let launchJsonReader: ILaunchJsonReader; let interpreterService: TypeMoq.IMock; setup(async () => { @@ -83,13 +80,7 @@ suite('Unit Tests - Debug Launcher', () => { .setup((c) => c.get(TypeMoq.It.isValue(IDebugEnvironmentVariablesService))) .returns(() => debugEnvHelper.object); - launchJsonReader = new LaunchJsonReader(filesystem.object, workspaceService.object); - - debugLauncher = new DebugLauncher( - serviceContainer.object, - getNewResolver(configService.object), - launchJsonReader, - ); + debugLauncher = new DebugLauncher(serviceContainer.object, getNewResolver(configService.object)); }); function getNewResolver(configService: IConfigurationService) { const validator = TypeMoq.Mock.ofType( From f5ec3cb8a6ba60f563569f46798e01e220af9a04 Mon Sep 17 00:00:00 2001 From: Paula Camargo Date: Mon, 14 Nov 2022 14:45:40 -0800 Subject: [PATCH 2/4] Remove DI from childProcess --- .../dynamicdebugConfigurationService.ts | 46 ++++++------ .../hooks/childProcessAttachService.ts | 23 +++--- .../childProcessAttachService.unit.test.ts | 70 +++++++++---------- 3 files changed, 69 insertions(+), 70 deletions(-) diff --git a/src/client/debugger/extension/configuration/dynamicdebugConfigurationService.ts b/src/client/debugger/extension/configuration/dynamicdebugConfigurationService.ts index 4fb2e7e0c225..d13cf48c2a6d 100644 --- a/src/client/debugger/extension/configuration/dynamicdebugConfigurationService.ts +++ b/src/client/debugger/extension/configuration/dynamicdebugConfigurationService.ts @@ -4,11 +4,10 @@ 'use strict'; import * as path from 'path'; -import { inject, injectable } from 'inversify'; +import * as fs from 'fs-extra'; +import { injectable } from 'inversify'; import { CancellationToken, DebugConfiguration, WorkspaceFolder } from 'vscode'; import { IDynamicDebugConfigurationService } from '../types'; -import { IFileSystem } from '../../../common/platform/types'; -import { IPathUtils } from '../../../common/types'; import { DebuggerTypeName } from '../../constants'; import { asyncFilter } from '../../../common/utils/arrayUtils'; @@ -16,8 +15,7 @@ const workspaceFolderToken = '${workspaceFolder}'; @injectable() export class DynamicPythonDebugConfigurationService implements IDynamicDebugConfigurationService { - constructor(@inject(IFileSystem) private fs: IFileSystem, @inject(IPathUtils) private pathUtils: IPathUtils) {} - + // eslint-disable-next-line class-methods-use-this public async provideDebugConfigurations( folder: WorkspaceFolder, _token?: CancellationToken, @@ -32,20 +30,20 @@ export class DynamicPythonDebugConfigurationService implements IDynamicDebugConf justMyCode: true, }); - const djangoManagePath = await this.getDjangoPath(folder); + const djangoManagePath = await DynamicPythonDebugConfigurationService.getDjangoPath(folder); if (djangoManagePath) { providers.push({ name: 'Python: Django', type: DebuggerTypeName, request: 'launch', - program: `${workspaceFolderToken}${this.pathUtils.separator}${djangoManagePath}`, + program: `${workspaceFolderToken}${path.sep}${djangoManagePath}`, args: ['runserver'], django: true, justMyCode: true, }); } - const flaskPath = await this.getFlaskPath(folder); + const flaskPath = await DynamicPythonDebugConfigurationService.getFlaskPath(folder); if (flaskPath) { providers.push({ name: 'Python: Flask', @@ -62,12 +60,9 @@ export class DynamicPythonDebugConfigurationService implements IDynamicDebugConf }); } - let fastApiPath = await this.getFastApiPath(folder); + let fastApiPath = await DynamicPythonDebugConfigurationService.getFastApiPath(folder); if (fastApiPath) { - fastApiPath = path - .relative(folder.uri.fsPath, fastApiPath) - .replaceAll(this.pathUtils.separator, '.') - .replace('.py', ''); + fastApiPath = path.relative(folder.uri.fsPath, fastApiPath).replaceAll(path.sep, '.').replace('.py', ''); providers.push({ name: 'Python: FastAPI', type: DebuggerTypeName, @@ -82,9 +77,9 @@ export class DynamicPythonDebugConfigurationService implements IDynamicDebugConf return providers; } - private async getDjangoPath(folder: WorkspaceFolder) { + private static async getDjangoPath(folder: WorkspaceFolder) { const regExpression = /execute_from_command_line\(/; - const possiblePaths = await this.getPossiblePaths( + const possiblePaths = await DynamicPythonDebugConfigurationService.getPossiblePaths( folder, ['manage.py', '*/manage.py', 'app.py', '*/app.py'], regExpression, @@ -92,9 +87,9 @@ export class DynamicPythonDebugConfigurationService implements IDynamicDebugConf return possiblePaths.length ? path.relative(folder.uri.fsPath, possiblePaths[0]) : null; } - private async getFastApiPath(folder: WorkspaceFolder) { + private static async getFastApiPath(folder: WorkspaceFolder) { const regExpression = /app\s*=\s*FastAPI\(/; - const fastApiPaths = await this.getPossiblePaths( + const fastApiPaths = await DynamicPythonDebugConfigurationService.getPossiblePaths( folder, ['main.py', 'app.py', '*/main.py', '*/app.py', '*/*/main.py', '*/*/app.py'], regExpression, @@ -103,9 +98,9 @@ export class DynamicPythonDebugConfigurationService implements IDynamicDebugConf return fastApiPaths.length ? fastApiPaths[0] : null; } - private async getFlaskPath(folder: WorkspaceFolder) { + private static async getFlaskPath(folder: WorkspaceFolder) { const regExpression = /app(?:lication)?\s*=\s*(?:flask\.)?Flask\(|def\s+(?:create|make)_app\(/; - const flaskPaths = await this.getPossiblePaths( + const flaskPaths = await DynamicPythonDebugConfigurationService.getPossiblePaths( folder, ['__init__.py', 'app.py', 'wsgi.py', '*/__init__.py', '*/app.py', '*/wsgi.py'], regExpression, @@ -114,16 +109,23 @@ export class DynamicPythonDebugConfigurationService implements IDynamicDebugConf return flaskPaths.length ? flaskPaths[0] : null; } - private async getPossiblePaths(folder: WorkspaceFolder, globPatterns: string[], regex: RegExp): Promise { + private static async getPossiblePaths( + folder: WorkspaceFolder, + globPatterns: string[], + regex: RegExp, + ): Promise { const foundPathsPromises = (await Promise.allSettled( globPatterns.map( - async (pattern): Promise => this.fs.search(path.join(folder.uri.fsPath, pattern)), + async (pattern): Promise => + (await fs.pathExists(path.join(folder.uri.fsPath, pattern))) + ? [path.join(folder.uri.fsPath, pattern)] + : [], ), )) as { status: string; value: [] }[]; const possiblePaths: string[] = []; foundPathsPromises.forEach((result) => possiblePaths.push(...result.value)); const finalPaths = await asyncFilter(possiblePaths, async (possiblePath) => - regex.exec((await this.fs.readFile(possiblePath)).toString()), + regex.exec((await fs.readFile(possiblePath)).toString()), ); return finalPaths; diff --git a/src/client/debugger/extension/hooks/childProcessAttachService.ts b/src/client/debugger/extension/hooks/childProcessAttachService.ts index 4493c3b7145e..07077fda8e0d 100644 --- a/src/client/debugger/extension/hooks/childProcessAttachService.ts +++ b/src/client/debugger/extension/hooks/childProcessAttachService.ts @@ -5,13 +5,15 @@ import { inject, injectable } from 'inversify'; import { DebugConfiguration, DebugSession, WorkspaceFolder } from 'vscode'; -import { IApplicationShell, IDebugService, IWorkspaceService } from '../../../common/application/types'; +import { IDebugService } from '../../../common/application/types'; import { noop } from '../../../common/utils/misc'; import { captureTelemetry } from '../../../telemetry'; import { EventName } from '../../../telemetry/constants'; import { AttachRequestArguments } from '../../types'; import { IChildProcessAttachService } from './types'; import * as nls from 'vscode-nls'; +import { showErrorMessage } from '../../../common/vscodeApis/windowApis'; +import { getWorkspaceFolders } from '../../../common/vscodeApis/workspaceApis'; const localize: nls.LocalizeFunc = nls.loadMessageBundle(); @@ -24,11 +26,7 @@ const localize: nls.LocalizeFunc = nls.loadMessageBundle(); */ @injectable() export class ChildProcessAttachService implements IChildProcessAttachService { - constructor( - @inject(IApplicationShell) private readonly appShell: IApplicationShell, - @inject(IDebugService) private readonly debugService: IDebugService, - @inject(IWorkspaceService) private readonly workspaceService: IWorkspaceService, - ) {} + constructor(@inject(IDebugService) private readonly debugService: IDebugService) {} @captureTelemetry(EventName.DEBUGGER_ATTACH_TO_CHILD_PROCESS) public async attach(data: AttachRequestArguments & DebugConfiguration, parentSession: DebugSession): Promise { @@ -37,11 +35,9 @@ export class ChildProcessAttachService implements IChildProcessAttachService { const folder = this.getRelatedWorkspaceFolder(debugConfig); const launched = await this.debugService.startDebugging(folder, debugConfig, parentSession); if (!launched) { - this.appShell - .showErrorMessage( - localize('debuggerError', 'Failed to launch debugger for child process {0}', processId), - ) - .then(noop, noop); + showErrorMessage( + localize('debuggerError', 'Failed to launch debugger for child process {0}', processId), + ).then(noop, noop); } } @@ -49,10 +45,11 @@ export class ChildProcessAttachService implements IChildProcessAttachService { config: AttachRequestArguments & DebugConfiguration, ): WorkspaceFolder | undefined { const workspaceFolder = config.workspaceFolder; - const hasWorkspaceFolders = (this.workspaceService.workspaceFolders?.length || 0) > 0; + + const hasWorkspaceFolders = (getWorkspaceFolders()?.length || 0) > 0; if (!hasWorkspaceFolders || !workspaceFolder) { return; } - return this.workspaceService.workspaceFolders!.find((ws) => ws.uri.fsPath === workspaceFolder); + return getWorkspaceFolders()!.find((ws) => ws.uri.fsPath === workspaceFolder); } } diff --git a/src/test/debugger/extension/hooks/childProcessAttachService.unit.test.ts b/src/test/debugger/extension/hooks/childProcessAttachService.unit.test.ts index b4056d0c04f2..2ab9d3e30d2c 100644 --- a/src/test/debugger/extension/hooks/childProcessAttachService.unit.test.ts +++ b/src/test/debugger/extension/hooks/childProcessAttachService.unit.test.ts @@ -4,30 +4,30 @@ 'use strict'; import { expect } from 'chai'; +import * as sinon from 'sinon'; import { anything, capture, instance, mock, verify, when } from 'ts-mockito'; import { Uri, WorkspaceFolder } from 'vscode'; -import { ApplicationShell } from '../../../../client/common/application/applicationShell'; import { DebugService } from '../../../../client/common/application/debugService'; -import { IApplicationShell, IDebugService, IWorkspaceService } from '../../../../client/common/application/types'; -import { WorkspaceService } from '../../../../client/common/application/workspace'; +import { IDebugService } from '../../../../client/common/application/types'; +import * as workspaceApis from '../../../../client/common/vscodeApis/workspaceApis'; import { ChildProcessAttachService } from '../../../../client/debugger/extension/hooks/childProcessAttachService'; import { AttachRequestArguments, LaunchRequestArguments } from '../../../../client/debugger/types'; +import * as windowApis from '../../../../client/common/vscodeApis/windowApis'; suite('Debug - Attach to Child Process', () => { - let shell: IApplicationShell; let debugService: IDebugService; - let workspaceService: IWorkspaceService; let attachService: ChildProcessAttachService; + let getWorkspaceFoldersStub: sinon.SinonStub; + let showErrorMessageStub: sinon.SinonStub; setup(() => { - shell = mock(ApplicationShell); debugService = mock(DebugService); - workspaceService = mock(WorkspaceService); - attachService = new ChildProcessAttachService( - instance(shell), - instance(debugService), - instance(workspaceService), - ); + attachService = new ChildProcessAttachService(instance(debugService)); + getWorkspaceFoldersStub = sinon.stub(workspaceApis, 'getWorkspaceFolders'); + showErrorMessageStub = sinon.stub(windowApis, 'showErrorMessage'); + }); + teardown(() => { + sinon.restore(); }); test('Message is not displayed if debugger is launched', async () => { @@ -39,15 +39,15 @@ suite('Debug - Attach to Child Process', () => { subProcessId: 2, }; const session: any = {}; - when(workspaceService.workspaceFolders).thenReturn(undefined); + getWorkspaceFoldersStub.returns(undefined); when(debugService.startDebugging(anything(), anything(), anything())).thenResolve(true as any); - when(shell.showErrorMessage(anything())).thenResolve(); + showErrorMessageStub.returns(undefined); await attachService.attach(data, session); - verify(workspaceService.workspaceFolders).once(); + sinon.assert.calledOnce(getWorkspaceFoldersStub); verify(debugService.startDebugging(anything(), anything(), anything())).once(); - verify(shell.showErrorMessage(anything())).never(); + sinon.assert.notCalled(showErrorMessageStub); }); test('Message is displayed if debugger is not launched', async () => { const data: AttachRequestArguments = { @@ -59,15 +59,15 @@ suite('Debug - Attach to Child Process', () => { }; const session: any = {}; - when(workspaceService.workspaceFolders).thenReturn(undefined); + getWorkspaceFoldersStub.returns(undefined); when(debugService.startDebugging(anything(), anything(), anything())).thenResolve(false as any); - when(shell.showErrorMessage(anything())).thenResolve(); + showErrorMessageStub.resolves(() => {}); await attachService.attach(data, session); - verify(workspaceService.workspaceFolders).once(); + sinon.assert.calledOnce(getWorkspaceFoldersStub); verify(debugService.startDebugging(anything(), anything(), anything())).once(); - verify(shell.showErrorMessage(anything())).once(); + sinon.assert.calledOnce(showErrorMessageStub); }); test('Use correct workspace folder', async () => { const rightWorkspaceFolder: WorkspaceFolder = { name: '1', index: 1, uri: Uri.file('a') }; @@ -84,14 +84,14 @@ suite('Debug - Attach to Child Process', () => { }; const session: any = {}; - when(workspaceService.workspaceFolders).thenReturn([wkspace1, rightWorkspaceFolder, wkspace2]); + getWorkspaceFoldersStub.returns([wkspace1, rightWorkspaceFolder, wkspace2]); when(debugService.startDebugging(rightWorkspaceFolder, anything(), anything())).thenResolve(true as any); await attachService.attach(data, session); - verify(workspaceService.workspaceFolders).atLeast(1); + sinon.assert.called(getWorkspaceFoldersStub); verify(debugService.startDebugging(rightWorkspaceFolder, anything(), anything())).once(); - verify(shell.showErrorMessage(anything())).never(); + sinon.assert.notCalled(showErrorMessageStub); }); test('Use empty workspace folder if right one is not found', async () => { const rightWorkspaceFolder: WorkspaceFolder = { name: '1', index: 1, uri: Uri.file('a') }; @@ -108,14 +108,14 @@ suite('Debug - Attach to Child Process', () => { }; const session: any = {}; - when(workspaceService.workspaceFolders).thenReturn([wkspace1, wkspace2]); + getWorkspaceFoldersStub.returns([wkspace1, wkspace2]); when(debugService.startDebugging(undefined, anything(), anything())).thenResolve(true as any); await attachService.attach(data, session); - verify(workspaceService.workspaceFolders).atLeast(1); + sinon.assert.called(getWorkspaceFoldersStub); verify(debugService.startDebugging(undefined, anything(), anything())).once(); - verify(shell.showErrorMessage(anything())).never(); + sinon.assert.notCalled(showErrorMessageStub); }); test('Validate debug config is passed as is', async () => { const data: LaunchRequestArguments | AttachRequestArguments = { @@ -131,17 +131,17 @@ suite('Debug - Attach to Child Process', () => { debugConfig.host = 'localhost'; const session: any = {}; - when(workspaceService.workspaceFolders).thenReturn(undefined); + getWorkspaceFoldersStub.returns(undefined); when(debugService.startDebugging(undefined, anything(), anything())).thenResolve(true as any); await attachService.attach(data, session); - verify(workspaceService.workspaceFolders).once(); + sinon.assert.calledOnce(getWorkspaceFoldersStub); verify(debugService.startDebugging(undefined, anything(), anything())).once(); const [, secondArg, thirdArg] = capture(debugService.startDebugging).last(); expect(secondArg).to.deep.equal(debugConfig); expect(thirdArg).to.deep.equal(session); - verify(shell.showErrorMessage(anything())).never(); + sinon.assert.notCalled(showErrorMessageStub); }); test('Pass data as is if data is attach debug configuration', async () => { const data: AttachRequestArguments = { @@ -152,17 +152,17 @@ suite('Debug - Attach to Child Process', () => { const session: any = {}; const debugConfig = JSON.parse(JSON.stringify(data)); - when(workspaceService.workspaceFolders).thenReturn(undefined); + getWorkspaceFoldersStub.returns(undefined); when(debugService.startDebugging(undefined, anything(), anything())).thenResolve(true as any); await attachService.attach(data, session); - verify(workspaceService.workspaceFolders).once(); + sinon.assert.calledOnce(getWorkspaceFoldersStub); verify(debugService.startDebugging(undefined, anything(), anything())).once(); const [, secondArg, thirdArg] = capture(debugService.startDebugging).last(); expect(secondArg).to.deep.equal(debugConfig); expect(thirdArg).to.deep.equal(session); - verify(shell.showErrorMessage(anything())).never(); + sinon.assert.notCalled(showErrorMessageStub); }); test('Validate debug config when parent/root parent was attached', async () => { const data: AttachRequestArguments = { @@ -180,16 +180,16 @@ suite('Debug - Attach to Child Process', () => { debugConfig.request = 'attach'; const session: any = {}; - when(workspaceService.workspaceFolders).thenReturn(undefined); + getWorkspaceFoldersStub.returns(undefined); when(debugService.startDebugging(undefined, anything(), anything())).thenResolve(true as any); await attachService.attach(data, session); - verify(workspaceService.workspaceFolders).once(); + sinon.assert.calledOnce(getWorkspaceFoldersStub); verify(debugService.startDebugging(undefined, anything(), anything())).once(); const [, secondArg, thirdArg] = capture(debugService.startDebugging).last(); expect(secondArg).to.deep.equal(debugConfig); expect(thirdArg).to.deep.equal(session); - verify(shell.showErrorMessage(anything())).never(); + sinon.assert.notCalled(showErrorMessageStub); }); }); From 5d85c183a0bbc7403c7ca13e85ae43cb8c68b0fb Mon Sep 17 00:00:00 2001 From: Paula Camargo Date: Tue, 15 Nov 2022 11:48:42 -0800 Subject: [PATCH 3/4] Fix tests in debugLauncher --- src/client/testing/common/debugLauncher.ts | 7 +- .../testing/common/debugLauncher.unit.test.ts | 76 ++++++++++++------- 2 files changed, 50 insertions(+), 33 deletions(-) diff --git a/src/client/testing/common/debugLauncher.ts b/src/client/testing/common/debugLauncher.ts index b166a7935962..611b20e345f7 100644 --- a/src/client/testing/common/debugLauncher.ts +++ b/src/client/testing/common/debugLauncher.ts @@ -1,7 +1,6 @@ import { inject, injectable, named } from 'inversify'; import * as path from 'path'; -import * as vscode from 'vscode'; import { DebugConfiguration, Uri, WorkspaceFolder } from 'vscode'; import { IApplicationShell, IDebugService } from '../../common/application/types'; import { EXTENSION_ROOT_DIR } from '../../common/constants'; @@ -16,7 +15,7 @@ import { TestProvider } from '../types'; import { ITestDebugLauncher, LaunchOptions } from './types'; import * as nls from 'vscode-nls'; import { getConfigurationsForWorkspace } from '../../debugger/extension/configuration/launch.json/launchJsonReader'; -import { getWorkspaceFolder } from '../../common/vscodeApis/workspaceApis'; +import { getWorkspaceFolder, getWorkspaceFolders } from '../../common/vscodeApis/workspaceApis'; const localize: nls.LocalizeFunc = nls.loadMessageBundle(); @@ -70,7 +69,7 @@ export class DebugLauncher implements ITestDebugLauncher { } } private resolveWorkspaceFolder(cwd: string): WorkspaceFolder { - const hasWorkspaceFolders = (vscode.workspace.workspaceFolders?.length || 0) > 0; + const hasWorkspaceFolders = (getWorkspaceFolders()?.length || 0) > 0; if (!hasWorkspaceFolders) { throw new Error('Please open a workspace'); } @@ -78,7 +77,7 @@ export class DebugLauncher implements ITestDebugLauncher { const cwdUri = cwd ? Uri.file(cwd) : undefined; let workspaceFolder = getWorkspaceFolder(cwdUri); if (!workspaceFolder) { - workspaceFolder = vscode.workspace.workspaceFolders![0]; + workspaceFolder = getWorkspaceFolders()![0]; } return workspaceFolder; } diff --git a/src/test/testing/common/debugLauncher.unit.test.ts b/src/test/testing/common/debugLauncher.unit.test.ts index 9a6778aaf75d..1929beaaaeed 100644 --- a/src/test/testing/common/debugLauncher.unit.test.ts +++ b/src/test/testing/common/debugLauncher.unit.test.ts @@ -6,13 +6,15 @@ import { expect, use } from 'chai'; import * as chaiAsPromised from 'chai-as-promised'; import * as path from 'path'; +import * as sinon from 'sinon'; import * as TypeMoq from 'typemoq'; +import * as fs from 'fs-extra'; +import * as workspaceApis from '../../../client/common/vscodeApis/workspaceApis'; import { CancellationTokenSource, DebugConfiguration, Uri, WorkspaceFolder } from 'vscode'; import { IInvalidPythonPathInDebuggerService } from '../../../client/application/diagnostics/types'; -import { IApplicationShell, IDebugService, IWorkspaceService } from '../../../client/common/application/types'; +import { IApplicationShell, IDebugService } from '../../../client/common/application/types'; import { EXTENSION_ROOT_DIR } from '../../../client/common/constants'; import '../../../client/common/extensions'; -import { IFileSystem, IPlatformService } from '../../../client/common/platform/types'; import { IConfigurationService, IPythonSettings } from '../../../client/common/types'; import { DebuggerTypeName } from '../../../client/debugger/constants'; import { IDebugEnvironmentVariablesService } from '../../../client/debugger/extension/configuration/resolvers/helper'; @@ -34,12 +36,13 @@ suite('Unit Tests - Debug Launcher', () => { let unitTestSettings: TypeMoq.IMock; let debugLauncher: DebugLauncher; let debugService: TypeMoq.IMock; - let workspaceService: TypeMoq.IMock; - let platformService: TypeMoq.IMock; - let filesystem: TypeMoq.IMock; let settings: TypeMoq.IMock; let debugEnvHelper: TypeMoq.IMock; let interpreterService: TypeMoq.IMock; + let getWorkspaceFolderStub: sinon.SinonStub; + let getWorkspaceFoldersStub: sinon.SinonStub; + let pathExistsStub: sinon.SinonStub; + let readFileStub: sinon.SinonStub; setup(async () => { interpreterService = TypeMoq.Mock.ofType(); @@ -51,19 +54,18 @@ suite('Unit Tests - Debug Launcher', () => { debugService = TypeMoq.Mock.ofType(undefined, TypeMoq.MockBehavior.Strict); serviceContainer.setup((c) => c.get(TypeMoq.It.isValue(IDebugService))).returns(() => debugService.object); + getWorkspaceFolderStub = sinon.stub(workspaceApis, 'getWorkspaceFolder'); + getWorkspaceFoldersStub = sinon.stub(workspaceApis, 'getWorkspaceFolders'); + pathExistsStub = sinon.stub(fs, 'pathExists'); + readFileStub = sinon.stub(fs, 'readFile'); - workspaceService = TypeMoq.Mock.ofType(undefined, TypeMoq.MockBehavior.Strict); - serviceContainer - .setup((c) => c.get(TypeMoq.It.isValue(IWorkspaceService))) - .returns(() => workspaceService.object); - - platformService = TypeMoq.Mock.ofType(undefined, TypeMoq.MockBehavior.Strict); - serviceContainer - .setup((c) => c.get(TypeMoq.It.isValue(IPlatformService))) - .returns(() => platformService.object); + // platformService = TypeMoq.Mock.ofType(undefined, TypeMoq.MockBehavior.Strict); + // serviceContainer + // .setup((c) => c.get(TypeMoq.It.isValue(IPlatformService))) + // .returns(() => platformService.object); - filesystem = TypeMoq.Mock.ofType(undefined, TypeMoq.MockBehavior.Strict); - serviceContainer.setup((c) => c.get(TypeMoq.It.isValue(IFileSystem))).returns(() => filesystem.object); + // filesystem = TypeMoq.Mock.ofType(undefined, TypeMoq.MockBehavior.Strict); + // serviceContainer.setup((c) => c.get(TypeMoq.It.isValue(IFileSystem))).returns(() => filesystem.object); const appShell = TypeMoq.Mock.ofType(undefined, TypeMoq.MockBehavior.Strict); appShell.setup((a) => a.showErrorMessage(TypeMoq.It.isAny())).returns(() => Promise.resolve(undefined)); @@ -82,6 +84,11 @@ suite('Unit Tests - Debug Launcher', () => { debugLauncher = new DebugLauncher(serviceContainer.object, getNewResolver(configService.object)); }); + + teardown(() => { + sinon.restore(); + }); + function getNewResolver(configService: IConfigurationService) { const validator = TypeMoq.Mock.ofType( undefined, @@ -102,7 +109,7 @@ suite('Unit Tests - Debug Launcher', () => { expected: DebugConfiguration, testProvider: TestProvider, ) { - platformService.setup((p) => p.isWindows).returns(() => /^win/.test(process.platform)); + // platformService.setup((p) => p.isWindows).returns(() => /^win/.test(process.platform)); interpreterService .setup((i) => i.getActiveInterpreter(TypeMoq.It.isAny())) .returns(() => Promise.resolve(({ path: 'python' } as unknown) as PythonEnvironment)); @@ -175,22 +182,28 @@ suite('Unit Tests - Debug Launcher', () => { const testLaunchScript = getTestLauncherScript(testProvider); const workspaceFolders = [createWorkspaceFolder(options.cwd), createWorkspaceFolder('five/six/seven')]; - workspaceService.setup((u) => u.workspaceFolders).returns(() => workspaceFolders); - workspaceService.setup((u) => u.getWorkspaceFolder(TypeMoq.It.isAny())).returns(() => workspaceFolders[0]); + // workspaceService.setup((u) => u.workspaceFolders).returns(() => workspaceFolders); + getWorkspaceFoldersStub.returns(workspaceFolders); + // workspaceService.setup((u) => u.getWorkspaceFolder(TypeMoq.It.isAny())).returns(() => workspaceFolders[0]); + getWorkspaceFolderStub.returns(workspaceFolders[0]); if (!debugConfigs) { - filesystem.setup((fs) => fs.fileExists(TypeMoq.It.isAny())).returns(() => Promise.resolve(false)); + // filesystem.setup((fs) => fs.fileExists(TypeMoq.It.isAny())).returns(() => Promise.resolve(false)); + pathExistsStub.resolves(false); } else { - filesystem.setup((fs) => fs.fileExists(TypeMoq.It.isAny())).returns(() => Promise.resolve(true)); + // filesystem.setup((fs) => fs.fileExists(TypeMoq.It.isAny())).returns(() => Promise.resolve(true)); + pathExistsStub.resolves(true); + if (typeof debugConfigs !== 'string') { debugConfigs = JSON.stringify({ version: '0.1.0', configurations: debugConfigs, }); } - filesystem - .setup((fs) => fs.readFile(TypeMoq.It.isAny())) - .returns(() => Promise.resolve(debugConfigs as string)); + // filesystem + // .setup((fs) => fs.readFile(TypeMoq.It.isAny())) + // .returns(() => Promise.resolve(debugConfigs as string)); + readFileStub.resolves(debugConfigs as string); } if (!expected) { @@ -289,7 +302,8 @@ suite('Unit Tests - Debug Launcher', () => { debugService.verifyAll(); }); test(`Must throw an exception if there are no workspaces ${testTitleSuffix}`, async () => { - workspaceService.setup((u) => u.workspaceFolders).returns(() => undefined); + // workspaceService.setup((u) => u.workspaceFolders).returns(() => undefined); + getWorkspaceFoldersStub.returns(undefined); debugService .setup((d) => d.startDebugging(TypeMoq.It.isAny(), TypeMoq.It.isAny())) .returns(() => Promise.resolve(undefined as any)) @@ -579,8 +593,10 @@ suite('Unit Tests - Debug Launcher', () => { const workspaceFolder = { name: 'abc', index: 0, uri: Uri.file(__filename) }; const filename = path.join(workspaceFolder.uri.fsPath, '.vscode', 'launch.json'); const jsonc = '{"version":"1234", "configurations":[1,2,],}'; - filesystem.setup((fs) => fs.fileExists(TypeMoq.It.isValue(filename))).returns(() => Promise.resolve(true)); - filesystem.setup((fs) => fs.readFile(TypeMoq.It.isValue(filename))).returns(() => Promise.resolve(jsonc)); + pathExistsStub.resolves(true); + // filesystem.setup((fs) => fs.fileExists(TypeMoq.It.isValue(filename))).returns(() => Promise.resolve(true)); + readFileStub.withArgs(filename).resolves(jsonc); + // filesystem.setup((fs) => fs.readFile(TypeMoq.It.isValue(filename))).returns(() => Promise.resolve(jsonc)); const configs = await debugLauncher.readAllDebugConfigs(workspaceFolder); @@ -591,8 +607,10 @@ suite('Unit Tests - Debug Launcher', () => { const filename = path.join(workspaceFolder.uri.fsPath, '.vscode', 'launch.json'); const jsonc = '{"version":"1234"'; - filesystem.setup((fs) => fs.fileExists(TypeMoq.It.isValue(filename))).returns(() => Promise.resolve(true)); - filesystem.setup((fs) => fs.readFile(TypeMoq.It.isValue(filename))).returns(() => Promise.resolve(jsonc)); + pathExistsStub.resolves(true); + // filesystem.setup((fs) => fs.fileExists(TypeMoq.It.isValue(filename))).returns(() => Promise.resolve(true)); + readFileStub.withArgs(filename).resolves(jsonc); + // filesystem.setup((fs) => fs.readFile(TypeMoq.It.isValue(filename))).returns(() => Promise.resolve(jsonc)); const configs = await debugLauncher.readAllDebugConfigs(workspaceFolder); From 0302ee39fe6314487a20a155de6017b7f7069c1a Mon Sep 17 00:00:00 2001 From: Paula Camargo Date: Tue, 15 Nov 2022 11:51:31 -0800 Subject: [PATCH 4/4] Fix code --- .../testing/common/debugLauncher.unit.test.ts | 21 ------------------- 1 file changed, 21 deletions(-) diff --git a/src/test/testing/common/debugLauncher.unit.test.ts b/src/test/testing/common/debugLauncher.unit.test.ts index 1929beaaaeed..dfe9e8ce5e99 100644 --- a/src/test/testing/common/debugLauncher.unit.test.ts +++ b/src/test/testing/common/debugLauncher.unit.test.ts @@ -59,14 +59,6 @@ suite('Unit Tests - Debug Launcher', () => { pathExistsStub = sinon.stub(fs, 'pathExists'); readFileStub = sinon.stub(fs, 'readFile'); - // platformService = TypeMoq.Mock.ofType(undefined, TypeMoq.MockBehavior.Strict); - // serviceContainer - // .setup((c) => c.get(TypeMoq.It.isValue(IPlatformService))) - // .returns(() => platformService.object); - - // filesystem = TypeMoq.Mock.ofType(undefined, TypeMoq.MockBehavior.Strict); - // serviceContainer.setup((c) => c.get(TypeMoq.It.isValue(IFileSystem))).returns(() => filesystem.object); - const appShell = TypeMoq.Mock.ofType(undefined, TypeMoq.MockBehavior.Strict); appShell.setup((a) => a.showErrorMessage(TypeMoq.It.isAny())).returns(() => Promise.resolve(undefined)); serviceContainer.setup((c) => c.get(TypeMoq.It.isValue(IApplicationShell))).returns(() => appShell.object); @@ -109,7 +101,6 @@ suite('Unit Tests - Debug Launcher', () => { expected: DebugConfiguration, testProvider: TestProvider, ) { - // platformService.setup((p) => p.isWindows).returns(() => /^win/.test(process.platform)); interpreterService .setup((i) => i.getActiveInterpreter(TypeMoq.It.isAny())) .returns(() => Promise.resolve(({ path: 'python' } as unknown) as PythonEnvironment)); @@ -182,16 +173,12 @@ suite('Unit Tests - Debug Launcher', () => { const testLaunchScript = getTestLauncherScript(testProvider); const workspaceFolders = [createWorkspaceFolder(options.cwd), createWorkspaceFolder('five/six/seven')]; - // workspaceService.setup((u) => u.workspaceFolders).returns(() => workspaceFolders); getWorkspaceFoldersStub.returns(workspaceFolders); - // workspaceService.setup((u) => u.getWorkspaceFolder(TypeMoq.It.isAny())).returns(() => workspaceFolders[0]); getWorkspaceFolderStub.returns(workspaceFolders[0]); if (!debugConfigs) { - // filesystem.setup((fs) => fs.fileExists(TypeMoq.It.isAny())).returns(() => Promise.resolve(false)); pathExistsStub.resolves(false); } else { - // filesystem.setup((fs) => fs.fileExists(TypeMoq.It.isAny())).returns(() => Promise.resolve(true)); pathExistsStub.resolves(true); if (typeof debugConfigs !== 'string') { @@ -200,9 +187,6 @@ suite('Unit Tests - Debug Launcher', () => { configurations: debugConfigs, }); } - // filesystem - // .setup((fs) => fs.readFile(TypeMoq.It.isAny())) - // .returns(() => Promise.resolve(debugConfigs as string)); readFileStub.resolves(debugConfigs as string); } @@ -302,7 +286,6 @@ suite('Unit Tests - Debug Launcher', () => { debugService.verifyAll(); }); test(`Must throw an exception if there are no workspaces ${testTitleSuffix}`, async () => { - // workspaceService.setup((u) => u.workspaceFolders).returns(() => undefined); getWorkspaceFoldersStub.returns(undefined); debugService .setup((d) => d.startDebugging(TypeMoq.It.isAny(), TypeMoq.It.isAny())) @@ -594,9 +577,7 @@ suite('Unit Tests - Debug Launcher', () => { const filename = path.join(workspaceFolder.uri.fsPath, '.vscode', 'launch.json'); const jsonc = '{"version":"1234", "configurations":[1,2,],}'; pathExistsStub.resolves(true); - // filesystem.setup((fs) => fs.fileExists(TypeMoq.It.isValue(filename))).returns(() => Promise.resolve(true)); readFileStub.withArgs(filename).resolves(jsonc); - // filesystem.setup((fs) => fs.readFile(TypeMoq.It.isValue(filename))).returns(() => Promise.resolve(jsonc)); const configs = await debugLauncher.readAllDebugConfigs(workspaceFolder); @@ -608,9 +589,7 @@ suite('Unit Tests - Debug Launcher', () => { const jsonc = '{"version":"1234"'; pathExistsStub.resolves(true); - // filesystem.setup((fs) => fs.fileExists(TypeMoq.It.isValue(filename))).returns(() => Promise.resolve(true)); readFileStub.withArgs(filename).resolves(jsonc); - // filesystem.setup((fs) => fs.readFile(TypeMoq.It.isValue(filename))).returns(() => Promise.resolve(jsonc)); const configs = await debugLauncher.readAllDebugConfigs(workspaceFolder);