Skip to content

Commit

Permalink
Revert "Consistent disposal of receivers across adapters (#21759)"
Browse files Browse the repository at this point in the history
This reverts commit b299ec9.
  • Loading branch information
eleanorjboyd committed Aug 14, 2023
1 parent 9a60b2e commit f45362c
Show file tree
Hide file tree
Showing 4 changed files with 23 additions and 37 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -33,30 +33,27 @@ export class PytestTestDiscoveryAdapter implements ITestDiscoveryAdapter {

async discoverTests(uri: Uri, executionFactory?: IPythonExecutionFactory): Promise<DiscoveredTestPayload> {
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<void> {
async runPytestDiscovery(uri: Uri, executionFactory?: IPythonExecutionFactory): Promise<void> {
const deferred = createDeferred<DiscoveredTestPayload>();
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;
Expand Down Expand Up @@ -96,6 +93,7 @@ export class PytestTestDiscoveryAdapter implements ITestDiscoveryAdapter {
});
result?.proc?.on('exit', () => {
deferredExec.resolve({ stdout: '', stderr: '' });
this.testServer.deleteUUID(uuid);
deferred.resolve();
});

Expand Down
22 changes: 6 additions & 16 deletions src/client/testing/testController/pytest/pytestExecutionAdapter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,28 +43,19 @@ export class PytestTestExecutionAdapter implements ITestExecutionAdapter {
): Promise<ExecutionTestPayload> {
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.
Expand All @@ -84,7 +75,6 @@ export class PytestTestExecutionAdapter implements ITestExecutionAdapter {
debugBool?: boolean,
executionFactory?: IPythonExecutionFactory,
debugLauncher?: ITestDebugLauncher,
disposeDataReceiver?: (testServer: ITestServer) => void,
): Promise<ExecutionTestPayload> {
const deferred = createDeferred<ExecutionTestPayload>();
const relativePathToPytest = 'pythonFiles';
Expand Down Expand Up @@ -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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand All @@ -60,7 +60,7 @@ export class UnittestTestExecutionAdapter implements ITestExecutionAdapter {
uuid: string,
runInstance?: TestRun,
debugBool?: boolean,
disposeDataReceiver?: (testServer: ITestServer) => void,
dispose?: () => void,
): Promise<ExecutionTestPayload> {
const settings = this.configSettings.getSettings(uri);
const { unittestArgs } = settings.testing;
Expand All @@ -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.
Expand Down

0 comments on commit f45362c

Please sign in to comment.