From c725079ccdda5c351cafbfe35874a4054e9d647e Mon Sep 17 00:00:00 2001 From: eleanorjboyd Date: Tue, 1 Oct 2024 15:45:29 -0700 Subject: [PATCH 1/6] merge conflicts --- python_files/tests/pytestadapter/helpers.py | 3 -- python_files/unittestadapter/discovery.py | 5 --- .../unittestadapter/django_test_runner.py | 4 --- python_files/unittestadapter/execution.py | 34 ++++++++----------- python_files/unittestadapter/pvsc_utils.py | 3 +- python_files/vscode_pytest/__init__.py | 2 +- .../testController/common/resultResolver.ts | 15 +++----- .../testing/testController/common/types.ts | 6 ---- .../testing/testController/common/utils.ts | 22 ++++-------- .../pytest/pytestDiscoveryAdapter.ts | 19 ++++------- .../pytest/pytestExecutionAdapter.ts | 31 ++++++----------- .../unittest/testDiscoveryAdapter.ts | 20 ++++------- .../unittest/testExecutionAdapter.ts | 21 ++++-------- .../testController/payloadTestCases.ts | 13 ++----- .../pytestExecutionAdapter.unit.test.ts | 4 --- .../resultResolver.unit.test.ts | 34 ++++++------------- .../testCancellationRunAdapters.unit.test.ts | 8 +---- .../testExecutionAdapter.unit.test.ts | 4 --- 18 files changed, 71 insertions(+), 177 deletions(-) diff --git a/python_files/tests/pytestadapter/helpers.py b/python_files/tests/pytestadapter/helpers.py index 991c7efbc60c..a730140f3a3a 100644 --- a/python_files/tests/pytestadapter/helpers.py +++ b/python_files/tests/pytestadapter/helpers.py @@ -85,9 +85,6 @@ def process_data_received(data: str) -> List[Dict[str, Any]]: else: json_messages.append(json_data["params"]) - last_json = json_messages.pop(-1) - if "eot" not in last_json: - raise ValueError("Last JSON messages does not contain 'eot' as its last payload.") return json_messages # return the list of json messages, only the params part without the EOT token diff --git a/python_files/unittestadapter/discovery.py b/python_files/unittestadapter/discovery.py index 660dda0b292c..ce8251218743 100644 --- a/python_files/unittestadapter/discovery.py +++ b/python_files/unittestadapter/discovery.py @@ -16,7 +16,6 @@ # If I use from utils then there will be an import error in test_discovery.py. from unittestadapter.pvsc_utils import ( # noqa: E402 DiscoveryPayloadDict, - EOTPayloadDict, VSCodeUnittestError, build_test_tree, parse_unittest_args, @@ -129,7 +128,6 @@ def discover_tests( # collect args for Django discovery runner. args = argv[index + 1 :] or [] django_discovery_runner(manage_py_path, args) - # eot payload sent within Django runner. except Exception as e: error_msg = f"Error configuring Django test runner: {e}" print(error_msg, file=sys.stderr) @@ -139,6 +137,3 @@ def discover_tests( payload = discover_tests(start_dir, pattern, top_level_dir) # Post this discovery payload. send_post_request(payload, test_run_pipe) - # Post EOT token. - eot_payload: EOTPayloadDict = {"command_type": "discovery", "eot": True} - send_post_request(eot_payload, test_run_pipe) diff --git a/python_files/unittestadapter/django_test_runner.py b/python_files/unittestadapter/django_test_runner.py index 4225e2c8fa65..c1cca7ac2780 100644 --- a/python_files/unittestadapter/django_test_runner.py +++ b/python_files/unittestadapter/django_test_runner.py @@ -13,7 +13,6 @@ from execution import UnittestTestResult # noqa: E402 from pvsc_utils import ( # noqa: E402 DiscoveryPayloadDict, - EOTPayloadDict, VSCodeUnittestError, build_test_tree, send_post_request, @@ -64,9 +63,6 @@ def run_tests(self, test_labels, **kwargs): # Send discovery payload. send_post_request(payload, test_run_pipe) - # Send EOT token. - eot_payload: EOTPayloadDict = {"command_type": "discovery", "eot": True} - send_post_request(eot_payload, test_run_pipe) return 0 # Skip actual test execution, return 0 as no tests were run. except Exception as e: error_msg = ( diff --git a/python_files/unittestadapter/execution.py b/python_files/unittestadapter/execution.py index 7884c80d84d9..3586bb23644d 100644 --- a/python_files/unittestadapter/execution.py +++ b/python_files/unittestadapter/execution.py @@ -17,6 +17,16 @@ os.environ[path_var_name] = ( sysconfig.get_paths()["scripts"] + os.pathsep + os.environ[path_var_name] ) +sys.path.append("/Users/eleanorboyd/vscode-python/.nox/install_python_libs/lib/python3.10") +sys.path.append("/Users/eleanorboyd/vscode-python-debugger") +sys.path.append("/Users/eleanorboyd/vscode-python-debugger/bundled") +sys.path.append("/Users/eleanorboyd/vscode-python-debugger/bundled/libs") + +import debugpy # noqa: E402 + +debugpy.connect(5678) +debugpy.breakpoint() # noqa: E702 + script_dir = pathlib.Path(__file__).parent sys.path.append(os.fspath(script_dir)) @@ -25,7 +35,6 @@ from unittestadapter.pvsc_utils import ( # noqa: E402 CoveragePayloadDict, - EOTPayloadDict, ExecutionPayloadDict, FileCoverageInfo, TestExecutionStatus, @@ -60,21 +69,6 @@ def startTest(self, test: unittest.TestCase): # noqa: N802 def stopTestRun(self): # noqa: N802 super().stopTestRun() - # After stopping the test run, send EOT - test_run_pipe = os.getenv("TEST_RUN_PIPE") - if os.getenv("MANAGE_PY_PATH"): - # only send this if it is a Django run - if not test_run_pipe: - print( - "UNITTEST ERROR: TEST_RUN_PIPE is not set at the time of unittest trying to send data. " - f"TEST_RUN_PIPE = {test_run_pipe}\n", - file=sys.stderr, - ) - raise VSCodeUnittestError( - "UNITTEST ERROR: TEST_RUN_PIPE is not set at the time of unittest trying to send data. " - ) - eot_payload: EOTPayloadDict = {"command_type": "execution", "eot": True} - send_post_request(eot_payload, test_run_pipe) def addError( # noqa: N802 self, @@ -270,14 +264,15 @@ def run_tests( def execute_eot_and_cleanup(): - eot_payload: EOTPayloadDict = {"command_type": "execution", "eot": True} - send_post_request(eot_payload, test_run_pipe) if __socket: __socket.close() __socket = None -atexit.register(execute_eot_and_cleanup) +__atexit_registered = False +if not __atexit_registered: + atexit.register(execute_eot_and_cleanup) + __atexit_registered = True def send_run_data(raw_data, test_run_pipe): @@ -361,7 +356,6 @@ def send_run_data(raw_data, test_run_pipe): print("MANAGE_PY_PATH env var set, running Django test suite.") args = argv[index + 1 :] or [] django_execution_runner(manage_py_path, test_ids, args) - # the django run subprocesses sends the eot payload. else: # Perform regular unittest execution. payload = run_tests( diff --git a/python_files/unittestadapter/pvsc_utils.py b/python_files/unittestadapter/pvsc_utils.py index 8246c580f3ad..315b603e8d82 100644 --- a/python_files/unittestadapter/pvsc_utils.py +++ b/python_files/unittestadapter/pvsc_utils.py @@ -314,7 +314,7 @@ def parse_unittest_args( def send_post_request( - payload: Union[ExecutionPayloadDict, DiscoveryPayloadDict, EOTPayloadDict, CoveragePayloadDict], + payload: Union[ExecutionPayloadDict, DiscoveryPayloadDict, CoveragePayloadDict], test_run_pipe: Optional[str], ): """ @@ -341,6 +341,7 @@ def send_post_request( __writer = socket_manager.PipeManager(test_run_pipe) __writer.connect() except Exception as error: + print("attempting to write", payload) error_msg = f"Error attempting to connect to extension named pipe {test_run_pipe}[vscode-unittest]: {error}" __writer = None raise VSCodeUnittestError(error_msg) from error diff --git a/python_files/vscode_pytest/__init__.py b/python_files/vscode_pytest/__init__.py index 92a803190886..a120a3690471 100644 --- a/python_files/vscode_pytest/__init__.py +++ b/python_files/vscode_pytest/__init__.py @@ -873,7 +873,7 @@ def default(self, o): def send_post_request( - payload: ExecutionPayloadDict | DiscoveryPayloadDict | EOTPayloadDict | CoveragePayloadDict, + payload: ExecutionPayloadDict | DiscoveryPayloadDict | CoveragePayloadDict, cls_encoder=None, ): """ diff --git a/src/client/testing/testController/common/resultResolver.ts b/src/client/testing/testController/common/resultResolver.ts index 0788c224b0cc..8ebf1a23ad80 100644 --- a/src/client/testing/testController/common/resultResolver.ts +++ b/src/client/testing/testController/common/resultResolver.ts @@ -20,19 +20,18 @@ import * as util from 'util'; import { CoveragePayload, DiscoveredTestPayload, - EOTTestPayload, + ExecutionTestPayload, ITestResultResolver, } from './types'; import { TestProvider } from '../../types'; -import { traceError, traceVerbose } from '../../../logging'; +import { traceError } from '../../../logging'; import { Testing } from '../../../common/utils/localize'; import { clearAllChildren, createErrorTestItem, getTestCaseNodes } from './testItemUtilities'; import { sendTelemetryEvent } from '../../../telemetry'; import { EventName } from '../../../telemetry/constants'; import { splitLines } from '../../../common/stringUtils'; import { buildErrorNodeOptions, populateTestTree, splitTestNameWithRegex } from './utils'; -import { Deferred } from '../../../common/utils/async'; export class PythonResultResolver implements ITestResultResolver { testController: TestController; @@ -58,14 +57,8 @@ export class PythonResultResolver implements ITestResultResolver { this.vsIdToRunId = new Map(); } - public resolveDiscovery( - payload: DiscoveredTestPayload | EOTTestPayload, - deferredTillEOT: Deferred, - token?: CancellationToken, - ): void { - if ('eot' in payload && payload.eot === true) { - deferredTillEOT.resolve(); - } else if (!payload) { + public resolveDiscovery(payload: DiscoveredTestPayload, token?: CancellationToken): void { + if (!payload) { // No test data is available } else { this._resolveDiscovery(payload as DiscoveredTestPayload, token); diff --git a/src/client/testing/testController/common/types.ts b/src/client/testing/testController/common/types.ts index 0942d9d2588c..d42e98197af6 100644 --- a/src/client/testing/testController/common/types.ts +++ b/src/client/testing/testController/common/types.ts @@ -16,7 +16,6 @@ import { import { ITestDebugLauncher, TestDiscoveryOptions } from '../../common/types'; import { IPythonExecutionFactory } from '../../../common/process/types'; import { EnvironmentVariables } from '../../../common/variables/types'; -import { Deferred } from '../../../common/utils/async'; export type TestRunInstanceOptions = TestRunOptions & { exclude?: readonly TestItem[]; @@ -259,11 +258,6 @@ export type DiscoveredTestPayload = { error?: string[]; }; -export type EOTTestPayload = { - commandType: 'discovery' | 'execution'; - eot: boolean; -}; - export type CoveragePayload = { coverage: boolean; cwd: string; diff --git a/src/client/testing/testController/common/utils.ts b/src/client/testing/testController/common/utils.ts index cf82a2ebd1c1..d386d953b933 100644 --- a/src/client/testing/testController/common/utils.ts +++ b/src/client/testing/testController/common/utils.ts @@ -16,7 +16,6 @@ import { DiscoveredTestItem, DiscoveredTestNode, DiscoveredTestPayload, - EOTTestPayload, ExecutionTestPayload, ITestResultResolver, } from './types'; @@ -193,7 +192,7 @@ export async function startTestIdsNamedPipe(testIds: string[]): Promise } interface ExecutionResultMessage extends Message { - params: ExecutionTestPayload | EOTTestPayload; + params: ExecutionTestPayload; } /** @@ -227,7 +226,7 @@ export async function writeTestIdsFile(testIds: string[]): Promise { } export async function startRunResultNamedPipe( - dataReceivedCallback: (payload: ExecutionTestPayload | EOTTestPayload) => void, + dataReceivedCallback: (payload: ExecutionTestPayload) => void, deferredTillServerClose: Deferred, cancellationToken?: CancellationToken, ): Promise<{ name: string } & Disposable> { @@ -259,8 +258,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); }), ); server.serverOnClosePromise().then(() => { @@ -275,11 +273,11 @@ export async function startRunResultNamedPipe( } interface DiscoveryResultMessage extends Message { - params: DiscoveredTestPayload | EOTTestPayload; + params: DiscoveredTestPayload; } export async function startDiscoveryNamedPipe( - callback: (payload: DiscoveredTestPayload | EOTTestPayload) => void, + callback: (payload: DiscoveredTestPayload) => void, cancellationToken?: CancellationToken, ): Promise<{ name: string } & Disposable> { traceVerbose('Starting Test Discovery named pipe'); @@ -302,10 +300,9 @@ export async function startDiscoveryNamedPipe( }), 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(true)); traceVerbose(`Test Discovery named pipe ${pipeName} closed`); dispose(); }), @@ -475,13 +472,6 @@ export function createDiscoveryErrorPayload( }; } -export function createEOTPayload(executionBool: boolean): EOTTestPayload { - return { - commandType: executionBool ? 'execution' : 'discovery', - eot: true, - } as EOTTestPayload; -} - /** * Splits a test name into its parent test name and subtest unique section. * diff --git a/src/client/testing/testController/pytest/pytestDiscoveryAdapter.ts b/src/client/testing/testController/pytest/pytestDiscoveryAdapter.ts index dd7bc9b21847..02734e32b8e3 100644 --- a/src/client/testing/testController/pytest/pytestDiscoveryAdapter.ts +++ b/src/client/testing/testController/pytest/pytestDiscoveryAdapter.ts @@ -9,14 +9,13 @@ import { SpawnOptions, } from '../../../common/process/types'; import { IConfigurationService, ITestOutputChannel } from '../../../common/types'; -import { Deferred, createDeferred } from '../../../common/utils/async'; +import { Deferred } from '../../../common/utils/async'; import { EXTENSION_ROOT_DIR } from '../../../constants'; import { traceError, traceInfo, traceVerbose, traceWarn } from '../../../logging'; -import { DiscoveredTestPayload, EOTTestPayload, ITestDiscoveryAdapter, ITestResultResolver } from '../common/types'; +import { DiscoveredTestPayload, ITestDiscoveryAdapter, ITestResultResolver } from '../common/types'; import { MESSAGE_ON_TESTING_OUTPUT_MOVE, createDiscoveryErrorPayload, - createEOTPayload, createTestingDeferred, fixLogLinesNoTrailing, startDiscoveryNamedPipe, @@ -37,16 +36,14 @@ export class PytestTestDiscoveryAdapter implements ITestDiscoveryAdapter { ) {} async discoverTests(uri: Uri, executionFactory?: IPythonExecutionFactory): Promise { - const deferredTillEOT: Deferred = createDeferred(); - - const { name, dispose } = await startDiscoveryNamedPipe((data: DiscoveredTestPayload | EOTTestPayload) => { - this.resultResolver?.resolveDiscovery(data, deferredTillEOT); + const { name, dispose } = await startDiscoveryNamedPipe((data: DiscoveredTestPayload) => { + this.resultResolver?.resolveDiscovery(data); }); try { - await this.runPytestDiscovery(uri, name, deferredTillEOT, executionFactory); + await this.runPytestDiscovery(uri, name, executionFactory); } finally { - await deferredTillEOT.promise; + // await deferredTillEOT.promise; traceVerbose('deferredTill EOT resolved'); dispose(); } @@ -58,7 +55,6 @@ export class PytestTestDiscoveryAdapter implements ITestDiscoveryAdapter { async runPytestDiscovery( uri: Uri, discoveryPipeName: string, - deferredTillEOT: Deferred, executionFactory?: IPythonExecutionFactory, ): Promise { const relativePathToPytest = 'python_files'; @@ -143,8 +139,7 @@ export class PytestTestDiscoveryAdapter implements ITestDiscoveryAdapter { traceError( `Subprocess exited unsuccessfully with exit code ${code} and signal ${signal} on workspace ${uri.fsPath}. Creating and sending error discovery payload`, ); - this.resultResolver?.resolveDiscovery(createDiscoveryErrorPayload(code, signal, cwd), deferredTillEOT); - this.resultResolver?.resolveDiscovery(createEOTPayload(false), deferredTillEOT); + this.resultResolver?.resolveDiscovery(createDiscoveryErrorPayload(code, signal, cwd)); } // deferredTillEOT is resolved when all data sent on stdout and stderr is received, close event is only called when this occurs // due to the sync reading of the output. diff --git a/src/client/testing/testController/pytest/pytestExecutionAdapter.ts b/src/client/testing/testController/pytest/pytestExecutionAdapter.ts index bfaaab9d6586..0f286a0983ab 100644 --- a/src/client/testing/testController/pytest/pytestExecutionAdapter.ts +++ b/src/client/testing/testController/pytest/pytestExecutionAdapter.ts @@ -7,13 +7,7 @@ import { ChildProcess } from 'child_process'; import { IConfigurationService, ITestOutputChannel } from '../../../common/types'; import { Deferred } from '../../../common/utils/async'; import { traceError, traceInfo, traceVerbose } from '../../../logging'; -import { - CoveragePayload, - EOTTestPayload, - ExecutionTestPayload, - ITestExecutionAdapter, - ITestResultResolver, -} from '../common/types'; +import { ExecutionTestPayload, ITestExecutionAdapter, ITestResultResolver } from '../common/types'; import { ExecutionFactoryCreateWithEnvironmentOptions, IPythonExecutionFactory, @@ -43,13 +37,13 @@ export class PytestTestExecutionAdapter implements ITestExecutionAdapter { debugLauncher?: ITestDebugLauncher, ): Promise { // deferredTillEOT awaits EOT message and deferredTillServerClose awaits named pipe server close - const deferredTillEOT: Deferred = utils.createTestingDeferred(); + // const deferredTillEOT: Deferred = utils.createTestingDeferred(); const deferredTillServerClose: Deferred = utils.createTestingDeferred(); // create callback to handle data received on the named pipe - const dataReceivedCallback = (data: ExecutionTestPayload | EOTTestPayload | CoveragePayload) => { + const dataReceivedCallback = (data: ExecutionTestPayload) => { if (runInstance && !runInstance.token.isCancellationRequested) { - this.resultResolver?.resolveExecution(data, runInstance, deferredTillEOT); + this.resultResolver?.resolveExecution(data, runInstance); } else { traceError(`No run instance found, cannot resolve execution, for workspace ${uri.fsPath}.`); } @@ -62,7 +56,7 @@ export class PytestTestExecutionAdapter implements ITestExecutionAdapter { runInstance?.token.onCancellationRequested(() => { traceInfo(`Test run cancelled, resolving 'till EOT' deferred for ${uri.fsPath}.`); // if canceled, stop listening for results - deferredTillEOT.resolve(); + // deferredTillEOT.resolve(); serverDispose(); // this will resolve deferredTillServerClose const executionPayload: ExecutionTestPayload = { @@ -78,7 +72,7 @@ export class PytestTestExecutionAdapter implements ITestExecutionAdapter { uri, testIds, name, - deferredTillEOT, + // deferredTillEOT, serverDispose, runInstance, profileKind, @@ -87,7 +81,7 @@ export class PytestTestExecutionAdapter implements ITestExecutionAdapter { ); } finally { // wait for to send EOT - await deferredTillEOT.promise; + // await deferredTillEOT.promise; await deferredTillServerClose.promise; } @@ -105,7 +99,7 @@ export class PytestTestExecutionAdapter implements ITestExecutionAdapter { uri: Uri, testIds: string[], resultNamedPipeName: string, - deferredTillEOT: Deferred, + // deferredTillEOT: Deferred, serverDispose: () => void, runInstance?: TestRun, profileKind?: TestRunProfileKind, @@ -178,7 +172,7 @@ export class PytestTestExecutionAdapter implements ITestExecutionAdapter { traceInfo(`Running DEBUG pytest with arguments: ${testArgs} for workspace ${uri.fsPath} \r\n`); await debugLauncher!.launchDebugger(launchOptions, () => { serverDispose(); // this will resolve deferredTillServerClose - deferredTillEOT?.resolve(); + // deferredTillEOT?.resolve(); }); } else { // deferredTillExecClose is resolved when all stdout and stderr is read @@ -238,12 +232,7 @@ export class PytestTestExecutionAdapter implements ITestExecutionAdapter { this.resultResolver?.resolveExecution( utils.createExecutionErrorPayload(code, signal, testIds, cwd), runInstance, - deferredTillEOT, - ); - this.resultResolver?.resolveExecution( - utils.createEOTPayload(true), - runInstance, - deferredTillEOT, + // deferredTillEOT, ); } // this doesn't work, it instead directs us to the noop one which is defined first diff --git a/src/client/testing/testController/unittest/testDiscoveryAdapter.ts b/src/client/testing/testController/unittest/testDiscoveryAdapter.ts index 8e6edcc16b56..2a65dc2fbc21 100644 --- a/src/client/testing/testController/unittest/testDiscoveryAdapter.ts +++ b/src/client/testing/testController/unittest/testDiscoveryAdapter.ts @@ -7,7 +7,6 @@ import { IConfigurationService, ITestOutputChannel } from '../../../common/types import { EXTENSION_ROOT_DIR } from '../../../constants'; import { DiscoveredTestPayload, - EOTTestPayload, ITestDiscoveryAdapter, ITestResultResolver, TestCommandOptions, @@ -24,7 +23,6 @@ import { import { MESSAGE_ON_TESTING_OUTPUT_MOVE, createDiscoveryErrorPayload, - createEOTPayload, fixLogLinesNoTrailing, startDiscoveryNamedPipe, } from '../common/utils'; @@ -46,10 +44,10 @@ 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 deferredTillEOT: Deferred = createDeferred(); + // const deferredTillEOT: Deferred = createDeferred(); - const { name, dispose } = await startDiscoveryNamedPipe((data: DiscoveredTestPayload | EOTTestPayload) => { - this.resultResolver?.resolveDiscovery(data, deferredTillEOT); + const { name, dispose } = await startDiscoveryNamedPipe((data: DiscoveredTestPayload) => { + this.resultResolver?.resolveDiscovery(data); }); // set up env with the pipe name @@ -68,9 +66,9 @@ export class UnittestTestDiscoveryAdapter implements ITestDiscoveryAdapter { }; try { - await this.runDiscovery(uri, options, name, cwd, deferredTillEOT, executionFactory); + await this.runDiscovery(uri, options, name, cwd, executionFactory); } finally { - await deferredTillEOT.promise; + // await deferredTillEOT.promise; traceVerbose('deferredTill EOT resolved'); dispose(); } @@ -85,7 +83,7 @@ export class UnittestTestDiscoveryAdapter implements ITestDiscoveryAdapter { options: TestCommandOptions, testRunPipeName: string, cwd: string, - deferredTillEOT: Deferred, + // deferredTillEOT: Deferred, executionFactory?: IPythonExecutionFactory, ): Promise { // get and edit env vars @@ -146,11 +144,7 @@ export class UnittestTestDiscoveryAdapter implements ITestDiscoveryAdapter { traceError( `Subprocess exited unsuccessfully with exit code ${code} and signal ${signal} on workspace ${uri.fsPath}. Creating and sending error discovery payload`, ); - this.resultResolver?.resolveDiscovery( - createDiscoveryErrorPayload(code, signal, cwd), - deferredTillEOT, - ); - this.resultResolver?.resolveDiscovery(createEOTPayload(false), deferredTillEOT); + this.resultResolver?.resolveDiscovery(createDiscoveryErrorPayload(code, signal, cwd)); } deferredTillExecClose.resolve(); }); diff --git a/src/client/testing/testController/unittest/testExecutionAdapter.ts b/src/client/testing/testController/unittest/testExecutionAdapter.ts index 8e5277fe68d9..8873edba76a7 100644 --- a/src/client/testing/testController/unittest/testExecutionAdapter.ts +++ b/src/client/testing/testController/unittest/testExecutionAdapter.ts @@ -8,7 +8,6 @@ import { IConfigurationService, ITestOutputChannel } from '../../../common/types import { Deferred, createDeferred } from '../../../common/utils/async'; import { EXTENSION_ROOT_DIR } from '../../../constants'; import { - EOTTestPayload, ExecutionTestPayload, ITestExecutionAdapter, ITestResultResolver, @@ -49,13 +48,12 @@ export class UnittestTestExecutionAdapter implements ITestExecutionAdapter { debugLauncher?: ITestDebugLauncher, ): Promise { // deferredTillEOT awaits EOT message and deferredTillServerClose awaits named pipe server close - const deferredTillEOT: Deferred = utils.createTestingDeferred(); const deferredTillServerClose: Deferred = utils.createTestingDeferred(); // create callback to handle data received on the named pipe - const dataReceivedCallback = (data: ExecutionTestPayload | EOTTestPayload) => { + const dataReceivedCallback = (data: ExecutionTestPayload) => { if (runInstance && !runInstance.token.isCancellationRequested) { - this.resultResolver?.resolveExecution(data, runInstance, deferredTillEOT); + this.resultResolver?.resolveExecution(data, runInstance); } else { traceError(`No run instance found, cannot resolve execution, for workspace ${uri.fsPath}.`); } @@ -68,7 +66,7 @@ export class UnittestTestExecutionAdapter implements ITestExecutionAdapter { runInstance?.token.onCancellationRequested(() => { console.log(`Test run cancelled, resolving 'till EOT' deferred for ${uri.fsPath}.`); // if canceled, stop listening for results - deferredTillEOT.resolve(); + // deferredTillEOT.resolve(); // if canceled, close the server, resolves the deferredTillAllServerClose deferredTillServerClose.resolve(); serverDispose(); @@ -78,7 +76,6 @@ export class UnittestTestExecutionAdapter implements ITestExecutionAdapter { uri, testIds, resultNamedPipeName, - deferredTillEOT, serverDispose, runInstance, profileKind, @@ -89,7 +86,7 @@ export class UnittestTestExecutionAdapter implements ITestExecutionAdapter { traceError(`Error in running unittest tests: ${error}`); } finally { // wait for EOT - await deferredTillEOT.promise; + // await deferredTillEOT.promise; await deferredTillServerClose.promise; } const executionPayload: ExecutionTestPayload = { @@ -104,7 +101,7 @@ export class UnittestTestExecutionAdapter implements ITestExecutionAdapter { uri: Uri, testIds: string[], resultNamedPipeName: string, - deferredTillEOT: Deferred, + // deferredTillEOT: Deferred, serverDispose: () => void, runInstance?: TestRun, profileKind?: TestRunProfileKind, @@ -181,7 +178,7 @@ export class UnittestTestExecutionAdapter implements ITestExecutionAdapter { } await debugLauncher.launchDebugger(launchOptions, () => { serverDispose(); // this will resolve the deferredTillAllServerClose - deferredTillEOT?.resolve(); + // deferredTillEOT?.resolve(); }); } else { // This means it is running the test @@ -232,12 +229,6 @@ export class UnittestTestExecutionAdapter implements ITestExecutionAdapter { this.resultResolver?.resolveExecution( utils.createExecutionErrorPayload(code, signal, testIds, cwd), runInstance, - deferredTillEOT, - ); - this.resultResolver?.resolveExecution( - utils.createEOTPayload(true), - runInstance, - deferredTillEOT, ); } serverDispose(); diff --git a/src/test/testing/testController/payloadTestCases.ts b/src/test/testing/testController/payloadTestCases.ts index af33b46c5a36..7f2f5e23bfc3 100644 --- a/src/test/testing/testController/payloadTestCases.ts +++ b/src/test/testing/testController/payloadTestCases.ts @@ -3,12 +3,6 @@ export interface DataWithPayloadChunks { data: string; } -const EOT_PAYLOAD = `Content-Length: 42 -Content-Type: application/json -Request-uuid: fake-u-u-i-d - -{"command_type": "execution", "eot": true}`; - const SINGLE_UNITTEST_SUBTEST = { cwd: '/home/runner/work/vscode-python/vscode-python/path with spaces/src/testTestingRootWkspc/largeWorkspace', status: 'success', @@ -84,7 +78,7 @@ export function PAYLOAD_SINGLE_CHUNK(uuid: string): DataWithPayloadChunks { const payload = createPayload(uuid, SINGLE_UNITTEST_SUBTEST); return { - payloadArray: [payload, EOT_PAYLOAD], + payloadArray: [payload], data: JSON.stringify(SINGLE_UNITTEST_SUBTEST.result), }; } @@ -99,7 +93,7 @@ export function PAYLOAD_MULTI_CHUNK(uuid: string): DataWithPayloadChunks { result += JSON.stringify(SINGLE_UNITTEST_SUBTEST.result); } return { - payloadArray: [payload, EOT_PAYLOAD], + payloadArray: [payload], data: result, }; } @@ -116,7 +110,6 @@ export function PAYLOAD_ONLY_HEADER_MULTI_CHUNK(uuid: string): DataWithPayloadCh const payload2 = val.substring(firstSpaceIndex); payloadArray.push(payload1); payloadArray.push(payload2); - payloadArray.push(EOT_PAYLOAD); return { payloadArray, data: result, @@ -128,7 +121,6 @@ export function PAYLOAD_SPLIT_ACROSS_CHUNKS_ARRAY(uuid: string): DataWithPayload const payload = createPayload(uuid, SINGLE_PYTEST_PAYLOAD); const splitPayload = splitIntoRandomSubstrings(payload); const finalResult = JSON.stringify(SINGLE_PYTEST_PAYLOAD.result); - splitPayload.push(EOT_PAYLOAD); return { payloadArray: splitPayload, data: finalResult, @@ -143,7 +135,6 @@ export function PAYLOAD_SPLIT_MULTI_CHUNK_ARRAY(uuid: string): DataWithPayloadCh JSON.stringify(SINGLE_PYTEST_PAYLOAD_TWO.result), ); - splitPayload.push(EOT_PAYLOAD); return { payloadArray: splitPayload, data: finalResult, diff --git a/src/test/testing/testController/pytest/pytestExecutionAdapter.unit.test.ts b/src/test/testing/testController/pytest/pytestExecutionAdapter.unit.test.ts index 8ab701ad6f57..d4f3cf0d3c6a 100644 --- a/src/test/testing/testController/pytest/pytestExecutionAdapter.unit.test.ts +++ b/src/test/testing/testController/pytest/pytestExecutionAdapter.unit.test.ts @@ -243,7 +243,6 @@ suite('pytest test execution adapter', () => { }); test('Debug launched correctly for pytest', async () => { const deferred3 = createDeferred(); - const deferredEOT = createDeferred(); utilsWriteTestIdsFileStub.callsFake(() => { deferred3.resolve(); return Promise.resolve('testIdPipe-mockName'); @@ -252,10 +251,7 @@ suite('pytest test execution adapter', () => { .setup((dl) => dl.launchDebugger(typeMoq.It.isAny(), typeMoq.It.isAny())) .returns(async () => { traceInfo('stubs launch debugger'); - deferredEOT.resolve(); }); - const utilsCreateEOTStub: sinon.SinonStub = sinon.stub(util, 'createTestingDeferred'); - utilsCreateEOTStub.callsFake(() => deferredEOT); const testRun = typeMoq.Mock.ofType(); testRun .setup((t) => t.token) diff --git a/src/test/testing/testController/resultResolver.unit.test.ts b/src/test/testing/testController/resultResolver.unit.test.ts index 5ecf75987b3c..108edb45da7e 100644 --- a/src/test/testing/testController/resultResolver.unit.test.ts +++ b/src/test/testing/testController/resultResolver.unit.test.ts @@ -14,7 +14,6 @@ import { import * as testItemUtilities from '../../../client/testing/testController/common/testItemUtilities'; import * as ResultResolver from '../../../client/testing/testController/common/resultResolver'; import * as util from '../../../client/testing/testController/common/utils'; -import { Deferred, createDeferred } from '../../../client/common/utils/async'; import { traceLog } from '../../../client/logging'; suite('Result Resolver tests', () => { @@ -89,8 +88,7 @@ suite('Result Resolver tests', () => { const populateTestTreeStub = sinon.stub(util, 'populateTestTree').returns(); // call resolve discovery - const deferredTillEOT: Deferred = createDeferred(); - resultResolver.resolveDiscovery(payload, deferredTillEOT, cancelationToken); + resultResolver.resolveDiscovery(payload, cancelationToken); // assert the stub functions were called with the correct parameters @@ -129,8 +127,7 @@ suite('Result Resolver tests', () => { const createErrorTestItemStub = sinon.stub(testItemUtilities, 'createErrorTestItem').returns(blankTestItem); // call resolve discovery - const deferredTillEOT: Deferred = createDeferred(); - resultResolver.resolveDiscovery(payload, deferredTillEOT, cancelationToken); + resultResolver.resolveDiscovery(payload, cancelationToken); // assert the stub functions were called with the correct parameters @@ -175,8 +172,7 @@ suite('Result Resolver tests', () => { // stub out functionality of populateTestTreeStub which is called in resolveDiscovery const populateTestTreeStub = sinon.stub(util, 'populateTestTree').returns(); // call resolve discovery - const deferredTillEOT: Deferred = createDeferred(); - resultResolver.resolveDiscovery(payload, deferredTillEOT, cancelationToken); + resultResolver.resolveDiscovery(payload, cancelationToken); // assert the stub functions were called with the correct parameters @@ -239,10 +235,8 @@ suite('Result Resolver tests', () => { const deleteSpy = sinon.spy(testController.items, 'delete'); const replaceSpy = sinon.spy(testController.items, 'replace'); // call resolve discovery - let deferredTillEOT: Deferred = createDeferred(); - resultResolver.resolveDiscovery(regPayload, deferredTillEOT, cancelationToken); - deferredTillEOT = createDeferred(); - resultResolver.resolveDiscovery(errorPayload, deferredTillEOT, cancelationToken); + resultResolver.resolveDiscovery(regPayload, cancelationToken); + resultResolver.resolveDiscovery(errorPayload, cancelationToken); // assert the stub functions were called with the correct parameters @@ -375,8 +369,7 @@ suite('Result Resolver tests', () => { }; // call resolveExecution - const deferredTillEOT: Deferred = createDeferred(); - resultResolver.resolveExecution(successPayload, runInstance.object, deferredTillEOT); + resultResolver.resolveExecution(successPayload, runInstance.object); // verify that the passed function was called for the single test item assert.ok(generatedId); @@ -416,8 +409,7 @@ suite('Result Resolver tests', () => { }; // call resolveExecution - const deferredTillEOT: Deferred = createDeferred(); - resultResolver.resolveExecution(successPayload, runInstance.object, deferredTillEOT); + resultResolver.resolveExecution(successPayload, runInstance.object); // verify that the passed function was called for the single test item runInstance.verify((r) => r.failed(typemoq.It.isAny(), typemoq.It.isAny()), typemoq.Times.once()); @@ -456,8 +448,7 @@ suite('Result Resolver tests', () => { }; // call resolveExecution - const deferredTillEOT: Deferred = createDeferred(); - resultResolver.resolveExecution(successPayload, runInstance.object, deferredTillEOT); + resultResolver.resolveExecution(successPayload, runInstance.object); // verify that the passed function was called for the single test item runInstance.verify((r) => r.skipped(typemoq.It.isAny()), typemoq.Times.once()); @@ -496,8 +487,7 @@ suite('Result Resolver tests', () => { }; // call resolveExecution - const deferredTillEOT: Deferred = createDeferred(); - resultResolver.resolveExecution(successPayload, runInstance.object, deferredTillEOT); + resultResolver.resolveExecution(successPayload, runInstance.object); // verify that the passed function was called for the single test item runInstance.verify((r) => r.errored(typemoq.It.isAny(), typemoq.It.isAny()), typemoq.Times.once()); @@ -536,8 +526,7 @@ suite('Result Resolver tests', () => { }; // call resolveExecution - const deferredTillEOT: Deferred = createDeferred(); - resultResolver.resolveExecution(successPayload, runInstance.object, deferredTillEOT); + resultResolver.resolveExecution(successPayload, runInstance.object); // verify that the passed function was called for the single test item runInstance.verify((r) => r.passed(typemoq.It.isAny()), typemoq.Times.once()); @@ -558,8 +547,7 @@ suite('Result Resolver tests', () => { error: 'error', }; - const deferredTillEOT: Deferred = createDeferred(); - resultResolver.resolveExecution(errorPayload, runInstance.object, deferredTillEOT); + resultResolver.resolveExecution(errorPayload, runInstance.object); // verify that none of these functions are called diff --git a/src/test/testing/testController/testCancellationRunAdapters.unit.test.ts b/src/test/testing/testController/testCancellationRunAdapters.unit.test.ts index 41f2fe257681..314314fc28af 100644 --- a/src/test/testing/testController/testCancellationRunAdapters.unit.test.ts +++ b/src/test/testing/testController/testCancellationRunAdapters.unit.test.ts @@ -111,15 +111,9 @@ suite('Execution Flow Run Adapters', () => { }); // mock EOT token & ExecClose token - const deferredEOT = createDeferred(); const deferredExecClose = createDeferred(); const utilsCreateEOTStub: sinon.SinonStub = sinon.stub(util, 'createTestingDeferred'); - utilsCreateEOTStub.callsFake(() => { - if (utilsCreateEOTStub.callCount === 1) { - return deferredEOT; - } - return deferredExecClose; - }); + utilsCreateEOTStub.callsFake(() => deferredExecClose); // define adapter and run tests const testAdapter = createAdapter(adapter, configService, typeMoq.Mock.ofType().object); diff --git a/src/test/testing/testController/unittest/testExecutionAdapter.unit.test.ts b/src/test/testing/testController/unittest/testExecutionAdapter.unit.test.ts index 88292c2254d8..ba7ac23b2109 100644 --- a/src/test/testing/testController/unittest/testExecutionAdapter.unit.test.ts +++ b/src/test/testing/testController/unittest/testExecutionAdapter.unit.test.ts @@ -242,7 +242,6 @@ suite('Unittest test execution adapter', () => { }); test('Debug launched correctly for unittest', async () => { const deferred3 = createDeferred(); - const deferredEOT = createDeferred(); utilsWriteTestIdsFileStub.callsFake(() => { deferred3.resolve(); return Promise.resolve('testIdPipe-mockName'); @@ -251,10 +250,7 @@ suite('Unittest test execution adapter', () => { .setup((dl) => dl.launchDebugger(typeMoq.It.isAny(), typeMoq.It.isAny())) .returns(async () => { traceInfo('stubs launch debugger'); - deferredEOT.resolve(); }); - const utilsCreateEOTStub: sinon.SinonStub = sinon.stub(util, 'createTestingDeferred'); - utilsCreateEOTStub.callsFake(() => deferredEOT); const testRun = typeMoq.Mock.ofType(); testRun .setup((t) => t.token) From a1383db167a5ead634c356510f864a00c87af9e8 Mon Sep 17 00:00:00 2001 From: eleanorjboyd Date: Tue, 1 Oct 2024 15:50:00 -0700 Subject: [PATCH 2/6] fix errors --- python_files/unittestadapter/execution.py | 10 --------- .../testController/common/resultResolver.ts | 22 ++++--------------- .../testing/testController/common/types.ts | 12 ++-------- .../unittest/testDiscoveryAdapter.ts | 2 +- 4 files changed, 7 insertions(+), 39 deletions(-) diff --git a/python_files/unittestadapter/execution.py b/python_files/unittestadapter/execution.py index 3586bb23644d..34bf2a360ce0 100644 --- a/python_files/unittestadapter/execution.py +++ b/python_files/unittestadapter/execution.py @@ -17,16 +17,6 @@ os.environ[path_var_name] = ( sysconfig.get_paths()["scripts"] + os.pathsep + os.environ[path_var_name] ) -sys.path.append("/Users/eleanorboyd/vscode-python/.nox/install_python_libs/lib/python3.10") -sys.path.append("/Users/eleanorboyd/vscode-python-debugger") -sys.path.append("/Users/eleanorboyd/vscode-python-debugger/bundled") -sys.path.append("/Users/eleanorboyd/vscode-python-debugger/bundled/libs") - -import debugpy # noqa: E402 - -debugpy.connect(5678) -debugpy.breakpoint() # noqa: E702 - script_dir = pathlib.Path(__file__).parent sys.path.append(os.fspath(script_dir)) diff --git a/src/client/testing/testController/common/resultResolver.ts b/src/client/testing/testController/common/resultResolver.ts index 8ebf1a23ad80..d2b8fcaa24a5 100644 --- a/src/client/testing/testController/common/resultResolver.ts +++ b/src/client/testing/testController/common/resultResolver.ts @@ -17,15 +17,9 @@ import { Range, } from 'vscode'; import * as util from 'util'; -import { - CoveragePayload, - DiscoveredTestPayload, - - ExecutionTestPayload, - ITestResultResolver, -} from './types'; +import { CoveragePayload, DiscoveredTestPayload, ExecutionTestPayload, ITestResultResolver } from './types'; import { TestProvider } from '../../types'; -import { traceError } from '../../../logging'; +import { traceError, traceVerbose } from '../../../logging'; import { Testing } from '../../../common/utils/localize'; import { clearAllChildren, createErrorTestItem, getTestCaseNodes } from './testItemUtilities'; import { sendTelemetryEvent } from '../../../telemetry'; @@ -110,16 +104,8 @@ export class PythonResultResolver implements ITestResultResolver { }); } - public resolveExecution( - payload: ExecutionTestPayload | EOTTestPayload | CoveragePayload, - runInstance: TestRun, - deferredTillEOT: Deferred, - ): void { - if ('eot' in payload && payload.eot === true) { - // eot sent once per connection - traceVerbose('EOT received, resolving deferredTillServerClose'); - deferredTillEOT.resolve(); - } else if ('coverage' in payload) { + public resolveExecution(payload: ExecutionTestPayload | CoveragePayload, runInstance: TestRun): void { + if ('coverage' in payload) { // coverage data is sent once per connection traceVerbose('Coverage data received.'); this._resolveCoverage(payload as CoveragePayload, runInstance); diff --git a/src/client/testing/testController/common/types.ts b/src/client/testing/testController/common/types.ts index d42e98197af6..0de3ff8ad0c0 100644 --- a/src/client/testing/testController/common/types.ts +++ b/src/client/testing/testController/common/types.ts @@ -197,16 +197,8 @@ export interface ITestResultResolver { vsIdToRunId: Map; detailedCoverageMap: Map; - resolveDiscovery( - payload: DiscoveredTestPayload | EOTTestPayload, - deferredTillEOT: Deferred, - token?: CancellationToken, - ): void; - resolveExecution( - payload: ExecutionTestPayload | EOTTestPayload | CoveragePayload, - runInstance: TestRun, - deferredTillEOT: Deferred, - ): void; + resolveDiscovery(payload: DiscoveredTestPayload, token?: CancellationToken): void; + resolveExecution(payload: ExecutionTestPayload | CoveragePayload, runInstance: TestRun): void; _resolveDiscovery(payload: DiscoveredTestPayload, token?: CancellationToken): void; _resolveExecution(payload: ExecutionTestPayload, runInstance: TestRun): void; _resolveCoverage(payload: CoveragePayload, runInstance: TestRun): void; diff --git a/src/client/testing/testController/unittest/testDiscoveryAdapter.ts b/src/client/testing/testController/unittest/testDiscoveryAdapter.ts index 2a65dc2fbc21..217a77afeaf9 100644 --- a/src/client/testing/testController/unittest/testDiscoveryAdapter.ts +++ b/src/client/testing/testController/unittest/testDiscoveryAdapter.ts @@ -12,7 +12,7 @@ import { TestCommandOptions, TestDiscoveryCommand, } from '../common/types'; -import { Deferred, createDeferred } from '../../../common/utils/async'; +import { createDeferred } from '../../../common/utils/async'; import { EnvironmentVariables, IEnvironmentVariablesProvider } from '../../../common/variables/types'; import { ExecutionFactoryCreateWithEnvironmentOptions, From 1c203908f3e1ce52d56137350a4027ff9bfd7504 Mon Sep 17 00:00:00 2001 From: eleanorjboyd Date: Tue, 1 Oct 2024 16:00:11 -0700 Subject: [PATCH 3/6] cleanup --- python_files/tests/pytestadapter/helpers.py | 4 +--- python_files/unittestadapter/pvsc_utils.py | 7 ------- python_files/vscode_pytest/__init__.py | 11 ----------- 3 files changed, 1 insertion(+), 21 deletions(-) diff --git a/python_files/tests/pytestadapter/helpers.py b/python_files/tests/pytestadapter/helpers.py index a730140f3a3a..7972eedd0919 100644 --- a/python_files/tests/pytestadapter/helpers.py +++ b/python_files/tests/pytestadapter/helpers.py @@ -71,7 +71,6 @@ def process_data_received(data: str) -> List[Dict[str, Any]]: This function also: - Checks that the jsonrpc value is 2.0 - - Checks that the last JSON message contains the `eot` token. """ json_messages = [] remaining = data @@ -85,7 +84,7 @@ def process_data_received(data: str) -> List[Dict[str, Any]]: else: json_messages.append(json_data["params"]) - return json_messages # return the list of json messages, only the params part without the EOT token + return json_messages # return the list of json messages def parse_rpc_message(data: str) -> Tuple[Dict[str, str], str]: @@ -93,7 +92,6 @@ def parse_rpc_message(data: str) -> Tuple[Dict[str, str], str]: A single rpc payload is in the format: content-length: #LEN# \r\ncontent-type: application/json\r\n\r\n{"jsonrpc": "2.0", "params": ENTIRE_DATA} - with EOT params: "params": {"command_type": "discovery", "eot": true} returns: json_data: A single rpc payload of JSON data from the server. diff --git a/python_files/unittestadapter/pvsc_utils.py b/python_files/unittestadapter/pvsc_utils.py index 315b603e8d82..cbe4c0e749b4 100644 --- a/python_files/unittestadapter/pvsc_utils.py +++ b/python_files/unittestadapter/pvsc_utils.py @@ -74,13 +74,6 @@ class ExecutionPayloadDict(TypedDict): error: NotRequired[str] -class EOTPayloadDict(TypedDict): - """A dictionary that is used to send a end of transmission post request to the server.""" - - command_type: Literal["discovery", "execution"] - eot: bool - - class FileCoverageInfo(TypedDict): lines_covered: List[int] lines_missed: List[int] diff --git a/python_files/vscode_pytest/__init__.py b/python_files/vscode_pytest/__init__.py index a120a3690471..ca06bf174418 100644 --- a/python_files/vscode_pytest/__init__.py +++ b/python_files/vscode_pytest/__init__.py @@ -455,10 +455,6 @@ def pytest_sessionfinish(session, exitstatus): ) send_post_request(payload) - command_type = "discovery" if IS_DISCOVERY else "execution" - payload_eot: EOTPayloadDict = {"command_type": command_type, "eot": True} - send_post_request(payload_eot) - def build_test_tree(session: pytest.Session) -> TestNode: """Builds a tree made up of testing nodes from the pytest session. @@ -782,13 +778,6 @@ class CoveragePayloadDict(Dict): error: str | None # Currently unused need to check -class EOTPayloadDict(TypedDict): - """A dictionary that is used to send a end of transmission post request to the server.""" - - command_type: Literal["discovery", "execution"] - eot: bool - - def get_node_path(node: Any) -> pathlib.Path: """A function that returns the path of a node given the switch to pathlib.Path. From 30037afa6bdfd12e9d035826e1f1e872e7813be2 Mon Sep 17 00:00:00 2001 From: eleanorjboyd Date: Wed, 2 Oct 2024 08:32:10 -0700 Subject: [PATCH 4/6] fix launch debugger tests --- .../pytestExecutionAdapter.unit.test.ts | 20 +++++++------------ .../testExecutionAdapter.unit.test.ts | 20 +++++++------------ 2 files changed, 14 insertions(+), 26 deletions(-) diff --git a/src/test/testing/testController/pytest/pytestExecutionAdapter.unit.test.ts b/src/test/testing/testController/pytest/pytestExecutionAdapter.unit.test.ts index d4f3cf0d3c6a..9e0f0d3d6302 100644 --- a/src/test/testing/testController/pytest/pytestExecutionAdapter.unit.test.ts +++ b/src/test/testing/testController/pytest/pytestExecutionAdapter.unit.test.ts @@ -243,14 +243,15 @@ suite('pytest test execution adapter', () => { }); test('Debug launched correctly for pytest', async () => { const deferred3 = createDeferred(); - utilsWriteTestIdsFileStub.callsFake(() => { - deferred3.resolve(); - return Promise.resolve('testIdPipe-mockName'); - }); + utilsWriteTestIdsFileStub.callsFake(() => Promise.resolve('testIdPipe-mockName')); debugLauncher .setup((dl) => dl.launchDebugger(typeMoq.It.isAny(), typeMoq.It.isAny())) - .returns(async () => { + .returns(async (_opts, callback) => { traceInfo('stubs launch debugger'); + if (typeof callback === 'function') { + deferred3.resolve(); + callback(); + } }); const testRun = typeMoq.Mock.ofType(); testRun @@ -264,14 +265,7 @@ suite('pytest test execution adapter', () => { const uri = Uri.file(myTestPath); const outputChannel = typeMoq.Mock.ofType(); adapter = new PytestTestExecutionAdapter(configService, outputChannel.object); - await adapter.runTests( - uri, - [], - TestRunProfileKind.Debug, - testRun.object, - execFactory.object, - debugLauncher.object, - ); + adapter.runTests(uri, [], TestRunProfileKind.Debug, testRun.object, execFactory.object, debugLauncher.object); await deferred3.promise; debugLauncher.verify( (x) => diff --git a/src/test/testing/testController/unittest/testExecutionAdapter.unit.test.ts b/src/test/testing/testController/unittest/testExecutionAdapter.unit.test.ts index ba7ac23b2109..d763cbcdff92 100644 --- a/src/test/testing/testController/unittest/testExecutionAdapter.unit.test.ts +++ b/src/test/testing/testController/unittest/testExecutionAdapter.unit.test.ts @@ -242,14 +242,15 @@ suite('Unittest test execution adapter', () => { }); test('Debug launched correctly for unittest', async () => { const deferred3 = createDeferred(); - utilsWriteTestIdsFileStub.callsFake(() => { - deferred3.resolve(); - return Promise.resolve('testIdPipe-mockName'); - }); + utilsWriteTestIdsFileStub.callsFake(() => Promise.resolve('testIdPipe-mockName')); debugLauncher .setup((dl) => dl.launchDebugger(typeMoq.It.isAny(), typeMoq.It.isAny())) - .returns(async () => { + .returns(async (_opts, callback) => { traceInfo('stubs launch debugger'); + if (typeof callback === 'function') { + deferred3.resolve(); + callback(); + } }); const testRun = typeMoq.Mock.ofType(); testRun @@ -263,14 +264,7 @@ suite('Unittest test execution adapter', () => { const uri = Uri.file(myTestPath); const outputChannel = typeMoq.Mock.ofType(); adapter = new UnittestTestExecutionAdapter(configService, outputChannel.object); - await adapter.runTests( - uri, - [], - TestRunProfileKind.Debug, - testRun.object, - execFactory.object, - debugLauncher.object, - ); + adapter.runTests(uri, [], TestRunProfileKind.Debug, testRun.object, execFactory.object, debugLauncher.object); await deferred3.promise; debugLauncher.verify( (x) => From e4d759ccec1cea86b70aa01a095b53ee6f73455c Mon Sep 17 00:00:00 2001 From: eleanorjboyd Date: Wed, 2 Oct 2024 08:45:55 -0700 Subject: [PATCH 5/6] cleanup --- python_files/unittestadapter/execution.py | 10 +--------- python_files/unittestadapter/pvsc_utils.py | 1 - .../pytest/pytestDiscoveryAdapter.ts | 3 --- .../pytest/pytestExecutionAdapter.ts | 12 +----------- .../unittest/testDiscoveryAdapter.ts | 5 ----- .../unittest/testExecutionAdapter.ts | 10 ++-------- .../testCancellationRunAdapters.unit.test.ts | 16 ---------------- 7 files changed, 4 insertions(+), 53 deletions(-) diff --git a/python_files/unittestadapter/execution.py b/python_files/unittestadapter/execution.py index 34bf2a360ce0..644b233fc530 100644 --- a/python_files/unittestadapter/execution.py +++ b/python_files/unittestadapter/execution.py @@ -253,16 +253,8 @@ def run_tests( return payload -def execute_eot_and_cleanup(): - if __socket: - __socket.close() - - __socket = None -__atexit_registered = False -if not __atexit_registered: - atexit.register(execute_eot_and_cleanup) - __atexit_registered = True +atexit.register(lambda: __socket.close() if __socket else None) def send_run_data(raw_data, test_run_pipe): diff --git a/python_files/unittestadapter/pvsc_utils.py b/python_files/unittestadapter/pvsc_utils.py index cbe4c0e749b4..09e61ff40518 100644 --- a/python_files/unittestadapter/pvsc_utils.py +++ b/python_files/unittestadapter/pvsc_utils.py @@ -334,7 +334,6 @@ def send_post_request( __writer = socket_manager.PipeManager(test_run_pipe) __writer.connect() except Exception as error: - print("attempting to write", payload) error_msg = f"Error attempting to connect to extension named pipe {test_run_pipe}[vscode-unittest]: {error}" __writer = None raise VSCodeUnittestError(error_msg) from error diff --git a/src/client/testing/testController/pytest/pytestDiscoveryAdapter.ts b/src/client/testing/testController/pytest/pytestDiscoveryAdapter.ts index 02734e32b8e3..2162c1fe6e71 100644 --- a/src/client/testing/testController/pytest/pytestDiscoveryAdapter.ts +++ b/src/client/testing/testController/pytest/pytestDiscoveryAdapter.ts @@ -43,8 +43,6 @@ export class PytestTestDiscoveryAdapter implements ITestDiscoveryAdapter { try { await this.runPytestDiscovery(uri, name, executionFactory); } finally { - // await deferredTillEOT.promise; - traceVerbose('deferredTill EOT resolved'); dispose(); } // this is only a placeholder to handle function overloading until rewrite is finished @@ -141,7 +139,6 @@ export class PytestTestDiscoveryAdapter implements ITestDiscoveryAdapter { ); this.resultResolver?.resolveDiscovery(createDiscoveryErrorPayload(code, signal, cwd)); } - // deferredTillEOT is resolved when all data sent on stdout and stderr is received, close event is only called when this occurs // due to the sync reading of the output. deferredTillExecClose?.resolve(); }); diff --git a/src/client/testing/testController/pytest/pytestExecutionAdapter.ts b/src/client/testing/testController/pytest/pytestExecutionAdapter.ts index 0f286a0983ab..bcd97f450b58 100644 --- a/src/client/testing/testController/pytest/pytestExecutionAdapter.ts +++ b/src/client/testing/testController/pytest/pytestExecutionAdapter.ts @@ -36,8 +36,6 @@ export class PytestTestExecutionAdapter implements ITestExecutionAdapter { executionFactory?: IPythonExecutionFactory, debugLauncher?: ITestDebugLauncher, ): Promise { - // deferredTillEOT awaits EOT message and deferredTillServerClose awaits named pipe server close - // const deferredTillEOT: Deferred = utils.createTestingDeferred(); const deferredTillServerClose: Deferred = utils.createTestingDeferred(); // create callback to handle data received on the named pipe @@ -54,9 +52,8 @@ export class PytestTestExecutionAdapter implements ITestExecutionAdapter { runInstance?.token, // token to cancel ); runInstance?.token.onCancellationRequested(() => { - traceInfo(`Test run cancelled, resolving 'till EOT' deferred for ${uri.fsPath}.`); + traceInfo(`Test run cancelled, resolving 'TillServerClose' deferred for ${uri.fsPath}.`); // if canceled, stop listening for results - // deferredTillEOT.resolve(); serverDispose(); // this will resolve deferredTillServerClose const executionPayload: ExecutionTestPayload = { @@ -72,7 +69,6 @@ export class PytestTestExecutionAdapter implements ITestExecutionAdapter { uri, testIds, name, - // deferredTillEOT, serverDispose, runInstance, profileKind, @@ -80,8 +76,6 @@ export class PytestTestExecutionAdapter implements ITestExecutionAdapter { debugLauncher, ); } finally { - // wait for to send EOT - // await deferredTillEOT.promise; await deferredTillServerClose.promise; } @@ -99,7 +93,6 @@ export class PytestTestExecutionAdapter implements ITestExecutionAdapter { uri: Uri, testIds: string[], resultNamedPipeName: string, - // deferredTillEOT: Deferred, serverDispose: () => void, runInstance?: TestRun, profileKind?: TestRunProfileKind, @@ -172,7 +165,6 @@ export class PytestTestExecutionAdapter implements ITestExecutionAdapter { traceInfo(`Running DEBUG pytest with arguments: ${testArgs} for workspace ${uri.fsPath} \r\n`); await debugLauncher!.launchDebugger(launchOptions, () => { serverDispose(); // this will resolve deferredTillServerClose - // deferredTillEOT?.resolve(); }); } else { // deferredTillExecClose is resolved when all stdout and stderr is read @@ -232,14 +224,12 @@ export class PytestTestExecutionAdapter implements ITestExecutionAdapter { this.resultResolver?.resolveExecution( utils.createExecutionErrorPayload(code, signal, testIds, cwd), runInstance, - // deferredTillEOT, ); } // this doesn't work, it instead directs us to the noop one which is defined first // potentially this is due to the server already being close, if this is the case? serverDispose(); // this will resolve deferredTillServerClose } - // deferredTillEOT is resolved when all data sent on stdout and stderr is received, close event is only called when this occurs // due to the sync reading of the output. deferredTillExecClose.resolve(); }); diff --git a/src/client/testing/testController/unittest/testDiscoveryAdapter.ts b/src/client/testing/testController/unittest/testDiscoveryAdapter.ts index 217a77afeaf9..c6364c9e3481 100644 --- a/src/client/testing/testController/unittest/testDiscoveryAdapter.ts +++ b/src/client/testing/testController/unittest/testDiscoveryAdapter.ts @@ -44,8 +44,6 @@ 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 deferredTillEOT: Deferred = createDeferred(); - const { name, dispose } = await startDiscoveryNamedPipe((data: DiscoveredTestPayload) => { this.resultResolver?.resolveDiscovery(data); }); @@ -68,8 +66,6 @@ export class UnittestTestDiscoveryAdapter implements ITestDiscoveryAdapter { try { await this.runDiscovery(uri, options, name, cwd, executionFactory); } finally { - // await deferredTillEOT.promise; - traceVerbose('deferredTill EOT resolved'); dispose(); } // placeholder until after the rewrite is adopted @@ -83,7 +79,6 @@ export class UnittestTestDiscoveryAdapter implements ITestDiscoveryAdapter { options: TestCommandOptions, testRunPipeName: string, cwd: string, - // deferredTillEOT: Deferred, executionFactory?: IPythonExecutionFactory, ): Promise { // get and edit env vars diff --git a/src/client/testing/testController/unittest/testExecutionAdapter.ts b/src/client/testing/testController/unittest/testExecutionAdapter.ts index 8873edba76a7..285f045f3e33 100644 --- a/src/client/testing/testController/unittest/testExecutionAdapter.ts +++ b/src/client/testing/testController/unittest/testExecutionAdapter.ts @@ -47,7 +47,7 @@ export class UnittestTestExecutionAdapter implements ITestExecutionAdapter { executionFactory?: IPythonExecutionFactory, debugLauncher?: ITestDebugLauncher, ): Promise { - // deferredTillEOT awaits EOT message and deferredTillServerClose awaits named pipe server close + // deferredTillServerClose awaits named pipe server close const deferredTillServerClose: Deferred = utils.createTestingDeferred(); // create callback to handle data received on the named pipe @@ -64,10 +64,8 @@ export class UnittestTestExecutionAdapter implements ITestExecutionAdapter { runInstance?.token, // token to cancel ); runInstance?.token.onCancellationRequested(() => { - console.log(`Test run cancelled, resolving 'till EOT' deferred for ${uri.fsPath}.`); + console.log(`Test run cancelled, resolving 'till TillAllServerClose' deferred for ${uri.fsPath}.`); // if canceled, stop listening for results - // deferredTillEOT.resolve(); - // if canceled, close the server, resolves the deferredTillAllServerClose deferredTillServerClose.resolve(); serverDispose(); }); @@ -85,8 +83,6 @@ export class UnittestTestExecutionAdapter implements ITestExecutionAdapter { } catch (error) { traceError(`Error in running unittest tests: ${error}`); } finally { - // wait for EOT - // await deferredTillEOT.promise; await deferredTillServerClose.promise; } const executionPayload: ExecutionTestPayload = { @@ -101,7 +97,6 @@ export class UnittestTestExecutionAdapter implements ITestExecutionAdapter { uri: Uri, testIds: string[], resultNamedPipeName: string, - // deferredTillEOT: Deferred, serverDispose: () => void, runInstance?: TestRun, profileKind?: TestRunProfileKind, @@ -178,7 +173,6 @@ export class UnittestTestExecutionAdapter implements ITestExecutionAdapter { } await debugLauncher.launchDebugger(launchOptions, () => { serverDispose(); // this will resolve the deferredTillAllServerClose - // deferredTillEOT?.resolve(); }); } else { // This means it is running the test diff --git a/src/test/testing/testController/testCancellationRunAdapters.unit.test.ts b/src/test/testing/testController/testCancellationRunAdapters.unit.test.ts index 314314fc28af..96f15f0b91f7 100644 --- a/src/test/testing/testController/testCancellationRunAdapters.unit.test.ts +++ b/src/test/testing/testController/testCancellationRunAdapters.unit.test.ts @@ -110,11 +110,6 @@ suite('Execution Flow Run Adapters', () => { } }); - // mock EOT token & ExecClose token - const deferredExecClose = createDeferred(); - const utilsCreateEOTStub: sinon.SinonStub = sinon.stub(util, 'createTestingDeferred'); - utilsCreateEOTStub.callsFake(() => deferredExecClose); - // define adapter and run tests const testAdapter = createAdapter(adapter, configService, typeMoq.Mock.ofType().object); await testAdapter.runTests( @@ -185,17 +180,6 @@ suite('Execution Flow Run Adapters', () => { } }); - // mock EOT token & ExecClose token - const deferredEOT = createDeferred(); - const deferredExecClose = createDeferred(); - const utilsCreateEOTStub: sinon.SinonStub = sinon.stub(util, 'createTestingDeferred'); - utilsCreateEOTStub.callsFake(() => { - if (utilsCreateEOTStub.callCount === 1) { - return deferredEOT; - } - return deferredExecClose; - }); - // debugLauncher mocked debugLauncher .setup((dl) => dl.launchDebugger(typeMoq.It.isAny(), typeMoq.It.isAny())) From f37390b2b28049815c99d1f0f6af8f08feb28e70 Mon Sep 17 00:00:00 2001 From: eleanorjboyd Date: Wed, 2 Oct 2024 08:49:07 -0700 Subject: [PATCH 6/6] fix import --- .../testing/testController/unittest/testDiscoveryAdapter.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/client/testing/testController/unittest/testDiscoveryAdapter.ts b/src/client/testing/testController/unittest/testDiscoveryAdapter.ts index c6364c9e3481..b2047f96a01f 100644 --- a/src/client/testing/testController/unittest/testDiscoveryAdapter.ts +++ b/src/client/testing/testController/unittest/testDiscoveryAdapter.ts @@ -26,7 +26,7 @@ import { fixLogLinesNoTrailing, startDiscoveryNamedPipe, } from '../common/utils'; -import { traceError, traceInfo, traceLog, traceVerbose } from '../../../logging'; +import { traceError, traceInfo, traceLog } from '../../../logging'; /** * Wrapper class for unittest test discovery. This is where we call `runTestCommand`.