FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

test_runner: top-level diagnostics not ommited when running with --test by pulkit-30 · Pull Request #46441 · nodejs/node · GitHub

/ node Public

test_runner: top-level diagnostics not ommited when running with --test - #46441

Merged
nodejs-github-bot merged 6 commits into
nodejs:mainfrom
pulkit-30:fix-45910
Feb 2, 2023
ConversationCommits (6)ChecksFiles changed
Merged

test_runner: top-level diagnostics not ommited when running with --test#46441
nodejs-github-bot merged 6 commits into
nodejs:mainfrom
pulkit-30:fix-45910

Conversation

pulkit-30 commented Jan 31, 2023 •
edited
Loading

Copy link
Copy Markdown
Contributor

fix: #45910

Should not omit top-level diagnostics when running test with --test flag.

Changes

added a logic #checkNestedComment() to filter top-level diagnostics only in cases they are redundant and show them
test:

test('should not omit top-level diagnostics', () => setImmediate(() => done()));

output:

TAP version 13
# Subtest: *
    # Subtest: should not omit top-level diagnostics
    ok 1 - should not omit top-level diagnostics
      ---
      duration_ms: 2.072958
      ...
    1..1
    # Warning: Test "should not omit top-level diagnostics" 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.
not ok 1 - *
  ---
  duration_ms: 57.882541
  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 59.641875

I am not sure about test failing
when running test without --test flag, test output is:

TAP version 13
# Subtest: should not omit top-level diagnostics
ok 1 - should not omit top-level diagnostics
  ---
  duration_ms: 2.544125
  ...
1..1
# Warning: Test "should not omit top-level diagnostics" 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 6.197042

nodejs-github-bot commented Jan 31, 2023

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/test_runner

nodejs-github-bot added dont-land-on-v14.x needs-ci PRs that need a full CI run. test_runner Issues and PRs related to the test runner subsystem. labels Jan 31, 2023
MoLow reviewed Jan 31, 2023
View reviewed changes
Comment thread lib/internal/test_runner/runner.js Outdated
MoLow reviewed Jan 31, 2023
View reviewed changes
Comment thread test/message/test_runner_output.js Outdated
MoLow reviewed Jan 31, 2023
View reviewed changes
Comment thread lib/internal/test_runner/runner.js Outdated
cjihrig reviewed Jan 31, 2023
View reviewed changes
Comment thread lib/internal/test_runner/runner.js Outdated
Comment thread lib/internal/test_runner/runner.js Outdated
ljharb reviewed Jan 31, 2023
View reviewed changes
Comment thread lib/internal/test_runner/runner.js Outdated
aduh95 reviewed Jan 31, 2023
View reviewed changes
Comment thread lib/internal/test_runner/runner.js
aduh95 added author ready PRs that have at least one approval, no outstanding review comments, and a CI started. request-ci Add this label to start a Jenkins CI on a PR. labels Feb 1, 2023
github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Feb 1, 2023

This comment was marked as outdated.

Sign in to view

This comment was marked as outdated.

Sign in to view

nodejs-github-bot commented Feb 1, 2023

Copy link
Copy Markdown
Collaborator

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

MoLow added commit-queue Add this label to land a pull request using GitHub Actions. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. labels Feb 1, 2023
cjihrig mentioned this pull request Feb 1, 2023
Merged
nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Feb 2, 2023
nodejs-github-bot merged commit 61c65b0 into nodejs:main Feb 2, 2023

nodejs-github-bot commented Feb 2, 2023

Copy link
Copy Markdown
Collaborator

Landed in 61c65b0

MoLow mentioned this pull request Feb 2, 2023
Merged
github-actions Bot mentioned this pull request Feb 5, 2023
Open
22 tasks
MoLow pushed a commit to MoLow/node-core-test that referenced this pull request Feb 7, 2023
test_runner: top-level diagnostics not ommited when running with --test
29d5a24 
PR-URL: nodejs/node#46441
Fixes: nodejs/node#45910
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
(cherry picked from commit 61c65b066b098cf47f89206212864ec1cddb8782)
MoLow pushed a commit to MoLow/node-core-test that referenced this pull request Feb 7, 2023
fix: top-level diagnostics not ommited when running with --test
2434b16 
PR-URL: nodejs/node#46441
Fixes: nodejs/node#45910
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
(cherry picked from commit 61c65b066b098cf47f89206212864ec1cddb8782)
MoLow pushed a commit to nodejs/node-core-test that referenced this pull request Feb 8, 2023
fix: top-level diagnostics not ommited when running with --test
d6f071a 
PR-URL: nodejs/node#46441
Fixes: nodejs/node#45910
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
(cherry picked from commit 61c65b066b098cf47f89206212864ec1cddb8782)
MoLow mentioned this pull request Feb 18, 2023
Closed
MylesBorins pushed a commit that referenced this pull request Feb 18, 2023
test_runner: top-level diagnostics not ommited when running with --test
b31aabb 
PR-URL: #46441
Fixes: #45910
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
MylesBorins mentioned this pull request Feb 19, 2023
Merged
MoLow pushed a commit to MoLow/node that referenced this pull request Feb 25, 2023
test_runner: top-level diagnostics not ommited when running with --test
e1f5a3a 
PR-URL: nodejs#46441
Fixes: nodejs#45910
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
juanarbol pushed a commit that referenced this pull request Mar 3, 2023
test_runner: top-level diagnostics not ommited when running with --test
a262a67 
PR-URL: #46441
Backport-PR-URL: #46839
Fixes: #45910
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
juanarbol mentioned this pull request Mar 3, 2023
Merged
juanarbol pushed a commit that referenced this pull request Mar 5, 2023
test_runner: top-level diagnostics not ommited when running with --test
c5f16fb 
PR-URL: #46441
Backport-PR-URL: #46839
Fixes: #45910
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

ljharb ljharb left review comments
cjihrig cjihrig left review comments
benjamingr benjamingr approved these changes
MoLow MoLow approved these changes
aduh95 aduh95 approved these changes
+1 more reviewer
tony-go tony-go approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

author ready PRs that have at least one approval, no outstanding review comments, and a CI started. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. needs-ci PRs that need a full CI run. test_runner Issues and PRs related to the test runner subsystem.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

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

8 participants

Back | FazBrowse Home | New Git URL