Skip to content

Commit

Permalink
Allow pytest to use correct interpreter from getActiveInterpreter (#2…
Browse files Browse the repository at this point in the history
…4250)

Resolves: #24122
Related: #24190,
#24127

I think the culprit was we were not passing in interpreter when we call
createActivatedEnvironment.
  • Loading branch information
anthonykim1 authored and karthiknadig committed Oct 7, 2024
1 parent 7513198 commit ff65066
Show file tree
Hide file tree
Showing 5 changed files with 30 additions and 4 deletions.
8 changes: 7 additions & 1 deletion src/client/testing/testController/common/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import {
import { ITestDebugLauncher, TestDiscoveryOptions } from '../../common/types';
import { IPythonExecutionFactory } from '../../../common/process/types';
import { EnvironmentVariables } from '../../../common/variables/types';
import { PythonEnvironment } from '../../../pythonEnvironments/info';

export type TestRunInstanceOptions = TestRunOptions & {
exclude?: readonly TestItem[];
Expand Down Expand Up @@ -206,7 +207,11 @@ export interface ITestResultResolver {
export interface ITestDiscoveryAdapter {
// ** first line old method signature, second line new method signature
discoverTests(uri: Uri): Promise<DiscoveredTestPayload>;
discoverTests(uri: Uri, executionFactory: IPythonExecutionFactory): Promise<DiscoveredTestPayload>;
discoverTests(
uri: Uri,
executionFactory: IPythonExecutionFactory,
interpreter?: PythonEnvironment,
): Promise<DiscoveredTestPayload>;
}

// interface for execution/runner adapter
Expand All @@ -220,6 +225,7 @@ export interface ITestExecutionAdapter {
runInstance?: TestRun,
executionFactory?: IPythonExecutionFactory,
debugLauncher?: ITestDebugLauncher,
interpreter?: PythonEnvironment,
): Promise<ExecutionTestPayload>;
}

Expand Down
4 changes: 4 additions & 0 deletions src/client/testing/testController/controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -276,6 +276,7 @@ export class PythonTestController implements ITestController, IExtensionSingleAc
this.testController,
this.refreshCancellation.token,
this.pythonExecFactory,
await this.interpreterService.getActiveInterpreter(workspace.uri),
);
} else {
traceError('Unable to find test adapter for workspace.');
Expand All @@ -297,6 +298,7 @@ export class PythonTestController implements ITestController, IExtensionSingleAc
this.testController,
this.refreshCancellation.token,
this.pythonExecFactory,
await this.interpreterService.getActiveInterpreter(workspace.uri),
);
} else {
traceError('Unable to find test adapter for workspace.');
Expand Down Expand Up @@ -455,6 +457,7 @@ export class PythonTestController implements ITestController, IExtensionSingleAc
request.profile?.kind,
this.pythonExecFactory,
this.debugLauncher,
await this.interpreterService.getActiveInterpreter(workspace.uri),
);
}
return this.pytest.runTests(
Expand Down Expand Up @@ -483,6 +486,7 @@ export class PythonTestController implements ITestController, IExtensionSingleAc
request.profile?.kind,
this.pythonExecFactory,
this.debugLauncher,
await this.interpreterService.getActiveInterpreter(workspace.uri),
);
}
// below is old way of running unittest execution
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import {
hasSymlinkParent,
} from '../common/utils';
import { IEnvironmentVariablesProvider } from '../../../common/variables/types';
import { PythonEnvironment } from '../../../pythonEnvironments/info';

