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.

Sign up for GitHub

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

Jump to bottom

test_runner: refactor spec reporter to generator function #50177

Closed
wants to merge 3 commits into from
Closed

test_runner: refactor spec reporter to generator function #50177

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
dba68c0
test_runner: refactor spec reporter to generator function
himself65 Oct 13, 2023
e218ad6
fixup! fix: remove this
himself65 Oct 13, 2023
331f9a7
fixup! fix: code
himself65 Oct 14, 2023
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
64 changes: 31 additions & 33 deletions lib/internal/test_runner/reporter/spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@ const {
SafeMap,
StringPrototypeRepeat,
} = primordials;

const assert = require('assert');
const Transform = require('internal/streams/transform');
const { inspectWithNoCustomRetry } = require('internal/errors');
const { green, blue, red, white, gray, shouldColorize } = require('internal/util/colors');
const { kSubtestsFailed } = require('internal/test_runner/test');
Expand All @@ -27,6 +27,7 @@ const colors = {
'test:pass': green,
'test:diagnostic': blue,
};

const symbols = {
'__proto__': null,
'test:fail': '\u2716 ',
Expand All @@ -36,27 +37,25 @@ const symbols = {
'arrow:right': '\u25B6 ',
'hyphen:minus': '\uFE63 ',
};
class SpecReporter extends Transform {
#stack = [];
#reported = [];
#indentMemo = new SafeMap();
#failedTests = [];
#cwd = process.cwd();

constructor() {
super({ __proto__: null, writableObjectMode: true });
}

#indent(nesting) {
let value = this.#indentMemo.get(nesting);
module.exports = async function* specReporter(source) {
himself65 marked this conversation as resolved.
Show resolved Hide resolved
let stack = [];
let reported = [];
let indentMemo = new SafeMap();
let failedTests = [];
let cwd = process.cwd();

function indent(nesting) {
Copy link
Member

Choose a reason for hiding this comment

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

this means all of these functions are defined anew every time this reporter is called, instead of just once at module startup.

Copy link
Member Author

Choose a reason for hiding this comment

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

these functions require closure variables

Copy link
Member

Choose a reason for hiding this comment

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

then if you're switching to a non-class-based model, the functions should take everything as arguments, so they can be defined at module level.

let value = indentMemo.get(nesting);
if (value === undefined) {
value = StringPrototypeRepeat(' ', nesting);
this.#indentMemo.set(nesting, value);
indentMemo.set(nesting, value);
}

return value;
}
#formatError(error, indent) {

function formatError(error, indent) {
if (!error) return '';
const err = error.code === 'ERR_TEST_FAILURE' ? error.cause : error;
const message = ArrayPrototypeJoin(
Expand All @@ -66,7 +65,8 @@ class SpecReporter extends Transform {
), `\n${indent} `);
return `\n${indent} ${message}\n`;
}
#formatTestReport(type, data, prefix = '', indent = '', hasChildren = false) {

function formatTestReport(type, data, prefix = '', indent = '', hasChildren = false) {
let color = colors[type] ?? white;
let symbol = symbols[type] ?? ' ';
const { skip, todo } = data;
Expand All @@ -89,7 +89,8 @@ class SpecReporter extends Transform {
}
return `${prefix}${indent}${color}${symbol}${title}${white}${error}`;
}
#handleTestReportEvent(type, data) {

function handleTestReportEvent(type, data) {
const subtest = ArrayPrototypeShift(this.#stack); // This is the matching `test:start` event
if (subtest) {
assert(subtest.type === 'test:start');
Expand All @@ -113,7 +114,8 @@ class SpecReporter extends Transform {
const indent = this.#indent(data.nesting);
return `${this.#formatTestReport(type, data, prefix, indent, hasChildren)}\n`;
}
#handleEvent({ type, data }) {

function handleEvent({ type, data }) {
switch (type) {
case 'test:fail':
if (data.details?.error?.failureType !== kSubtestsFailed) {
Expand All @@ -134,25 +136,21 @@ class SpecReporter extends Transform {
return getCoverageReport(this.#indent(data.nesting), data.summary, symbols['test:coverage'], blue, true);
}
}
_transform({ type, data }, encoding, callback) {
callback(null, this.#handleEvent({ __proto__: null, type, data }));

for await (const event of source) {
yield handleEvent(event);
}
_flush(callback) {
if (this.#failedTests.length === 0) {
callback(null, '');
return;
}

if (failedTests.length !== 0) {
const results = [`\n${colors['test:fail']}${symbols['test:fail']}failing tests:${white}\n`];
for (let i = 0; i < this.#failedTests.length; i++) {
const test = this.#failedTests[i];
const relPath = relative(this.#cwd, test.file);
const formattedErr = this.#formatTestReport('test:fail', test);
for (let i = 0; i < failedTests.length; i++) {
const test = failedTests[i];
const relPath = relative(cwd, test.file);
const formattedErr = formatTestReport('test:fail', test);
const location = `test at ${relPath}:${test.line}:${test.column}`;

ArrayPrototypePush(results, location, formattedErr);
}
callback(null, ArrayPrototypeJoin(results, '\n'));
yield ArrayPrototypeJoin(results, '\n');
}
}

module.exports = SpecReporter;
};
Loading