From c6c47f7ebf0b9516204733dfc93cf2a0e3549d80 Mon Sep 17 00:00:00 2001 From: eleanorjboyd Date: Mon, 28 Oct 2024 11:26:38 -0700 Subject: [PATCH] minor updates to fix EOT removal and unittest working --- python_files/testing_tools/socket_manager.py | 8 +++----- python_files/unittestadapter/pvsc_utils.py | 11 ++++++----- python_files/vscode_pytest/__init__.py | 4 ++-- src/client/common/pipes/namedPipes.ts | 12 +++++++++--- .../testing/testController/common/utils.ts | 5 ++--- .../pytest/pytestExecutionAdapter.ts | 9 ++------- .../unittest/testDiscoveryAdapter.ts | 4 ++-- .../unittest/testExecutionAdapter.ts | 19 ++++++++++--------- 8 files changed, 36 insertions(+), 36 deletions(-) diff --git a/python_files/testing_tools/socket_manager.py b/python_files/testing_tools/socket_manager.py index b47e39a743fc..f143ac111cdb 100644 --- a/python_files/testing_tools/socket_manager.py +++ b/python_files/testing_tools/socket_manager.py @@ -26,15 +26,13 @@ def connect(self): def close(self): self._writer.close() - self._reader.close() + if hasattr(self, "_reader"): + self._reader.close() def write(self, data: str): - try: # for windows, is should only use \n\n - request = ( - f"""content-length: {len(data)}\ncontent-type: application/json\n\n{data}""" - ) + request = f"""content-length: {len(data)}\ncontent-type: application/json\n\n{data}""" self._writer.write(request) self._writer.flush() except Exception as e: diff --git a/python_files/unittestadapter/pvsc_utils.py b/python_files/unittestadapter/pvsc_utils.py index 09e61ff40518..16473f5691eb 100644 --- a/python_files/unittestadapter/pvsc_utils.py +++ b/python_files/unittestadapter/pvsc_utils.py @@ -307,7 +307,7 @@ def parse_unittest_args( def send_post_request( - payload: Union[ExecutionPayloadDict, DiscoveryPayloadDict, CoveragePayloadDict], + payload: ExecutionPayloadDict | DiscoveryPayloadDict | CoveragePayloadDict, test_run_pipe: Optional[str], ): """ @@ -331,10 +331,10 @@ def send_post_request( if __writer is None: try: - __writer = socket_manager.PipeManager(test_run_pipe) - __writer.connect() + __writer = open(test_run_pipe, "w", encoding="utf-8", newline="\r\n") # noqa: SIM115, PTH123 except Exception as error: error_msg = f"Error attempting to connect to extension named pipe {test_run_pipe}[vscode-unittest]: {error}" + print(error_msg, file=sys.stderr) __writer = None raise VSCodeUnittestError(error_msg) from error @@ -343,10 +343,11 @@ def send_post_request( "params": payload, } data = json.dumps(rpc) - try: if __writer: - __writer.write(data) + request = f"""content-length: {len(data)}\ncontent-type: application/json\n\n{data}""" + __writer.write(request) + __writer.flush() else: print( f"Connection error[vscode-unittest], writer is None \n[vscode-unittest] data: \n{data} \n", diff --git a/python_files/vscode_pytest/__init__.py b/python_files/vscode_pytest/__init__.py index 739d9609b7db..335be247263b 100644 --- a/python_files/vscode_pytest/__init__.py +++ b/python_files/vscode_pytest/__init__.py @@ -448,7 +448,7 @@ def pytest_sessionfinish(session, exitstatus): result=file_coverage_map, error=None, ) - send_post_request(payload) + send_message(payload) def build_test_tree(session: pytest.Session) -> TestNode: @@ -858,7 +858,7 @@ def default(self, o): return super().default(o) -def send_post_request( +def send_message( payload: ExecutionPayloadDict | DiscoveryPayloadDict | CoveragePayloadDict, cls_encoder=None, ): diff --git a/src/client/common/pipes/namedPipes.ts b/src/client/common/pipes/namedPipes.ts index d796cbee8096..9fd21fead2d5 100644 --- a/src/client/common/pipes/namedPipes.ts +++ b/src/client/common/pipes/namedPipes.ts @@ -75,7 +75,9 @@ export async function createWriterPipe(pipeName: string, token?: CancellationTok await mkfifo(pipeName); try { await fs.chmod(pipeName, 0o666); - } catch {} + } catch { + // Intentionally ignored + } const writer = fs.createWriteStream(pipeName, { encoding: 'utf-8', }); @@ -89,6 +91,7 @@ class CombinedReader implements rpc.MessageReader { private _onPartialMessage = new rpc.Emitter(); + // eslint-disable-next-line @typescript-eslint/no-empty-function private _callback: rpc.DataCallback = () => {}; private _disposables: rpc.Disposable[] = []; @@ -107,6 +110,7 @@ class CombinedReader implements rpc.MessageReader { listen(callback: rpc.DataCallback): rpc.Disposable { this._callback = callback; + // eslint-disable-next-line no-return-assign, @typescript-eslint/no-empty-function return new Disposable(() => (this._callback = () => {})); } @@ -167,7 +171,7 @@ export async function createReaderPipe(pipeName: string, token?: CancellationTok server.listen(pipeName); if (token) { token.onCancellationRequested(() => { - if (server.listening) { + if (server.listening) { server.close(); } deferred.reject(new CancellationError()); @@ -180,7 +184,9 @@ export async function createReaderPipe(pipeName: string, token?: CancellationTok await mkfifo(pipeName); try { await fs.chmod(pipeName, 0o666); - } catch {} + } catch { + // Intentionally ignored + } const reader = fs.createReadStream(pipeName, { encoding: 'utf-8' }); return new rpc.StreamMessageReader(reader, 'utf-8'); } diff --git a/src/client/testing/testController/common/utils.ts b/src/client/testing/testController/common/utils.ts index a34ac013ba52..8ca5bf469c12 100644 --- a/src/client/testing/testController/common/utils.ts +++ b/src/client/testing/testController/common/utils.ts @@ -256,7 +256,7 @@ export async function startRunResultNamedPipe( reader.listen((data: Message) => { traceVerbose(`Test Result named pipe ${pipeName} received data`); // if EOT, call decrement connection count (callback) - dataReceivedCallback((data as ExecutionResultMessage).params as ExecutionTestPayload | EOTTestPayload); + dataReceivedCallback((data as ExecutionResultMessage).params as ExecutionTestPayload); }), reader.onClose(() => { // this is called once the server close, once per run instance @@ -306,10 +306,9 @@ export async function startDiscoveryNamedPipe( reader, reader.listen((data: Message) => { traceVerbose(`Test Discovery named pipe ${pipeName} received data`); - callback((data as DiscoveryResultMessage).params as DiscoveredTestPayload | EOTTestPayload); + callback((data as DiscoveryResultMessage).params as DiscoveredTestPayload); }), reader.onClose(() => { - callback(createEOTPayload(false)); traceVerbose(`Test Discovery named pipe ${pipeName} closed`); disposable.dispose(); }), diff --git a/src/client/testing/testController/pytest/pytestExecutionAdapter.ts b/src/client/testing/testController/pytest/pytestExecutionAdapter.ts index 9cf4dad8a0c2..a7578045b84a 100644 --- a/src/client/testing/testController/pytest/pytestExecutionAdapter.ts +++ b/src/client/testing/testController/pytest/pytestExecutionAdapter.ts @@ -1,7 +1,7 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. -import { TestRun, TestRunProfileKind, Uri } from 'vscode'; +import { CancellationTokenSource, TestRun, TestRunProfileKind, Uri } from 'vscode'; import * as path from 'path'; import { ChildProcess } from 'child_process'; import { IConfigurationService, ITestOutputChannel } from '../../../common/types'; @@ -58,9 +58,6 @@ export class PytestTestExecutionAdapter implements ITestExecutionAdapter { ); runInstance?.token.onCancellationRequested(() => { traceInfo(`Test run cancelled, resolving 'TillServerClose' deferred for ${uri.fsPath}.`); - // if canceled, stop listening for results - serverDispose(); // this will resolve deferredTillServerClose - const executionPayload: ExecutionTestPayload = { cwd: uri.fsPath, status: 'success', @@ -74,7 +71,6 @@ export class PytestTestExecutionAdapter implements ITestExecutionAdapter { uri, testIds, name, - deferredTillEOT, cSource, runInstance, profileKind, @@ -100,7 +96,7 @@ export class PytestTestExecutionAdapter implements ITestExecutionAdapter { uri: Uri, testIds: string[], resultNamedPipeName: string, - serverDispose: () => void, + serverCancel: CancellationTokenSource, runInstance?: TestRun, profileKind?: TestRunProfileKind, executionFactory?: IPythonExecutionFactory, @@ -174,7 +170,6 @@ export class PytestTestExecutionAdapter implements ITestExecutionAdapter { traceInfo(`Running DEBUG pytest with arguments: ${testArgs} for workspace ${uri.fsPath} \r\n`); await debugLauncher!.launchDebugger(launchOptions, () => { serverCancel.cancel(); - deferredTillEOT?.resolve(); }); } else { // deferredTillExecClose is resolved when all stdout and stderr is read diff --git a/src/client/testing/testController/unittest/testDiscoveryAdapter.ts b/src/client/testing/testController/unittest/testDiscoveryAdapter.ts index b2047f96a01f..ba52d1ffd57b 100644 --- a/src/client/testing/testController/unittest/testDiscoveryAdapter.ts +++ b/src/client/testing/testController/unittest/testDiscoveryAdapter.ts @@ -44,7 +44,7 @@ export class UnittestTestDiscoveryAdapter implements ITestDiscoveryAdapter { const { unittestArgs } = settings.testing; const cwd = settings.testing.cwd && settings.testing.cwd.length > 0 ? settings.testing.cwd : uri.fsPath; - const { name, dispose } = await startDiscoveryNamedPipe((data: DiscoveredTestPayload) => { + const name = await startDiscoveryNamedPipe((data: DiscoveredTestPayload) => { this.resultResolver?.resolveDiscovery(data); }); @@ -66,7 +66,7 @@ export class UnittestTestDiscoveryAdapter implements ITestDiscoveryAdapter { try { await this.runDiscovery(uri, options, name, cwd, executionFactory); } finally { - dispose(); + // none } // 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 285f045f3e33..e72d877e7617 100644 --- a/src/client/testing/testController/unittest/testExecutionAdapter.ts +++ b/src/client/testing/testController/unittest/testExecutionAdapter.ts @@ -2,7 +2,7 @@ // Licensed under the MIT License. import * as path from 'path'; -import { TestRun, TestRunProfileKind, Uri } from 'vscode'; +import { CancellationTokenSource, TestRun, TestRunProfileKind, Uri } from 'vscode'; import { ChildProcess } from 'child_process'; import { IConfigurationService, ITestOutputChannel } from '../../../common/types'; import { Deferred, createDeferred } from '../../../common/utils/async'; @@ -58,23 +58,24 @@ export class UnittestTestExecutionAdapter implements ITestExecutionAdapter { traceError(`No run instance found, cannot resolve execution, for workspace ${uri.fsPath}.`); } }; - const { name: resultNamedPipeName, dispose: serverDispose } = await utils.startRunResultNamedPipe( + const cSource = new CancellationTokenSource(); + runInstance?.token.onCancellationRequested(() => cSource.cancel()); + const name = await utils.startRunResultNamedPipe( dataReceivedCallback, // callback to handle data received deferredTillServerClose, // deferred to resolve when server closes - runInstance?.token, // token to cancel + cSource.token, // token to cancel ); runInstance?.token.onCancellationRequested(() => { console.log(`Test run cancelled, resolving 'till TillAllServerClose' deferred for ${uri.fsPath}.`); // if canceled, stop listening for results deferredTillServerClose.resolve(); - serverDispose(); }); try { await this.runTestsNew( uri, testIds, - resultNamedPipeName, - serverDispose, + name, + cSource, runInstance, profileKind, executionFactory, @@ -97,7 +98,7 @@ export class UnittestTestExecutionAdapter implements ITestExecutionAdapter { uri: Uri, testIds: string[], resultNamedPipeName: string, - serverDispose: () => void, + serverCancel: CancellationTokenSource, runInstance?: TestRun, profileKind?: TestRunProfileKind, executionFactory?: IPythonExecutionFactory, @@ -172,7 +173,7 @@ export class UnittestTestExecutionAdapter implements ITestExecutionAdapter { throw new Error('Debug launcher is not defined'); } await debugLauncher.launchDebugger(launchOptions, () => { - serverDispose(); // this will resolve the deferredTillAllServerClose + serverCancel.cancel(); }); } else { // This means it is running the test @@ -225,9 +226,9 @@ export class UnittestTestExecutionAdapter implements ITestExecutionAdapter { runInstance, ); } - serverDispose(); } deferredTillExecClose.resolve(); + serverCancel.cancel(); }); await deferredTillExecClose.promise; }