Skip to content

Commit

Permalink
minor updates to fix EOT removal and unittest working
Browse files Browse the repository at this point in the history
  • Loading branch information
eleanorjboyd committed Oct 28, 2024
1 parent 5451fe6 commit c6c47f7
Show file tree
Hide file tree
Showing 8 changed files with 36 additions and 36 deletions.
8 changes: 3 additions & 5 deletions python_files/testing_tools/socket_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,15 +26,13 @@ def connect(self):

def close(self):
self._writer.close()
self._reader.close()
if hasattr(self, "_reader"):
self._reader.close()

def write(self, data: str):

try:
# for windows, is should only use \n\n
request = (
f"""content-length: {len(data)}\ncontent-type: application/json\n\n{data}"""
)
request = f"""content-length: {len(data)}\ncontent-type: application/json\n\n{data}"""
self._writer.write(request)
self._writer.flush()
except Exception as e:
Expand Down
11 changes: 6 additions & 5 deletions python_files/unittestadapter/pvsc_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -307,7 +307,7 @@ def parse_unittest_args(


def send_post_request(
payload: Union[ExecutionPayloadDict, DiscoveryPayloadDict, CoveragePayloadDict],
payload: ExecutionPayloadDict | DiscoveryPayloadDict | CoveragePayloadDict,
test_run_pipe: Optional[str],
):
"""
Expand All @@ -331,10 +331,10 @@ def send_post_request(

if __writer is None:
try:
__writer = socket_manager.PipeManager(test_run_pipe)
__writer.connect()
__writer = open(test_run_pipe, "w", encoding="utf-8", newline="\r\n") # noqa: SIM115, PTH123
except Exception as error:
error_msg = f"Error attempting to connect to extension named pipe {test_run_pipe}[vscode-unittest]: {error}"
print(error_msg, file=sys.stderr)
__writer = None
raise VSCodeUnittestError(error_msg) from error

Expand All @@ -343,10 +343,11 @@ def send_post_request(
"params": payload,
}
data = json.dumps(rpc)

try:
if __writer:
__writer.write(data)
request = f"""content-length: {len(data)}\ncontent-type: application/json\n\n{data}"""
__writer.write(request)
__writer.flush()
else:
print(
f"Connection error[vscode-unittest], writer is None \n[vscode-unittest] data: \n{data} \n",
Expand Down
4 changes: 2 additions & 2 deletions python_files/vscode_pytest/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -448,7 +448,7 @@ def pytest_sessionfinish(session, exitstatus):
result=file_coverage_map,
error=None,
)
send_post_request(payload)
send_message(payload)


def build_test_tree(session: pytest.Session) -> TestNode:
Expand Down Expand Up @@ -858,7 +858,7 @@ def default(self, o):
return super().default(o)


def send_post_request(
def send_message(
payload: ExecutionPayloadDict | DiscoveryPayloadDict | CoveragePayloadDict,
cls_encoder=None,
):
Expand Down
12 changes: 9 additions & 3 deletions src/client/common/pipes/namedPipes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,9 @@ export async function createWriterPipe(pipeName: string, token?: CancellationTok
await mkfifo(pipeName);
try {
await fs.chmod(pipeName, 0o666);
} catch {}
} catch {
// Intentionally ignored
}
const writer = fs.createWriteStream(pipeName, {
encoding: 'utf-8',
});
Expand All @@ -89,6 +91,7 @@ class CombinedReader implements rpc.MessageReader {

private _onPartialMessage = new rpc.Emitter<rpc.PartialMessageInfo>();

// eslint-disable-next-line @typescript-eslint/no-empty-function
private _callback: rpc.DataCallback = () => {};

private _disposables: rpc.Disposable[] = [];
Expand All @@ -107,6 +110,7 @@ class CombinedReader implements rpc.MessageReader {

listen(callback: rpc.DataCallback): rpc.Disposable {
this._callback = callback;
// eslint-disable-next-line no-return-assign, @typescript-eslint/no-empty-function
return new Disposable(() => (this._callback = () => {}));
}

Expand Down Expand Up @@ -167,7 +171,7 @@ export async function createReaderPipe(pipeName: string, token?: CancellationTok
server.listen(pipeName);
if (token) {
token.onCancellationRequested(() => {
if (server.listening) {
if (server.listening) {
server.close();
}
deferred.reject(new CancellationError());
Expand All @@ -180,7 +184,9 @@ export async function createReaderPipe(pipeName: string, token?: CancellationTok
await mkfifo(pipeName);
try {
await fs.chmod(pipeName, 0o666);
} catch {}
} catch {
// Intentionally ignored
}
const reader = fs.createReadStream(pipeName, { encoding: 'utf-8' });
return new rpc.StreamMessageReader(reader, 'utf-8');
}
5 changes: 2 additions & 3 deletions src/client/testing/testController/common/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -256,7 +256,7 @@ export async function startRunResultNamedPipe(
reader.listen((data: Message) => {
traceVerbose(`Test Result named pipe ${pipeName} received data`);
// if EOT, call decrement connection count (callback)
dataReceivedCallback((data as ExecutionResultMessage).params as ExecutionTestPayload | EOTTestPayload);
dataReceivedCallback((data as ExecutionResultMessage).params as ExecutionTestPayload);
}),
reader.onClose(() => {
// this is called once the server close, once per run instance
Expand Down Expand Up @@ -306,10 +306,9 @@ export async function startDiscoveryNamedPipe(
reader,
reader.listen((data: Message) => {
traceVerbose(`Test Discovery named pipe ${pipeName} received data`);
callback((data as DiscoveryResultMessage).params as DiscoveredTestPayload | EOTTestPayload);
callback((data as DiscoveryResultMessage).params as DiscoveredTestPayload);
}),
reader.onClose(() => {
callback(createEOTPayload(false));
traceVerbose(`Test Discovery named pipe ${pipeName} closed`);
disposable.dispose();
}),
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.

import { TestRun, TestRunProfileKind, Uri } from 'vscode';
import { CancellationTokenSource, TestRun, TestRunProfileKind, Uri } from 'vscode';
import * as path from 'path';
import { ChildProcess } from 'child_process';
import { IConfigurationService, ITestOutputChannel } from '../../../common/types';
Expand Down Expand Up @@ -58,9 +58,6 @@ export class PytestTestExecutionAdapter implements ITestExecutionAdapter {
);
runInstance?.token.onCancellationRequested(() => {
traceInfo(`Test run cancelled, resolving 'TillServerClose' deferred for ${uri.fsPath}.`);
// if canceled, stop listening for results
serverDispose(); // this will resolve deferredTillServerClose

const executionPayload: ExecutionTestPayload = {
cwd: uri.fsPath,
status: 'success',
Expand All @@ -74,7 +71,6 @@ export class PytestTestExecutionAdapter implements ITestExecutionAdapter {
uri,
testIds,
name,
deferredTillEOT,
cSource,
runInstance,
profileKind,
Expand All @@ -100,7 +96,7 @@ export class PytestTestExecutionAdapter implements ITestExecutionAdapter {
uri: Uri,
testIds: string[],
resultNamedPipeName: string,
serverDispose: () => void,
serverCancel: CancellationTokenSource,
runInstance?: TestRun,
profileKind?: TestRunProfileKind,
executionFactory?: IPythonExecutionFactory,
Expand Down Expand Up @@ -174,7 +170,6 @@ export class PytestTestExecutionAdapter implements ITestExecutionAdapter {
traceInfo(`Running DEBUG pytest with arguments: ${testArgs} for workspace ${uri.fsPath} \r\n`);
await debugLauncher!.launchDebugger(launchOptions, () => {
serverCancel.cancel();
deferredTillEOT?.resolve();
});
} else {
// deferredTillExecClose is resolved when all stdout and stderr is read
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ export class UnittestTestDiscoveryAdapter implements ITestDiscoveryAdapter {
const { unittestArgs } = settings.testing;
const cwd = settings.testing.cwd && settings.testing.cwd.length > 0 ? settings.testing.cwd : uri.fsPath;

const { name, dispose } = await startDiscoveryNamedPipe((data: DiscoveredTestPayload) => {
const name = await startDiscoveryNamedPipe((data: DiscoveredTestPayload) => {
this.resultResolver?.resolveDiscovery(data);
});

Expand All @@ -66,7 +66,7 @@ export class UnittestTestDiscoveryAdapter implements ITestDiscoveryAdapter {
try {
await this.runDiscovery(uri, options, name, cwd, executionFactory);
} finally {
dispose();
// none
}
// placeholder until after the rewrite is adopted
// TODO: remove after adoption.
Expand Down
19 changes: 10 additions & 9 deletions src/client/testing/testController/unittest/testExecutionAdapter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
// Licensed under the MIT License.

import * as path from 'path';
import { TestRun, TestRunProfileKind, Uri } from 'vscode';
import { CancellationTokenSource, TestRun, TestRunProfileKind, Uri } from 'vscode';
import { ChildProcess } from 'child_process';
import { IConfigurationService, ITestOutputChannel } from '../../../common/types';
import { Deferred, createDeferred } from '../../../common/utils/async';
Expand Down Expand Up @@ -58,23 +58,24 @@ export class UnittestTestExecutionAdapter implements ITestExecutionAdapter {
traceError(`No run instance found, cannot resolve execution, for workspace ${uri.fsPath}.`);
}
};
const { name: resultNamedPipeName, dispose: serverDispose } = await utils.startRunResultNamedPipe(
const cSource = new CancellationTokenSource();
runInstance?.token.onCancellationRequested(() => cSource.cancel());
const name = await utils.startRunResultNamedPipe(
dataReceivedCallback, // callback to handle data received
deferredTillServerClose, // deferred to resolve when server closes
runInstance?.token, // token to cancel
cSource.token, // token to cancel
);
runInstance?.token.onCancellationRequested(() => {
console.log(`Test run cancelled, resolving 'till TillAllServerClose' deferred for ${uri.fsPath}.`);
// if canceled, stop listening for results
deferredTillServerClose.resolve();
serverDispose();
});
try {
await this.runTestsNew(
uri,
testIds,
resultNamedPipeName,
serverDispose,
name,
cSource,
runInstance,
profileKind,
executionFactory,
Expand All @@ -97,7 +98,7 @@ export class UnittestTestExecutionAdapter implements ITestExecutionAdapter {
uri: Uri,
testIds: string[],
resultNamedPipeName: string,
serverDispose: () => void,
serverCancel: CancellationTokenSource,
runInstance?: TestRun,
profileKind?: TestRunProfileKind,
executionFactory?: IPythonExecutionFactory,
Expand Down Expand Up @@ -172,7 +173,7 @@ export class UnittestTestExecutionAdapter implements ITestExecutionAdapter {
throw new Error('Debug launcher is not defined');
}
await debugLauncher.launchDebugger(launchOptions, () => {
serverDispose(); // this will resolve the deferredTillAllServerClose
serverCancel.cancel();
});
} else {
// This means it is running the test
Expand Down Expand Up @@ -225,9 +226,9 @@ export class UnittestTestExecutionAdapter implements ITestExecutionAdapter {
runInstance,
);
}
serverDispose();
}
deferredTillExecClose.resolve();
serverCancel.cancel();
});
await deferredTillExecClose.promise;
}
Expand Down

0 comments on commit c6c47f7

Please sign in to comment.