test_runner: delegate stderr and stdout formatting to reporter #48045

HinataKah0 · 2023-05-17T10:26:16Z

Introduce new TestsStream events test:stderr and test:stdout
to delegate stderr and stdout (e.g. console.log()) formatting
to the reporter. And patch existing reporters to:

Spec: output the message as it is
TAP: stay the same with existing test:diagnostic

Fixes: #48011

nodejs-github-bot · 2023-05-17T10:26:21Z

Review requested:

@nodejs/test_runner

lib/internal/test_runner/tests_stream.js

doc/api/test.md

MoLow · 2023-05-17T14:40:48Z

lib/internal/test_runner/runner.js

@@ -285,7 +285,7 @@ class FileTest extends Test {
        const message = messages[i];
        this.addToReport({
          __proto__: null,
-          type: 'test:diagnostic',
+          type: 'test:stdout',
          data: { __proto__: null, nesting: 0, file: this.name, message },


as you have mentioned, nesting is redundant here.
also, I think we should stop breaking this into lines and delegate that to the TAP reporter (and leave as is in spec reporter) - but that dosnt have to happen in this PR

I think we should stop breaking this into lines and delegate that to the TAP reporter (and leave as is in spec reporter)

Sorry I don't get the spec reporter part. By "leaving as it is" are you referring to printing \n as \\n?

Sorry I don't get the spec reporter part. By "leaving as it is" are you referring to printing \n as \n?

I was referring to the output being split into lines before passing it to the reporter:

node/lib/internal/test_runner/runner.js

Line 283 in 3473edc

const messages = RegExpPrototypeSymbolSplit(kSplitLine, nonSerialized.toString('utf-8'));

but as written, it is also ok to not include it in this PR

I'll address it in separate PR 👍

lib/internal/test_runner/runner.js

test/fixtures/test-runner/output/stdout.js

HinataKah0 · 2023-05-19T02:29:32Z

hopefully, with #48045 we can make node --test test.js behave the same as node test.js in terms of console logs

Hi @MoLow , so we want to make the output format of both with & without --test same, right? Maybe without the prefix # or the symbol?

doc/api/test.md

MoLow · 2023-05-19T05:18:07Z

so we want to make the output format of both with & without --test same, right? Maybe without the prefix # or the symbol?

I think TAP should stay the same as today, and spec should just pipe the original data. that is the goal

HinataKah0 · 2023-05-19T15:03:10Z

For the Spec reporter, it looks like this:

I don't think we have any test for console.log Spec reporter because after I changed to output the original data as it is, it doesn't break any test.

Introduce new `TestsStream` events `test:stderr` and `test:stdout` to delegate `stderr` and `stdout` (e.g. `console.log()`) formatting to the reporter. And patch existing reporters to: - Spec: output the message as it is - TAP: stay the same with existing `test:diagnostic`

cjihrig · 2023-05-21T13:44:15Z

doc/api/test.md

+### Event: `'test:stderr'`
+
+* `data` {Object}
+  * `file` {string} The path of the test file.


For the test:diagnostic event, this can also be undefined. Is that possible for these new events?

no. test:diagnostic can be produced inside the REPL. these new events are only useful with --test

nodejs-github-bot · 2023-05-22T21:43:43Z

CI: https://ci.nodejs.org/job/node-test-pull-request/51916/

nodejs-github-bot · 2023-05-23T03:37:44Z

CI: https://ci.nodejs.org/job/node-test-pull-request/51920/

nodejs-github-bot · 2023-05-23T04:44:50Z

Landed in 1229a22

Introduce new `TestsStream` events `test:stderr` and `test:stdout` to delegate `stderr` and `stdout` (e.g. `console.log()`) formatting to the reporter. And patch existing reporters to: - Spec: output the message as it is - TAP: stay the same with existing `test:diagnostic` PR-URL: #48045 Fixes: #48011 Reviewed-By: Moshe Atlow <moshe@atlow.co.il> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

Introduce new `TestsStream` events `test:stderr` and `test:stdout` to delegate `stderr` and `stdout` (e.g. `console.log()`) formatting to the reporter. And patch existing reporters to: - Spec: output the message as it is - TAP: stay the same with existing `test:diagnostic` PR-URL: nodejs#48045 Fixes: nodejs#48011 Reviewed-By: Moshe Atlow <moshe@atlow.co.il> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

nodejs-github-bot added lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run. labels May 17, 2023

HinataKah0 commented May 17, 2023

View reviewed changes

lib/internal/test_runner/tests_stream.js Outdated Show resolved Hide resolved

cjihrig reviewed May 17, 2023

View reviewed changes

doc/api/test.md Outdated Show resolved Hide resolved

doc/api/test.md Show resolved Hide resolved

MoLow reviewed May 17, 2023

View reviewed changes

doc/api/test.md Show resolved Hide resolved

MoLow reviewed May 17, 2023

View reviewed changes

lib/internal/test_runner/runner.js Show resolved Hide resolved

MoLow reviewed May 17, 2023

View reviewed changes

test/fixtures/test-runner/output/stdout.js Outdated Show resolved Hide resolved

MoLow added the test_runner Issues and PRs related to the test runner subsystem. label May 18, 2023

MoLow mentioned this pull request May 18, 2023

test_runner: align stdout and std error with and without --test #48057

Closed

HinataKah0 force-pushed the tests-stream-new-event branch from 6e503ca to 3473edc Compare May 19, 2023 02:30

MoLow reviewed May 19, 2023

View reviewed changes

doc/api/test.md Outdated Show resolved Hide resolved

MoLow reviewed May 19, 2023

View reviewed changes

doc/api/test.md Outdated Show resolved Hide resolved

HinataKah0 force-pushed the tests-stream-new-event branch from 3473edc to 16f1020 Compare May 19, 2023 14:58

HinataKah0 changed the title ~~test_runner: delegate stdout formatting to reporter~~ test_runner: delegate stderr and stdout formatting to reporter May 19, 2023

HinataKah0 force-pushed the tests-stream-new-event branch from 16f1020 to 67f3543 Compare May 21, 2023 09:02

MoLow approved these changes May 21, 2023

View reviewed changes

cjihrig reviewed May 21, 2023

View reviewed changes

MoLow requested a review from cjihrig May 22, 2023 15:51

cjihrig approved these changes May 22, 2023

View reviewed changes

MoLow added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. request-ci Add this label to start a Jenkins CI on a PR. labels May 22, 2023

github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label May 22, 2023

github-actions bot mentioned this pull request May 23, 2023

CI Reliability 2023-05-23 nodejs/reliability#574

Open

22 tasks

MoLow added the commit-queue Add this label to land a pull request using GitHub Actions. label May 23, 2023

nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label May 23, 2023

nodejs-github-bot merged commit 1229a22 into nodejs:main May 23, 2023

HinataKah0 deleted the tests-stream-new-event branch May 23, 2023 13:37

HinataKah0 mentioned this pull request May 23, 2023

test_runner: delegate split message to reporter #48140

Closed

github-actions bot mentioned this pull request May 31, 2023

CI Reliability 2023-05-31 nodejs/reliability#582

Open

22 tasks

targos mentioned this pull request Jun 4, 2023

v20.3.0 release proposal #48332

Merged

danielleadams mentioned this pull request Jul 10, 2023

v18.17.0 release proposal #48694

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

test_runner: delegate stderr and stdout formatting to reporter #48045

test_runner: delegate stderr and stdout formatting to reporter #48045

HinataKah0 commented May 17, 2023 •

edited

Loading

nodejs-github-bot commented May 17, 2023

MoLow May 17, 2023 •

edited

Loading

HinataKah0 May 19, 2023

MoLow May 19, 2023

HinataKah0 May 19, 2023

HinataKah0 commented May 19, 2023

MoLow commented May 19, 2023

HinataKah0 commented May 19, 2023 •

edited

Loading

cjihrig May 21, 2023

MoLow May 21, 2023

nodejs-github-bot commented May 22, 2023

nodejs-github-bot commented May 23, 2023

nodejs-github-bot commented May 23, 2023

test_runner: delegate stderr and stdout formatting to reporter #48045

test_runner: delegate stderr and stdout formatting to reporter #48045

Conversation

HinataKah0 commented May 17, 2023 • edited Loading

nodejs-github-bot commented May 17, 2023

MoLow May 17, 2023 • edited Loading

Choose a reason for hiding this comment

HinataKah0 May 19, 2023

Choose a reason for hiding this comment

MoLow May 19, 2023

Choose a reason for hiding this comment

HinataKah0 May 19, 2023

Choose a reason for hiding this comment

HinataKah0 commented May 19, 2023

MoLow commented May 19, 2023

HinataKah0 commented May 19, 2023 • edited Loading

cjihrig May 21, 2023

Choose a reason for hiding this comment

MoLow May 21, 2023

Choose a reason for hiding this comment

nodejs-github-bot commented May 22, 2023

nodejs-github-bot commented May 23, 2023

nodejs-github-bot commented May 23, 2023

HinataKah0 commented May 17, 2023 •

edited

Loading

MoLow May 17, 2023 •

edited

Loading

HinataKah0 commented May 19, 2023 •

edited

Loading