Skip to content

Commit 7b4bc29

Browse files
eleanorjboydeleanorjboyd
authored
Overload methods testing rewrite (#21082)
I now overload the method signature for discovery and execution so instead the only place where the new rewrite code needs to be enabled is in the controller where it calls either the old method signature without the`pythonExecFactory` or provides it to use the new code.
1 parent 77c63f1 commit 7b4bc29

File tree

6 files changed

+255
-240
lines changed

6 files changed

+255
-240
lines changed

‎src/client/testing/testController/common/types.ts

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,8 @@ import {
1212
Uri,
1313
WorkspaceFolder,
1414
} from 'vscode';
15-
// ** import { IPythonExecutionFactory } from '../../../common/process/types';
1615
import { TestDiscoveryOptions } from '../../common/types';
16+
import { IPythonExecutionFactory } from '../../../common/process/types';
1717

1818
export type TestRunInstanceOptions = TestRunOptions & {
1919
exclude?: readonly TestItem[];
@@ -179,21 +179,21 @@ export interface ITestServer {
179179
}
180180

181181
export interface ITestDiscoveryAdapter {
182-
// ** Uncomment second line and comment out first line to use the new discovery method.
182+
// ** first line old method signature, second line new method signature
183183
discoverTests(uri: Uri): Promise<DiscoveredTestPayload>;
184-
// discoverTests(uri: Uri, executionFactory: IPythonExecutionFactory): Promise<DiscoveredTestPayload>;
184+
discoverTests(uri: Uri, executionFactory: IPythonExecutionFactory): Promise<DiscoveredTestPayload>;
185185
}
186186

187187
// interface for execution/runner adapter
188188
export interface ITestExecutionAdapter {
189-
// ** Uncomment second line and comment out first line to use the new execution method.
189+
// ** first line old method signature, second line new method signature
190190
runTests(uri: Uri, testIds: string[], debugBool?: boolean): Promise<ExecutionTestPayload>;
191-
// runTests(
192-
// uri: Uri,
193-
// testIds: string[],
194-
// debugBool?: boolean,
195-
// executionFactory?: IPythonExecutionFactory,
196-
// ): Promise<ExecutionTestPayload>;
191+
runTests(
192+
uri: Uri,
193+
testIds: string[],
194+
debugBool?: boolean,
195+
executionFactory?: IPythonExecutionFactory,
196+
): Promise<ExecutionTestPayload>;
197197
}
198198

199199
// Same types as in pythonFiles/unittestadapter/utils.py

‎src/client/testing/testController/controller.ts

Lines changed: 62 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -241,39 +241,46 @@ export class PythonTestController implements ITestController, IExtensionSingleAc
241241
if (uri) {
242242
const settings = this.configSettings.getSettings(uri);
243243
traceVerbose(`Testing: Refreshing test data for ${uri.fsPath}`);
244+
const rewriteTestingEnabled = process.env.ENABLE_PYTHON_TESTING_REWRITE;
244245
if (settings.testing.pytestEnabled) {
245246
// Ensure we send test telemetry if it gets disabled again
246247
this.sendTestDisabledTelemetry = true;
247-
// ** uncomment ~231 - 241 to NEW new test discovery mechanism
248-
// const workspace = this.workspaceService.getWorkspaceFolder(uri);
249-
// traceVerbose(`Discover tests for workspace name: ${workspace?.name} - uri: ${uri.fsPath}`);
250-
// const testAdapter =
251-
// this.testAdapters.get(uri) || (this.testAdapters.values().next().value as WorkspaceTestAdapter);
252-
// testAdapter.discoverTests(
253-
// this.testController,
254-
// this.refreshCancellation.token,
255-
// this.testAdapters.size > 1,
256-
// this.workspaceService.workspaceFile?.fsPath,
257-
// this.pythonExecFactory,
258-
// );
259-
// uncomment ~243 to use OLD test discovery mechanism
260-
await this.pytest.refreshTestData(this.testController, uri, this.refreshCancellation.token);
248+
if (rewriteTestingEnabled) {
249+
// ** rewriteTestingEnabled set to true to use NEW test discovery mechanism
250+
const workspace = this.workspaceService.getWorkspaceFolder(uri);
251+
traceVerbose(`Discover tests for workspace name: ${workspace?.name} - uri: ${uri.fsPath}`);
252+
const testAdapter =
253+
this.testAdapters.get(uri) || (this.testAdapters.values().next().value as WorkspaceTestAdapter);
254+
testAdapter.discoverTests(
255+
this.testController,
256+
this.refreshCancellation.token,
257+
this.testAdapters.size > 1,
258+
this.workspaceService.workspaceFile?.fsPath,
259+
this.pythonExecFactory,
260+
);
261+
} else {
262+
// else use OLD test discovery mechanism
263+
await this.pytest.refreshTestData(this.testController, uri, this.refreshCancellation.token);
264+
}
261265
} else if (settings.testing.unittestEnabled) {
262266
// ** Ensure we send test telemetry if it gets disabled again
263267
this.sendTestDisabledTelemetry = true;
264-
// uncomment ~248 - 258 to NEW new test discovery mechanism
265-
// const workspace = this.workspaceService.getWorkspaceFolder(uri);
266-
// traceVerbose(`Discover tests for workspace name: ${workspace?.name} - uri: ${uri.fsPath}`);
267-
// const testAdapter =
268-
// this.testAdapters.get(uri) || (this.testAdapters.values().next().value as WorkspaceTestAdapter);
269-
// testAdapter.discoverTests(
270-
// this.testController,
271-
// this.refreshCancellation.token,
272-
// this.testAdapters.size > 1,
273-
// this.workspaceService.workspaceFile?.fsPath,
274-
// );
275-
// uncomment ~260 to use OLD test discovery mechanism
276-
await this.unittest.refreshTestData(this.testController, uri, this.refreshCancellation.token);
268+
if (rewriteTestingEnabled) {
269+
// ** rewriteTestingEnabled set to true to use NEW test discovery mechanism
270+
const workspace = this.workspaceService.getWorkspaceFolder(uri);
271+
traceVerbose(`Discover tests for workspace name: ${workspace?.name} - uri: ${uri.fsPath}`);
272+
const testAdapter =
273+
this.testAdapters.get(uri) || (this.testAdapters.values().next().value as WorkspaceTestAdapter);
274+
testAdapter.discoverTests(
275+
this.testController,
276+
this.refreshCancellation.token,
277+
this.testAdapters.size > 1,
278+
this.workspaceService.workspaceFile?.fsPath,
279+
);
280+
} else {
281+
// else use OLD test discovery mechanism
282+
await this.unittest.refreshTestData(this.testController, uri, this.refreshCancellation.token);
283+
}
277284
} else {
278285
if (this.sendTestDisabledTelemetry) {
279286
this.sendTestDisabledTelemetry = false;
@@ -384,25 +391,26 @@ export class PythonTestController implements ITestController, IExtensionSingleAc
384391

385392
const settings = this.configSettings.getSettings(workspace.uri);
386393
if (testItems.length > 0) {
394+
const rewriteTestingEnabled = process.env.ENABLE_PYTHON_TESTING_REWRITE;
387395
if (settings.testing.pytestEnabled) {
388396
sendTelemetryEvent(EventName.UNITTEST_RUN, undefined, {
389397
tool: 'pytest',
390398
debugging: request.profile?.kind === TestRunProfileKind.Debug,
391399
});
392-
// ** new execution runner/adapter
393-
// const testAdapter =
394-
// this.testAdapters.get(workspace.uri) ||
395-
// (this.testAdapters.values().next().value as WorkspaceTestAdapter);
396-
// return testAdapter.executeTests(
397-
// this.testController,
398-
// runInstance,
399-
// testItems,
400-
// token,
401-
// request.profile?.kind === TestRunProfileKind.Debug,
402-
// this.pythonExecFactory,
403-
// );
404-
405-
// below is old way of running pytest execution
400+
// ** rewriteTestingEnabled set to true to use NEW test discovery mechanism
401+
if (rewriteTestingEnabled) {
402+
const testAdapter =
403+
this.testAdapters.get(workspace.uri) ||
404+
(this.testAdapters.values().next().value as WorkspaceTestAdapter);
405+
return testAdapter.executeTests(
406+
this.testController,
407+
runInstance,
408+
testItems,
409+
token,
410+
request.profile?.kind === TestRunProfileKind.Debug,
411+
this.pythonExecFactory,
412+
);
413+
}
406414
return this.pytest.runTests(
407415
{
408416
includes: testItems,
@@ -415,23 +423,23 @@ export class PythonTestController implements ITestController, IExtensionSingleAc
415423
);
416424
}
417425
if (settings.testing.unittestEnabled) {
418-
// potentially squeeze in the new execution way here?
419426
sendTelemetryEvent(EventName.UNITTEST_RUN, undefined, {
420427
tool: 'unittest',
421428
debugging: request.profile?.kind === TestRunProfileKind.Debug,
422429
});
423-
// new execution runner/adapter
424-
// const testAdapter =
425-
// this.testAdapters.get(workspace.uri) ||
426-
// (this.testAdapters.values().next().value as WorkspaceTestAdapter);
427-
// return testAdapter.executeTests(
428-
// this.testController,
429-
// runInstance,
430-
// testItems,
431-
// token,
432-
// request.profile?.kind === TestRunProfileKind.Debug,
433-
// );
434-
430+
// ** rewriteTestingEnabled set to true to use NEW test discovery mechanism
431+
if (rewriteTestingEnabled) {
432+
const testAdapter =
433+
this.testAdapters.get(workspace.uri) ||
434+
(this.testAdapters.values().next().value as WorkspaceTestAdapter);
435+
return testAdapter.executeTests(
436+
this.testController,
437+
runInstance,
438+
testItems,
439+
token,
440+
request.profile?.kind === TestRunProfileKind.Debug,
441+
);
442+
}
435443
// below is old way of running unittest execution
436444
return this.unittest.runTests(
437445
{

‎src/client/testing/testController/pytest/pytestDiscoveryAdapter.ts

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -37,20 +37,19 @@ export class PytestTestDiscoveryAdapter implements ITestDiscoveryAdapter {
3737
}
3838
}
3939

40-
// ** Old version of discover tests.
41-
discoverTests(uri: Uri): Promise<DiscoveredTestPayload> {
40+
discoverTests(uri: Uri, executionFactory?: IPythonExecutionFactory): Promise<DiscoveredTestPayload> {
41+
if (executionFactory !== undefined) {
42+
// ** new version of discover tests.
43+
const settings = this.configSettings.getSettings(uri);
44+
const { pytestArgs } = settings.testing;
45+
traceVerbose(pytestArgs);
46+
return this.runPytestDiscovery(uri, executionFactory);
47+
}
48+
// if executionFactory is undefined, we are using the old method signature of discover tests.
4249
traceVerbose(uri);
4350
this.deferred = createDeferred<DiscoveredTestPayload>();
4451
return this.deferred.promise;
4552
}
46-
// Uncomment this version of the function discoverTests to use the new discovery method.
47-
// public async discoverTests(uri: Uri, executionFactory: IPythonExecutionFactory): Promise<DiscoveredTestPayload> {
48-
// const settings = this.configSettings.getSettings(uri);
49-
// const { pytestArgs } = settings.testing;
50-
// traceVerbose(pytestArgs);
51-
52-
// return this.runPytestDiscovery(uri, executionFactory);
53-
// }
5453

5554
async runPytestDiscovery(uri: Uri, executionFactory: IPythonExecutionFactory): Promise<DiscoveredTestPayload> {
5655
const deferred = createDeferred<DiscoveredTestPayload>();

‎src/client/testing/testController/pytest/pytestExecutionAdapter.ts

Lines changed: 61 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,17 @@
22
// Licensed under the MIT License.
33

44
import { Uri } from 'vscode';
5+
import path from 'path';
56
import { IConfigurationService, ITestOutputChannel } from '../../../common/types';
67
import { createDeferred, Deferred } from '../../../common/utils/async';
78
import { traceVerbose } from '../../../logging';
89
import { DataReceivedEvent, ExecutionTestPayload, ITestExecutionAdapter, ITestServer } from '../common/types';
10+
import {
11+
ExecutionFactoryCreateWithEnvironmentOptions,
12+
IPythonExecutionFactory,
13+
SpawnOptions,
14+
} from '../../../common/process/types';
15+
import { EXTENSION_ROOT_DIR } from '../../../constants';
916

1017
/**
1118
* Wrapper Class for pytest test execution. This is where we call `runTestCommand`?
@@ -32,70 +39,68 @@ export class PytestTestExecutionAdapter implements ITestExecutionAdapter {
3239
}
3340
}
3441

35-
// ** Old version of discover tests.
36-
async runTests(uri: Uri, testIds: string[], debugBool?: boolean): Promise<ExecutionTestPayload> {
42+
async runTests(
43+
uri: Uri,
44+
testIds: string[],
45+
debugBool?: boolean,
46+
executionFactory?: IPythonExecutionFactory,
47+
): Promise<ExecutionTestPayload> {
3748
traceVerbose(uri, testIds, debugBool);
38-
// TODO:Remove this line after enabling runs
49+
if (executionFactory !== undefined) {
50+
// ** new version of run tests.
51+
return this.runTestsNew(uri, testIds, debugBool, executionFactory);
52+
}
53+
// if executionFactory is undefined, we are using the old method signature of run tests.
3954
this.outputChannel.appendLine('Running tests.');
4055
this.deferred = createDeferred<ExecutionTestPayload>();
4156
return this.deferred.promise;
4257
}
43-
}
44-
45-
// public async runTests(
46-
// uri: Uri,
47-
// testIds: string[],
48-
// debugBool?: boolean,
49-
// executionFactory?: IPythonExecutionFactory,
50-
// ): Promise<ExecutionTestPayload> {
51-
// const deferred = createDeferred<ExecutionTestPayload>();
52-
// const relativePathToPytest = 'pythonFiles';
53-
// const fullPluginPath = path.join(EXTENSION_ROOT_DIR, relativePathToPytest);
54-
// this.configSettings.isTestExecution();
55-
// const uuid = this.testServer.createUUID(uri.fsPath);
56-
// this.promiseMap.set(uuid, deferred);
57-
// const settings = this.configSettings.getSettings(uri);
58-
// const { pytestArgs } = settings.testing;
59-
60-
// const pythonPathParts: string[] = process.env.PYTHONPATH?.split(path.delimiter) ?? [];
61-
// const pythonPathCommand = [fullPluginPath, ...pythonPathParts].join(path.delimiter);
6258

63-
// const spawnOptions: SpawnOptions = {
64-
// cwd: uri.fsPath,
65-
// throwOnStdErr: true,
66-
// extraVariables: {
67-
// PYTHONPATH: pythonPathCommand,
68-
// TEST_UUID: uuid.toString(),
69-
// TEST_PORT: this.testServer.getPort().toString(),
70-
// },
71-
// outputChannel: this.outputChannel,
72-
// };
59+
private async runTestsNew(
60+
uri: Uri,
61+
testIds: string[],
62+
debugBool?: boolean,
63+
executionFactory?: IPythonExecutionFactory,
64+
): Promise<ExecutionTestPayload> {
65+
const deferred = createDeferred<ExecutionTestPayload>();
66+
const relativePathToPytest = 'pythonFiles';
67+
const fullPluginPath = path.join(EXTENSION_ROOT_DIR, relativePathToPytest);
68+
this.configSettings.isTestExecution();
69+
const uuid = this.testServer.createUUID(uri.fsPath);
70+
this.promiseMap.set(uuid, deferred);
71+
const settings = this.configSettings.getSettings(uri);
72+
const { pytestArgs } = settings.testing;
7373

74-
// // Create the Python environment in which to execute the command.
75-
// const creationOptions: ExecutionFactoryCreateWithEnvironmentOptions = {
76-
// allowEnvironmentFetchExceptions: false,
77-
// resource: uri,
78-
// };
79-
// // need to check what will happen in the exec service is NOT defined and is null
80-
// const execService = await executionFactory?.createActivatedEnvironment(creationOptions);
74+
const pythonPathParts: string[] = process.env.PYTHONPATH?.split(path.delimiter) ?? [];
75+
const pythonPathCommand = [fullPluginPath, ...pythonPathParts].join(path.delimiter);
8176

82-
// const testIdsString = testIds.join(' ');
83-
// console.debug('what to do with debug bool?', debugBool);
84-
// try {
85-
// execService?.exec(['-m', 'pytest', '-p', 'vscode_pytest', testIdsString].concat(pytestArgs), spawnOptions);
86-
// } catch (ex) {
87-
// console.error(ex);
88-
// }
77+
const spawnOptions: SpawnOptions = {
78+
cwd: uri.fsPath,
79+
throwOnStdErr: true,
80+
extraVariables: {
81+
PYTHONPATH: pythonPathCommand,
82+
TEST_UUID: uuid.toString(),
83+
TEST_PORT: this.testServer.getPort().toString(),
84+
},
85+
outputChannel: this.outputChannel,
86+
};
8987

90-
// return deferred.promise;
91-
// }
92-
// }
88+
// Create the Python environment in which to execute the command.
89+
const creationOptions: ExecutionFactoryCreateWithEnvironmentOptions = {
90+
allowEnvironmentFetchExceptions: false,
91+
resource: uri,
92+
};
93+
// need to check what will happen in the exec service is NOT defined and is null
94+
const execService = await executionFactory?.createActivatedEnvironment(creationOptions);
9395

94-
// function buildExecutionCommand(args: string[]): TestExecutionCommand {
95-
// const executionScript = path.join(EXTENSION_ROOT_DIR, 'pythonFiles', 'unittestadapter', 'execution.py');
96+
const testIdsString = testIds.join(' ');
97+
console.debug('what to do with debug bool?', debugBool);
98+
try {
99+
execService?.exec(['-m', 'pytest', '-p', 'vscode_pytest', testIdsString].concat(pytestArgs), spawnOptions);
100+
} catch (ex) {
101+
console.error(ex);
102+
}
96103

97-
// return {
98-
// script: executionScript,
99-
// args: ['--udiscovery', ...args],
100-
// };
101-
// }
104+
return deferred.promise;
105+
}
106+
}

0 commit comments

Comments
 (0)