Skip to content

Commit cd771ca

Browse files
eleanorjboydeleanorjboyd
authored
Refactor test and discovery methods to remove overload (#25572)
1 parent 2ce21a0 commit cd771ca

File tree

8 files changed

+83
-84
lines changed

8 files changed

+83
-84
lines changed

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

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -155,26 +155,22 @@ export interface ITestResultResolver {
155155
_resolveCoverage(payload: CoveragePayload, runInstance: TestRun): void;
156156
}
157157
export interface ITestDiscoveryAdapter {
158-
// ** first line old method signature, second line new method signature
159-
discoverTests(uri: Uri): Promise<void>;
160158
discoverTests(
161159
uri: Uri,
162-
executionFactory?: IPythonExecutionFactory,
160+
executionFactory: IPythonExecutionFactory,
163161
token?: CancellationToken,
164162
interpreter?: PythonEnvironment,
165163
): Promise<void>;
166164
}
167165

168166
// interface for execution/runner adapter
169167
export interface ITestExecutionAdapter {
170-
// ** first line old method signature, second line new method signature
171-
runTests(uri: Uri, testIds: string[], profileKind?: boolean | TestRunProfileKind): Promise<void>;
172168
runTests(
173169
uri: Uri,
174170
testIds: string[],
175-
profileKind?: boolean | TestRunProfileKind,
176-
runInstance?: TestRun,
177-
executionFactory?: IPythonExecutionFactory,
171+
profileKind: boolean | TestRunProfileKind | undefined,
172+
runInstance: TestRun,
173+
executionFactory: IPythonExecutionFactory,
178174
debugLauncher?: ITestDebugLauncher,
179175
interpreter?: PythonEnvironment,
180176
): Promise<void>;

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -276,8 +276,8 @@ export class PythonTestController implements ITestController, IExtensionSingleAc
276276
}
277277
await testAdapter.discoverTests(
278278
this.testController,
279-
this.refreshCancellation.token,
280279
this.pythonExecFactory,
280+
this.refreshCancellation.token,
281281
await this.interpreterService.getActiveInterpreter(workspace.uri),
282282
);
283283
} else {
@@ -302,8 +302,8 @@ export class PythonTestController implements ITestController, IExtensionSingleAc
302302
}
303303
await testAdapter.discoverTests(
304304
this.testController,
305-
this.refreshCancellation.token,
306305
this.pythonExecFactory,
306+
this.refreshCancellation.token,
307307
await this.interpreterService.getActiveInterpreter(workspace.uri),
308308
);
309309
} else {
@@ -453,9 +453,9 @@ export class PythonTestController implements ITestController, IExtensionSingleAc
453453
this.testController,
454454
runInstance,
455455
testItems,
456+
this.pythonExecFactory,
456457
token,
457458
request.profile?.kind,
458-
this.pythonExecFactory,
459459
this.debugLauncher,
460460
await this.interpreterService.getActiveInterpreter(workspace.uri),
461461
);
@@ -470,9 +470,9 @@ export class PythonTestController implements ITestController, IExtensionSingleAc
470470
this.testController,
471471
runInstance,
472472
testItems,
473+
this.pythonExecFactory,
473474
token,
474475
request.profile?.kind,
475-
this.pythonExecFactory,
476476
this.debugLauncher,
477477
await this.interpreterService.getActiveInterpreter(workspace.uri),
478478
);

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ export class PytestTestDiscoveryAdapter implements ITestDiscoveryAdapter {
3838

3939
async discoverTests(
4040
uri: Uri,
41-
executionFactory?: IPythonExecutionFactory,
41+
executionFactory: IPythonExecutionFactory,
4242
token?: CancellationToken,
4343
interpreter?: PythonEnvironment,
4444
): Promise<void> {
@@ -69,7 +69,7 @@ export class PytestTestDiscoveryAdapter implements ITestDiscoveryAdapter {
6969
uri: Uri,
7070
discoveryPipeName: string,
7171
cSource: CancellationTokenSource,
72-
executionFactory?: IPythonExecutionFactory,
72+
executionFactory: IPythonExecutionFactory,
7373
interpreter?: PythonEnvironment,
7474
token?: CancellationToken,
7575
): Promise<void> {
@@ -170,7 +170,7 @@ export class PytestTestDiscoveryAdapter implements ITestDiscoveryAdapter {
170170
resource: uri,
171171
interpreter,
172172
};
173-
const execService = await executionFactory?.createActivatedEnvironment(creationOptions);
173+
const execService = await executionFactory.createActivatedEnvironment(creationOptions);
174174

175175
const execInfo = await execService?.getExecutablePath();
176176
traceVerbose(`Executable path for pytest discovery: ${execInfo}.`);

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

Lines changed: 17 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -32,9 +32,9 @@ export class PytestTestExecutionAdapter implements ITestExecutionAdapter {
3232
async runTests(
3333
uri: Uri,
3434
testIds: string[],
35-
profileKind?: TestRunProfileKind,
36-
runInstance?: TestRun,
37-
executionFactory?: IPythonExecutionFactory,
35+
profileKind: boolean | TestRunProfileKind | undefined,
36+
runInstance: TestRun,
37+
executionFactory: IPythonExecutionFactory,
3838
debugLauncher?: ITestDebugLauncher,
3939
interpreter?: PythonEnvironment,
4040
): Promise<void> {
@@ -49,14 +49,14 @@ export class PytestTestExecutionAdapter implements ITestExecutionAdapter {
4949
}
5050
};
5151
const cSource = new CancellationTokenSource();
52-
runInstance?.token.onCancellationRequested(() => cSource.cancel());
52+
runInstance.token.onCancellationRequested(() => cSource.cancel());
5353

5454
const name = await utils.startRunResultNamedPipe(
5555
dataReceivedCallback, // callback to handle data received
5656
deferredTillServerClose, // deferred to resolve when server closes
5757
cSource.token, // token to cancel
5858
);
59-
runInstance?.token.onCancellationRequested(() => {
59+
runInstance.token.onCancellationRequested(() => {
6060
traceInfo(`Test run cancelled, resolving 'TillServerClose' deferred for ${uri.fsPath}.`);
6161
});
6262

@@ -82,9 +82,9 @@ export class PytestTestExecutionAdapter implements ITestExecutionAdapter {
8282
testIds: string[],
8383
resultNamedPipeName: string,
8484
serverCancel: CancellationTokenSource,
85-
runInstance?: TestRun,
86-
profileKind?: TestRunProfileKind,
87-
executionFactory?: IPythonExecutionFactory,
85+
runInstance: TestRun,
86+
profileKind: boolean | TestRunProfileKind | undefined,
87+
executionFactory: IPythonExecutionFactory,
8888
debugLauncher?: ITestDebugLauncher,
8989
interpreter?: PythonEnvironment,
9090
): Promise<ExecutionTestPayload> {
@@ -114,7 +114,7 @@ export class PytestTestExecutionAdapter implements ITestExecutionAdapter {
114114
interpreter,
115115
};
116116
// need to check what will happen in the exec service is NOT defined and is null
117-
const execService = await executionFactory?.createActivatedEnvironment(creationOptions);
117+
const execService = await executionFactory.createActivatedEnvironment(creationOptions);
118118

119119
const execInfo = await execService?.getExecutablePath();
120120
traceVerbose(`Executable path for pytest execution: ${execInfo}.`);
@@ -144,14 +144,14 @@ export class PytestTestExecutionAdapter implements ITestExecutionAdapter {
144144
cwd,
145145
throwOnStdErr: true,
146146
env: mutableEnv,
147-
token: runInstance?.token,
147+
token: runInstance.token,
148148
};
149149

150150
if (debugBool) {
151151
const launchOptions: LaunchOptions = {
152152
cwd,
153153
args: testArgs,
154-
token: runInstance?.token,
154+
token: runInstance.token,
155155
testProvider: PYTEST_PROVIDER,
156156
runTestIdsPort: testIdsFileName,
157157
pytestPort: resultNamedPipeName,
@@ -181,19 +181,19 @@ export class PytestTestExecutionAdapter implements ITestExecutionAdapter {
181181
args: runArgs,
182182
env: (mutableEnv as unknown) as { [key: string]: string },
183183
});
184-
runInstance?.token.onCancellationRequested(() => {
184+
runInstance.token.onCancellationRequested(() => {
185185
traceInfo(`Test run cancelled, killing pytest subprocess for workspace ${uri.fsPath}`);
186186
proc.kill();
187187
deferredTillExecClose.resolve();
188188
serverCancel.cancel();
189189
});
190190
proc.stdout.on('data', (data) => {
191191
const out = utils.fixLogLinesNoTrailing(data.toString());
192-
runInstance?.appendOutput(out);
192+
runInstance.appendOutput(out);
193193
});
194194
proc.stderr.on('data', (data) => {
195195
const out = utils.fixLogLinesNoTrailing(data.toString());
196-
runInstance?.appendOutput(out);
196+
runInstance.appendOutput(out);
197197
});
198198
proc.onExit((code, signal) => {
199199
if (code !== 0) {
@@ -218,7 +218,7 @@ export class PytestTestExecutionAdapter implements ITestExecutionAdapter {
218218

219219
let resultProc: ChildProcess | undefined;
220220

221-
runInstance?.token.onCancellationRequested(() => {
221+
runInstance.token.onCancellationRequested(() => {
222222
traceInfo(`Test run cancelled, killing pytest subprocess for workspace ${uri.fsPath}`);
223223
// if the resultProc exists just call kill on it which will handle resolving the ExecClose deferred, otherwise resolve the deferred here.
224224
if (resultProc) {
@@ -235,11 +235,11 @@ export class PytestTestExecutionAdapter implements ITestExecutionAdapter {
235235
// Displays output to user and ensure the subprocess doesn't run into buffer overflow.
236236
result?.proc?.stdout?.on('data', (data) => {
237237
const out = utils.fixLogLinesNoTrailing(data.toString());
238-
runInstance?.appendOutput(out);
238+
runInstance.appendOutput(out);
239239
});
240240
result?.proc?.stderr?.on('data', (data) => {
241241
const out = utils.fixLogLinesNoTrailing(data.toString());
242-
runInstance?.appendOutput(out);
242+
runInstance.appendOutput(out);
243243
});
244244
result?.proc?.on('exit', (code, signal) => {
245245
if (code !== 0) {

‎src/client/testing/testController/unittest/testDiscoveryAdapter.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ export class UnittestTestDiscoveryAdapter implements ITestDiscoveryAdapter {
3838

3939
public async discoverTests(
4040
uri: Uri,
41-
executionFactory?: IPythonExecutionFactory,
41+
executionFactory: IPythonExecutionFactory,
4242
token?: CancellationToken,
4343
): Promise<void> {
4444
const settings = this.configSettings.getSettings(uri);
@@ -89,7 +89,7 @@ export class UnittestTestDiscoveryAdapter implements ITestDiscoveryAdapter {
8989
testRunPipeName: string,
9090
cwd: string,
9191
cSource: CancellationTokenSource,
92-
executionFactory?: IPythonExecutionFactory,
92+
executionFactory: IPythonExecutionFactory,
9393
): Promise<void> {
9494
// get and edit env vars
9595
const mutableEnv = {
@@ -157,7 +157,7 @@ export class UnittestTestDiscoveryAdapter implements ITestDiscoveryAdapter {
157157
allowEnvironmentFetchExceptions: false,
158158
resource: options.workspaceFolder,
159159
};
160-
const execService = await executionFactory?.createActivatedEnvironment(creationOptions);
160+
const execService = await executionFactory.createActivatedEnvironment(creationOptions);
161161
const execInfo = await execService?.getExecutablePath();
162162
traceVerbose(`Executable path for unittest discovery: ${execInfo}.`);
163163

‎src/client/testing/testController/unittest/testExecutionAdapter.ts‎

Lines changed: 17 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -42,9 +42,9 @@ export class UnittestTestExecutionAdapter implements ITestExecutionAdapter {
4242
public async runTests(
4343
uri: Uri,
4444
testIds: string[],
45-
profileKind?: TestRunProfileKind,
46-
runInstance?: TestRun,
47-
executionFactory?: IPythonExecutionFactory,
45+
profileKind: boolean | TestRunProfileKind | undefined,
46+
runInstance: TestRun,
47+
executionFactory: IPythonExecutionFactory,
4848
debugLauncher?: ITestDebugLauncher,
4949
): Promise<void> {
5050
// deferredTillServerClose awaits named pipe server close
@@ -59,13 +59,13 @@ export class UnittestTestExecutionAdapter implements ITestExecutionAdapter {
5959
}
6060
};
6161
const cSource = new CancellationTokenSource();
62-
runInstance?.token.onCancellationRequested(() => cSource.cancel());
62+
runInstance.token.onCancellationRequested(() => cSource.cancel());
6363
const name = await utils.startRunResultNamedPipe(
6464
dataReceivedCallback, // callback to handle data received
6565
deferredTillServerClose, // deferred to resolve when server closes
6666
cSource.token, // token to cancel
6767
);
68-
runInstance?.token.onCancellationRequested(() => {
68+
runInstance.token.onCancellationRequested(() => {
6969
console.log(`Test run cancelled, resolving 'till TillAllServerClose' deferred for ${uri.fsPath}.`);
7070
// if canceled, stop listening for results
7171
deferredTillServerClose.resolve();
@@ -93,9 +93,9 @@ export class UnittestTestExecutionAdapter implements ITestExecutionAdapter {
9393
testIds: string[],
9494
resultNamedPipeName: string,
9595
serverCancel: CancellationTokenSource,
96-
runInstance?: TestRun,
97-
profileKind?: TestRunProfileKind,
98-
executionFactory?: IPythonExecutionFactory,
96+
runInstance: TestRun,
97+
profileKind: boolean | TestRunProfileKind | undefined,
98+
executionFactory: IPythonExecutionFactory,
9999
debugLauncher?: ITestDebugLauncher,
100100
): Promise<ExecutionTestPayload> {
101101
const settings = this.configSettings.getSettings(uri);
@@ -119,9 +119,9 @@ export class UnittestTestExecutionAdapter implements ITestExecutionAdapter {
119119
workspaceFolder: uri,
120120
command,
121121
cwd,
122-
profileKind,
122+
profileKind: typeof profileKind === 'boolean' ? undefined : profileKind,
123123
testIds,
124-
token: runInstance?.token,
124+
token: runInstance.token,
125125
};
126126
traceLog(`Running UNITTEST execution for the following test ids: ${testIds}`);
127127

@@ -145,7 +145,7 @@ export class UnittestTestExecutionAdapter implements ITestExecutionAdapter {
145145
allowEnvironmentFetchExceptions: false,
146146
resource: options.workspaceFolder,
147147
};
148-
const execService = await executionFactory?.createActivatedEnvironment(creationOptions);
148+
const execService = await executionFactory.createActivatedEnvironment(creationOptions);
149149

150150
const execInfo = await execService?.getExecutablePath();
151151
traceVerbose(`Executable path for unittest execution: ${execInfo}.`);
@@ -193,19 +193,19 @@ export class UnittestTestExecutionAdapter implements ITestExecutionAdapter {
193193
args,
194194
env: (mutableEnv as unknown) as { [key: string]: string },
195195
});
196-
runInstance?.token.onCancellationRequested(() => {
196+
runInstance.token.onCancellationRequested(() => {
197197
traceInfo(`Test run cancelled, killing unittest subprocess for workspace ${uri.fsPath}`);
198198
proc.kill();
199199
deferredTillExecClose.resolve();
200200
serverCancel.cancel();
201201
});
202202
proc.stdout.on('data', (data) => {
203203
const out = utils.fixLogLinesNoTrailing(data.toString());
204-
runInstance?.appendOutput(out);
204+
runInstance.appendOutput(out);
205205
});
206206
proc.stderr.on('data', (data) => {
207207
const out = utils.fixLogLinesNoTrailing(data.toString());
208-
runInstance?.appendOutput(out);
208+
runInstance.appendOutput(out);
209209
});
210210
proc.onExit((code, signal) => {
211211
if (code !== 0) {
@@ -228,7 +228,7 @@ export class UnittestTestExecutionAdapter implements ITestExecutionAdapter {
228228

229229
let resultProc: ChildProcess | undefined;
230230

231-
runInstance?.token.onCancellationRequested(() => {
231+
runInstance.token.onCancellationRequested(() => {
232232
traceInfo(`Test run cancelled, killing unittest subprocess for workspace ${cwd}.`);
233233
// if the resultProc exists just call kill on it which will handle resolving the ExecClose deferred, otherwise resolve the deferred here.
234234
if (resultProc) {
@@ -246,11 +246,11 @@ export class UnittestTestExecutionAdapter implements ITestExecutionAdapter {
246246

247247
result?.proc?.stdout?.on('data', (data) => {
248248
const out = fixLogLinesNoTrailing(data.toString());
249-
runInstance?.appendOutput(`${out}`);
249+
runInstance.appendOutput(`${out}`);
250250
});
251251
result?.proc?.stderr?.on('data', (data) => {
252252
const out = fixLogLinesNoTrailing(data.toString());
253-
runInstance?.appendOutput(`${out}`);
253+
runInstance.appendOutput(`${out}`);
254254
});
255255

256256
result?.proc?.on('exit', (code, signal) => {

0 commit comments

Comments
 (0)