test_runner: add support for null and date value output

[malthe]

This adds support for null and Date value output in the TAP test runner reporter.

Previously, a null value would not get included at all while a Date value would be rendered as an empty value.

Note that while Date objects can have properties, the v8 serialization functionality does not carry over such properties. For this reason, we don't target such values with this change.

[nodejs-github-bot]

Review requested:

• @nodejs/test_runner

[bnb]

Note that while Date objects can have properties, the v8 serialization functionality does not carry over such properties. For this reason, we don't target such values with this change.

This is probably worth adding to documentation in this PR? I can see someone being very confused about this rather easily.

[MoLow]

This is probably worth adding to documentation in this PR? I can see someone being very confused about this rather easily.

what confusion can there be? this only affects how test reports present dates. it does the same thing util.inspect does

[malthe]

@bnb it was probably confusing to me only because I was trying to find edge cases for the purpose of writing a test case with good coverage – in real life, probably very few people will try to make assertions against such frankendates for example.

We can document it but honestly it's probably just noise for most people. It was a bit hidden how the test runner serializes your objects but at the same time, how else would it really work given that we're spawning parallel processes?

[malthe]

I mean your suggestion is good but if you don’t edit that suggested change and I accept it then the tests will kick off and fail. tor. 29. feb. 2024 kl. 22.47 skrev Antoine du Hamel < ***@***.***>:

…

***@***.**** commented on this pull request. ------------------------------ In lib/internal/test_runner/reporter/tap.js <#51920 (comment)>: > + if (value instanceof Date) { + // YAML uses the ISO-8601 standard to express dates. + result += ' ' + value.toISOString(); + } The solution seems simple: just remove the unused Date assignment. Or am I missing something? — Reply to this email directly, view it on GitHub <#51920 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAGOJK7KUAIRSCEMY5GIQTYV6QXVAVCNFSM6AAAAABD7PLGD6VHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMYTSMBZHEZTCOBVGQ> . You are receiving this because you authored the thread.Message ID: ***@***.***>

[nodejs-github-bot]

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

[nodejs-github-bot]

Commit Queue failed

- Loading data for nodejs/node/pull/51920
✔  Done loading data for nodejs/node/pull/51920
----------------------------------- PR info ------------------------------------
Title      test_runner: add support for null and date value output (#51920)
Author     Malthe Borch  (@malthe)
Branch     malthe:reporter-tap-null-date -> nodejs:main
Labels     author ready, needs-ci, test_runner
Commits    2
 - test_runner: add support for null and date value output
 - test_runner: use internal date check function
Committers 1
 - Malthe Borch 
PR-URL: https://github.com/nodejs/node/pull/51920
Reviewed-By: Moshe Atlow 
Reviewed-By: Chemi Atlow 
Reviewed-By: Benjamin Gruenbaum 
Reviewed-By: Antoine du Hamel 
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/51920
Reviewed-By: Moshe Atlow 
Reviewed-By: Chemi Atlow 
Reviewed-By: Benjamin Gruenbaum 
Reviewed-By: Antoine du Hamel 
--------------------------------------------------------------------------------
   ℹ  This PR was created on Thu, 29 Feb 2024 08:28:37 GMT
   ✔  Approvals: 4
   ✔  - Moshe Atlow (@MoLow) (TSC): https://github.com/nodejs/node/pull/51920#pullrequestreview-1908565190
   ✔  - Chemi Atlow (@atlowChemi): https://github.com/nodejs/node/pull/51920#pullrequestreview-1910453248
   ✔  - Benjamin Gruenbaum (@benjamingr) (TSC): https://github.com/nodejs/node/pull/51920#pullrequestreview-1910610877
   ✔  - Antoine du Hamel (@aduh95) (TSC): https://github.com/nodejs/node/pull/51920#pullrequestreview-1910660709
   ✔  Last GitHub CI successful
   ℹ  Last Full PR CI on 2024-03-01T07:20:23Z: https://ci.nodejs.org/job/node-test-pull-request/57520/
- Querying data for job/node-test-pull-request/57520/
   ✔  Last Jenkins CI successful
--------------------------------------------------------------------------------
   ✔  No git cherry-pick in progress
   ✔  No git am in progress
   ✔  No git rebase in progress
--------------------------------------------------------------------------------
- Bringing origin/main up to date...
From https://github.com/nodejs/node
 * branch                  main       -> FETCH_HEAD
✔  origin/main is now up-to-date
- Downloading patch for 51920
From https://github.com/nodejs/node
 * branch                  refs/pull/51920/merge -> FETCH_HEAD
✔  Fetched commits as 14293814a772..15e3f5c26172
--------------------------------------------------------------------------------
[main 0d8f8664db] test_runner: add support for null and date value output
 Author: Malthe Borch 
 Date: Wed Feb 28 12:29:45 2024 +0100
 8 files changed, 173 insertions(+), 17 deletions(-)
[main bc6a7c8bc6] test_runner: use internal date check function
 Author: Malthe Borch 
 Date: Fri Mar 1 06:39:06 2024 +0000
 1 file changed, 3 insertions(+), 3 deletions(-)
   ✔  Patches applied
There are 2 commits in the PR. Attempting autorebase.
Rebasing (2/4)
Executing: git node land --amend --yes

--------------------------------- New Message ----------------------------------

test_runner: add support for null and date value output
PR-URL: #51920

Reviewed-By: Moshe Atlow moshe@atlow.co.il

Reviewed-By: Chemi Atlow chemi@atlow.co.il

Reviewed-By: Benjamin Gruenbaum benjamingr@gmail.com

Reviewed-By: Antoine du Hamel duhamelantoine1995@gmail.com
[detached HEAD 914ddb0f46] test_runner: add support for null and date value output

Author: Malthe Borch mborch@gmail.com

Date: Wed Feb 28 12:29:45 2024 +0100

8 files changed, 173 insertions(+), 17 deletions(-)

Rebasing (3/4)

Rebasing (4/4)
Executing: git node land --amend --yes

--------------------------------- New Message ----------------------------------

test_runner: use internal date check function
This fixes an issue where a value could be instanceof Date without actually being a native date.
Co-authored-by: Antoine du Hamel duhamelantoine1995@gmail.com

PR-URL: #51920

Reviewed-By: Moshe Atlow moshe@atlow.co.il

Reviewed-By: Chemi Atlow chemi@atlow.co.il

Reviewed-By: Benjamin Gruenbaum benjamingr@gmail.com

Reviewed-By: Antoine du Hamel duhamelantoine1995@gmail.com
[detached HEAD c2e4725124] test_runner: use internal date check function

Author: Malthe Borch mborch@gmail.com

Date: Fri Mar 1 06:39:06 2024 +0000

1 file changed, 3 insertions(+), 3 deletions(-)
Successfully rebased and updated refs/heads/main.
ℹ  Add commit-queue-squash label to land the PR as one commit, or commit-queue-rebase to land as separate commits.

https://github.com/nodejs/node/actions/runs/8121573407

[nodejs-github-bot]

Landed in 8451990