Skip to content

Commit 81f3397

Browse files
Copiloteleanorjboydanthonykim1
authored
Enhance pytest installation flow and error handling with Environment Extension integration (#25252)
## Overview This PR addresses the issue where pytest configuration attempts would proceed without user confirmation when pytest is not installed, and provides better error messages when pytest installation issues occur. ## Changes Made ### 1. Enhanced User Prompt for pytest Installation **Before**: Extension would silently attempt to install pytest without user input. **After**: Shows a user-friendly prompt when pytest is selected but not installed: ``` pytest selected but not installed. Would you like to install pytest? [Install pytest] [Ignore] ``` ### 2. Python Environments Extension Integration When the Python Environments extension is available: - Uses the `managePackages` API for proper environment-targeted installation - Ensures pytest is installed in the correct Python environment - Provides better integration with the extension ecosystem **New Class**: `PytestInstallationHelper` handles the enhanced installation flow with fallback to traditional installer when the environment extension is not available. ## Technical Implementation - **New**: `src/client/testing/configuration/pytestInstallationHelper.ts` - Handles enhanced installation flow - **Enhanced**: `src/client/testing/configuration/pytest/testConfigurationManager.ts` - Integrates new installation helper - **Enhanced**: `src/client/testing/testController/common/utils.ts` - Improved error message detection - **Comprehensive test coverage** with unit tests for all scenarios Fixes #[25251](#25251). also fixes #17772 --------- Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: eleanorjboyd <[email protected]> Co-authored-by: anthonykim1 <[email protected]>
1 parent 285734a commit 81f3397

File tree

5 files changed

+322
-4
lines changed

5 files changed

+322
-4
lines changed

src/client/testing/configuration/pytest/testConfigurationManager.ts

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,12 +3,19 @@ import { QuickPickItem, Uri } from 'vscode';
33
import { IFileSystem } from '../../../common/platform/types';
44
import { Product } from '../../../common/types';
55
import { IServiceContainer } from '../../../ioc/types';
6+
import { IApplicationShell } from '../../../common/application/types';
67
import { TestConfigurationManager } from '../../common/testConfigurationManager';
78
import { ITestConfigSettingsService } from '../../common/types';
9+
import { PytestInstallationHelper } from '../pytestInstallationHelper';
10+
import { traceInfo } from '../../../logging';
811

912
export class ConfigurationManager extends TestConfigurationManager {
13+
private readonly pytestInstallationHelper: PytestInstallationHelper;
14+
1015
constructor(workspace: Uri, serviceContainer: IServiceContainer, cfg?: ITestConfigSettingsService) {
1116
super(workspace, Product.pytest, serviceContainer, cfg);
17+
const appShell = serviceContainer.get<IApplicationShell>(IApplicationShell);
18+
this.pytestInstallationHelper = new PytestInstallationHelper(appShell);
1219
}
1320

1421
public async requiresUserToConfigure(wkspace: Uri): Promise<boolean> {
@@ -42,10 +49,22 @@ export class ConfigurationManager extends TestConfigurationManager {
4249
args.push(testDir);
4350
}
4451
const installed = await this.installer.isInstalled(Product.pytest);
52+
await this.testConfigSettingsService.updateTestArgs(wkspace.fsPath, Product.pytest, args);
4553
if (!installed) {
46-
await this.installer.install(Product.pytest);
54+
// Check if Python Environments extension is available for enhanced installation flow
55+
if (this.pytestInstallationHelper.isEnvExtensionAvailable()) {
56+
traceInfo('pytest not installed, prompting user with environment extension integration');
57+
const installAttempted = await this.pytestInstallationHelper.promptToInstallPytest(wkspace);
58+
if (!installAttempted) {
59+
// User chose to ignore or installation failed
60+
return;
61+
}
62+
} else {
63+
// Fall back to traditional installer
64+
traceInfo('pytest not installed, falling back to traditional installer');
65+
await this.installer.install(Product.pytest);
66+
}
4767
}
48-
await this.testConfigSettingsService.updateTestArgs(wkspace.fsPath, Product.pytest, args);
4968
}
5069

5170
private async getConfigFiles(rootDir: string): Promise<string[]> {
Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,95 @@
1+
// Copyright (c) Microsoft Corporation. All rights reserved.
2+
// Licensed under the MIT License.
3+
4+
import { Uri, l10n } from 'vscode';
5+
import { IApplicationShell } from '../../common/application/types';
6+
import { traceInfo, traceError } from '../../logging';
7+
import { useEnvExtension, getEnvExtApi } from '../../envExt/api.internal';
8+
import { getEnvironment } from '../../envExt/api.internal';
9+
10+
/**
11+
* Helper class to handle pytest installation using the appropriate method
12+
* based on whether the Python Environments extension is available.
13+
*/
14+
export class PytestInstallationHelper {
15+
constructor(private readonly appShell: IApplicationShell) {}
16+
17+
/**
18+
* Prompts the user to install pytest with appropriate installation method.
19+
* @param workspaceUri The workspace URI where pytest should be installed
20+
* @returns Promise that resolves to true if installation was attempted, false otherwise
21+
*/
22+
async promptToInstallPytest(workspaceUri: Uri): Promise<boolean> {
23+
const message = l10n.t('pytest selected but not installed. Would you like to install pytest?');
24+
const installOption = l10n.t('Install pytest');
25+
26+
const selection = await this.appShell.showInformationMessage(message, { modal: true }, installOption);
27+
28+
if (selection === installOption) {
29+
return this.installPytest(workspaceUri);
30+
}
31+
32+
return false;
33+
}
34+
35+
/**
36+
* Installs pytest using the appropriate method based on available extensions.
37+
* @param workspaceUri The workspace URI where pytest should be installed
38+
* @returns Promise that resolves to true if installation was successful, false otherwise
39+
*/
40+
private async installPytest(workspaceUri: Uri): Promise<boolean> {
41+
try {
42+
if (useEnvExtension()) {
43+
return this.installPytestWithEnvExtension(workspaceUri);
44+
} else {
45+
// Fall back to traditional installer if environments extension is not available
46+
traceInfo(
47+
'Python Environments extension not available, installation cannot proceed via environment extension',
48+
);
49+
return false;
50+
}
51+
} catch (error) {
52+
traceError('Error installing pytest:', error);
53+
return false;
54+
}
55+
}
56+
57+
/**
58+
* Installs pytest using the Python Environments extension.
59+
* @param workspaceUri The workspace URI where pytest should be installed
60+
* @returns Promise that resolves to true if installation was successful, false otherwise
61+
*/
62+
private async installPytestWithEnvExtension(workspaceUri: Uri): Promise<boolean> {
63+
try {
64+
const envExtApi = await getEnvExtApi();
65+
const environment = await getEnvironment(workspaceUri);
66+
67+
if (!environment) {
68+
traceError('No Python environment found for workspace:', workspaceUri.fsPath);
69+
await this.appShell.showErrorMessage(
70+
l10n.t('No Python environment found. Please set up a Python environment first.'),
71+
);
72+
return false;
73+
}
74+
75+
traceInfo('Installing pytest using Python Environments extension...');
76+
await envExtApi.managePackages(environment, {
77+
install: ['pytest'],
78+
});
79+
80+
traceInfo('pytest installation completed successfully');
81+
return true;
82+
} catch (error) {
83+
traceError('Failed to install pytest using Python Environments extension:', error);
84+
return false;
85+
}
86+
}
87+
88+
/**
89+
* Checks if the Python Environments extension is available for package management.
90+
* @returns True if the extension is available, false otherwise
91+
*/
92+
isEnvExtensionAvailable(): boolean {
93+
return useEnvExtension();
94+
}
95+
}

src/client/testing/testController/common/utils.ts

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -174,12 +174,34 @@ export async function startDiscoveryNamedPipe(
174174
return pipeName;
175175
}
176176

177+
/**
178+
* Detects if an error message indicates that pytest is not installed.
179+
* @param message The error message to check
180+
* @returns True if the error indicates pytest is not installed
181+
*/
182+
function isPytestNotInstalledError(message: string): boolean {
183+
return (
184+
(message.includes('ModuleNotFoundError') && message.includes('pytest')) ||
185+
(message.includes('No module named') && message.includes('pytest')) ||
186+
(message.includes('ImportError') && message.includes('pytest'))
187+
);
188+
}
189+
177190
export function buildErrorNodeOptions(uri: Uri, message: string, testType: string): ErrorTestItemOptions {
178-
const labelText = testType === 'pytest' ? 'pytest Discovery Error' : 'Unittest Discovery Error';
191+
let labelText = testType === 'pytest' ? 'pytest Discovery Error' : 'Unittest Discovery Error';
192+
let errorMessage = message;
193+
194+
// Provide more specific error message if pytest is not installed
195+
if (testType === 'pytest' && isPytestNotInstalledError(message)) {
196+
labelText = 'pytest Not Installed';
197+
errorMessage =
198+
'pytest is not installed in the selected Python environment. Please install pytest to enable test discovery and execution.';
199+
}
200+
179201
return {
180202
id: `DiscoveryError:${uri.fsPath}`,
181203
label: `${labelText} [${path.basename(uri.fsPath)}]`,
182-
error: message,
204+
error: errorMessage,
183205
};
184206
}
185207

Lines changed: 131 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,131 @@
1+
// Copyright (c) Microsoft Corporation. All rights reserved.
2+
// Licensed under the MIT License.
3+
4+
import { expect } from 'chai';
5+
import * as sinon from 'sinon';
6+
import { Uri } from 'vscode';
7+
import * as TypeMoq from 'typemoq';
8+
import { IApplicationShell } from '../../../client/common/application/types';
9+
import { PytestInstallationHelper } from '../../../client/testing/configuration/pytestInstallationHelper';
10+
import * as envExtApi from '../../../client/envExt/api.internal';
11+
12+
suite('PytestInstallationHelper', () => {
13+
let appShell: TypeMoq.IMock<IApplicationShell>;
14+
let helper: PytestInstallationHelper;
15+
let useEnvExtensionStub: sinon.SinonStub;
16+
let getEnvExtApiStub: sinon.SinonStub;
17+
let getEnvironmentStub: sinon.SinonStub;
18+
19+
const workspaceUri = Uri.file('/test/workspace');
20+
21+
setup(() => {
22+
appShell = TypeMoq.Mock.ofType<IApplicationShell>();
23+
helper = new PytestInstallationHelper(appShell.object);
24+
25+
useEnvExtensionStub = sinon.stub(envExtApi, 'useEnvExtension');
26+
getEnvExtApiStub = sinon.stub(envExtApi, 'getEnvExtApi');
27+
getEnvironmentStub = sinon.stub(envExtApi, 'getEnvironment');
28+
});
29+
30+
teardown(() => {
31+
sinon.restore();
32+
});
33+
34+
test('promptToInstallPytest should return false if user selects ignore', async () => {
35+
appShell
36+
.setup((a) =>
37+
a.showInformationMessage(
38+
TypeMoq.It.isAny(),
39+
TypeMoq.It.isAny(),
40+
TypeMoq.It.isAny(),
41+
TypeMoq.It.isAny(),
42+
),
43+
)
44+
.returns(() => Promise.resolve('Ignore'))
45+
.verifiable(TypeMoq.Times.once());
46+
47+
const result = await helper.promptToInstallPytest(workspaceUri);
48+
49+
expect(result).to.be.false;
50+
appShell.verifyAll();
51+
});
52+
53+
test('promptToInstallPytest should return false if user cancels', async () => {
54+
appShell
55+
.setup((a) =>
56+
a.showInformationMessage(
57+
TypeMoq.It.isAny(),
58+
TypeMoq.It.isAny(),
59+
TypeMoq.It.isAny(),
60+
TypeMoq.It.isAny(),
61+
),
62+
)
63+
.returns(() => Promise.resolve(undefined))
64+
.verifiable(TypeMoq.Times.once());
65+
66+
const result = await helper.promptToInstallPytest(workspaceUri);
67+
68+
expect(result).to.be.false;
69+
appShell.verifyAll();
70+
});
71+
72+
test('isEnvExtensionAvailable should return result from useEnvExtension', () => {
73+
useEnvExtensionStub.returns(true);
74+
75+
const result = helper.isEnvExtensionAvailable();
76+
77+
expect(result).to.be.true;
78+
expect(useEnvExtensionStub.calledOnce).to.be.true;
79+
});
80+
81+
test('promptToInstallPytest should return false if env extension not available', async () => {
82+
useEnvExtensionStub.returns(false);
83+
84+
appShell
85+
.setup((a) =>
86+
a.showInformationMessage(
87+
TypeMoq.It.isAny(),
88+
TypeMoq.It.isAny(),
89+
TypeMoq.It.isAny(),
90+
TypeMoq.It.isAny(),
91+
),
92+
)
93+
.returns(() => Promise.resolve('Install pytest'))
94+
.verifiable(TypeMoq.Times.once());
95+
96+
const result = await helper.promptToInstallPytest(workspaceUri);
97+
98+
expect(result).to.be.false;
99+
appShell.verifyAll();
100+
});
101+
102+
test('promptToInstallPytest should attempt installation when env extension is available', async () => {
103+
useEnvExtensionStub.returns(true);
104+
105+
const mockEnvironment = { envId: { id: 'test-env', managerId: 'test-manager' } };
106+
const mockEnvExtApi = {
107+
managePackages: sinon.stub().resolves(),
108+
};
109+
110+
getEnvExtApiStub.resolves(mockEnvExtApi);
111+
getEnvironmentStub.resolves(mockEnvironment);
112+
113+
appShell
114+
.setup((a) =>
115+
a.showInformationMessage(
116+
TypeMoq.It.is((msg: string) => msg.includes('pytest selected but not installed')),
117+
TypeMoq.It.isAny(),
118+
TypeMoq.It.isAny(),
119+
TypeMoq.It.isAny(),
120+
),
121+
)
122+
.returns(() => Promise.resolve('Install pytest'))
123+
.verifiable(TypeMoq.Times.once());
124+
125+
const result = await helper.promptToInstallPytest(workspaceUri);
126+
127+
expect(result).to.be.true;
128+
expect(mockEnvExtApi.managePackages.calledOnceWithExactly(mockEnvironment, { install: ['pytest'] })).to.be.true;
129+
appShell.verifyAll();
130+
});
131+
});
Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
// Copyright (c) Microsoft Corporation. All rights reserved.
2+
// Licensed under the MIT License.
3+
4+
import { expect } from 'chai';
5+
import { Uri } from 'vscode';
6+
import { buildErrorNodeOptions } from '../../../../client/testing/testController/common/utils';
7+
8+
suite('buildErrorNodeOptions - pytest not installed detection', () => {
9+
const workspaceUri = Uri.file('/test/workspace');
10+
11+
test('Should detect pytest ModuleNotFoundError and provide specific message', () => {
12+
const errorMessage =
13+
'Traceback (most recent call last):\n File "<string>", line 1, in <module>\n import pytest\nModuleNotFoundError: No module named \'pytest\'';
14+
15+
const result = buildErrorNodeOptions(workspaceUri, errorMessage, 'pytest');
16+
17+
expect(result.label).to.equal('pytest Not Installed [workspace]');
18+
expect(result.error).to.equal(
19+
'pytest is not installed in the selected Python environment. Please install pytest to enable test discovery and execution.',
20+
);
21+
});
22+
23+
test('Should detect pytest ImportError and provide specific message', () => {
24+
const errorMessage = 'ImportError: No module named pytest';
25+
26+
const result = buildErrorNodeOptions(workspaceUri, errorMessage, 'pytest');
27+
28+
expect(result.label).to.equal('pytest Not Installed [workspace]');
29+
expect(result.error).to.equal(
30+
'pytest is not installed in the selected Python environment. Please install pytest to enable test discovery and execution.',
31+
);
32+
});
33+
34+
test('Should use generic error for non-pytest-related errors', () => {
35+
const errorMessage = 'Some other error occurred';
36+
37+
const result = buildErrorNodeOptions(workspaceUri, errorMessage, 'pytest');
38+
39+
expect(result.label).to.equal('pytest Discovery Error [workspace]');
40+
expect(result.error).to.equal('Some other error occurred');
41+
});
42+
43+
test('Should use generic error for unittest errors', () => {
44+
const errorMessage = "ModuleNotFoundError: No module named 'pytest'";
45+
46+
const result = buildErrorNodeOptions(workspaceUri, errorMessage, 'unittest');
47+
48+
expect(result.label).to.equal('Unittest Discovery Error [workspace]');
49+
expect(result.error).to.equal("ModuleNotFoundError: No module named 'pytest'");
50+
});
51+
});

0 commit comments

Comments
 (0)