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: top-level diagnostics are ommited when running with --test #45910

Closed
MoLow opened this issue Dec 19, 2022 · 12 comments · Fixed by #46441
Closed

Test runner: top-level diagnostics are ommited when running with --test #45910

MoLow opened this issue Dec 19, 2022 · 12 comments · Fixed by #46441
Labels
good first issue Issues that are suitable for first-time contributors. test_runner Issues and PRs related to the test runner subsystem.

Comments

@MoLow
Copy link
Member

MoLow commented Dec 19, 2022

Version

v20.0.0-pre

Platform

Darwin Moshes-MBP.localdomain 21.1.0 Darwin Kernel Version 21.1.0: Wed Oct 13 17:33:01 PDT 2021; root:xnu-8019.41.5~1/RELEASE_ARM64_T6000 arm64

Subsystem

test_runner

What steps will reproduce the bug?

run node --test test.js
where tets.js is

const test = require('node:test');
test(() => setImmediate(() => done()));

How often does it reproduce? Is there a required condition?

always

What is the expected behavior?

the same as when running without --test:

TAP version 13
# Subtest: <anonymous>
ok 1 - <anonymous>
  ---
  duration_ms: 2.272166
  ...
1..1
# Warning: Test "<anonymous>" generated asynchronous activity after the test ended. This activity created the error "ReferenceError: done is not defined" and would have caused the test to fail, but instead triggered an uncaughtException event.
# tests 1
# pass 1
# fail 0
# cancelled 0
# skipped 0
# todo 0
# duration_ms 5.046833

What do you see instead?

TAP version 13
# Subtest: /Users/moshe/repos/node/a.js
    # Subtest: <anonymous>
    ok 1 - <anonymous>
      ---
      duration_ms: 2.224708
      ...
    1..1
not ok 1 - /Users/moshe/repos/node/a.js
  ---
  duration_ms: 81.555291
  failureType: 'subtestsFailed'
  exitCode: 1
  error: 'test failed'
  code: 'ERR_TEST_FAILURE'
  ...
1..1
# tests 1
# pass 0
# fail 1
# cancelled 0
# skipped 0
# todo 0
# duration_ms 82.435834

missing # Warning: Test "<anonymous>" generated asynchronous activity after the test ended. This activity created the error "ReferenceError: done is not defined" and would have caused the test to fail, but instead triggered an uncaughtException event.

Additional information

diagnostics were ignored intentionally so the duration_ms, tests, pass etc don't appear twice:

if (nesting === 1) {
// Ignore file top level diagnostics
break;
}

we should find some way to filter top-level diagnostics only in cases they are redundant

@MoLow MoLow added good first issue Issues that are suitable for first-time contributors. test_runner Issues and PRs related to the test runner subsystem. labels Dec 19, 2022
@cjihrig
Copy link
Contributor

cjihrig commented Dec 19, 2022

cc @manekinekko

@MoLow MoLow changed the title Test runner: top level diagnostics are ommited when running with --test Test runner: diagnostics are ommited when running with --test Dec 19, 2022
@MoLow MoLow changed the title Test runner: diagnostics are ommited when running with --test Test runner: top-level diagnostics are ommited when running with --test Dec 19, 2022
@MoLow
Copy link
Member Author

MoLow commented Dec 19, 2022

I initially thought this and #45911 were the same issue but some debugging showed that #45911 originates inside the TAP parser whereas this issue comes from here

if (nesting === 1) {
// Ignore file top level diagnostics
break;
}

@manekinekko
Copy link
Contributor

manekinekko commented Dec 20, 2022

For anyone looking to work on this issue, I've commented on #45911. Both issues are related.

#45911 (comment)

@Ayush-Dutt-Sharma
Copy link

@MoLow claim

@ashutosh887
Copy link

Hi @manekinekko

I would like to work on this Issue!

@cjihrig
Copy link
Contributor

cjihrig commented Dec 23, 2022

@ashutosh887 I noticed that you commented on at least 3 issues that you would like to work on them. Feel free to work on any available issues, but please do not try to "claim" them or otherwise lick the cookie. That creates problems in the event that you aren't able to complete everything you've signed up for.

@MrJithil
Copy link
Member

Please feel free to work on it. I closed the one I created as it's not the way we needed.

@ashutosh887
Copy link

@cjihrig No it's not like that
I'm really willing to contribute but finding hard to get started

@italojs
Copy link
Contributor

italojs commented Jan 25, 2023

@ashutosh887 are you still interested to take it? what is your main difficulty? we could try to help you

@MoLow if @ashutosh887 does not reply, I'm the next in the queue to work on this ;)

@MoLow
Copy link
Member Author

MoLow commented Jan 25, 2023

@italojs AFAIAC you can work on this

@italojs
Copy link
Contributor

italojs commented Jan 26, 2023

nice, could you assign it to me, please?

@MoLow
Copy link
Member Author

MoLow commented Jan 26, 2023

@italojs no need to assign, you can simply go ahead and create a PR

nodejs-github-bot pushed a commit that referenced this issue Feb 2, 2023
PR-URL: #46441
Fixes: #45910
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
MoLow pushed a commit to MoLow/node-core-test that referenced this issue Feb 7, 2023
PR-URL: nodejs/node#46441
Fixes: nodejs/node#45910
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
(cherry picked from commit 61c65b066b098cf47f89206212864ec1cddb8782)
MoLow pushed a commit to MoLow/node-core-test that referenced this issue Feb 7, 2023
PR-URL: nodejs/node#46441
Fixes: nodejs/node#45910
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
(cherry picked from commit 61c65b066b098cf47f89206212864ec1cddb8782)
MoLow pushed a commit to nodejs/node-core-test that referenced this issue Feb 8, 2023
PR-URL: nodejs/node#46441
Fixes: nodejs/node#45910
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
(cherry picked from commit 61c65b066b098cf47f89206212864ec1cddb8782)
MoLow pushed a commit to MoLow/node that referenced this issue Feb 18, 2023
PR-URL: nodejs#46441
Fixes: nodejs#45910
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
MylesBorins pushed a commit that referenced this issue Feb 18, 2023
PR-URL: #46441
Fixes: #45910
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
MoLow pushed a commit to MoLow/node that referenced this issue Feb 25, 2023
PR-URL: nodejs#46441
Fixes: nodejs#45910
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
MoLow pushed a commit to MoLow/node that referenced this issue Feb 25, 2023
PR-URL: nodejs#46441
Fixes: nodejs#45910
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
MoLow pushed a commit to MoLow/node that referenced this issue Feb 25, 2023
PR-URL: nodejs#46441
Fixes: nodejs#45910
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
juanarbol pushed a commit that referenced this issue Mar 3, 2023
PR-URL: #46441
Backport-PR-URL: #46839
Fixes: #45910
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
juanarbol pushed a commit that referenced this issue Mar 5, 2023
PR-URL: #46441
Backport-PR-URL: #46839
Fixes: #45910
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Issues that are suitable for first-time contributors. test_runner Issues and PRs related to the test runner subsystem.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants