From 303ea6dd52381fd737ddcc173ca3d5f44a8487ba Mon Sep 17 00:00:00 2001 From: eleanorjboyd Date: Fri, 11 Aug 2023 12:45:39 -0700 Subject: [PATCH] Revert "Consistent disposal of receivers across adapters (#21759)" This reverts commit b299ec97a74e12fa8f87425973739d19f955838c. --- .../pytest/pytestDiscoveryAdapter.ts | 16 ++++++-------- .../pytest/pytestExecutionAdapter.ts | 22 +++++-------------- .../unittest/testDiscoveryAdapter.ts | 9 +++----- .../unittest/testExecutionAdapter.ts | 13 ++++++----- 4 files changed, 23 insertions(+), 37 deletions(-) diff --git a/src/client/testing/testController/pytest/pytestDiscoveryAdapter.ts b/src/client/testing/testController/pytest/pytestDiscoveryAdapter.ts index 44ab3746dde4..f8e3490e854f 100644 --- a/src/client/testing/testController/pytest/pytestDiscoveryAdapter.ts +++ b/src/client/testing/testController/pytest/pytestDiscoveryAdapter.ts @@ -33,30 +33,27 @@ export class PytestTestDiscoveryAdapter implements ITestDiscoveryAdapter { async discoverTests(uri: Uri, executionFactory?: IPythonExecutionFactory): Promise { const settings = this.configSettings.getSettings(uri); - const uuid = this.testServer.createUUID(uri.fsPath); const { pytestArgs } = settings.testing; traceVerbose(pytestArgs); - const dataReceivedDisposable = this.testServer.onDiscoveryDataReceived((e: DataReceivedEvent) => { + const disposable = this.testServer.onDiscoveryDataReceived((e: DataReceivedEvent) => { + // cancelation token ? this.resultResolver?.resolveDiscovery(JSON.parse(e.data)); }); - const disposeDataReceiver = function (testServer: ITestServer) { - testServer.deleteUUID(uuid); - dataReceivedDisposable.dispose(); - }; try { - await this.runPytestDiscovery(uri, uuid, executionFactory); + await this.runPytestDiscovery(uri, executionFactory); } finally { - disposeDataReceiver(this.testServer); + disposable.dispose(); } // this is only a placeholder to handle function overloading until rewrite is finished const discoveryPayload: DiscoveredTestPayload = { cwd: uri.fsPath, status: 'success' }; return discoveryPayload; } - async runPytestDiscovery(uri: Uri, uuid: string, executionFactory?: IPythonExecutionFactory): Promise { + async runPytestDiscovery(uri: Uri, executionFactory?: IPythonExecutionFactory): Promise { const deferred = createDeferred(); const relativePathToPytest = 'pythonFiles'; const fullPluginPath = path.join(EXTENSION_ROOT_DIR, relativePathToPytest); + const uuid = this.testServer.createUUID(uri.fsPath); const settings = this.configSettings.getSettings(uri); const { pytestArgs } = settings.testing; const cwd = settings.testing.cwd && settings.testing.cwd.length > 0 ? settings.testing.cwd : uri.fsPath; @@ -96,6 +93,7 @@ export class PytestTestDiscoveryAdapter implements ITestDiscoveryAdapter { }); result?.proc?.on('exit', () => { deferredExec.resolve({ stdout: '', stderr: '' }); + this.testServer.deleteUUID(uuid); deferred.resolve(); }); diff --git a/src/client/testing/testController/pytest/pytestExecutionAdapter.ts b/src/client/testing/testController/pytest/pytestExecutionAdapter.ts index 4a9a57b16fed..ffc2b705e1a3 100644 --- a/src/client/testing/testController/pytest/pytestExecutionAdapter.ts +++ b/src/client/testing/testController/pytest/pytestExecutionAdapter.ts @@ -43,28 +43,19 @@ export class PytestTestExecutionAdapter implements ITestExecutionAdapter { ): Promise { const uuid = this.testServer.createUUID(uri.fsPath); traceVerbose(uri, testIds, debugBool); - const dataReceivedDisposable = this.testServer.onRunDataReceived((e: DataReceivedEvent) => { + const disposedDataReceived = this.testServer.onRunDataReceived((e: DataReceivedEvent) => { if (runInstance) { this.resultResolver?.resolveExecution(JSON.parse(e.data), runInstance); } }); - const disposeDataReceiver = function (testServer: ITestServer) { + const dispose = function (testServer: ITestServer) { testServer.deleteUUID(uuid); - dataReceivedDisposable.dispose(); + disposedDataReceived.dispose(); }; runInstance?.token.onCancellationRequested(() => { - disposeDataReceiver(this.testServer); + dispose(this.testServer); }); - await this.runTestsNew( - uri, - testIds, - uuid, - runInstance, - debugBool, - executionFactory, - debugLauncher, - disposeDataReceiver, - ); + await this.runTestsNew(uri, testIds, uuid, runInstance, debugBool, executionFactory, debugLauncher); // placeholder until after the rewrite is adopted // TODO: remove after adoption. @@ -84,7 +75,6 @@ export class PytestTestExecutionAdapter implements ITestExecutionAdapter { debugBool?: boolean, executionFactory?: IPythonExecutionFactory, debugLauncher?: ITestDebugLauncher, - disposeDataReceiver?: (testServer: ITestServer) => void, ): Promise { const deferred = createDeferred(); const relativePathToPytest = 'pythonFiles'; @@ -177,8 +167,8 @@ export class PytestTestExecutionAdapter implements ITestExecutionAdapter { result?.proc?.on('exit', () => { deferredExec.resolve({ stdout: '', stderr: '' }); + this.testServer.deleteUUID(uuid); deferred.resolve(); - disposeDataReceiver?.(this.testServer); }); await deferredExec.promise; } diff --git a/src/client/testing/testController/unittest/testDiscoveryAdapter.ts b/src/client/testing/testController/unittest/testDiscoveryAdapter.ts index 1cbad7ef65ef..b49ac3dabd0e 100644 --- a/src/client/testing/testController/unittest/testDiscoveryAdapter.ts +++ b/src/client/testing/testController/unittest/testDiscoveryAdapter.ts @@ -43,16 +43,13 @@ export class UnittestTestDiscoveryAdapter implements ITestDiscoveryAdapter { outChannel: this.outputChannel, }; - const dataReceivedDisposable = this.testServer.onDiscoveryDataReceived((e: DataReceivedEvent) => { + const disposable = this.testServer.onDiscoveryDataReceived((e: DataReceivedEvent) => { this.resultResolver?.resolveDiscovery(JSON.parse(e.data)); }); - const disposeDataReceiver = function (testServer: ITestServer) { - testServer.deleteUUID(uuid); - dataReceivedDisposable.dispose(); - }; await this.callSendCommand(options, () => { - disposeDataReceiver(this.testServer); + this.testServer.deleteUUID(uuid); + disposable.dispose(); }); // placeholder until after the rewrite is adopted // TODO: remove after adoption. diff --git a/src/client/testing/testController/unittest/testExecutionAdapter.ts b/src/client/testing/testController/unittest/testExecutionAdapter.ts index 9af9e593c246..4cd392f93a43 100644 --- a/src/client/testing/testController/unittest/testExecutionAdapter.ts +++ b/src/client/testing/testController/unittest/testExecutionAdapter.ts @@ -42,14 +42,14 @@ export class UnittestTestExecutionAdapter implements ITestExecutionAdapter { this.resultResolver?.resolveExecution(JSON.parse(e.data), runInstance); } }); - const disposeDataReceiver = function (testServer: ITestServer) { - testServer.deleteUUID(uuid); + const dispose = function () { disposedDataReceived.dispose(); }; runInstance?.token.onCancellationRequested(() => { - disposeDataReceiver(this.testServer); + this.testServer.deleteUUID(uuid); + dispose(); }); - await this.runTestsNew(uri, testIds, uuid, runInstance, debugBool, disposeDataReceiver); + await this.runTestsNew(uri, testIds, uuid, runInstance, debugBool, dispose); const executionPayload: ExecutionTestPayload = { cwd: uri.fsPath, status: 'success', error: '' }; return executionPayload; } @@ -60,7 +60,7 @@ export class UnittestTestExecutionAdapter implements ITestExecutionAdapter { uuid: string, runInstance?: TestRun, debugBool?: boolean, - disposeDataReceiver?: (testServer: ITestServer) => void, + dispose?: () => void, ): Promise { const settings = this.configSettings.getSettings(uri); const { unittestArgs } = settings.testing; @@ -84,8 +84,9 @@ export class UnittestTestExecutionAdapter implements ITestExecutionAdapter { const runTestIdsPort = await startTestIdServer(testIds); await this.testServer.sendCommand(options, runTestIdsPort.toString(), runInstance, () => { + this.testServer.deleteUUID(uuid); deferred.resolve(); - disposeDataReceiver?.(this.testServer); + dispose?.(); }); // placeholder until after the rewrite is adopted // TODO: remove after adoption.