Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test_runner: use v8.serialize instead of TAP #47867

Merged
merged 6 commits into from
May 15, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion doc/api/cli.md
Original file line number Diff line number Diff line change
Expand Up @@ -2321,7 +2321,8 @@ on unsupported platforms will not be fixed.
### `NODE_TEST_CONTEXT=value`

If `value` equals `'child'`, test reporter options will be overridden and test
output will be sent to stdout in the TAP format.
output will be sent to stdout in the TAP format. If any other value is provided,
Node.js makes no guarantees about the reporter format used or its stability.

### `NODE_TLS_REJECT_UNAUTHORIZED=value`

Expand Down
39 changes: 39 additions & 0 deletions lib/internal/test_runner/reporter/v8-serializer.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
'use strict';

const { DefaultSerializer } = require('v8');
const { Buffer } = require('buffer');
const { serializeError } = require('internal/error_serdes');


module.exports = async function* v8Reporter(source) {
const serializer = new DefaultSerializer();

for await (const item of source) {
const originalError = item.data.details?.error;
if (originalError) {
// Error is overriden with a serialized version, so that it can be
// deserialized in the parent process.
// Error is restored after serialization.
item.data.details.error = serializeError(originalError);
MoLow marked this conversation as resolved.
Show resolved Hide resolved
}
// Add 4 bytes, to later populate with message length
serializer.writeRawBytes(Buffer.allocUnsafe(4));
serializer.writeHeader();
serializer.writeValue(item);

if (originalError) {
item.data.details.error = originalError;
}

const serializedMessage = serializer.releaseBuffer();
const serializedMessageLength = serializedMessage.length - 4;

serializedMessage.set([
serializedMessageLength >> 24 & 0xFF,
serializedMessageLength >> 16 & 0xFF,
serializedMessageLength >> 8 & 0xFF,
serializedMessageLength & 0xFF,
], 0);
yield serializedMessage;
}
};
219 changes: 124 additions & 95 deletions lib/internal/test_runner/runner.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,27 +7,31 @@ const {
ArrayPrototypeIncludes,
ArrayPrototypeMap,
ArrayPrototypePush,
ArrayPrototypeShift,
ArrayPrototypeSlice,
ArrayPrototypeSome,
ArrayPrototypeSort,
ObjectAssign,
PromisePrototypeThen,
SafePromiseAll,
SafePromiseAllReturnVoid,
SafePromiseAllSettledReturnVoid,
PromiseResolve,
RegExpPrototypeSymbolSplit,
SafeMap,
SafeSet,
StringPrototypeIndexOf,
StringPrototypeSlice,
StringPrototypeStartsWith,
TypedArrayPrototypeSubarray,
} = primordials;

const { spawn } = require('child_process');
const { readdirSync, statSync } = require('fs');
const { finished } = require('internal/streams/end-of-stream');
const { DefaultDeserializer, DefaultSerializer } = require('v8');
// TODO(aduh95): switch to internal/readline/interface when backporting to Node.js 16.x is no longer a concern.
const { createInterface } = require('readline');
const { deserializeError } = require('internal/error_serdes');
const { Buffer } = require('buffer');
const { FilesWatcher } = require('internal/watch_mode/files_watcher');
const console = require('internal/console/global');
const {
Expand All @@ -40,6 +44,7 @@ const { validateArray, validateBoolean, validateFunction } = require('internal/v
const { getInspectPort, isUsingInspector, isInspectorMessage } = require('internal/util/inspector');
const { isRegExp } = require('internal/util/types');
const { kEmptyObject } = require('internal/util');
const { kEmitMessage } = require('internal/test_runner/tests_stream');
const { createTestTree } = require('internal/test_runner/harness');
const {
kAborted,
Expand All @@ -49,9 +54,6 @@ const {
kTestTimeoutFailure,
Test,
} = require('internal/test_runner/test');
const { TapParser } = require('internal/test_runner/tap_parser');
const { YAMLToJs } = require('internal/test_runner/yaml_to_js');
const { TokenKind } = require('internal/test_runner/tap_lexer');

const {
convertStringToRegExp,
Expand Down Expand Up @@ -153,92 +155,62 @@ function getRunArgs({ path, inspectPort, testNamePatterns }) {
return argv;
}

const serializer = new DefaultSerializer();
serializer.writeHeader();
const v8Header = serializer.releaseBuffer();
const kSerializedSizeHeader = 4;

class FileTest extends Test {
#buffer = [];
#messageBuffer = [];
#messageBufferSize = 0;
#reportedChildren = 0;
failedSubtests = false;
#skipReporting() {
return this.#reportedChildren > 0 && (!this.error || this.error.failureType === kSubtestsFailed);
}
#checkNestedComment({ comment }) {
#checkNestedComment(comment) {
const firstSpaceIndex = StringPrototypeIndexOf(comment, ' ');
if (firstSpaceIndex === -1) return false;
const secondSpaceIndex = StringPrototypeIndexOf(comment, ' ', firstSpaceIndex + 1);
return secondSpaceIndex === -1 &&
ArrayPrototypeIncludes(kDiagnosticsFilterArgs, StringPrototypeSlice(comment, 0, firstSpaceIndex));
}
#handleReportItem({ kind, node, comments, nesting = 0 }) {
if (comments) {
ArrayPrototypeForEach(comments, (comment) => this.reporter.diagnostic(nesting, this.name, comment));
}
switch (kind) {
case TokenKind.TAP_VERSION:
// TODO(manekinekko): handle TAP version coming from the parser.
// this.reporter.version(node.version);
break;

case TokenKind.TAP_PLAN:
if (nesting === 0 && this.#skipReporting()) {
break;
}
this.reporter.plan(nesting, this.name, node.end - node.start + 1);
break;

case TokenKind.TAP_SUBTEST_POINT:
this.reporter.start(nesting, this.name, node.name);
break;

case TokenKind.TAP_TEST_POINT: {

const { todo, skip, pass } = node.status;

let directive;

if (skip) {
directive = this.reporter.getSkip(node.reason || true);
} else if (todo) {
directive = this.reporter.getTodo(node.reason || true);
} else {
directive = kEmptyObject;
}

const diagnostics = YAMLToJs(node.diagnostics);
const cancelled = kCanceledTests.has(diagnostics.error?.failureType);
const testNumber = nesting === 0 ? (this.root.harness.counters.topLevel + 1) : node.id;
const method = pass ? 'ok' : 'fail';
this.reporter[method](nesting, this.name, testNumber, node.description, diagnostics, directive);
countCompletedTest({
name: node.description,
finished: true,
skipped: skip,
isTodo: todo,
passed: pass,
cancelled,
nesting,
reportedType: diagnostics.type,
}, this.root.harness);
break;

#handleReportItem(item) {
const isTopLevel = item.data.nesting === 0;
if (isTopLevel) {
if (item.type === 'test:plan' && this.#skipReporting()) {
return;
}
case TokenKind.COMMENT:
if (nesting === 0 && this.#checkNestedComment(node)) {
// Ignore file top level diagnostics
break;
}
this.reporter.diagnostic(nesting, this.name, node.comment);
break;

case TokenKind.UNKNOWN:
this.reporter.diagnostic(nesting, this.name, node.value);
break;
if (item.type === 'test:diagnostic' && this.#checkNestedComment(item.data.message)) {
return;
}
}
if (item.data.details?.error) {
item.data.details.error = deserializeError(item.data.details.error);
}
if (item.type === 'test:pass' || item.type === 'test:fail') {
item.data.testNumber = isTopLevel ? (this.root.harness.counters.topLevel + 1) : item.data.testNumber;
countCompletedTest({
MoLow marked this conversation as resolved.
Show resolved Hide resolved
__proto__: null,
name: item.data.name,
finished: true,
skipped: item.data.skip !== undefined,
isTodo: item.data.todo !== undefined,
passed: item.type === 'test:pass',
cancelled: kCanceledTests.has(item.data.details?.error?.failureType),
nesting: item.data.nesting,
reportedType: item.data.details?.type,
}, this.root.harness);
}
this.reporter[kEmitMessage](item.type, item.data);
}
#accumulateReportItem({ kind, node, comments, nesting = 0 }) {
if (kind !== TokenKind.TAP_TEST_POINT) {
#accumulateReportItem(item) {
if (item.type !== 'test:pass' && item.type !== 'test:fail') {
return;
}
this.#reportedChildren++;
if (nesting === 0 && !node.status.pass) {
if (item.data.nesting === 0 && item.type === 'test:fail') {
this.failedSubtests = true;
}
}
Expand All @@ -248,14 +220,79 @@ class FileTest extends Test {
this.#buffer = [];
}
}
addToReport(ast) {
this.#accumulateReportItem(ast);
addToReport(item) {
this.#accumulateReportItem(item);
if (!this.isClearToSend()) {
ArrayPrototypePush(this.#buffer, ast);
ArrayPrototypePush(this.#buffer, item);
return;
}
this.#drainBuffer();
this.#handleReportItem(ast);
this.#handleReportItem(item);
}
parseMessage(readData) {
if (readData.length === 0) return;

ArrayPrototypePush(this.#messageBuffer, readData);
this.#messageBufferSize += readData.length;
MoLow marked this conversation as resolved.
Show resolved Hide resolved

// Index 0 should always be present because we just pushed data into it.
let messageBufferHead = this.#messageBuffer[0];
let headerIndex = messageBufferHead.indexOf(v8Header);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
let headerIndex = messageBufferHead.indexOf(v8Header);
let headerIndex = ArrayPrototypeIndexOf(messageBufferHead, v8Header);

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

messageBufferHead is a Buffer

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If it's a Buffer, you should be able to use Uint8ArrayPrototypeIndexOf

Copy link
Member Author

@MoLow MoLow May 14, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have tried but:

./node --expose-internals -p "require('internal/test/binding').primordials.Uint8ArrayPrototypeIndexOf()" 

results in require(...).primordials.Uint8ArrayPrototypeIndexOf is not a function
and

./node --expose-internals -p "require('internal/test/binding').primordials.TypedArrayPrototypeIndexOf(Buffer.from('Hello'), Buffer.from('ll'))"

results in -1

This comment was marked as outdated.

let nonSerialized = Buffer.alloc(0);

while (messageBufferHead && headerIndex !== kSerializedSizeHeader) {
const nonSerializedData = headerIndex === -1 ?
messageBufferHead :
messageBufferHead.slice(0, headerIndex - kSerializedSizeHeader);
MoLow marked this conversation as resolved.
Show resolved Hide resolved
nonSerialized = Buffer.concat([nonSerialized, nonSerializedData]);
this.#messageBufferSize -= nonSerializedData.length;
MoLow marked this conversation as resolved.
Show resolved Hide resolved
if (headerIndex === -1) {
ArrayPrototypeShift(this.#messageBuffer);
} else {
this.#messageBuffer[0] = messageBufferHead.subarray(headerIndex - kSerializedSizeHeader);
MoLow marked this conversation as resolved.
Show resolved Hide resolved
}
messageBufferHead = this.#messageBuffer[0];
headerIndex = messageBufferHead?.indexOf(v8Header);
MoLow marked this conversation as resolved.
Show resolved Hide resolved
}

if (nonSerialized.length > 0) {
const messages = RegExpPrototypeSymbolSplit(/\r?\n/, nonSerialized.toString('utf-8'));
messages.forEach((message) => {
MoLow marked this conversation as resolved.
Show resolved Hide resolved
this.addToReport({
__proto__: null,
type: 'test:diagnostic',
data: { __proto__: null, nesting: 0, file: this.name, message },
});
});
MoLow marked this conversation as resolved.
Show resolved Hide resolved
}

while (messageBufferHead?.length >= kSerializedSizeHeader) {
// We call `readUInt32BE` manually here, because this is faster than first converting
// it to a buffer and using `readUInt32BE` on that.
const fullMessageSize = (
messageBufferHead[0] << 24 |
messageBufferHead[1] << 16 |
messageBufferHead[2] << 8 |
messageBufferHead[3]
) + kSerializedSizeHeader;

if (this.#messageBufferSize < fullMessageSize) break;

const concatenatedBuffer = this.#messageBuffer.length === 1 ?
this.#messageBuffer[0] : Buffer.concat(this.#messageBuffer, this.#messageBufferSize);

const deserializer = new DefaultDeserializer(
TypedArrayPrototypeSubarray(concatenatedBuffer, kSerializedSizeHeader, fullMessageSize),
);

messageBufferHead = TypedArrayPrototypeSubarray(concatenatedBuffer, fullMessageSize);
this.#messageBufferSize = messageBufferHead.length;
MoLow marked this conversation as resolved.
Show resolved Hide resolved
this.#messageBuffer = this.#messageBufferSize !== 0 ? [messageBufferHead] : [];

deserializer.readHeader();
const item = deserializer.readValue();
this.addToReport(item);
}
}
reportStarted() {}
report() {
Expand All @@ -275,7 +312,7 @@ function runTestFile(path, root, inspectPort, filesWatcher, testNamePatterns) {
const subtest = root.createSubtest(FileTest, path, async (t) => {
const args = getRunArgs({ path, inspectPort, testNamePatterns });
const stdio = ['pipe', 'pipe', 'pipe'];
const env = { ...process.env, NODE_TEST_CONTEXT: 'child' };
const env = { ...process.env, NODE_TEST_CONTEXT: 'child-v8' };
if (filesWatcher) {
stdio.push('ipc');
env.WATCH_REPORT_DEPENDENCIES = '1';
Expand All @@ -292,6 +329,10 @@ function runTestFile(path, root, inspectPort, filesWatcher, testNamePatterns) {
err = error;
});

child.stdout.on('data', (data) => {
subtest.parseMessage(data);
});

const rl = createInterface({ input: child.stderr });
rl.on('line', (line) => {
if (isInspectorMessage(line)) {
Expand All @@ -303,26 +344,14 @@ function runTestFile(path, root, inspectPort, filesWatcher, testNamePatterns) {
// surface stderr lines as TAP diagnostics to improve the DX. Inject
// each line into the test output as an unknown token as if it came
// from the TAP parser.
const node = {
kind: TokenKind.UNKNOWN,
node: {
value: line,
},
};

subtest.addToReport(node);
});

const parser = new TapParser();

child.stdout.pipe(parser).on('data', (ast) => {
subtest.addToReport(ast);
subtest.addToReport({
__proto__: null,
type: 'test:diagnostic',
data: { __proto__: null, nesting: 0, file: path, message: line },
});
});

const { 0: { 0: code, 1: signal } } = await SafePromiseAll([
once(child, 'exit', { signal: t.signal }),
finished(parser, { signal: t.signal }),
]);
const { 0: code, 1: signal } = await once(child, 'exit', { signal: t.signal });

runningProcesses.delete(path);
runningSubtests.delete(path);
Expand Down
Loading