Skip to content

Commit 9576411

Browse files
remove di in debugger prompt
1 parent 49e38ff commit 9576411

File tree

4 files changed

+64
-49
lines changed

4 files changed

+64
-49
lines changed
Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
// Copyright (c) Microsoft Corporation. All rights reserved.
2+
// Licensed under the MIT License.
3+
4+
'use strict';
5+
6+
import { env, Uri } from 'vscode';
7+
8+
export function launch(url: string): void {
9+
env.openExternal(Uri.parse(url));
10+
}

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

Lines changed: 10 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -3,33 +3,26 @@
33

44
'use strict';
55

6-
import { inject, injectable } from 'inversify';
76
import { DebugAdapterTracker, DebugAdapterTrackerFactory, DebugSession, ProviderResult } from 'vscode';
87
import { DebugProtocol } from 'vscode-debugprotocol';
9-
import { IApplicationShell } from '../../../common/application/types';
10-
import { IBrowserService } from '../../../common/types';
118
import { Common, OutdatedDebugger } from '../../../common/utils/localize';
9+
import { showInformationMessage } from '../configuration/utils/common';
10+
import { launch } from './browser';
1211
import { IPromptShowState } from './types';
1312

1413
// This situation occurs when user connects to old containers or server where
1514
// the debugger they had installed was ptvsd. We should show a prompt to ask them to update.
1615
class OutdatedDebuggerPrompt implements DebugAdapterTracker {
17-
constructor(
18-
private promptCheck: IPromptShowState,
19-
private appShell: IApplicationShell,
20-
private browserService: IBrowserService,
21-
) {}
16+
constructor(private promptCheck: IPromptShowState) {}
2217

2318
public onDidSendMessage(message: DebugProtocol.ProtocolMessage) {
2419
if (this.promptCheck.shouldShowPrompt() && this.isPtvsd(message)) {
2520
const prompts = [Common.moreInfo];
26-
this.appShell
27-
.showInformationMessage(OutdatedDebugger.outdatedDebuggerMessage, ...prompts)
28-
.then((selection) => {
29-
if (selection === prompts[0]) {
30-
this.browserService.launch('https://aka.ms/migrateToDebugpy');
31-
}
32-
});
21+
showInformationMessage(OutdatedDebugger.outdatedDebuggerMessage, ...prompts).then((selection) => {
22+
if (selection === prompts[0]) {
23+
launch('https://aka.ms/migrateToDebugpy');
24+
}
25+
});
3326
}
3427
}
3528

@@ -68,16 +61,12 @@ class OutdatedDebuggerPromptState implements IPromptShowState {
6861
}
6962
}
7063

