Skip to content

Don't prompt to update inheritEnv when on Windows #7901

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Oct 10, 2019
Merged

Don't prompt to update inheritEnv when on Windows #7901

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b9b5e25
Don't prompt inheritEnv on windows
kimadeline Oct 10, 2019
37a39e3
Add verification for undefined workspace config
kimadeline Oct 10, 2019
bd549d8
Why did i commit that formatting change
kimadeline Oct 10, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion package.nls.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@
"Experiments.inGroup": "User belongs to experiment group '{0}'",
"Interpreters.RefreshingInterpreters": "Refreshing Python Interpreters",
"Interpreters.LoadingInterpreters": "Loading Python Interpreters",
"Interpreters.condaInheritEnvMessage": "We noticed you're using a conda environment. If you are experiencing issues with this environment in the integrated terminal, we suggest the \"terminal.integrated.inheritEnv\" setting to be changed to false. Would you like to update this setting?",
"Interpreters.condaInheritEnvMessage": "We noticed you're using a conda environment. If you are experiencing issues with this environment in the integrated terminal, we recommend that you let the Python extension change \"terminal.integrated.inheritEnv\" to false in your user settings.",
"Logging.CurrentWorkingDirectory": "cwd:",
"Common.doNotShowAgain": "Do not show again",
"Common.reload": "Reload",
Expand Down
2 changes: 1 addition & 1 deletion src/client/common/utils/localize.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ export namespace Experiments {
export namespace Interpreters {
export const loading = localize('Interpreters.LoadingInterpreters', 'Loading Python Interpreters');
export const refreshing = localize('Interpreters.RefreshingInterpreters', 'Refreshing Python Interpreters');
export const condaInheritEnvMessage = localize('Interpreters.condaInheritEnvMessage', 'We noticed you\'re using a conda environment. If you are experiencing issues with this environment in the integrated terminal, we suggest the \"terminal.integrated.inheritEnv\" setting to be changed to false. Would you like to update this setting?');
export const condaInheritEnvMessage = localize('Interpreters.condaInheritEnvMessage', 'We noticed you\'re using a conda environment. If you are experiencing issues with this environment in the integrated terminal, we recommend that you let the Python extension change \"terminal.integrated.inheritEnv\" to false in your user settings.');
export const environmentPromptMessage = localize('Interpreters.environmentPromptMessage', 'We noticed a new virtual environment has been created. Do you want to select it for the workspace folder?');
export const selectInterpreterTip = localize('Interpreters.selectInterpreterTip', 'Tip: you can change the Python interpreter used by the Python extension by clicking on the Python version in the status bar');
}
Expand Down
7 changes: 6 additions & 1 deletion src/client/interpreter/virtualEnvs/condaInheritEnvPrompt.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import { ConfigurationTarget, Uri } from 'vscode';
import { IExtensionActivationService } from '../../activation/types';
import { IApplicationShell, IWorkspaceService } from '../../common/application/types';
import { traceDecorators, traceError } from '../../common/logger';
import { IPlatformService } from '../../common/platform/types';
import { IBrowserService, IPersistentStateFactory } from '../../common/types';
import { Common, InteractiveShiftEnterBanner, Interpreters } from '../../common/utils/localize';
import { sendTelemetryEvent } from '../../telemetry';
Expand All @@ -22,8 +23,9 @@ export class CondaInheritEnvPrompt implements IExtensionActivationService {
@inject(IBrowserService) private browserService: IBrowserService,
@inject(IApplicationShell) private readonly appShell: IApplicationShell,
@inject(IPersistentStateFactory) private readonly persistentStateFactory: IPersistentStateFactory,
@inject(IPlatformService) private readonly platformService: IPlatformService,
@optional() public hasPromptBeenShownInCurrentSession: boolean = false
) { }
) {}

