Remove DI from factory

src/client/debugger/extension/adapter/browser.ts

@@ -0,0 +1,10 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT License.
+
+'use strict';
+
+import { env, Uri } from 'vscode';
+
+export function launch(url: string): void {
+    env.openExternal(Uri.parse(url));
+}

src/client/debugger/extension/adapter/factory.ts

@@ -12,7 +12,6 @@ import {
     DebugSession,
     WorkspaceFolder,
 } from 'vscode';
-import { IApplicationShell } from '../../../common/application/types';
 import { EXTENSION_ROOT_DIR } from '../../../constants';
 import { IInterpreterService } from '../../../interpreter/contracts';
 import { traceLog, traceVerbose } from '../../../logging';
@@ -22,15 +21,13 @@ import { EventName } from '../../../telemetry/constants';
 import { AttachRequestArguments, LaunchRequestArguments } from '../../types';
 import { IDebugAdapterDescriptorFactory } from '../types';
 import * as nls from 'vscode-nls';
+import { showErrorMessage } from '../configuration/utils/common';
 
 const localize: nls.LocalizeFunc = nls.loadMessageBundle();
 
 @injectable()
 export class DebugAdapterDescriptorFactory implements IDebugAdapterDescriptorFactory {
-    constructor(
-        @inject(IInterpreterService) private readonly interpreterService: IInterpreterService,
-        @inject(IApplicationShell) private readonly appShell: IApplicationShell,
-    ) {}
+    constructor(@inject(IInterpreterService) private readonly interpreterService: IInterpreterService) {}
 
     public async createDebugAdapterDescriptor(
         session: DebugSession,
@@ -161,7 +158,7 @@ export class DebugAdapterDescriptorFactory implements IDebugAdapterDescriptorFac
      * @memberof DebugAdapterDescriptorFactory
      */
     private async notifySelectInterpreter() {
-        await this.appShell.showErrorMessage(
+        await showErrorMessage(
             localize('interpreterError', 'Please install Python or select a Python Interpreter to use the debugger.'),
         );
     }

src/client/debugger/extension/adapter/outdatedDebuggerPrompt.ts

@@ -2,34 +2,27 @@
 // Licensed under the MIT License.
 
 'use strict';
-
-import { inject, injectable } from 'inversify';
+import { injectable } from 'inversify';
 import { DebugAdapterTracker, DebugAdapterTrackerFactory, DebugSession, ProviderResult } from 'vscode';
 import { DebugProtocol } from 'vscode-debugprotocol';
-import { IApplicationShell } from '../../../common/application/types';
-import { IBrowserService } from '../../../common/types';
 import { Common, OutdatedDebugger } from '../../../common/utils/localize';
+import { showInformationMessage } from '../configuration/utils/common';
+import { launch } from './browser';
 import { IPromptShowState } from './types';
 
 // This situation occurs when user connects to old containers or server where
 // the debugger they had installed was ptvsd. We should show a prompt to ask them to update.
 class OutdatedDebuggerPrompt implements DebugAdapterTracker {
-    constructor(
-        private promptCheck: IPromptShowState,
-        private appShell: IApplicationShell,
-        private browserService: IBrowserService,
-    ) {}
+    constructor(private promptCheck: IPromptShowState) {}
 
     public onDidSendMessage(message: DebugProtocol.ProtocolMessage) {
         if (this.promptCheck.shouldShowPrompt() && this.isPtvsd(message)) {
             const prompts = [Common.moreInfo];
-            this.appShell
-                .showInformationMessage(OutdatedDebugger.outdatedDebuggerMessage, ...prompts)
-                .then((selection) => {
-                    if (selection === prompts[0]) {
-                        this.browserService.launch('https://aka.ms/migrateToDebugpy');
-                    }
-                });
+            showInformationMessage(OutdatedDebugger.outdatedDebuggerMessage, ...prompts).then((selection) => {
+                if (selection === prompts[0]) {
+                    launch('https://aka.ms/migrateToDebugpy');
+                }
+            });
         }
     }
 
@@ -71,13 +64,10 @@ class OutdatedDebuggerPromptState implements IPromptShowState {
 @injectable()
 export class OutdatedDebuggerPromptFactory implements DebugAdapterTrackerFactory {
     private readonly promptCheck: OutdatedDebuggerPromptState;
-    constructor(
-        @inject(IApplicationShell) private readonly appShell: IApplicationShell,
-        @inject(IBrowserService) private browserService: IBrowserService,
-    ) {
+    constructor() {
         this.promptCheck = new OutdatedDebuggerPromptState();
     }
     public createDebugAdapterTracker(_session: DebugSession): ProviderResult<DebugAdapterTracker> {
-        return new OutdatedDebuggerPrompt(this.promptCheck, this.appShell, this.browserService);
+        return new OutdatedDebuggerPrompt(this.promptCheck);
     }
 }

src/client/debugger/extension/configuration/utils/common.ts

@@ -46,3 +46,11 @@ export function getActiveTextEditor(): TextEditor | undefined {
     const { activeTextEditor } = window;
     return activeTextEditor;
 }
+
+export function showInformationMessage(message: string, ...items: string[]): Thenable<any> {
+    return window.showInformationMessage(message, ...items);
+}
+
+export function showErrorMessage(message: string, ...items: string[]): Thenable<any> {
+    return window.showErrorMessage(message, ...items);
+}

src/test/debugger/extension/adapter/factory.unit.test.ts

@@ -7,13 +7,11 @@ import * as assert from 'assert';
 import { expect, use } from 'chai';
 import * as chaiAsPromised from 'chai-as-promised';
 import * as path from 'path';
-
+import * as sinon from 'sinon';
 import rewiremock from 'rewiremock';
 import { SemVer } from 'semver';
-import { anyString, anything, instance, mock, verify, when } from 'ts-mockito';
+import { anything, instance, mock, verify, when } from 'ts-mockito';
 import { DebugAdapterExecutable, DebugAdapterServer, DebugConfiguration, DebugSession, WorkspaceFolder } from 'vscode';
-import { ApplicationShell } from '../../../../client/common/application/applicationShell';
-import { IApplicationShell } from '../../../../client/common/application/types';
 import { ConfigurationService } from '../../../../client/common/configuration/service';
 import { IPythonSettings } from '../../../../client/common/types';
 import { Architecture } from '../../../../client/common/utils/platform';
@@ -25,13 +23,14 @@ import { InterpreterService } from '../../../../client/interpreter/interpreterSe
 import { EnvironmentType } from '../../../../client/pythonEnvironments/info';
 import { clearTelemetryReporter } from '../../../../client/telemetry';
 import { EventName } from '../../../../client/telemetry/constants';
+import * as common from '../../../../client/debugger/extension/configuration/utils/common';
 
 use(chaiAsPromised);
 
 suite('Debugging - Adapter Factory', () => {
     let factory: IDebugAdapterDescriptorFactory;
     let interpreterService: IInterpreterService;
-    let appShell: IApplicationShell;
+    let showErrorMessageStub: sinon.SinonStub;
 
     const nodeExecutable = undefined;
     const debugAdapterPath = path.join(EXTENSION_ROOT_DIR, 'pythonFiles', 'lib', 'python', 'debugpy', 'adapter');
@@ -63,19 +62,19 @@ suite('Debugging - Adapter Factory', () => {
         process.env.VSC_PYTHON_CI_TEST = undefined;
         rewiremock.enable();
         rewiremock('@vscode/extension-telemetry').with({ default: Reporter });
+        showErrorMessageStub = sinon.stub(common, 'showErrorMessage');
 
         const configurationService = mock(ConfigurationService);
         when(configurationService.getSettings(undefined)).thenReturn(({
             experiments: { enabled: true },
         } as any) as IPythonSettings);
 
         interpreterService = mock(InterpreterService);
-        appShell = mock(ApplicationShell);
 
         when(interpreterService.getInterpreterDetails(pythonPath)).thenResolve(interpreter);
         when(interpreterService.getInterpreters(anything())).thenReturn([interpreter]);
 
-        factory = new DebugAdapterDescriptorFactory(instance(interpreterService), instance(appShell));
+        factory = new DebugAdapterDescriptorFactory(instance(interpreterService));
     });
 
     teardown(() => {
@@ -86,6 +85,7 @@ suite('Debugging - Adapter Factory', () => {
         Reporter.measures = [];
         rewiremock.disable();
         clearTelemetryReporter();
+        sinon.restore();
     });
 
     function createSession(config: Partial<DebugConfiguration>, workspaceFolder?: WorkspaceFolder): DebugSession {
@@ -136,7 +136,7 @@ suite('Debugging - Adapter Factory', () => {
         const promise = factory.createDebugAdapterDescriptor(session, nodeExecutable);
 
         await expect(promise).to.eventually.be.rejectedWith('Debug Adapter Executable not provided');
-        verify(appShell.showErrorMessage(anyString())).once();
+        sinon.assert.calledOnce(showErrorMessageStub);
     });
 
     test('Return Debug Adapter server if request is "attach", and port is specified directly', async () => {

src/test/debugger/extension/adapter/outdatedDebuggerPrompt.unit.test.ts

@@ -4,23 +4,23 @@
 'use strict';
 
 import * as assert from 'assert';
-import { anyString, anything, instance, mock, verify, when } from 'ts-mockito';
+import * as sinon from 'sinon';
+import { anyString, anything, mock, when } from 'ts-mockito';
 import { DebugSession, WorkspaceFolder } from 'vscode';
 import { DebugProtocol } from 'vscode-debugprotocol';
-import { ApplicationShell } from '../../../../client/common/application/applicationShell';
-import { IApplicationShell } from '../../../../client/common/application/types';
 import { ConfigurationService } from '../../../../client/common/configuration/service';
-import { BrowserService } from '../../../../client/common/net/browser';
-import { IBrowserService, IPythonSettings } from '../../../../client/common/types';
 import { createDeferred, sleep } from '../../../../client/common/utils/async';
 import { Common } from '../../../../client/common/utils/localize';
 import { OutdatedDebuggerPromptFactory } from '../../../../client/debugger/extension/adapter/outdatedDebuggerPrompt';
 import { clearTelemetryReporter } from '../../../../client/telemetry';
+import * as browser from '../../../../client/debugger/extension/adapter/browser';
+import * as common from '../../../../client/debugger/extension/configuration/utils/common';
+import { IPythonSettings } from '../../../../client/common/types';
 
 suite('Debugging - Outdated Debugger Prompt tests.', () => {
     let promptFactory: OutdatedDebuggerPromptFactory;
-    let appShell: IApplicationShell;
-    let browserService: IBrowserService;
+    let showInformationMessageStub: sinon.SinonStub;
+    let browserLaunchStub: sinon.SinonStub;
 
     const ptvsdOutputEvent: DebugProtocol.OutputEvent = {
         seq: 1,
@@ -42,12 +42,14 @@ suite('Debugging - Outdated Debugger Prompt tests.', () => {
             experiments: { enabled: true },
         } as any) as IPythonSettings);
 
-        appShell = mock(ApplicationShell);
-        browserService = mock(BrowserService);
-        promptFactory = new OutdatedDebuggerPromptFactory(instance(appShell), instance(browserService));
+        showInformationMessageStub = sinon.stub(common, 'showInformationMessage');
+        browserLaunchStub = sinon.stub(browser, 'launch');
+
+        promptFactory = new OutdatedDebuggerPromptFactory();
     });
 
     teardown(() => {
+        sinon.restore();
         clearTelemetryReporter();
     });
 
@@ -68,32 +70,37 @@ suite('Debugging - Outdated Debugger Prompt tests.', () => {
     }
 
     test('Show prompt when attaching to ptvsd, more info is NOT clicked', async () => {
-        when(appShell.showInformationMessage(anything(), anything())).thenReturn(Promise.resolve(undefined));
-
+        showInformationMessageStub.returns(Promise.resolve(undefined));
         const session = createSession();
         const prompter = await promptFactory.createDebugAdapterTracker(session);
         if (prompter) {
             prompter.onDidSendMessage!(ptvsdOutputEvent);
         }
 
-        verify(browserService.launch(anyString())).never();
+        browserLaunchStub.neverCalledWith(anyString());
+
         // First call should show info once
-        verify(appShell.showInformationMessage(anything(), anything())).once();
+
+        sinon.assert.calledOnce(showInformationMessageStub);
         assert(prompter);
 
         prompter!.onDidSendMessage!(ptvsdOutputEvent);
         // Can't use deferred promise here
         await sleep(1);
 
-        verify(browserService.launch(anyString())).never();
+        browserLaunchStub.neverCalledWith(anyString());
         // Second time it should not be called, so overall count is one.
-        verify(appShell.showInformationMessage(anything(), anything())).once();
+        sinon.assert.calledOnce(showInformationMessageStub);
     });
 
     test('Show prompt when attaching to ptvsd, more info is clicked', async () => {
-        when(appShell.showInformationMessage(anything(), anything())).thenReturn(Promise.resolve(Common.moreInfo));
+        showInformationMessageStub.returns(Promise.resolve(Common.moreInfo));
+
         const deferred = createDeferred();
-        when(browserService.launch(anything())).thenCall(() => deferred.resolve());
+        browserLaunchStub.callsFake(() => deferred.resolve());
+        browserLaunchStub.onCall(1).callsFake(() => {
+            return new Promise(() => deferred.resolve());
+        });
 
         const session = createSession();
         const prompter = await promptFactory.createDebugAdapterTracker(session);
@@ -102,22 +109,24 @@ suite('Debugging - Outdated Debugger Prompt tests.', () => {
         prompter!.onDidSendMessage!(ptvsdOutputEvent);
         await deferred.promise;
 
-        verify(browserService.launch(anything())).once();
+        sinon.assert.calledOnce(browserLaunchStub);
+
         // First call should show info once
-        verify(appShell.showInformationMessage(anything(), anything())).once();
+        sinon.assert.calledOnce(showInformationMessageStub);
 
         prompter!.onDidSendMessage!(ptvsdOutputEvent);
         // The second call does not go through the same path. So we just give enough time for the
         // operation to complete.
         await sleep(1);
 
-        verify(browserService.launch(anyString())).once();
+        sinon.assert.calledOnce(browserLaunchStub);
+
         // Second time it should not be called, so overall count is one.
-        verify(appShell.showInformationMessage(anything(), anything())).once();
+        sinon.assert.calledOnce(showInformationMessageStub);
     });
 
     test("Don't show prompt attaching to debugpy", async () => {
-        when(appShell.showInformationMessage(anything(), anything())).thenReturn(Promise.resolve(undefined));
+        showInformationMessageStub.returns(Promise.resolve(undefined));
 
         const session = createSession();
         const prompter = await promptFactory.createDebugAdapterTracker(session);
@@ -127,7 +136,7 @@ suite('Debugging - Outdated Debugger Prompt tests.', () => {
         // Can't use deferred promise here
         await sleep(1);
 
-        verify(appShell.showInformationMessage(anything(), anything())).never();
+        showInformationMessageStub.neverCalledWith(anything(), anything());
     });
 
     const someRequest: DebugProtocol.RunInTerminalRequest = {
@@ -155,7 +164,7 @@ suite('Debugging - Outdated Debugger Prompt tests.', () => {
 
     [someRequest, someEvent, someOutputEvent].forEach((message) => {
         test(`Don't show prompt when non-telemetry events are seen: ${JSON.stringify(message)}`, async () => {
-            when(appShell.showInformationMessage(anything(), anything())).thenReturn(Promise.resolve(undefined));
+            showInformationMessageStub.returns(Promise.resolve(undefined));
 
             const session = createSession();
             const prompter = await promptFactory.createDebugAdapterTracker(session);
@@ -165,7 +174,7 @@ suite('Debugging - Outdated Debugger Prompt tests.', () => {
             // Can't use deferred promise here
             await sleep(1);
 
-            verify(appShell.showInformationMessage(anything(), anything())).never();
+            showInformationMessageStub.neverCalledWith(anything(), anything());
         });
     });
 });