Skip to content

Commit

Permalink
Add unit tests
Browse files Browse the repository at this point in the history
  • Loading branch information
Kartik Raj committed Oct 18, 2023
1 parent b71fb93 commit 7649be7
Show file tree
Hide file tree
Showing 4 changed files with 56 additions and 116 deletions.
5 changes: 1 addition & 4 deletions src/client/common/utils/localize.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ export namespace Common {
export const noIWillDoItLater = l10n.t('No, I will do it later');
export const notNow = l10n.t('Not now');
export const doNotShowAgain = l10n.t("Don't show again");
export const editSomething = l10n.t('Edit {0}');
export const reload = l10n.t('Reload');
export const moreInfo = l10n.t('More Info');
export const learnMore = l10n.t('Learn more');
Expand Down Expand Up @@ -200,12 +201,8 @@ export namespace Interpreters {
);
export const terminalDeactivateProgress = l10n.t('Editing {0}...');
export const terminalDeactivatePrompt = l10n.t(
'Deactivating virtual environments may not work by default due to a technical limitation in our activation approach, but it can be resolved with a few simple steps.',
);
export const terminalDeactivateShellSpecificPrompt = l10n.t(
'Deactivating virtual environments may not work by default due to a technical limitation in our activation approach, but it can be resolved by appending a line to "{0}". Be sure to restart the shell afterward. [Learn more](https://aka.ms/AAmx2ft).',
);
export const deactivateDoneButton = l10n.t('Done, it works');
export const activatedCondaEnvLaunch = l10n.t(
'We noticed VS Code was launched from an activated conda environment, would you like to select it?',
);
Expand Down
18 changes: 0 additions & 18 deletions src/client/telemetry/index.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1291,24 +1291,6 @@ export interface IEventNamePropertyMapping {
*/
selection: 'Allow' | 'Close' | undefined;
};
/**
* Telemetry event sent with details when user clicks the prompt with the following message:
*
* 'Deactivating virtual environments may not work by default due to a technical limitation in our activation approach, but it can be resolved with a few simple steps.'
*/
/* __GDPR__
"terminal_deactivate_prompt" : {
"selection" : { "classification": "SystemMetaData", "purpose": "FeatureInsight", "owner": "karrtikr" }
}
*/
[EventName.TERMINAL_DEACTIVATE_PROMPT]: {
/**
* `See Instructions` When 'See Instructions' option is selected
* `Done, it works` When 'Done, it works' option is selected
* `Don't show again` When 'Don't show again' option is selected
*/
selection: 'See Instructions' | 'Done, it works' | "Don't show again" | undefined;
};
/**
* Telemetry event sent with details when user attempts to run in interactive window when Jupyter is not installed.
*/
Expand Down
47 changes: 7 additions & 40 deletions src/client/terminals/envCollectionActivation/deactivatePrompt.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,7 @@
import { inject, injectable } from 'inversify';
import { Position, TextDocument, Uri, WorkspaceEdit, Range, TextEditorRevealType, ProgressLocation } from 'vscode';
import { IApplicationEnvironment, IApplicationShell, IDocumentManager } from '../../common/application/types';
import {
IBrowserService,
IDisposableRegistry,
IExperimentService,
IPersistentState,
IPersistentStateFactory,
} from '../../common/types';
import { IDisposableRegistry, IExperimentService, IPersistentStateFactory } from '../../common/types';
import { Common, Interpreters } from '../../common/utils/localize';
import { IExtensionSingleActivationService } from '../../activation/types';
import { inTerminalEnvVarExperiment } from '../../common/experiments/helpers';
Expand All @@ -20,8 +14,6 @@ import { identifyShellFromShellPath } from '../../common/terminal/shellDetectors
import { TerminalShellType } from '../../common/terminal/types';
import { IFileSystem } from '../../common/platform/types';
import { traceError } from '../../logging';
import { sendTelemetryEvent } from '../../telemetry';
import { EventName } from '../../telemetry/constants';
import { shellExec } from '../../common/process/rawProcessApis';
import { createDeferred, sleep } from '../../common/utils/async';
import { getDeactivateShellInfo } from './deactivateScripts';
Expand All @@ -42,15 +34,13 @@ export class TerminalDeactivateLimitationPrompt implements IExtensionSingleActiv
@inject(IPersistentStateFactory) private readonly persistentStateFactory: IPersistentStateFactory,
@inject(IDisposableRegistry) private readonly disposableRegistry: IDisposableRegistry,
@inject(IInterpreterService) private readonly interpreterService: IInterpreterService,
@inject(IBrowserService) private readonly browserService: IBrowserService,
@inject(IApplicationEnvironment) private readonly appEnvironment: IApplicationEnvironment,
@inject(IFileSystem) private readonly fs: IFileSystem,
@inject(IDocumentManager) private readonly documentManager: IDocumentManager,
@inject(IApplicationShell) private readonly shell: IApplicationShell,
@inject(IExperimentService) private readonly experimentService: IExperimentService,
) {
this.codeCLI = this.appEnvironment.channel === 'insiders' ? 'code-insiders' : 'code';
this.progressService = new ProgressService(this.shell);
this.progressService = new ProgressService(this.appShell);
}