public async activate(resource: Uri): Promise<void> {
this.initializeInBackground(resource).ignoreErrors();
Expand Down Expand Up @@ -65,6 +67,9 @@ export class CondaInheritEnvPrompt implements IExtensionActivationService {
if (this.hasPromptBeenShownInCurrentSession) {
return false;
}
if (this.platformService.isWindows) {
return false;
}
const interpreter = await this.interpreterService.getActiveInterpreter(resource);
if (!interpreter || interpreter.type !== InterpreterType.Conda) {
return false;
Expand Down
103 changes: 95 additions & 8 deletions src/test/interpreters/virtualEnvs/condaInheritEnvPrompt.unit.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import * as TypeMoq from 'typemoq';
import { ConfigurationTarget, Uri, WorkspaceConfiguration } from 'vscode';
import { IApplicationShell, IWorkspaceService } from '../../../client/common/application/types';
import { PersistentStateFactory } from '../../../client/common/persistentState';
import { IPlatformService } from '../../../client/common/platform/types';
import { IBrowserService, IPersistentState, IPersistentStateFactory } from '../../../client/common/types';
import { createDeferred, createDeferredFromPromise, sleep } from '../../../client/common/utils/async';
import { Common, InteractiveShiftEnterBanner, Interpreters } from '../../../client/common/utils/localize';
Expand All @@ -24,6 +25,7 @@ suite('Conda Inherit Env Prompt', async () => {
let workspaceService: TypeMoq.IMock<IWorkspaceService>;
let appShell: TypeMoq.IMock<IApplicationShell>;
let interpreterService: TypeMoq.IMock<IInterpreterService>;
let platformService: TypeMoq.IMock<IPlatformService>;
let browserService: TypeMoq.IMock<IBrowserService>;
let persistentStateFactory: IPersistentStateFactory;
let notificationPromptEnabled: TypeMoq.IMock<IPersistentState<any>>;
Expand All @@ -41,10 +43,26 @@ suite('Conda Inherit Env Prompt', async () => {
browserService = TypeMoq.Mock.ofType<IBrowserService>();
interpreterService = TypeMoq.Mock.ofType<IInterpreterService>();
persistentStateFactory = mock(PersistentStateFactory);
condaInheritEnvPrompt = new CondaInheritEnvPrompt(interpreterService.object, workspaceService.object, browserService.object, appShell.object, instance(persistentStateFactory));
platformService = TypeMoq.Mock.ofType<IPlatformService>();
condaInheritEnvPrompt = new CondaInheritEnvPrompt(
interpreterService.object,
workspaceService.object,
browserService.object,
appShell.object,
instance(persistentStateFactory),
platformService.object
);
});
test('Returns false if prompt has already been shown in the current session', async () => {
condaInheritEnvPrompt = new CondaInheritEnvPrompt(interpreterService.object, workspaceService.object, browserService.object, appShell.object, instance(persistentStateFactory), true);
condaInheritEnvPrompt = new CondaInheritEnvPrompt(
interpreterService.object,
workspaceService.object,
browserService.object,
appShell.object,
instance(persistentStateFactory),
platformService.object,
true
);
const workspaceConfig = TypeMoq.Mock.ofType<WorkspaceConfiguration>();
interpreterService
.setup(is => is.getActiveInterpreter(resource))
Expand All @@ -59,11 +77,25 @@ suite('Conda Inherit Env Prompt', async () => {
expect(condaInheritEnvPrompt.hasPromptBeenShownInCurrentSession).to.equal(true, 'Should be true');
verifyAll();
});
test('Returns false if on Windows', async () => {
platformService
.setup(ps => ps.isWindows)
.returns(() => true)
.verifiable(TypeMoq.Times.once());
const result = await condaInheritEnvPrompt.shouldShowPrompt(resource);
expect(result).to.equal(false, 'Prompt should not be shown');
expect(condaInheritEnvPrompt.hasPromptBeenShownInCurrentSession).to.equal(false, 'Should be false');
verifyAll();
});
test('Returns false if active interpreter is not of type Conda', async () => {
const interpreter = {
type: InterpreterType.Pipenv
};
const workspaceConfig = TypeMoq.Mock.ofType<WorkspaceConfiguration>();
platformService
.setup(ps => ps.isWindows)
.returns(() => false)
.verifiable(TypeMoq.Times.once());
interpreterService
.setup(is => is.getActiveInterpreter(resource))
.returns(() => Promise.resolve(interpreter) as any)
Expand All @@ -79,6 +111,10 @@ suite('Conda Inherit Env Prompt', async () => {
});
test('Returns false if no active interpreter is present', async () => {
const workspaceConfig = TypeMoq.Mock.ofType<WorkspaceConfiguration>();
platformService
.setup(ps => ps.isWindows)
.returns(() => false)
.verifiable(TypeMoq.Times.once());
interpreterService
.setup(is => is.getActiveInterpreter(resource))
.returns(() => Promise.resolve(undefined))
Expand All @@ -97,6 +133,10 @@ suite('Conda Inherit Env Prompt', async () => {
type: InterpreterType.Conda
};
const workspaceConfig = TypeMoq.Mock.ofType<WorkspaceConfiguration>();
platformService
.setup(ps => ps.isWindows)
.returns(() => false)
.verifiable(TypeMoq.Times.once());
interpreterService
.setup(is => is.getActiveInterpreter(resource))
.returns(() => Promise.resolve(interpreter) as any)
Expand All @@ -107,7 +147,8 @@ suite('Conda Inherit Env Prompt', async () => {
.verifiable(TypeMoq.Times.once());
workspaceConfig
.setup(ws => ws.inspect<boolean>('integrated.inheritEnv'))
.returns(() => undefined);
.returns(() => undefined)
.verifiable(TypeMoq.Times.once());
const result = await condaInheritEnvPrompt.shouldShowPrompt(resource);
expect(result).to.equal(false, 'Prompt should not be shown');
expect(condaInheritEnvPrompt.hasPromptBeenShownInCurrentSession).to.equal(false, 'Should be false');
Expand Down Expand Up @@ -144,6 +185,10 @@ suite('Conda Inherit Env Prompt', async () => {
type: InterpreterType.Conda
};
const workspaceConfig = TypeMoq.Mock.ofType<WorkspaceConfiguration>();
platformService
.setup(ps => ps.isWindows)
.returns(() => false)
.verifiable(TypeMoq.Times.once());
interpreterService
.setup(is => is.getActiveInterpreter(resource))
.returns(() => Promise.resolve(interpreter) as any)
Expand Down Expand Up @@ -171,6 +216,10 @@ suite('Conda Inherit Env Prompt', async () => {
workspaceFolderValue: undefined
};
const workspaceConfig = TypeMoq.Mock.ofType<WorkspaceConfiguration>();
platformService
.setup(ps => ps.isWindows)
.returns(() => false)
.verifiable(TypeMoq.Times.once());
interpreterService
.setup(is => is.getActiveInterpreter(resource))
.returns(() => Promise.resolve(interpreter) as any)
Expand All @@ -196,6 +245,7 @@ suite('Conda Inherit Env Prompt', async () => {
browserService = TypeMoq.Mock.ofType<IBrowserService>();
interpreterService = TypeMoq.Mock.ofType<IInterpreterService>();
persistentStateFactory = mock(PersistentStateFactory);
platformService = TypeMoq.Mock.ofType<IPlatformService>();
});

teardown(() => {
Expand All @@ -206,7 +256,14 @@ suite('Conda Inherit Env Prompt', async () => {
const initializeInBackgroundDeferred = createDeferred<void>();
initializeInBackground = sinon.stub(CondaInheritEnvPrompt.prototype, 'initializeInBackground');
initializeInBackground.callsFake(() => initializeInBackgroundDeferred.promise);
condaInheritEnvPrompt = new CondaInheritEnvPrompt(interpreterService.object, workspaceService.object, browserService.object, appShell.object, instance(persistentStateFactory));
condaInheritEnvPrompt = new CondaInheritEnvPrompt(
interpreterService.object,
workspaceService.object,
browserService.object,
appShell.object,
instance(persistentStateFactory),
platformService.object
);

const promise = condaInheritEnvPrompt.activate(resource);
const deferred = createDeferredFromPromise(promise);
Expand All @@ -223,7 +280,14 @@ suite('Conda Inherit Env Prompt', async () => {
test('Ignores errors raised by initializeInBackground()', async () => {
initializeInBackground = sinon.stub(CondaInheritEnvPrompt.prototype, 'initializeInBackground');
initializeInBackground.rejects(new Error('Kaboom'));
condaInheritEnvPrompt = new CondaInheritEnvPrompt(interpreterService.object, workspaceService.object, browserService.object, appShell.object, instance(persistentStateFactory));
condaInheritEnvPrompt = new CondaInheritEnvPrompt(
interpreterService.object,
workspaceService.object,
browserService.object,
appShell.object,
instance(persistentStateFactory),
platformService.object
);
await condaInheritEnvPrompt.activate(resource);
assert.ok(initializeInBackground.calledOnce);
});
Expand All @@ -238,6 +302,7 @@ suite('Conda Inherit Env Prompt', async () => {
browserService = TypeMoq.Mock.ofType<IBrowserService>();
interpreterService = TypeMoq.Mock.ofType<IInterpreterService>();
persistentStateFactory = mock(PersistentStateFactory);
platformService = TypeMoq.Mock.ofType<IPlatformService>();
});

teardown(() => {
Expand All @@ -249,7 +314,14 @@ suite('Conda Inherit Env Prompt', async () => {
shouldShowPrompt.callsFake(() => Promise.resolve(true));
promptAndUpdate = sinon.stub(CondaInheritEnvPrompt.prototype, 'promptAndUpdate');
promptAndUpdate.callsFake(() => Promise.resolve(undefined));
condaInheritEnvPrompt = new CondaInheritEnvPrompt(interpreterService.object, workspaceService.object, browserService.object, appShell.object, instance(persistentStateFactory));
condaInheritEnvPrompt = new CondaInheritEnvPrompt(
interpreterService.object,
workspaceService.object,
browserService.object,
appShell.object,
instance(persistentStateFactory),
platformService.object
);
await condaInheritEnvPrompt.initializeInBackground(resource);
assert.ok(shouldShowPrompt.calledOnce);
assert.ok(promptAndUpdate.calledOnce);
Expand All @@ -260,7 +332,14 @@ suite('Conda Inherit Env Prompt', async () => {
shouldShowPrompt.callsFake(() => Promise.resolve(false));
promptAndUpdate = sinon.stub(CondaInheritEnvPrompt.prototype, 'promptAndUpdate');
promptAndUpdate.callsFake(() => Promise.resolve(undefined));
condaInheritEnvPrompt = new CondaInheritEnvPrompt(interpreterService.object, workspaceService.object, browserService.object, appShell.object, instance(persistentStateFactory));
condaInheritEnvPrompt = new CondaInheritEnvPrompt(
interpreterService.object,
workspaceService.object,
browserService.object,
appShell.object,
instance(persistentStateFactory),
platformService.object
);
await condaInheritEnvPrompt.initializeInBackground(resource);
assert.ok(shouldShowPrompt.calledOnce);
assert.ok(promptAndUpdate.notCalled);
Expand All @@ -276,8 +355,16 @@ suite('Conda Inherit Env Prompt', async () => {
persistentStateFactory = mock(PersistentStateFactory);
browserService = TypeMoq.Mock.ofType<IBrowserService>();
notificationPromptEnabled = TypeMoq.Mock.ofType<IPersistentState<any>>();
platformService = TypeMoq.Mock.ofType<IPlatformService>();
when(persistentStateFactory.createGlobalPersistentState(condaInheritEnvPromptKey, true)).thenReturn(notificationPromptEnabled.object);
condaInheritEnvPrompt = new CondaInheritEnvPrompt(interpreterService.object, workspaceService.object, browserService.object, appShell.object, instance(persistentStateFactory));
condaInheritEnvPrompt = new CondaInheritEnvPrompt(
interpreterService.object,
workspaceService.object,
browserService.object,
appShell.object,
instance(persistentStateFactory),
platformService.object
);
});

test('Does not display prompt if it is disabled', async () => {
Expand Down