71-
@injectable()
7264
export class OutdatedDebuggerPromptFactory implements DebugAdapterTrackerFactory {
7365
private readonly promptCheck: OutdatedDebuggerPromptState;
74-
constructor(
75-
@inject(IApplicationShell) private readonly appShell: IApplicationShell,
76-
@inject(IBrowserService) private browserService: IBrowserService,
77-
) {
66+
constructor() {
7867
this.promptCheck = new OutdatedDebuggerPromptState();
7968
}
8069
public createDebugAdapterTracker(_session: DebugSession): ProviderResult<DebugAdapterTracker> {
81-
return new OutdatedDebuggerPrompt(this.promptCheck, this.appShell, this.browserService);
70+
return new OutdatedDebuggerPrompt(this.promptCheck);
8271
}
8372
}

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

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66

77
'use strict';
88

9-
import { WorkspaceFolder } from 'vscode';
9+
import { WorkspaceFolder, window } from 'vscode';
1010
import { getWorkspaceFolder } from './workspaceFolder';
1111

1212
/**
@@ -38,3 +38,7 @@ export function resolveVariables(
3838
return match && (match.indexOf('env.') > 0 || match.indexOf('env:') > 0) ? '' : match;
3939
});
4040
}
41+
42+
export function showInformationMessage(message: string, ...items: string[]): Thenable<any> {
43+
return window.showInformationMessage(message, ...items);
44+
}

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

Lines changed: 39 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -4,23 +4,23 @@
44
'use strict';
55

66
import * as assert from 'assert';
7-
import { anyString, anything, instance, mock, verify, when } from 'ts-mockito';
7+
import * as sinon from 'sinon';
8+
import { anyString, mock, when } from 'ts-mockito';
89
import { DebugSession, WorkspaceFolder } from 'vscode';
910
import { DebugProtocol } from 'vscode-debugprotocol';
10-
import { ApplicationShell } from '../../../../client/common/application/applicationShell';
11-
import { IApplicationShell } from '../../../../client/common/application/types';
1211
import { ConfigurationService } from '../../../../client/common/configuration/service';
13-
import { BrowserService } from '../../../../client/common/net/browser';
14-
import { IBrowserService, IPythonSettings } from '../../../../client/common/types';
1512
import { createDeferred, sleep } from '../../../../client/common/utils/async';
1613
import { Common } from '../../../../client/common/utils/localize';
1714
import { OutdatedDebuggerPromptFactory } from '../../../../client/debugger/extension/adapter/outdatedDebuggerPrompt';
1815
import { clearTelemetryReporter } from '../../../../client/telemetry';
16+
import * as browser from '../../../../client/debugger/extension/adapter/browser';
17+
import * as common from '../../../../client/debugger/extension/configuration/utils/common';
18+
import { IPythonSettings } from '../../../../client/common/types';
1919

2020
suite('Debugging - Outdated Debugger Prompt tests.', () => {
2121
let promptFactory: OutdatedDebuggerPromptFactory;
22-
let appShell: IApplicationShell;
23-
let browserService: IBrowserService;
22+
let showInformationMessageStub: sinon.SinonStub;
23+
let browserLaunchStub: sinon.SinonStub;
2424

2525
const ptvsdOutputEvent: DebugProtocol.OutputEvent = {
2626
seq: 1,
@@ -42,12 +42,14 @@ suite('Debugging - Outdated Debugger Prompt tests.', () => {
4242
experiments: { enabled: true },
4343
} as any) as IPythonSettings);
4444

45-
appShell = mock(ApplicationShell);
46-
browserService = mock(BrowserService);
47-
promptFactory = new OutdatedDebuggerPromptFactory(instance(appShell), instance(browserService));
45+
showInformationMessageStub = sinon.stub(common, 'showInformationMessage');
46+
browserLaunchStub = sinon.stub(browser, 'launch');
47+
48+
promptFactory = new OutdatedDebuggerPromptFactory();
4849
});
4950

5051
teardown(() => {
52+
sinon.restore();
5153
clearTelemetryReporter();
5254
});
5355

@@ -67,33 +69,38 @@ suite('Debugging - Outdated Debugger Prompt tests.', () => {
6769
};
6870
}
6971

70-
test('Show prompt when attaching to ptvsd, more info is NOT clicked', async () => {
71-
when(appShell.showInformationMessage(anything(), anything())).thenReturn(Promise.resolve(undefined));
72-
72+
test.only('Show prompt when attaching to ptvsd, more info is NOT clicked', async () => {
73+
// when(appShell.showInformationMessage(anything(), anything())).thenReturn(Promise.resolve(undefined));
74+
showInformationMessageStub.returns(Promise.resolve(undefined));
7375
const session = createSession();
7476
const prompter = await promptFactory.createDebugAdapterTracker(session);
7577
if (prompter) {
7678
prompter.onDidSendMessage!(ptvsdOutputEvent);
7779
}
7880

79-
verify(browserService.launch(anyString())).never();
81+
browserLaunchStub.neverCalledWith(anyString());
82+
8083
// First call should show info once
81-
verify(appShell.showInformationMessage(anything(), anything())).once();
84+
85+
sinon.assert.calledOnce(showInformationMessageStub);
8286
assert(prompter);
8387

8488
prompter!.onDidSendMessage!(ptvsdOutputEvent);
8589
// Can't use deferred promise here
8690
await sleep(1);
8791

88-
verify(browserService.launch(anyString())).never();
92+
browserLaunchStub.neverCalledWith(anyString());
8993
// Second time it should not be called, so overall count is one.
90-
verify(appShell.showInformationMessage(anything(), anything())).once();
94+
sinon.assert.calledOnce(showInformationMessageStub);
9195
});
9296

9397
test('Show prompt when attaching to ptvsd, more info is clicked', async () => {
94-
when(appShell.showInformationMessage(anything(), anything())).thenReturn(Promise.resolve(Common.moreInfo));
98+
// when(appShell.showInformationMessage(anything(), anything())).thenReturn(Promise.resolve(Common.moreInfo));
99+
showInformationMessageStub.returns(Promise.resolve(Common.moreInfo));
100+
95101
const deferred = createDeferred();
96-
when(browserService.launch(anything())).thenCall(() => deferred.resolve());
102+
// when(browserService.launch(anything())).thenCall(() => deferred.resolve());
103+
browserLaunchStub.callsFake(() => deferred.resolve());
97104

98105
const session = createSession();
99106
const prompter = await promptFactory.createDebugAdapterTracker(session);
@@ -102,22 +109,26 @@ suite('Debugging - Outdated Debugger Prompt tests.', () => {
102109
prompter!.onDidSendMessage!(ptvsdOutputEvent);
103110
await deferred.promise;
104111

105-
verify(browserService.launch(anything())).once();
112+
// verify(browserService.launch(anything())).once();
113+
sinon.assert.calledOnce(browserLaunchStub);
114+
106115
// First call should show info once
107-
verify(appShell.showInformationMessage(anything(), anything())).once();
116+
// verify(appShell.showInformationMessage(anything(), anything())).once();
117+
sinon.assert.calledOnce(showInformationMessageStub);
108118

109119
prompter!.onDidSendMessage!(ptvsdOutputEvent);
110120
// The second call does not go through the same path. So we just give enough time for the
111121
// operation to complete.
112122
await sleep(1);
113123

114-
verify(browserService.launch(anyString())).once();
124+
// verify(browserService.launch(anyString())).once();
115125
// Second time it should not be called, so overall count is one.
116-
verify(appShell.showInformationMessage(anything(), anything())).once();
126+
// verify(appShell.showInformationMessage(anything(), anything())).once();
117127
});
118128

119129
test("Don't show prompt attaching to debugpy", async () => {
120-
when(appShell.showInformationMessage(anything(), anything())).thenReturn(Promise.resolve(undefined));
130+
// when(appShell.showInformationMessage(anything(), anything())).thenReturn(Promise.resolve(undefined));
131+
showInformationMessageStub.returns(Promise.resolve(undefined));
121132

122133
const session = createSession();
123134
const prompter = await promptFactory.createDebugAdapterTracker(session);
@@ -127,7 +138,7 @@ suite('Debugging - Outdated Debugger Prompt tests.', () => {
127138
// Can't use deferred promise here
128139
await sleep(1);
129140

130-
verify(appShell.showInformationMessage(anything(), anything())).never();
141+
// verify(appShell.showInformationMessage(anything(), anything())).never();
131142
});
132143

133144
const someRequest: DebugProtocol.RunInTerminalRequest = {
@@ -155,7 +166,8 @@ suite('Debugging - Outdated Debugger Prompt tests.', () => {
155166

156167
[someRequest, someEvent, someOutputEvent].forEach((message) => {
157168
test(`Don't show prompt when non-telemetry events are seen: ${JSON.stringify(message)}`, async () => {
158-
when(appShell.showInformationMessage(anything(), anything())).thenReturn(Promise.resolve(undefined));
169+
// when(appShell.showInformationMessage(anything(), anything())).thenReturn(Promise.resolve(undefined));
170+
showInformationMessageStub.returns(Promise.resolve(undefined));
159171

160172
const session = createSession();
161173
const prompter = await promptFactory.createDebugAdapterTracker(session);
@@ -165,7 +177,7 @@ suite('Debugging - Outdated Debugger Prompt tests.', () => {
165177
// Can't use deferred promise here
166178
await sleep(1);
167179

168-
verify(appShell.showInformationMessage(anything(), anything())).never();
180+
// verify(appShell.showInformationMessage(anything(), anything())).never();
169181
});
170182
});
171183
});

0 commit comments

Comments
 (0)