diff --git a/python_files/tests/pytestadapter/helpers.py b/python_files/tests/pytestadapter/helpers.py index 991c7efbc60c..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,10 +84,7 @@ 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 + return json_messages # return the list of json messages def parse_rpc_message(data: str) -> Tuple[Dict[str, str], str]: @@ -96,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/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..644b233fc530 100644 --- a/python_files/unittestadapter/execution.py +++ b/python_files/unittestadapter/execution.py @@ -25,7 +25,6 @@ from unittestadapter.pvsc_utils import ( # noqa: E402 CoveragePayloadDict, - EOTPayloadDict, ExecutionPayloadDict, FileCoverageInfo, TestExecutionStatus, @@ -60,21 +59,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, @@ -269,15 +253,8 @@ def run_tests( return payload -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.register(lambda: __socket.close() if __socket else None) def send_run_data(raw_data, test_run_pipe): @@ -361,7 +338,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..09e61ff40518 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] @@ -314,7 +307,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], ): """ diff --git a/python_files/vscode_pytest/__init__.py b/python_files/vscode_pytest/__init__.py index 92a803190886..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. @@ -873,7 +862,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..d2b8fcaa24a5 100644 --- a/src/client/testing/testController/common/resultResolver.ts +++ b/src/client/testing/testController/common/resultResolver.ts @@ -17,13 +17,7 @@ import { Range, } from 'vscode'; import * as util from 'util'; -import { - CoveragePayload, - DiscoveredTestPayload, - EOTTestPayload, - ExecutionTestPayload, - ITestResultResolver, -} from './types'; +import { CoveragePayload, DiscoveredTestPayload, ExecutionTestPayload, ITestResultResolver } from './types'; import { TestProvider } from '../../types'; import { traceError, traceVerbose } from '../../../logging'; import { Testing } from '../../../common/utils/localize'; @@ -32,7 +26,6 @@ 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 +51,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); @@ -117,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 0942d9d2588c..0de3ff8ad0c0 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[]; @@ -198,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; @@ -259,11 +250,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..2162c1fe6e71 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,17 +36,13 @@ 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; - traceVerbose('deferredTill EOT resolved'); dispose(); } // this is only a placeholder to handle function overloading until rewrite is finished @@ -58,7 +53,6 @@ export class PytestTestDiscoveryAdapter implements ITestDiscoveryAdapter { async runPytestDiscovery( uri: Uri, discoveryPipeName: string, - deferredTillEOT: Deferred, executionFactory?: IPythonExecutionFactory, ): Promise { const relativePathToPytest = 'python_files'; @@ -143,10 +137,8 @@ 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. deferredTillExecClose?.resolve(); }); diff --git a/src/client/testing/testController/pytest/pytestExecutionAdapter.ts b/src/client/testing/testController/pytest/pytestExecutionAdapter.ts index bfaaab9d6586..bcd97f450b58 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, @@ -42,14 +36,12 @@ 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 - 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}.`); } @@ -60,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 = { @@ -78,7 +69,6 @@ export class PytestTestExecutionAdapter implements ITestExecutionAdapter { uri, testIds, name, - deferredTillEOT, serverDispose, runInstance, profileKind, @@ -86,8 +76,6 @@ export class PytestTestExecutionAdapter implements ITestExecutionAdapter { debugLauncher, ); } finally { - // wait for to send EOT - await deferredTillEOT.promise; await deferredTillServerClose.promise; } @@ -105,7 +93,6 @@ export class PytestTestExecutionAdapter implements ITestExecutionAdapter { uri: Uri, testIds: string[], resultNamedPipeName: string, - deferredTillEOT: Deferred, serverDispose: () => void, runInstance?: TestRun, profileKind?: TestRunProfileKind, @@ -178,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 @@ -238,19 +224,12 @@ export class PytestTestExecutionAdapter implements ITestExecutionAdapter { this.resultResolver?.resolveExecution( utils.createExecutionErrorPayload(code, signal, testIds, cwd), runInstance, - deferredTillEOT, - ); - this.resultResolver?.resolveExecution( - utils.createEOTPayload(true), - 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 8e6edcc16b56..b2047f96a01f 100644 --- a/src/client/testing/testController/unittest/testDiscoveryAdapter.ts +++ b/src/client/testing/testController/unittest/testDiscoveryAdapter.ts @@ -7,13 +7,12 @@ import { IConfigurationService, ITestOutputChannel } from '../../../common/types import { EXTENSION_ROOT_DIR } from '../../../constants'; import { DiscoveredTestPayload, - EOTTestPayload, ITestDiscoveryAdapter, ITestResultResolver, 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, @@ -24,11 +23,10 @@ import { import { MESSAGE_ON_TESTING_OUTPUT_MOVE, createDiscoveryErrorPayload, - createEOTPayload, 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`. @@ -46,10 +44,8 @@ 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 | 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,10 +64,8 @@ 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; - traceVerbose('deferredTill EOT resolved'); dispose(); } // placeholder until after the rewrite is adopted @@ -85,7 +79,6 @@ export class UnittestTestDiscoveryAdapter implements ITestDiscoveryAdapter { options: TestCommandOptions, testRunPipeName: string, cwd: string, - deferredTillEOT: Deferred, executionFactory?: IPythonExecutionFactory, ): Promise { // get and edit env vars @@ -146,11 +139,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..285f045f3e33 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, @@ -48,14 +47,13 @@ export class UnittestTestExecutionAdapter implements ITestExecutionAdapter { executionFactory?: IPythonExecutionFactory, debugLauncher?: ITestDebugLauncher, ): Promise { - // deferredTillEOT awaits EOT message and deferredTillServerClose awaits named pipe server close - const deferredTillEOT: Deferred = utils.createTestingDeferred(); + // deferredTillServerClose awaits named pipe server close 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}.`); } @@ -66,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(); }); @@ -78,7 +74,6 @@ export class UnittestTestExecutionAdapter implements ITestExecutionAdapter { uri, testIds, resultNamedPipeName, - deferredTillEOT, serverDispose, runInstance, profileKind, @@ -88,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 = { @@ -104,7 +97,6 @@ export class UnittestTestExecutionAdapter implements ITestExecutionAdapter { uri: Uri, testIds: string[], resultNamedPipeName: string, - deferredTillEOT: Deferred, serverDispose: () => void, runInstance?: TestRun, profileKind?: TestRunProfileKind, @@ -181,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 @@ -232,12 +223,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..9e0f0d3d6302 100644 --- a/src/test/testing/testController/pytest/pytestExecutionAdapter.unit.test.ts +++ b/src/test/testing/testController/pytest/pytestExecutionAdapter.unit.test.ts @@ -243,19 +243,16 @@ 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'); - }); + 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'); - deferredEOT.resolve(); + if (typeof callback === 'function') { + deferred3.resolve(); + callback(); + } }); - const utilsCreateEOTStub: sinon.SinonStub = sinon.stub(util, 'createTestingDeferred'); - utilsCreateEOTStub.callsFake(() => deferredEOT); const testRun = typeMoq.Mock.ofType(); testRun .setup((t) => t.token) @@ -268,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/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..96f15f0b91f7 100644 --- a/src/test/testing/testController/testCancellationRunAdapters.unit.test.ts +++ b/src/test/testing/testController/testCancellationRunAdapters.unit.test.ts @@ -110,17 +110,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; - }); - // define adapter and run tests const testAdapter = createAdapter(adapter, configService, typeMoq.Mock.ofType().object); await testAdapter.runTests( @@ -191,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())) diff --git a/src/test/testing/testController/unittest/testExecutionAdapter.unit.test.ts b/src/test/testing/testController/unittest/testExecutionAdapter.unit.test.ts index 88292c2254d8..d763cbcdff92 100644 --- a/src/test/testing/testController/unittest/testExecutionAdapter.unit.test.ts +++ b/src/test/testing/testController/unittest/testExecutionAdapter.unit.test.ts @@ -242,19 +242,16 @@ 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'); - }); + 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'); - deferredEOT.resolve(); + if (typeof callback === 'function') { + deferred3.resolve(); + callback(); + } }); - const utilsCreateEOTStub: sinon.SinonStub = sinon.stub(util, 'createTestingDeferred'); - utilsCreateEOTStub.callsFake(() => deferredEOT); const testRun = typeMoq.Mock.ofType(); testRun .setup((t) => t.token) @@ -267,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) =>