public async activate(): Promise<void> {
Expand Down Expand Up @@ -80,12 +70,12 @@ export class TerminalDeactivateLimitationPrompt implements IExtensionSingleActiv
if (interpreter?.type !== PythonEnvType.Virtual) {
return;
}
await this.notifyUsers(shellType).catch((ex) => traceError('Deactivate prompt failed', ex));
await this._notifyUsers(shellType).catch((ex) => traceError('Deactivate prompt failed', ex));
}),
);
}

private async notifyUsers(shellType: TerminalShellType): Promise<void> {
public async _notifyUsers(shellType: TerminalShellType): Promise<void> {
const notificationPromptEnabled = this.persistentStateFactory.createGlobalPersistentState(
`${terminalDeactivationPromptKey}-${shellType}`,
true,
Expand All @@ -95,13 +85,13 @@ export class TerminalDeactivateLimitationPrompt implements IExtensionSingleActiv
}
const scriptInfo = getDeactivateShellInfo(shellType);
if (!scriptInfo) {
await this.showGeneralNotification(notificationPromptEnabled);
// Shell integration is not supported for these shells, in which case this workaround won't work.
return;
}
const { initScript, source, destination } = scriptInfo;
const prompts = [`Edit ${initScript.displayName}`, Common.doNotShowAgain];
const prompts = [Common.editSomething.format(initScript.displayName), Common.doNotShowAgain];
const selection = await this.appShell.showWarningMessage(
Interpreters.terminalDeactivateShellSpecificPrompt.format(initScript.displayName),
Interpreters.terminalDeactivatePrompt.format(initScript.displayName),
...prompts,
);
if (!selection) {
Expand Down Expand Up @@ -151,27 +141,4 @@ ${content}
await shellExec(`${this.codeCLI} ${scriptPath}`, { shell: this.appEnvironment.shell });
return deferred.promise;
}

private async showGeneralNotification(notificationPromptEnabled: IPersistentState<boolean>): Promise<void> {
const prompts = [Common.seeInstructions, Interpreters.deactivateDoneButton, Common.doNotShowAgain];
const telemetrySelections: ['See Instructions', 'Done, it works', "Don't show again"] = [
'See Instructions',
'Done, it works',
"Don't show again",
];
const selection = await this.appShell.showWarningMessage(Interpreters.terminalDeactivatePrompt, ...prompts);
if (!selection) {
return;
}
sendTelemetryEvent(EventName.TERMINAL_DEACTIVATE_PROMPT, undefined, {
selection: selection ? telemetrySelections[prompts.indexOf(selection)] : undefined,
});
if (selection === prompts[0]) {
const url = `https://aka.ms/AAmx2ft`;
this.browserService.launch(url);
}
if (selection === prompts[1] || selection === prompts[2]) {
await notificationPromptEnabled.updateValue(false);
}
}
}
102 changes: 48 additions & 54 deletions src/test/terminals/envCollectionActivation/deactivatePrompt.unit.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,10 @@
'use strict';

import { mock, when, anything, instance, verify, reset } from 'ts-mockito';
import { EventEmitter, Terminal, TerminalDataWriteEvent, Uri } from 'vscode';
import { IApplicationEnvironment, IApplicationShell } from '../../../client/common/application/types';
import {
IBrowserService,
IExperimentService,
IPersistentState,
IPersistentStateFactory,
} from '../../../client/common/types';
import { EventEmitter, Terminal, TerminalDataWriteEvent, TextDocument, TextEditor, Uri } from 'vscode';
import * as sinon from 'sinon';
import { IApplicationEnvironment, IApplicationShell, IDocumentManager } from '../../../client/common/application/types';
import { IExperimentService, IPersistentState, IPersistentStateFactory } from '../../../client/common/types';
import { Common, Interpreters } from '../../../client/common/utils/localize';
import { TerminalEnvVarActivation } from '../../../client/common/experiments/groups';
import { sleep } from '../../core';
Expand All @@ -20,6 +16,8 @@ import { PythonEnvironment } from '../../../client/pythonEnvironments/info';
import { TerminalDeactivateLimitationPrompt } from '../../../client/terminals/envCollectionActivation/deactivatePrompt';
import { PythonEnvType } from '../../../client/pythonEnvironments/base/info';
import { TerminalShellType } from '../../../client/common/terminal/types';
import { IFileSystem } from '../../../client/common/platform/types';
import * as processApi from '../../../client/common/process/rawProcessApis';

suite('Terminal Deactivation Limitation Prompt', () => {
let shell: IApplicationShell;
Expand All @@ -29,19 +27,37 @@ suite('Terminal Deactivation Limitation Prompt', () => {
let deactivatePrompt: TerminalDeactivateLimitationPrompt;
let terminalWriteEvent: EventEmitter<TerminalDataWriteEvent>;
let notificationEnabled: IPersistentState<boolean>;
let browserService: IBrowserService;
let interpreterService: IInterpreterService;
const prompts = [Common.seeInstructions, Interpreters.deactivateDoneButton, Common.doNotShowAgain];
const expectedMessage = Interpreters.terminalDeactivatePrompt;
let documentManager: IDocumentManager;
let fs: IFileSystem;
const prompts = [Common.editSomething.format('~/.bashrc'), Common.doNotShowAgain];
const expectedMessage = Interpreters.terminalDeactivatePrompt.format('~/.bashrc');

setup(async () => {
const activeEditorEvent = new EventEmitter<TextEditor | undefined>();
const document = ({
uri: Uri.file(''),
getText: () => '',
} as unknown) as TextDocument;
sinon.stub(processApi, 'shellExec').callsFake(async (command: string) => {
if (command !== 'code ~/.bashrc') {
throw new Error(`Unexpected command: ${command}`);
}
sleep(1500).then(() => {
activeEditorEvent.fire(undefined);
activeEditorEvent.fire({ document } as TextEditor);
});
return { stdout: '' };
});
fs = mock<IFileSystem>();
documentManager = mock<IDocumentManager>();
when(documentManager.onDidChangeActiveTextEditor).thenReturn(activeEditorEvent.event);
shell = mock<IApplicationShell>();
interpreterService = mock<IInterpreterService>();
experimentService = mock<IExperimentService>();
persistentStateFactory = mock<IPersistentStateFactory>();
appEnvironment = mock<IApplicationEnvironment>();
when(appEnvironment.shell).thenReturn('bash');
browserService = mock<IBrowserService>();
notificationEnabled = mock<IPersistentState<boolean>>();
terminalWriteEvent = new EventEmitter<TerminalDataWriteEvent>();
when(persistentStateFactory.createGlobalPersistentState(anything(), true)).thenReturn(
Expand All @@ -54,8 +70,9 @@ suite('Terminal Deactivation Limitation Prompt', () => {
instance(persistentStateFactory),
[],
instance(interpreterService),
instance(browserService),
instance(appEnvironment),
instance(fs),
instance(documentManager),
instance(experimentService),
);
});
Expand Down Expand Up @@ -99,7 +116,7 @@ suite('Terminal Deactivation Limitation Prompt', () => {
terminalWriteEvent.fire({ data: 'Please deactivate me', terminal });
await sleep(1);

verify(shell.showWarningMessage(expectedMessage, ...prompts)).once();
verify(shell.showWarningMessage(expectedMessage, ...prompts)).never();
});

test('When not in experiment, do not show notification for the same', async () => {
Expand Down Expand Up @@ -184,47 +201,25 @@ suite('Terminal Deactivation Limitation Prompt', () => {
verify(notificationEnabled.updateValue(false)).once();
});

test('Disable notification if `Done, it works` is clicked', async () => {
const resource = Uri.file('a');
const terminal = ({
creationOptions: {
cwd: resource,
},
} as unknown) as Terminal;
when(notificationEnabled.value).thenReturn(true);
when(interpreterService.getActiveInterpreter(anything())).thenResolve(({
type: PythonEnvType.Virtual,
} as unknown) as PythonEnvironment);
when(shell.showWarningMessage(expectedMessage, ...prompts)).thenReturn(
Promise.resolve(Interpreters.deactivateDoneButton),
);

await deactivatePrompt.activate();
terminalWriteEvent.fire({ data: 'Please deactivate me', terminal });
await sleep(1);

verify(notificationEnabled.updateValue(false)).once();
});

test('Open link to workaround if `See instructions` is clicked', async () => {
const resource = Uri.file('a');
const terminal = ({
creationOptions: {
cwd: resource,
},
} as unknown) as Terminal;
test('Edit script correctly if `Edit <script>` button is clicked', async () => {
when(notificationEnabled.value).thenReturn(true);
when(interpreterService.getActiveInterpreter(anything())).thenResolve(({
type: PythonEnvType.Virtual,
} as unknown) as PythonEnvironment);
when(shell.showWarningMessage(expectedMessage, ...prompts)).thenReturn(Promise.resolve(Common.seeInstructions));

await deactivatePrompt.activate();
terminalWriteEvent.fire({ data: 'Please deactivate me', terminal });
await sleep(1);

when(shell.showWarningMessage(expectedMessage, ...prompts)).thenReturn(Promise.resolve(prompts[0]));
when(fs.copyFile(anything(), anything())).thenResolve();
when(shell.withProgress(anything(), anything())).thenResolve();
const editor = mock<TextEditor>();
// eslint-disable-next-line @typescript-eslint/no-explicit-any
when((editor as any).then).thenReturn(undefined);
when(documentManager.showTextDocument(anything())).thenReturn(Promise.resolve(editor));
when(editor.revealRange(anything(), anything())).thenReturn();
when(documentManager.applyEdit(anything())).thenReturn();

await deactivatePrompt._notifyUsers(TerminalShellType.bash);

verify(fs.copyFile(anything(), anything())).once();
verify(shell.withProgress(anything(), anything())).once();
verify(shell.showWarningMessage(expectedMessage, ...prompts)).once();
verify(browserService.launch(anything())).once();
verify(notificationEnabled.updateValue(false)).once();
verify(documentManager.applyEdit(anything())).once();
});

test('Do not perform any action if prompt is closed', async () => {
Expand All @@ -246,6 +241,5 @@ suite('Terminal Deactivation Limitation Prompt', () => {

verify(shell.showWarningMessage(expectedMessage, ...prompts)).once();
verify(notificationEnabled.updateValue(false)).never();
verify(browserService.launch(anything())).never();
});
});

0 comments on commit 7649be7

Please sign in to comment.