Skip to content

Commit 869dc64

Browse files
switch to using tcp for comm with server (#20981)
code written by @karthiknadig and @eleanorjboyd, which switches to using TCP as the communication channel between the test adapter in the extension and the node server that handles the discovery/running of python tests. --------- Co-authored-by: Karthik Nadig <[email protected]>
1 parent 2401e13 commit 869dc64

File tree

6 files changed

+193
-69
lines changed

6 files changed

+193
-69
lines changed

pythonFiles/unittestadapter/discovery.py

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -121,13 +121,16 @@ def discover_tests(
121121

122122
# Build the request data (it has to be a POST request or the Node side will not process it), and send it.
123123
addr = ("localhost", port)
124-
with socket_manager.SocketManager(addr) as s:
125-
data = json.dumps(payload)
126-
request = f"""POST / HTTP/1.1
127-
Host: localhost:{port}
128-
Content-Length: {len(data)}
124+
data = json.dumps(payload)
125+
request = f"""Content-Length: {len(data)}
129126
Content-Type: application/json
130127
Request-uuid: {uuid}
131128
132129
{data}"""
133-
result = s.socket.sendall(request.encode("utf-8")) # type: ignore
130+
try:
131+
with socket_manager.SocketManager(addr) as s:
132+
if s.socket is not None:
133+
s.socket.sendall(request.encode("utf-8"))
134+
except Exception as e:
135+
print(f"Error sending response: {e}")
136+
print(f"Request data: {request}")

pythonFiles/unittestadapter/execution.py

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -222,13 +222,16 @@ def run_tests(
222222

223223
# Build the request data (it has to be a POST request or the Node side will not process it), and send it.
224224
addr = ("localhost", port)
225-
with socket_manager.SocketManager(addr) as s:
226-
data = json.dumps(payload)
227-
request = f"""POST / HTTP/1.1
228-
Host: localhost:{port}
229-
Content-Length: {len(data)}
225+
data = json.dumps(payload)
226+
request = f"""Content-Length: {len(data)}
230227
Content-Type: application/json
231228
Request-uuid: {uuid}
232229
233230
{data}"""
234-
result = s.socket.sendall(request.encode("utf-8")) # type: ignore
231+
try:
232+
with socket_manager.SocketManager(addr) as s:
233+
if s.socket is not None:
234+
s.socket.sendall(request.encode("utf-8"))
235+
except Exception as e:
236+
print(f"Error sending response: {e}")
237+
print(f"Request data: {request}")

src/client/testing/testController/common/server.ts

Lines changed: 42 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
// Copyright (c) Microsoft Corporation. All rights reserved.
22
// Licensed under the MIT License.
33

4-
import * as http from 'http';
54
import * as net from 'net';
65
import * as crypto from 'crypto';
76
import { Disposable, Event, EventEmitter } from 'vscode';
@@ -14,53 +13,66 @@ import { traceLog } from '../../../logging';
1413
import { DataReceivedEvent, ITestServer, TestCommandOptions } from './types';
1514
import { ITestDebugLauncher, LaunchOptions } from '../../common/types';
1615
import { UNITTEST_PROVIDER } from '../../common/constants';
16+
import { jsonRPCHeaders, jsonRPCContent, JSONRPC_UUID_HEADER } from './utils';
1717

1818
export class PythonTestServer implements ITestServer, Disposable {
1919
private _onDataReceived: EventEmitter<DataReceivedEvent> = new EventEmitter<DataReceivedEvent>();
2020

2121
private uuids: Map<string, string>;
2222

23-
private server: http.Server;
23+
private server: net.Server;
2424

2525
private ready: Promise<void>;
2626

2727
constructor(private executionFactory: IPythonExecutionFactory, private debugLauncher: ITestDebugLauncher) {
2828
this.uuids = new Map();
2929

30-
const requestListener: http.RequestListener = async (request, response) => {
31-
const buffers = [];
32-
33-
try {
34-
for await (const chunk of request) {
35-
buffers.push(chunk);
36-
}
37-
38-
const data = Buffer.concat(buffers).toString();
39-
// grab the uuid from the header
40-
const indexRequestuuid = request.rawHeaders.indexOf('Request-uuid');
41-
const uuid = request.rawHeaders[indexRequestuuid + 1];
42-
response.end();
43-
44-
JSON.parse(data);
45-
// Check if the uuid we received exists in the list of active ones.
46-
// If yes, process the response, if not, ignore it.
47-
const cwd = this.uuids.get(uuid);
48-
if (cwd) {
49-
this._onDataReceived.fire({ uuid, data });
50-
this.uuids.delete(uuid);
30+
this.server = net.createServer((socket: net.Socket) => {
31+
socket.on('data', (data: Buffer) => {
32+
try {
33+
let rawData: string = data.toString();
34+
35+
while (rawData.length > 0) {
36+
const rpcHeaders = jsonRPCHeaders(rawData);
37+
const uuid = rpcHeaders.headers.get(JSONRPC_UUID_HEADER);
38+
rawData = rpcHeaders.remainingRawData;
39+
if (uuid) {
40+
const rpcContent = jsonRPCContent(rpcHeaders.headers, rawData);
41+
rawData = rpcContent.remainingRawData;
42+
const cwd = this.uuids.get(uuid);
43+
if (cwd) {
44+
this._onDataReceived.fire({ uuid, data: rpcContent.extractedJSON });
45+
this.uuids.delete(uuid);
46+
}
47+
} else {
48+
traceLog(`Error processing test server request: uuid not found`);
49+
this._onDataReceived.fire({ uuid: '', data: '' });
50+
return;
51+
}
52+
}
53+
} catch (ex) {
54+
traceLog(`Error processing test server request: ${ex} observe`);
55+
this._onDataReceived.fire({ uuid: '', data: '' });
5156
}
52-
} catch (ex) {
53-
traceLog(`Error processing test server request: ${ex} observe`);
54-
this._onDataReceived.fire({ uuid: '', data: '' });
55-
}
56-
};
57-
58-
this.server = http.createServer(requestListener);
57+
});
58+
});
5959
this.ready = new Promise((resolve, _reject) => {
6060
this.server.listen(undefined, 'localhost', () => {
6161
resolve();
6262
});
6363
});
64+
this.server.on('error', (ex) => {
65+
traceLog(`Error starting test server: ${ex}`);
66+
});
67+
this.server.on('close', () => {
68+
traceLog('Test server closed.');
69+
});
70+
this.server.on('listening', () => {
71+
traceLog('Test server listening.');
72+
});
73+
this.server.on('connection', () => {
74+
traceLog('Test server connected to a client.');
75+
});
6476
}
6577

6678
public serverReady(): Promise<void> {

src/client/testing/testController/common/utils.ts

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,3 +5,48 @@ export function fixLogLines(content: string): string {
55
const lines = content.split(/\r?\n/g);
66
return `${lines.join('\r\n')}\r\n`;
77
}
8+
export interface IJSONRPCContent {
9+
extractedJSON: string;
10+
remainingRawData: string;
11+
}
12+
13+
export interface IJSONRPCHeaders {
14+
headers: Map<string, string>;
15+
remainingRawData: string;
16+
}
17+
18+
export const JSONRPC_UUID_HEADER = 'Request-uuid';
19+
export const JSONRPC_CONTENT_LENGTH_HEADER = 'Content-Length';
20+
export const JSONRPC_CONTENT_TYPE_HEADER = 'Content-Type';
21+
22+
export function jsonRPCHeaders(rawData: string): IJSONRPCHeaders {
23+
const lines = rawData.split('\n');
24+
let remainingRawData = '';
25+
const headerMap = new Map<string, string>();
26+
for (let i = 0; i < lines.length; i += 1) {
27+
const line = lines[i];
28+
if (line === '') {
29+
remainingRawData = lines.slice(i + 1).join('\n');
30+
break;
31+
}
32+
const [key, value] = line.split(':');
33+
if ([JSONRPC_UUID_HEADER, JSONRPC_CONTENT_LENGTH_HEADER, JSONRPC_CONTENT_TYPE_HEADER].includes(key)) {
34+
headerMap.set(key.trim(), value.trim());
35+
}
36+
}
37+
38+
return {
39+
headers: headerMap,
40+
remainingRawData,
41+
};
42+
}
43+
44+
export function jsonRPCContent(headers: Map<string, string>, rawData: string): IJSONRPCContent {
45+
const length = parseInt(headers.get('Content-Length') ?? '0', 10);
46+
const data = rawData.slice(0, length);
47+
const remainingRawData = rawData.slice(length);
48+
return {
49+
extractedJSON: data,
50+
remainingRawData,
51+
};
52+
}

src/test/testing/testController/server.unit.test.ts

Lines changed: 21 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -2,15 +2,14 @@
22
// Licensed under the MIT License.
33

44
import * as assert from 'assert';
5-
import * as http from 'http';
5+
import * as net from 'net';
66
import * as sinon from 'sinon';
77
import * as crypto from 'crypto';
88
import { OutputChannel, Uri } from 'vscode';
99
import { IPythonExecutionFactory, IPythonExecutionService } from '../../../client/common/process/types';
10-
import { createDeferred } from '../../../client/common/utils/async';
1110
import { PythonTestServer } from '../../../client/testing/testController/common/server';
12-
import * as logging from '../../../client/logging';
1311
import { ITestDebugLauncher } from '../../../client/testing/common/types';
12+
import { createDeferred } from '../../../client/common/utils/async';
1413

1514
suite('Python Test Server', () => {
1615
const fakeUuid = 'fake-uuid';
@@ -21,13 +20,11 @@ suite('Python Test Server', () => {
2120
let sandbox: sinon.SinonSandbox;
2221
let execArgs: string[];
2322
let v4Stub: sinon.SinonStub;
24-
let traceLogStub: sinon.SinonStub;
2523
let debugLauncher: ITestDebugLauncher;
2624

2725
setup(() => {
2826
sandbox = sinon.createSandbox();
2927
v4Stub = sandbox.stub(crypto, 'randomUUID');
30-
traceLogStub = sandbox.stub(logging, 'traceLog');
3128

3229
v4Stub.returns(fakeUuid);
3330
stubExecutionService = ({
@@ -120,45 +117,42 @@ suite('Python Test Server', () => {
120117
});
121118

122119
test('If the server receives malformed data, it should display a log message, and not fire an event', async () => {
120+
let eventData: string | undefined;
121+
const client = new net.Socket();
123122
const deferred = createDeferred();
123+
124124
const options = {
125125
command: { script: 'myscript', args: ['-foo', 'foo'] },
126126
workspaceFolder: Uri.file('/foo/bar'),
127127
cwd: '/foo/bar',
128128
uuid: fakeUuid,
129129
};
130130

131-
let response;
131+
stubExecutionService = ({
132+
exec: async () => {
133+
client.connect(server.getPort());
134+
return Promise.resolve({ stdout: '', stderr: '' });
135+
},
136+
} as unknown) as IPythonExecutionService;
132137

133138
server = new PythonTestServer(stubExecutionFactory, debugLauncher);
134139
await server.serverReady();
135-
136140
server.onDataReceived(({ data }) => {
137-
response = data;
141+
eventData = data;
138142
deferred.resolve();
139143
});
140144

141-
await server.sendCommand(options);
142-
143-
// Send data back.
144-
const port = server.getPort();
145-
const requestOptions = {
146-
hostname: 'localhost',
147-
method: 'POST',
148-
port,
149-
headers: { 'Request-uuid': fakeUuid },
150-
};
151-
152-
const request = http.request(requestOptions, (res) => {
153-
res.setEncoding('utf8');
145+
client.on('connect', () => {
146+
console.log('Socket connected, local port:', client.localPort);
147+
client.write('malformed data');
148+
client.end();
149+
});
150+
client.on('error', (error) => {
151+
console.log('Socket connection error:', error);
154152
});
155-
const postData = '[test';
156-
request.write(postData);
157-
request.end();
158153

154+
await server.sendCommand(options);
159155
await deferred.promise;
160-
161-
sinon.assert.calledOnce(traceLogStub);
162-
assert.deepStrictEqual(response, '');
156+
assert.deepStrictEqual(eventData, '');
163157
});
164158
});
Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
// Copyright (c) Microsoft Corporation. All rights reserved.
2+
// Licensed under the MIT License.
3+
4+
import * as assert from 'assert';
5+
import {
6+
JSONRPC_CONTENT_LENGTH_HEADER,
7+
JSONRPC_CONTENT_TYPE_HEADER,
8+
JSONRPC_UUID_HEADER,
9+
jsonRPCContent,
10+
jsonRPCHeaders,
11+
} from '../../../client/testing/testController/common/utils';
12+
13+
suite('Test Controller Utils: JSON RPC', () => {
14+
test('Empty raw data string', async () => {
15+
const rawDataString = '';
16+
17+
const output = jsonRPCHeaders(rawDataString);
18+
assert.deepStrictEqual(output.headers.size, 0);
19+
assert.deepStrictEqual(output.remainingRawData, '');
20+
});
21+
22+
test('Valid data empty JSON', async () => {
23+
const rawDataString = `${JSONRPC_CONTENT_LENGTH_HEADER}: 2\n${JSONRPC_CONTENT_TYPE_HEADER}: application/json\n${JSONRPC_UUID_HEADER}: 1234\n\n{}`;
24+
25+
const rpcHeaders = jsonRPCHeaders(rawDataString);
26+
assert.deepStrictEqual(rpcHeaders.headers.size, 3);
27+
assert.deepStrictEqual(rpcHeaders.remainingRawData, '{}');
28+
const rpcContent = jsonRPCContent(rpcHeaders.headers, rpcHeaders.remainingRawData);
29+
assert.deepStrictEqual(rpcContent.extractedJSON, '{}');
30+
});
31+
32+
test('Valid data NO JSON', async () => {
33+
const rawDataString = `${JSONRPC_CONTENT_LENGTH_HEADER}: 0\n${JSONRPC_CONTENT_TYPE_HEADER}: application/json\n${JSONRPC_UUID_HEADER}: 1234\n\n`;
34+
35+
const rpcHeaders = jsonRPCHeaders(rawDataString);
36+
assert.deepStrictEqual(rpcHeaders.headers.size, 3);
37+
assert.deepStrictEqual(rpcHeaders.remainingRawData, '');
38+
const rpcContent = jsonRPCContent(rpcHeaders.headers, rpcHeaders.remainingRawData);
39+
assert.deepStrictEqual(rpcContent.extractedJSON, '');
40+
});
41+
42+
test('Valid data with full JSON', async () => {
43+
// this is just some random JSON
44+
const json =
45+
'{"jsonrpc": "2.0", "method": "initialize", "params": {"processId": 1234, "rootPath": "/home/user/project", "rootUri": "file:///home/user/project", "capabilities": {}}, "id": 0}';
46+
const rawDataString = `${JSONRPC_CONTENT_LENGTH_HEADER}: ${json.length}\n${JSONRPC_CONTENT_TYPE_HEADER}: application/json\n${JSONRPC_UUID_HEADER}: 1234\n\n${json}`;
47+
48+
const rpcHeaders = jsonRPCHeaders(rawDataString);
49+
assert.deepStrictEqual(rpcHeaders.headers.size, 3);
50+
assert.deepStrictEqual(rpcHeaders.remainingRawData, json);
51+
const rpcContent = jsonRPCContent(rpcHeaders.headers, rpcHeaders.remainingRawData);
52+
assert.deepStrictEqual(rpcContent.extractedJSON, json);
53+
});
54+
55+
test('Valid data with multiple JSON', async () => {
56+
const json =
57+
'{"jsonrpc": "2.0", "method": "initialize", "params": {"processId": 1234, "rootPath": "/home/user/project", "rootUri": "file:///home/user/project", "capabilities": {}}, "id": 0}';
58+
const rawDataString = `${JSONRPC_CONTENT_LENGTH_HEADER}: ${json.length}\n${JSONRPC_CONTENT_TYPE_HEADER}: application/json\n${JSONRPC_UUID_HEADER}: 1234\n\n${json}`;
59+
const rawDataString2 = rawDataString + rawDataString;
60+
61+
const rpcHeaders = jsonRPCHeaders(rawDataString2);
62+
assert.deepStrictEqual(rpcHeaders.headers.size, 3);
63+
const rpcContent = jsonRPCContent(rpcHeaders.headers, rpcHeaders.remainingRawData);
64+
assert.deepStrictEqual(rpcContent.extractedJSON, json);
65+
assert.deepStrictEqual(rpcContent.remainingRawData, rawDataString);
66+
});
67+
});

0 commit comments

Comments
 (0)