Skip to content

Refactor test and discovery methods to remove overload #25572

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 2 commits into from
Nov 10, 2025
Merged

Refactor test and discovery methods to remove overload #25572

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
fe08b64
exec refactor
eleanorjboyd Nov 10, 2025
3686e9a
refactor discovery
eleanorjboyd Nov 10, 2025
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
12 changes: 4 additions & 8 deletions src/client/testing/testController/common/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -155,26 +155,22 @@ export interface ITestResultResolver {
_resolveCoverage(payload: CoveragePayload, runInstance: TestRun): void;
}
export interface ITestDiscoveryAdapter {
// ** first line old method signature, second line new method signature
discoverTests(uri: Uri): Promise<void>;
discoverTests(
uri: Uri,
executionFactory?: IPythonExecutionFactory,
executionFactory: IPythonExecutionFactory,
token?: CancellationToken,
interpreter?: PythonEnvironment,
): Promise<void>;
}

// interface for execution/runner adapter
export interface ITestExecutionAdapter {
// ** first line old method signature, second line new method signature
runTests(uri: Uri, testIds: string[], profileKind?: boolean | TestRunProfileKind): Promise<void>;
runTests(
uri: Uri,
testIds: string[],
profileKind?: boolean | TestRunProfileKind,
runInstance?: TestRun,
executionFactory?: IPythonExecutionFactory,
profileKind: boolean | TestRunProfileKind | undefined,
runInstance: TestRun,
executionFactory: IPythonExecutionFactory,
debugLauncher?: ITestDebugLauncher,
interpreter?: PythonEnvironment,
): Promise<void>;
Expand Down
8 changes: 4 additions & 4 deletions src/client/testing/testController/controller.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -276,8 +276,8 @@ export class PythonTestController implements ITestController, IExtensionSingleAc
}
await testAdapter.discoverTests(
this.testController,
this.refreshCancellation.token,
this.pythonExecFactory,
this.refreshCancellation.token,
await this.interpreterService.getActiveInterpreter(workspace.uri),
);
} else {
Expand All @@ -302,8 +302,8 @@ export class PythonTestController implements ITestController, IExtensionSingleAc
}
await testAdapter.discoverTests(
this.testController,
this.refreshCancellation.token,
this.pythonExecFactory,
this.refreshCancellation.token,
await this.interpreterService.getActiveInterpreter(workspace.uri),
);
} else {
Expand Down Expand Up @@ -453,9 +453,9 @@ export class PythonTestController implements ITestController, IExtensionSingleAc
this.testController,
runInstance,
testItems,
this.pythonExecFactory,
token,
request.profile?.kind,
this.pythonExecFactory,
this.debugLauncher,
await this.interpreterService.getActiveInterpreter(workspace.uri),
);
Expand All @@ -470,9 +470,9 @@ export class PythonTestController implements ITestController, IExtensionSingleAc
this.testController,
runInstance,
testItems,
this.pythonExecFactory,
token,
request.profile?.kind,
this.pythonExecFactory,
this.debugLauncher,
await this.interpreterService.getActiveInterpreter(workspace.uri),
);
Expand Down
6 changes: 3 additions & 3 deletions src/client/testing/testController/pytest/pytestDiscoveryAdapter.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ export class PytestTestDiscoveryAdapter implements ITestDiscoveryAdapter {

async discoverTests(
uri: Uri,
executionFactory?: IPythonExecutionFactory,
executionFactory: IPythonExecutionFactory,
token?: CancellationToken,
interpreter?: PythonEnvironment,
): Promise<void> {
Expand Down Expand Up @@ -69,7 +69,7 @@ export class PytestTestDiscoveryAdapter implements ITestDiscoveryAdapter {
uri: Uri,
discoveryPipeName: string,
cSource: CancellationTokenSource,
executionFactory?: IPythonExecutionFactory,
executionFactory: IPythonExecutionFactory,
interpreter?: PythonEnvironment,
token?: CancellationToken,
): Promise<void> {
Expand Down Expand Up @@ -170,7 +170,7 @@ export class PytestTestDiscoveryAdapter implements ITestDiscoveryAdapter {
resource: uri,
interpreter,
};
const execService = await executionFactory?.createActivatedEnvironment(creationOptions);
const execService = await executionFactory.createActivatedEnvironment(creationOptions);

const execInfo = await execService?.getExecutablePath();
traceVerbose(`Executable path for pytest discovery: ${execInfo}.`);
Expand Down
34 changes: 17 additions & 17 deletions src/client/testing/testController/pytest/pytestExecutionAdapter.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,9 @@ export class PytestTestExecutionAdapter implements ITestExecutionAdapter {
async runTests(
uri: Uri,
testIds: string[],
profileKind?: TestRunProfileKind,
runInstance?: TestRun,
executionFactory?: IPythonExecutionFactory,
profileKind: boolean | TestRunProfileKind | undefined,
runInstance: TestRun,
executionFactory: IPythonExecutionFactory,
debugLauncher?: ITestDebugLauncher,
interpreter?: PythonEnvironment,
): Promise<void> {
Expand All @@ -49,14 +49,14 @@ export class PytestTestExecutionAdapter implements ITestExecutionAdapter {
}
};
const cSource = new CancellationTokenSource();
runInstance?.token.onCancellationRequested(() => cSource.cancel());
runInstance.token.onCancellationRequested(() => cSource.cancel());

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

Expand All @@ -82,9 +82,9 @@ export class PytestTestExecutionAdapter implements ITestExecutionAdapter {
testIds: string[],
resultNamedPipeName: string,
serverCancel: CancellationTokenSource,
runInstance?: TestRun,
profileKind?: TestRunProfileKind,
executionFactory?: IPythonExecutionFactory,
runInstance: TestRun,
profileKind: boolean | TestRunProfileKind | undefined,
executionFactory: IPythonExecutionFactory,
debugLauncher?: ITestDebugLauncher,
interpreter?: PythonEnvironment,
): Promise<ExecutionTestPayload> {
Expand Down Expand Up @@ -114,7 +114,7 @@ export class PytestTestExecutionAdapter implements ITestExecutionAdapter {
interpreter,
};
// need to check what will happen in the exec service is NOT defined and is null
const execService = await executionFactory?.createActivatedEnvironment(creationOptions);
const execService = await executionFactory.createActivatedEnvironment(creationOptions);

const execInfo = await execService?.getExecutablePath();
traceVerbose(`Executable path for pytest execution: ${execInfo}.`);
Expand Down Expand Up @@ -144,14 +144,14 @@ export class PytestTestExecutionAdapter implements ITestExecutionAdapter {
cwd,
throwOnStdErr: true,
env: mutableEnv,
token: runInstance?.token,
token: runInstance.token,
};

if (debugBool) {
const launchOptions: LaunchOptions = {
cwd,
args: testArgs,
token: runInstance?.token,
token: runInstance.token,
testProvider: PYTEST_PROVIDER,
runTestIdsPort: testIdsFileName,
pytestPort: resultNamedPipeName,
Expand Down Expand Up @@ -181,19 +181,19 @@ export class PytestTestExecutionAdapter implements ITestExecutionAdapter {
args: runArgs,
env: (mutableEnv as unknown) as { [key: string]: string },
});
runInstance?.token.onCancellationRequested(() => {
runInstance.token.onCancellationRequested(() => {
traceInfo(`Test run cancelled, killing pytest subprocess for workspace ${uri.fsPath}`);
proc.kill();
deferredTillExecClose.resolve();
serverCancel.cancel();
});
proc.stdout.on('data', (data) => {
const out = utils.fixLogLinesNoTrailing(data.toString());
runInstance?.appendOutput(out);
runInstance.appendOutput(out);
});
proc.stderr.on('data', (data) => {
const out = utils.fixLogLinesNoTrailing(data.toString());
runInstance?.appendOutput(out);
runInstance.appendOutput(out);
});
proc.onExit((code, signal) => {
if (code !== 0) {
Expand All @@ -218,7 +218,7 @@ export class PytestTestExecutionAdapter implements ITestExecutionAdapter {

let resultProc: ChildProcess | undefined;

runInstance?.token.onCancellationRequested(() => {
runInstance.token.onCancellationRequested(() => {
traceInfo(`Test run cancelled, killing pytest subprocess for workspace ${uri.fsPath}`);
// if the resultProc exists just call kill on it which will handle resolving the ExecClose deferred, otherwise resolve the deferred here.
if (resultProc) {
Expand All @@ -235,11 +235,11 @@ export class PytestTestExecutionAdapter implements ITestExecutionAdapter {
// Displays output to user and ensure the subprocess doesn't run into buffer overflow.
result?.proc?.stdout?.on('data', (data) => {
const out = utils.fixLogLinesNoTrailing(data.toString());
runInstance?.appendOutput(out);
runInstance.appendOutput(out);
});
result?.proc?.stderr?.on('data', (data) => {
const out = utils.fixLogLinesNoTrailing(data.toString());
runInstance?.appendOutput(out);
runInstance.appendOutput(out);
});
result?.proc?.on('exit', (code, signal) => {
if (code !== 0) {
Expand Down
6 changes: 3 additions & 3 deletions src/client/testing/testController/unittest/testDiscoveryAdapter.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ export class UnittestTestDiscoveryAdapter implements ITestDiscoveryAdapter {

public async discoverTests(
uri: Uri,
executionFactory?: IPythonExecutionFactory,
executionFactory: IPythonExecutionFactory,
token?: CancellationToken,
): Promise<void> {
const settings = this.configSettings.getSettings(uri);
Expand Down Expand Up @@ -89,7 +89,7 @@ export class UnittestTestDiscoveryAdapter implements ITestDiscoveryAdapter {
testRunPipeName: string,
cwd: string,
cSource: CancellationTokenSource,
executionFactory?: IPythonExecutionFactory,
executionFactory: IPythonExecutionFactory,
): Promise<void> {
// get and edit env vars
const mutableEnv = {
Expand Down Expand Up @@ -157,7 +157,7 @@ export class UnittestTestDiscoveryAdapter implements ITestDiscoveryAdapter {
allowEnvironmentFetchExceptions: false,
resource: options.workspaceFolder,
};
const execService = await executionFactory?.createActivatedEnvironment(creationOptions);
const execService = await executionFactory.createActivatedEnvironment(creationOptions);
const execInfo = await execService?.getExecutablePath();
traceVerbose(`Executable path for unittest discovery: ${execInfo}.`);

Expand Down
34 changes: 17 additions & 17 deletions src/client/testing/testController/unittest/testExecutionAdapter.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,9 @@ export class UnittestTestExecutionAdapter implements ITestExecutionAdapter {
public async runTests(
uri: Uri,
testIds: string[],
profileKind?: TestRunProfileKind,
runInstance?: TestRun,
executionFactory?: IPythonExecutionFactory,
profileKind: boolean | TestRunProfileKind | undefined,
runInstance: TestRun,
executionFactory: IPythonExecutionFactory,
debugLauncher?: ITestDebugLauncher,
): Promise<void> {
// deferredTillServerClose awaits named pipe server close
Expand All @@ -59,13 +59,13 @@ export class UnittestTestExecutionAdapter implements ITestExecutionAdapter {
}
};
const cSource = new CancellationTokenSource();
runInstance?.token.onCancellationRequested(() => cSource.cancel());
runInstance.token.onCancellationRequested(() => cSource.cancel());
const name = await utils.startRunResultNamedPipe(
dataReceivedCallback, // callback to handle data received
deferredTillServerClose, // deferred to resolve when server closes
cSource.token, // token to cancel
);
runInstance?.token.onCancellationRequested(() => {
runInstance.token.onCancellationRequested(() => {
console.log(`Test run cancelled, resolving 'till TillAllServerClose' deferred for ${uri.fsPath}.`);
// if canceled, stop listening for results
deferredTillServerClose.resolve();
Expand Down Expand Up @@ -93,9 +93,9 @@ export class UnittestTestExecutionAdapter implements ITestExecutionAdapter {
testIds: string[],
resultNamedPipeName: string,
serverCancel: CancellationTokenSource,
runInstance?: TestRun,
profileKind?: TestRunProfileKind,
executionFactory?: IPythonExecutionFactory,
runInstance: TestRun,
profileKind: boolean | TestRunProfileKind | undefined,
executionFactory: IPythonExecutionFactory,
debugLauncher?: ITestDebugLauncher,
): Promise<ExecutionTestPayload> {
const settings = this.configSettings.getSettings(uri);
Expand All @@ -119,9 +119,9 @@ export class UnittestTestExecutionAdapter implements ITestExecutionAdapter {
workspaceFolder: uri,
command,
cwd,
profileKind,
profileKind: typeof profileKind === 'boolean' ? undefined : profileKind,
testIds,
token: runInstance?.token,
token: runInstance.token,
};
traceLog(`Running UNITTEST execution for the following test ids: ${testIds}`);

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

const execInfo = await execService?.getExecutablePath();
traceVerbose(`Executable path for unittest execution: ${execInfo}.`);
Expand Down Expand Up @@ -193,19 +193,19 @@ export class UnittestTestExecutionAdapter implements ITestExecutionAdapter {
args,
env: (mutableEnv as unknown) as { [key: string]: string },
});
runInstance?.token.onCancellationRequested(() => {
runInstance.token.onCancellationRequested(() => {
traceInfo(`Test run cancelled, killing unittest subprocess for workspace ${uri.fsPath}`);
proc.kill();
deferredTillExecClose.resolve();
serverCancel.cancel();
});
proc.stdout.on('data', (data) => {
const out = utils.fixLogLinesNoTrailing(data.toString());
runInstance?.appendOutput(out);
runInstance.appendOutput(out);
});
proc.stderr.on('data', (data) => {
const out = utils.fixLogLinesNoTrailing(data.toString());
runInstance?.appendOutput(out);
runInstance.appendOutput(out);
});
proc.onExit((code, signal) => {
if (code !== 0) {
Expand All @@ -228,7 +228,7 @@ export class UnittestTestExecutionAdapter implements ITestExecutionAdapter {

let resultProc: ChildProcess | undefined;

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

result?.proc?.stdout?.on('data', (data) => {
const out = fixLogLinesNoTrailing(data.toString());
runInstance?.appendOutput(`${out}`);
runInstance.appendOutput(`${out}`);
});
result?.proc?.stderr?.on('data', (data) => {
const out = fixLogLinesNoTrailing(data.toString());
runInstance?.appendOutput(`${out}`);
runInstance.appendOutput(`${out}`);
});

result?.proc?.on('exit', (code, signal) => {
Expand Down
Loading