Skip to content

Commit ee59425

Browse files
committed
Revert "Revert "Consistent disposal of receivers across adapters (microsoft#21759)""
This reverts commit f0df531.
1 parent f0df531 commit ee59425

File tree

4 files changed

+37
-23
lines changed

4 files changed

+37
-23
lines changed

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

+9-7
Original file line numberDiff line numberDiff line change
@@ -33,27 +33,30 @@ export class PytestTestDiscoveryAdapter implements ITestDiscoveryAdapter {
3333

3434
async discoverTests(uri: Uri, executionFactory?: IPythonExecutionFactory): Promise<DiscoveredTestPayload> {
3535
const settings = this.configSettings.getSettings(uri);
36+
const uuid = this.testServer.createUUID(uri.fsPath);
3637
const { pytestArgs } = settings.testing;
3738
traceVerbose(pytestArgs);
38-
const disposable = this.testServer.onDiscoveryDataReceived((e: DataReceivedEvent) => {
39-
// cancelation token ?
39+
const dataReceivedDisposable = this.testServer.onDiscoveryDataReceived((e: DataReceivedEvent) => {
4040
this.resultResolver?.resolveDiscovery(JSON.parse(e.data));
4141
});
42+
const disposeDataReceiver = function (testServer: ITestServer) {
43+
testServer.deleteUUID(uuid);
44+
dataReceivedDisposable.dispose();
45+
};
4246
try {
43-
await this.runPytestDiscovery(uri, executionFactory);
47+
await this.runPytestDiscovery(uri, uuid, executionFactory);
4448
} finally {
45-
disposable.dispose();
49+
disposeDataReceiver(this.testServer);
4650
}
4751
// this is only a placeholder to handle function overloading until rewrite is finished
4852
const discoveryPayload: DiscoveredTestPayload = { cwd: uri.fsPath, status: 'success' };
4953
return discoveryPayload;
5054
}
5155

52-
async runPytestDiscovery(uri: Uri, executionFactory?: IPythonExecutionFactory): Promise<void> {
56+
async runPytestDiscovery(uri: Uri, uuid: string, executionFactory?: IPythonExecutionFactory): Promise<void> {
5357
const deferred = createDeferred<DiscoveredTestPayload>();
5458
const relativePathToPytest = 'pythonFiles';
5559
const fullPluginPath = path.join(EXTENSION_ROOT_DIR, relativePathToPytest);
56-
const uuid = this.testServer.createUUID(uri.fsPath);
5760
const settings = this.configSettings.getSettings(uri);
5861
const { pytestArgs } = settings.testing;
5962
const cwd = settings.testing.cwd && settings.testing.cwd.length > 0 ? settings.testing.cwd : uri.fsPath;
@@ -93,7 +96,6 @@ export class PytestTestDiscoveryAdapter implements ITestDiscoveryAdapter {
9396
});
9497
result?.proc?.on('exit', () => {
9598
deferredExec.resolve({ stdout: '', stderr: '' });
96-
this.testServer.deleteUUID(uuid);
9799
deferred.resolve();
98100
});
99101

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

+16-6
Original file line numberDiff line numberDiff line change
@@ -43,19 +43,28 @@ export class PytestTestExecutionAdapter implements ITestExecutionAdapter {
4343
): Promise<ExecutionTestPayload> {
4444
const uuid = this.testServer.createUUID(uri.fsPath);
4545
traceVerbose(uri, testIds, debugBool);
46-
const disposedDataReceived = this.testServer.onRunDataReceived((e: DataReceivedEvent) => {
46+
const dataReceivedDisposable = this.testServer.onRunDataReceived((e: DataReceivedEvent) => {
4747
if (runInstance) {
4848
this.resultResolver?.resolveExecution(JSON.parse(e.data), runInstance);
4949
}
5050
});
51-
const dispose = function (testServer: ITestServer) {
51+
const disposeDataReceiver = function (testServer: ITestServer) {
5252
testServer.deleteUUID(uuid);
53-
disposedDataReceived.dispose();
53+
dataReceivedDisposable.dispose();
5454
};
5555
runInstance?.token.onCancellationRequested(() => {
56-
dispose(this.testServer);
56+
disposeDataReceiver(this.testServer);
5757
});
58-
await this.runTestsNew(uri, testIds, uuid, runInstance, debugBool, executionFactory, debugLauncher);
58+
await this.runTestsNew(
59+
uri,
60+
testIds,
61+
uuid,
62+
runInstance,
63+
debugBool,
64+
executionFactory,
65+
debugLauncher,
66+
disposeDataReceiver,
67+
);
5968

6069
// placeholder until after the rewrite is adopted
6170
// TODO: remove after adoption.
@@ -75,6 +84,7 @@ export class PytestTestExecutionAdapter implements ITestExecutionAdapter {
7584
debugBool?: boolean,
7685
executionFactory?: IPythonExecutionFactory,
7786
debugLauncher?: ITestDebugLauncher,
87+
disposeDataReceiver?: (testServer: ITestServer) => void,
7888
): Promise<ExecutionTestPayload> {
7989
const deferred = createDeferred<ExecutionTestPayload>();
8090
const relativePathToPytest = 'pythonFiles';
@@ -167,8 +177,8 @@ export class PytestTestExecutionAdapter implements ITestExecutionAdapter {
167177

168178
result?.proc?.on('exit', () => {
169179
deferredExec.resolve({ stdout: '', stderr: '' });
170-
this.testServer.deleteUUID(uuid);
171180
deferred.resolve();
181+
disposeDataReceiver?.(this.testServer);
172182
});
173183
await deferredExec.promise;
174184
}

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

+6-3
Original file line numberDiff line numberDiff line change
@@ -43,13 +43,16 @@ export class UnittestTestDiscoveryAdapter implements ITestDiscoveryAdapter {
4343
outChannel: this.outputChannel,
4444
};
4545

46-
const disposable = this.testServer.onDiscoveryDataReceived((e: DataReceivedEvent) => {
46+
const dataReceivedDisposable = this.testServer.onDiscoveryDataReceived((e: DataReceivedEvent) => {
4747
this.resultResolver?.resolveDiscovery(JSON.parse(e.data));
4848
});
49+
const disposeDataReceiver = function (testServer: ITestServer) {
50+
testServer.deleteUUID(uuid);
51+
dataReceivedDisposable.dispose();
52+
};
4953

5054
await this.callSendCommand(options, () => {
51-
this.testServer.deleteUUID(uuid);
52-
disposable.dispose();
55+
disposeDataReceiver(this.testServer);
5356
});
5457
// placeholder until after the rewrite is adopted
5558
// TODO: remove after adoption.

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

+6-7
Original file line numberDiff line numberDiff line change
@@ -42,14 +42,14 @@ export class UnittestTestExecutionAdapter implements ITestExecutionAdapter {
4242
this.resultResolver?.resolveExecution(JSON.parse(e.data), runInstance);
4343
}
4444
});
45-
const dispose = function () {
45+
const disposeDataReceiver = function (testServer: ITestServer) {
46+
testServer.deleteUUID(uuid);
4647
disposedDataReceived.dispose();
4748
};
4849
runInstance?.token.onCancellationRequested(() => {
49-
this.testServer.deleteUUID(uuid);
50-
dispose();
50+
disposeDataReceiver(this.testServer);
5151
});
52-
await this.runTestsNew(uri, testIds, uuid, runInstance, debugBool, dispose);
52+
await this.runTestsNew(uri, testIds, uuid, runInstance, debugBool, disposeDataReceiver);
5353
const executionPayload: ExecutionTestPayload = { cwd: uri.fsPath, status: 'success', error: '' };
5454
return executionPayload;
5555
}
@@ -60,7 +60,7 @@ export class UnittestTestExecutionAdapter implements ITestExecutionAdapter {
6060
uuid: string,
6161
runInstance?: TestRun,
6262
debugBool?: boolean,
63-
dispose?: () => void,
63+
disposeDataReceiver?: (testServer: ITestServer) => void,
6464
): Promise<ExecutionTestPayload> {
6565
const settings = this.configSettings.getSettings(uri);
6666
const { unittestArgs } = settings.testing;
@@ -84,9 +84,8 @@ export class UnittestTestExecutionAdapter implements ITestExecutionAdapter {
8484
const runTestIdsPort = await startTestIdServer(testIds);
8585

8686
await this.testServer.sendCommand(options, runTestIdsPort.toString(), runInstance, () => {
87-
this.testServer.deleteUUID(uuid);
8887
deferred.resolve();
89-
dispose?.();
88+
disposeDataReceiver?.(this.testServer);
9089
});
9190
// placeholder until after the rewrite is adopted
9291
// TODO: remove after adoption.

0 commit comments

Comments
 (0)