diff --git a/src/client/testing/testController/common/types.ts b/src/client/testing/testController/common/types.ts index b4d95af6c30e..5c6796905024 100644 --- a/src/client/testing/testController/common/types.ts +++ b/src/client/testing/testController/common/types.ts @@ -155,11 +155,9 @@ 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; discoverTests( uri: Uri, - executionFactory?: IPythonExecutionFactory, + executionFactory: IPythonExecutionFactory, token?: CancellationToken, interpreter?: PythonEnvironment, ): Promise; @@ -167,14 +165,12 @@ export interface ITestDiscoveryAdapter { // 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; 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; diff --git a/src/client/testing/testController/controller.ts b/src/client/testing/testController/controller.ts index aefc97117da5..b38c9b0bcee1 100644 --- a/src/client/testing/testController/controller.ts +++ b/src/client/testing/testController/controller.ts @@ -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 { @@ -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 { @@ -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), ); @@ -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), ); diff --git a/src/client/testing/testController/pytest/pytestDiscoveryAdapter.ts b/src/client/testing/testController/pytest/pytestDiscoveryAdapter.ts index 04258ddbddf2..308c9ba1f9bc 100644 --- a/src/client/testing/testController/pytest/pytestDiscoveryAdapter.ts +++ b/src/client/testing/testController/pytest/pytestDiscoveryAdapter.ts @@ -38,7 +38,7 @@ export class PytestTestDiscoveryAdapter implements ITestDiscoveryAdapter { async discoverTests( uri: Uri, - executionFactory?: IPythonExecutionFactory, + executionFactory: IPythonExecutionFactory, token?: CancellationToken, interpreter?: PythonEnvironment, ): Promise { @@ -69,7 +69,7 @@ export class PytestTestDiscoveryAdapter implements ITestDiscoveryAdapter { uri: Uri, discoveryPipeName: string, cSource: CancellationTokenSource, - executionFactory?: IPythonExecutionFactory, + executionFactory: IPythonExecutionFactory, interpreter?: PythonEnvironment, token?: CancellationToken, ): Promise { @@ -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}.`); diff --git a/src/client/testing/testController/pytest/pytestExecutionAdapter.ts b/src/client/testing/testController/pytest/pytestExecutionAdapter.ts index 053c497c56e0..3b2f9f7de33a 100644 --- a/src/client/testing/testController/pytest/pytestExecutionAdapter.ts +++ b/src/client/testing/testController/pytest/pytestExecutionAdapter.ts @@ -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 { @@ -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}.`); }); @@ -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 { @@ -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}.`); @@ -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, @@ -181,7 +181,7 @@ 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(); @@ -189,11 +189,11 @@ export class PytestTestExecutionAdapter implements ITestExecutionAdapter { }); 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) { @@ -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) { @@ -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) { diff --git a/src/client/testing/testController/unittest/testDiscoveryAdapter.ts b/src/client/testing/testController/unittest/testDiscoveryAdapter.ts index 23d70568687f..a40e25153fbc 100644 --- a/src/client/testing/testController/unittest/testDiscoveryAdapter.ts +++ b/src/client/testing/testController/unittest/testDiscoveryAdapter.ts @@ -38,7 +38,7 @@ export class UnittestTestDiscoveryAdapter implements ITestDiscoveryAdapter { public async discoverTests( uri: Uri, - executionFactory?: IPythonExecutionFactory, + executionFactory: IPythonExecutionFactory, token?: CancellationToken, ): Promise { const settings = this.configSettings.getSettings(uri); @@ -89,7 +89,7 @@ export class UnittestTestDiscoveryAdapter implements ITestDiscoveryAdapter { testRunPipeName: string, cwd: string, cSource: CancellationTokenSource, - executionFactory?: IPythonExecutionFactory, + executionFactory: IPythonExecutionFactory, ): Promise { // get and edit env vars const mutableEnv = { @@ -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}.`); diff --git a/src/client/testing/testController/unittest/testExecutionAdapter.ts b/src/client/testing/testController/unittest/testExecutionAdapter.ts index 74572ea5c63c..cbc1d2985f84 100644 --- a/src/client/testing/testController/unittest/testExecutionAdapter.ts +++ b/src/client/testing/testController/unittest/testExecutionAdapter.ts @@ -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 { // deferredTillServerClose awaits named pipe server close @@ -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(); @@ -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 { const settings = this.configSettings.getSettings(uri); @@ -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}`); @@ -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}.`); @@ -193,7 +193,7 @@ 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(); @@ -201,11 +201,11 @@ export class UnittestTestExecutionAdapter implements ITestExecutionAdapter { }); 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) { @@ -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) { @@ -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) => { diff --git a/src/client/testing/testController/workspaceTestAdapter.ts b/src/client/testing/testController/workspaceTestAdapter.ts index a73acdaba5f0..75b9489f708e 100644 --- a/src/client/testing/testController/workspaceTestAdapter.ts +++ b/src/client/testing/testController/workspaceTestAdapter.ts @@ -42,9 +42,9 @@ export class WorkspaceTestAdapter { testController: TestController, runInstance: TestRun, includes: TestItem[], + executionFactory: IPythonExecutionFactory, token?: CancellationToken, profileKind?: boolean | TestRunProfileKind, - executionFactory?: IPythonExecutionFactory, debugLauncher?: ITestDebugLauncher, interpreter?: PythonEnvironment, ): Promise { @@ -73,20 +73,18 @@ export class WorkspaceTestAdapter { } }); const testCaseIds = Array.from(testCaseIdsSet); - // ** execution factory only defined for new rewrite way - if (executionFactory !== undefined) { - await this.executionAdapter.runTests( - this.workspaceUri, - testCaseIds, - profileKind, - runInstance, - executionFactory, - debugLauncher, - interpreter, - ); - } else { - await this.executionAdapter.runTests(this.workspaceUri, testCaseIds, profileKind); + if (executionFactory === undefined) { + throw new Error('Execution factory is required for test execution'); } + await this.executionAdapter.runTests( + this.workspaceUri, + testCaseIds, + profileKind, + runInstance, + executionFactory, + debugLauncher, + interpreter, + ); deferred.resolve(); } catch (ex) { // handle token and telemetry here @@ -116,8 +114,8 @@ export class WorkspaceTestAdapter { public async discoverTests( testController: TestController, + executionFactory: IPythonExecutionFactory, token?: CancellationToken, - executionFactory?: IPythonExecutionFactory, interpreter?: PythonEnvironment, ): Promise { sendTelemetryEvent(EventName.UNITTEST_DISCOVERING, undefined, { tool: this.testProvider }); @@ -132,12 +130,10 @@ export class WorkspaceTestAdapter { this.discovering = deferred; try { - // ** execution factory only defined for new rewrite way - if (executionFactory !== undefined) { - await this.discoveryAdapter.discoverTests(this.workspaceUri, executionFactory, token, interpreter); - } else { - await this.discoveryAdapter.discoverTests(this.workspaceUri); + if (executionFactory === undefined) { + throw new Error('Execution factory is required for test discovery'); } + await this.discoveryAdapter.discoverTests(this.workspaceUri, executionFactory, token, interpreter); deferred.resolve(); } catch (ex) { sendTelemetryEvent(EventName.UNITTEST_DISCOVERY_DONE, undefined, { tool: this.testProvider, failed: true }); diff --git a/src/test/testing/testController/workspaceTestAdapter.unit.test.ts b/src/test/testing/testController/workspaceTestAdapter.unit.test.ts index aac07793ca66..6d2895ca2979 100644 --- a/src/test/testing/testController/workspaceTestAdapter.unit.test.ts +++ b/src/test/testing/testController/workspaceTestAdapter.unit.test.ts @@ -147,7 +147,7 @@ suite('Workspace test adapter', () => { const testProvider = 'unittest'; execFactory = typemoq.Mock.ofType(); - await workspaceTestAdapter.discoverTests(testController, undefined, execFactory.object); + await workspaceTestAdapter.discoverTests(testController, execFactory.object); sinon.assert.calledWithMatch(createErrorTestItemStub, sinon.match.any, sinon.match.any); sinon.assert.calledWithMatch(buildErrorNodeOptionsStub, uriFoo, sinon.match.any, testProvider); @@ -166,7 +166,7 @@ suite('Workspace test adapter', () => { stubResultResolver, ); - await workspaceTestAdapter.discoverTests(testController, undefined, execFactory.object); + await workspaceTestAdapter.discoverTests(testController, execFactory.object); sinon.assert.calledOnce(discoverTestsStub); }); @@ -193,8 +193,8 @@ suite('Workspace test adapter', () => { ); // Try running discovery twice - const one = workspaceTestAdapter.discoverTests(testController); - const two = workspaceTestAdapter.discoverTests(testController); + const one = workspaceTestAdapter.discoverTests(testController, execFactory.object); + const two = workspaceTestAdapter.discoverTests(testController, execFactory.object); Promise.all([one, two]); @@ -215,7 +215,7 @@ suite('Workspace test adapter', () => { stubResultResolver, ); - await workspaceTestAdapter.discoverTests(testController, undefined, execFactory.object); + await workspaceTestAdapter.discoverTests(testController, execFactory.object); sinon.assert.calledWith(sendTelemetryStub, EventName.UNITTEST_DISCOVERY_DONE); assert.strictEqual(telemetryEvent.length, 2); @@ -238,7 +238,7 @@ suite('Workspace test adapter', () => { stubResultResolver, ); - await workspaceTestAdapter.discoverTests(testController); + await workspaceTestAdapter.discoverTests(testController, execFactory.object); sinon.assert.calledWith(sendTelemetryStub, EventName.UNITTEST_DISCOVERY_DONE); assert.strictEqual(telemetryEvent.length, 2); @@ -256,6 +256,7 @@ suite('Workspace test adapter', () => { let testControllerMock: typemoq.IMock; let telemetryEvent: { eventName: EventName; properties: Record }[] = []; let resultResolver: ResultResolver.PythonResultResolver; + let execFactory: typemoq.IMock; // Stubbed test controller (see comment around L.40) let testController: TestController; @@ -328,6 +329,7 @@ suite('Workspace test adapter', () => { executionTestsStub = sandbox.stub(UnittestTestExecutionAdapter.prototype, 'runTests'); sendTelemetryStub = sandbox.stub(Telemetry, 'sendTelemetryEvent').callsFake(mockSendTelemetryEvent); + execFactory = typemoq.Mock.ofType(); runInstance = typemoq.Mock.ofType(); const testProvider = 'pytest'; @@ -384,7 +386,12 @@ suite('Workspace test adapter', () => { testControllerMock = typemoq.Mock.ofType(); testControllerMock.setup((t) => t.items).returns(() => testItemCollectionMock.object); - await workspaceTestAdapter.executeTests(testController, runInstance.object, [mockTestItem1, mockTestItem2]); + await workspaceTestAdapter.executeTests( + testController, + runInstance.object, + [mockTestItem1, mockTestItem2], + execFactory.object, + ); runInstance.verify((r) => r.started(typemoq.It.isAny()), typemoq.Times.exactly(2)); }); @@ -400,7 +407,7 @@ suite('Workspace test adapter', () => { stubResultResolver, ); - await workspaceTestAdapter.executeTests(testController, runInstance.object, []); + await workspaceTestAdapter.executeTests(testController, runInstance.object, [], execFactory.object); sinon.assert.calledOnce(executionTestsStub); }); @@ -427,8 +434,8 @@ suite('Workspace test adapter', () => { ); // Try running discovery twice - const one = workspaceTestAdapter.executeTests(testController, runInstance.object, []); - const two = workspaceTestAdapter.executeTests(testController, runInstance.object, []); + const one = workspaceTestAdapter.executeTests(testController, runInstance.object, [], execFactory.object); + const two = workspaceTestAdapter.executeTests(testController, runInstance.object, [], execFactory.object); Promise.all([one, two]); @@ -467,7 +474,7 @@ suite('Workspace test adapter', () => { const buildErrorNodeOptionsStub = sinon.stub(util, 'buildErrorNodeOptions').returns(errorTestItemOptions); const testProvider = 'unittest'; - await workspaceTestAdapter.executeTests(testController, runInstance.object, []); + await workspaceTestAdapter.executeTests(testController, runInstance.object, [], execFactory.object); sinon.assert.calledWithMatch(createErrorTestItemStub, sinon.match.any, sinon.match.any); sinon.assert.calledWithMatch(buildErrorNodeOptionsStub, Uri.parse('foo'), sinon.match.any, testProvider); @@ -487,7 +494,7 @@ suite('Workspace test adapter', () => { stubResultResolver, ); - await workspaceTestAdapter.executeTests(testController, runInstance.object, []); + await workspaceTestAdapter.executeTests(testController, runInstance.object, [], execFactory.object); sinon.assert.calledWith(sendTelemetryStub, EventName.UNITTEST_RUN_ALL_FAILED); assert.strictEqual(telemetryEvent.length, 1);