Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
Prev Previous commit
Next Next commit
Revert "Consistent disposal of receivers across adapters (#21759)"
This reverts commit b299ec9.
  • Loading branch information
eleanorjboyd
eleanorjboyd committed Aug 14, 2023
commit f0df531a89c29ae40f35b1e1f59c5f6e510ba505
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