/**
* Wrapper class for unittest test discovery. This is where we call `runTestCommand`. #this seems incorrectly copied
Expand All @@ -35,13 +36,17 @@ export class PytestTestDiscoveryAdapter implements ITestDiscoveryAdapter {
private readonly envVarsService?: IEnvironmentVariablesProvider,
) {}

async discoverTests(uri: Uri, executionFactory?: IPythonExecutionFactory): Promise<DiscoveredTestPayload> {
async discoverTests(
uri: Uri,
executionFactory?: IPythonExecutionFactory,
interpreter?: PythonEnvironment,
): Promise<DiscoveredTestPayload> {
const { name, dispose } = await startDiscoveryNamedPipe((data: DiscoveredTestPayload) => {
this.resultResolver?.resolveDiscovery(data);
});

try {
await this.runPytestDiscovery(uri, name, executionFactory);
await this.runPytestDiscovery(uri, name, executionFactory, interpreter);
} finally {
dispose();
}
Expand All @@ -54,6 +59,7 @@ export class PytestTestDiscoveryAdapter implements ITestDiscoveryAdapter {
uri: Uri,
discoveryPipeName: string,
executionFactory?: IPythonExecutionFactory,
interpreter?: PythonEnvironment,
): Promise<void> {
const relativePathToPytest = 'python_files';
const fullPluginPath = path.join(EXTENSION_ROOT_DIR, relativePathToPytest);
Expand Down Expand Up @@ -100,6 +106,7 @@ export class PytestTestDiscoveryAdapter implements ITestDiscoveryAdapter {
const creationOptions: ExecutionFactoryCreateWithEnvironmentOptions = {
allowEnvironmentFetchExceptions: false,
resource: uri,
interpreter,
};
const execService = await executionFactory?.createActivatedEnvironment(creationOptions);
// delete UUID following entire discovery finishing.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import { PYTEST_PROVIDER } from '../../common/constants';
import { EXTENSION_ROOT_DIR } from '../../../common/constants';
import * as utils from '../common/utils';
import { IEnvironmentVariablesProvider } from '../../../common/variables/types';
import { PythonEnvironment } from '../../../pythonEnvironments/info';

export class PytestTestExecutionAdapter implements ITestExecutionAdapter {
constructor(
Expand All @@ -35,6 +36,7 @@ export class PytestTestExecutionAdapter implements ITestExecutionAdapter {
runInstance?: TestRun,
executionFactory?: IPythonExecutionFactory,
debugLauncher?: ITestDebugLauncher,
interpreter?: PythonEnvironment,
): Promise<ExecutionTestPayload> {
const deferredTillServerClose: Deferred<void> = utils.createTestingDeferred();

Expand Down Expand Up @@ -74,6 +76,7 @@ export class PytestTestExecutionAdapter implements ITestExecutionAdapter {
profileKind,
executionFactory,
debugLauncher,
interpreter,
);
} finally {
await deferredTillServerClose.promise;
Expand All @@ -98,6 +101,7 @@ export class PytestTestExecutionAdapter implements ITestExecutionAdapter {
profileKind?: TestRunProfileKind,
executionFactory?: IPythonExecutionFactory,
debugLauncher?: ITestDebugLauncher,
interpreter?: PythonEnvironment,
): Promise<ExecutionTestPayload> {
const relativePathToPytest = 'python_files';
const fullPluginPath = path.join(EXTENSION_ROOT_DIR, relativePathToPytest);
Expand All @@ -122,6 +126,7 @@ export class PytestTestExecutionAdapter implements ITestExecutionAdapter {
const creationOptions: ExecutionFactoryCreateWithEnvironmentOptions = {
allowEnvironmentFetchExceptions: false,
resource: uri,
interpreter,
};
// need to check what will happen in the exec service is NOT defined and is null
const execService = await executionFactory?.createActivatedEnvironment(creationOptions);
Expand Down
6 changes: 5 additions & 1 deletion src/client/testing/testController/workspaceTestAdapter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import { ITestDiscoveryAdapter, ITestExecutionAdapter, ITestResultResolver } fro
import { IPythonExecutionFactory } from '../../common/process/types';
import { ITestDebugLauncher } from '../common/types';
import { buildErrorNodeOptions } from './common/utils';
import { PythonEnvironment } from '../../pythonEnvironments/info';

/**
* This class exposes a test-provider-agnostic way of discovering tests.
Expand Down Expand Up @@ -45,6 +46,7 @@ export class WorkspaceTestAdapter {
profileKind?: boolean | TestRunProfileKind,
executionFactory?: IPythonExecutionFactory,
debugLauncher?: ITestDebugLauncher,
interpreter?: PythonEnvironment,
): Promise<void> {
if (this.executing) {
traceError('Test execution already in progress, not starting a new one.');
Expand Down Expand Up @@ -80,6 +82,7 @@ export class WorkspaceTestAdapter {
runInstance,
executionFactory,
debugLauncher,
interpreter,
);
} else {
await this.executionAdapter.runTests(this.workspaceUri, testCaseIds, profileKind);
Expand Down Expand Up @@ -115,6 +118,7 @@ export class WorkspaceTestAdapter {
testController: TestController,
token?: CancellationToken,
executionFactory?: IPythonExecutionFactory,
interpreter?: PythonEnvironment,
): Promise<void> {
sendTelemetryEvent(EventName.UNITTEST_DISCOVERING, undefined, { tool: this.testProvider });

Expand All @@ -130,7 +134,7 @@ export class WorkspaceTestAdapter {
try {
// ** execution factory only defined for new rewrite way
if (executionFactory !== undefined) {
await this.discoveryAdapter.discoverTests(this.workspaceUri, executionFactory);
await this.discoveryAdapter.discoverTests(this.workspaceUri, executionFactory, interpreter);
} else {
await this.discoveryAdapter.discoverTests(this.workspaceUri);
}
Expand Down

0 comments on commit ff65066

Please sign in to comment.