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: changing spec from class to function #48208

Closed
186 changes: 94 additions & 92 deletions lib/internal/test_runner/reporter/spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,114 +35,116 @@ const symbols = {
'arrow:right': '\u25B6 ',
'hyphen:minus': '\uFE63 ',
};
class SpecReporter extends Transform {
Copy link
Member

Choose a reason for hiding this comment

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

@MoLow @cjihrig Do we still want/need this?

if (reporter?.prototype && ObjectGetOwnPropertyDescriptor(reporter.prototype, 'constructor')) {
reporter = new reporter();
}

The docs do not show passing a constructor as an option...

Copy link
Member

Choose a reason for hiding this comment

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

yes, for custom reporters I guess

Copy link
Member

Choose a reason for hiding this comment

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

Then I guess we might want to add an example for that to the docs?

#stack = [];
#reported = [];
#indentMemo = new SafeMap();
#failedTests = [];
const stack = [];
const reported = [];
const indentMemo = new SafeMap();
const failedTests = [];
Copy link
Member

Choose a reason for hiding this comment

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

Doesn't this mean that calling run twice with spec now shares the failures between runs? (whereas previously you could instantiate a new reporter as per need)

Copy link
Member

Choose a reason for hiding this comment

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

I think I would expect const spec to be a function creating these vars and return the new Transform. Does that make sense?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

makes sense 👍🏻 will change

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hey @atlowChemi, I have updated the PR with the lates changes. Instead of using the new Transform, I have modified it to an async generator, similar to dot and tap reporters.


constructor() {
super({ writableObjectMode: true });
function indent(nesting) {
let value = indentMemo.get(nesting);
if (value === undefined) {
value = StringPrototypeRepeat(' ', nesting);
indentMemo.set(nesting, value);
}

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

return value;
function formatError(error, indent) {
if (!error) return '';
const err = error.code === 'ERR_TEST_FAILURE' ? error.cause : error;
const message = ArrayPrototypeJoin(
RegExpPrototypeSymbolSplit(
hardenRegExp(/\r?\n/),
inspectWithNoCustomRetry(err, inspectOptions),
), `\n${indent} `);
return `\n${indent} ${message}\n`;
}

function formatTestReport(type, data, prefix = '', indent = '', hasChildren = false, skippedSubtest = false) {
let color = colors[type] ?? white;
let symbol = symbols[type] ?? ' ';
const duration_ms = data.details?.duration_ms ? ` ${gray}(${data.details.duration_ms}ms)${white}` : '';
const title = `${data.name}${duration_ms}${skippedSubtest ? ' # SKIP' : ''}`;
if (hasChildren) {
// If this test has had children - it was already reported, so slightly modify the output
return `${prefix}${indent}${color}${symbols['arrow:right']}${white}${title}\n`;
}
#formatError(error, indent) {
if (!error) return '';
const err = error.code === 'ERR_TEST_FAILURE' ? error.cause : error;
const message = ArrayPrototypeJoin(
RegExpPrototypeSymbolSplit(
hardenRegExp(/\r?\n/),
inspectWithNoCustomRetry(err, inspectOptions),
), `\n${indent} `);
return `\n${indent} ${message}\n`;
const error = formatError(data.details?.error, indent);
if (skippedSubtest) {
color = gray;
symbol = symbols['hyphen:minus'];
}
#formatTestReport(type, data, prefix = '', indent = '', hasChildren = false, skippedSubtest = false) {
let color = colors[type] ?? white;
let symbol = symbols[type] ?? ' ';
const duration_ms = data.details?.duration_ms ? ` ${gray}(${data.details.duration_ms}ms)${white}` : '';
const title = `${data.name}${duration_ms}${skippedSubtest ? ' # SKIP' : ''}`;
if (hasChildren) {
// If this test has had children - it was already reported, so slightly modify the output
return `${prefix}${indent}${color}${symbols['arrow:right']}${white}${title}\n`;
}
const error = this.#formatError(data.details?.error, indent);
if (skippedSubtest) {
color = gray;
symbol = symbols['hyphen:minus'];
}
return `${prefix}${indent}${color}${symbol}${title}${white}${error}`;
return `${prefix}${indent}${color}${symbol}${title}${white}${error}`;
}

function handleTestReportEvent(type, data) {
const subtest = ArrayPrototypeShift(stack); // This is the matching `test:start` event
if (subtest) {
assert(subtest.type === 'test:start');
assert(subtest.data.nesting === data.nesting);
assert(subtest.data.name === data.name);
}
#handleTestReportEvent(type, data) {
const subtest = ArrayPrototypeShift(this.#stack); // This is the matching `test:start` event
if (subtest) {
assert(subtest.type === 'test:start');
assert(subtest.data.nesting === data.nesting);
assert(subtest.data.name === data.name);
}
let prefix = '';
while (this.#stack.length) {
// Report all the parent `test:start` events
const parent = ArrayPrototypePop(this.#stack);
assert(parent.type === 'test:start');
const msg = parent.data;
ArrayPrototypeUnshift(this.#reported, msg);
prefix += `${this.#indent(msg.nesting)}${symbols['arrow:right']}${msg.name}\n`;
}
let hasChildren = false;
if (this.#reported[0] && this.#reported[0].nesting === data.nesting && this.#reported[0].name === data.name) {
ArrayPrototypeShift(this.#reported);
hasChildren = true;
}
const skippedSubtest = subtest && data.skip && data.skip !== undefined;
const indent = this.#indent(data.nesting);
return `${this.#formatTestReport(type, data, prefix, indent, hasChildren, skippedSubtest)}\n`;
let prefix = '';
while (stack.length) {
// Report all the parent `test:start` events
const parent = ArrayPrototypePop(stack);
assert(parent.type === 'test:start');
const msg = parent.data;
ArrayPrototypeUnshift(reported, msg);
prefix += `${indent(msg.nesting)}${symbols['arrow:right']}${msg.name}\n`;
}
#handleEvent({ type, data }) {
switch (type) {
case 'test:fail':
if (data.details?.error?.failureType !== kSubtestsFailed) {
ArrayPrototypePush(this.#failedTests, data);
}
return this.#handleTestReportEvent(type, data);
case 'test:pass':
return this.#handleTestReportEvent(type, data);
case 'test:start':
ArrayPrototypeUnshift(this.#stack, { __proto__: null, data, type });
break;
case 'test:stderr':
case 'test:stdout':
return data.message;
case 'test:diagnostic':
return `${colors[type]}${this.#indent(data.nesting)}${symbols[type]}${data.message}${white}\n`;
case 'test:coverage':
return getCoverageReport(this.#indent(data.nesting), data.summary, symbols['test:coverage'], blue);
}
let hasChildren = false;
if (reported[0] && reported[0].nesting === data.nesting && reported[0].name === data.name) {
ArrayPrototypeShift(reported);
hasChildren = true;
}
_transform({ type, data }, encoding, callback) {
callback(null, this.#handleEvent({ type, data }));
const skippedSubtest = subtest && data.skip && data.skip !== undefined;
const _indent = indent(data.nesting);
return `${formatTestReport(type, data, prefix, _indent, hasChildren, skippedSubtest)}\n`;
}

function handleEvent({ type, data }) {
switch (type) {
case 'test:fail':
if (data.details?.error?.failureType !== kSubtestsFailed) {
ArrayPrototypePush(failedTests, data);
}
return handleTestReportEvent(type, data);
case 'test:pass':
return handleTestReportEvent(type, data);
case 'test:start':
ArrayPrototypeUnshift(stack, { __proto__: null, data, type });
break;
case 'test:stderr':
case 'test:stdout':
return `${data.message}\n`;
case 'test:diagnostic':
return `${colors[type]}${indent(data.nesting)}${symbols[type]}${data.message}${white}\n`;
case 'test:coverage':
return getCoverageReport(indent(data.nesting), data.summary, symbols['test:coverage'], blue);
}
_flush(callback) {
if (this.#failedTests.length === 0) {
}

const spec = new Transform({
writableObjectMode: true,
transform(event, encoding, callback) {
callback(null, handleEvent(event));
},
flush(callback) {
if (failedTests.length === 0) {
callback(null, '');
return;
}
const results = [`\n${colors['test:fail']}${symbols['test:fail']}failing tests:${white}\n`];
for (let i = 0; i < this.#failedTests.length; i++) {
ArrayPrototypePush(results, this.#formatTestReport(
for (let i = 0; i < failedTests.length; i++) {
ArrayPrototypePush(results, formatTestReport(
'test:fail',
this.#failedTests[i],
failedTests[i],
));
}
callback(null, ArrayPrototypeJoin(results, '\n'));
}
}
},
});

module.exports = SpecReporter;
module.exports = spec;
9 changes: 7 additions & 2 deletions test/parallel/test-runner-run.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -76,12 +76,17 @@ describe('require(\'node:test\').run', { concurrency: true }, () => {
});

it('should be piped with spec', async () => {
const specReporter = new spec();
const result = await run({ files: [join(testFixtures, 'test/random.cjs')] }).compose(specReporter).toArray();
const result = await run({ files: [join(testFixtures, 'test/random.cjs')] }).compose(spec).toArray();
const stringResults = result.map((bfr) => bfr.toString());
assert.match(stringResults[0], /this should pass/);
assert.match(stringResults[1], /tests 1/);
assert.match(stringResults[1], /suites 0/);
assert.match(stringResults[1], /pass 1/);
assert.match(stringResults[1], /fail 0/);
assert.match(stringResults[1], /cancelled 0/);
assert.match(stringResults[1], /skipped 0/);
assert.match(stringResults[1], /todo 0/);
assert.match(stringResults[1], /duration_ms \d+\.?\d*/);
});

it('should be piped with tap', async () => {
Expand Down