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

test_runner: make sure tap subtest is reported in order by MoLow · Pull Request #45220 · nodejs/node · GitHub

/ node Public

test_runner: make sure tap subtest is reported in order - #45220

Closed
MoLow wants to merge 3 commits into
nodejs:mainfrom
MoLow:test_runner_fix_subtest
ConversationCommits (3)ChecksFiles changed
Closed

test_runner: make sure tap subtest is reported in order#45220
MoLow wants to merge 3 commits into
nodejs:mainfrom
MoLow:test_runner_fix_subtest

Conversation

MoLow commented Oct 28, 2022

Copy link
Copy Markdown
Member

No description provided.

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 Oct 28, 2022
MoLow commented Oct 28, 2022
View reviewed changes
Comment thread lib/internal/test_runner/test.js
return;
}
this.#reportedSubtest = true;
this.parent.reportSubtest();

MoLow Oct 28, 2022

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

this is the actual fix.

MoLow commented Oct 28, 2022
View reviewed changes
Comment thread test/message/test_runner_describe_nested.out
@@ -0,0 +1,26 @@
TAP version 13

MoLow Oct 28, 2022

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

prior to the fix, output was

TAP version 13
    # Subtest: nested
        # Subtest: nested
        ok 1 - nested
          ---
          duration_ms: 0.132417
          ...
# Subtest: nested - no tests
        1..1
    ok 1 - nested
      ---
      duration_ms: 0.740375
      ...
    1..1
ok 1 - nested - no tests
  ---
  duration_ms: 1.359792
  ...
1..1
# tests 1
# pass 1
# fail 0
# cancelled 0
# skipped 0
# todo 0
# duration_ms 2.906166

MoLow commented Oct 28, 2022

Copy link
Copy Markdown
Member Author

CC @nodejs/test_runner

MoLow changed the title test_runner: make sure tap subtest is reported by order test_runner: make sure tap subtest is reported in order Oct 28, 2022
aduh95 reviewed Oct 28, 2022
View reviewed changes
Comment thread lib/internal/test_runner/test.js Outdated
cjihrig approved these changes Oct 28, 2022
View reviewed changes

cjihrig left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

LGTM. This change is part of the TAP parser PR, right?

Update lib/internal/test_runner/test.js
7fa3e01 
Co-authored-by: Antoine du Hamel <duhamelantoine1995@gmail.com>

MoLow commented Oct 29, 2022

Copy link
Copy Markdown
Member Author

This change is part of the TAP parser PR, right?

the part seperating reportSubtest to a function is, but the addition of this.parent.reportSubtest() is new in this PR only

MoLow added the request-ci Add this label to start a Jenkins CI on a PR. label Oct 29, 2022
github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Oct 29, 2022

nodejs-github-bot commented Oct 29, 2022

Copy link
Copy Markdown
Collaborator

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

MoLow added the commit-queue Add this label to land a pull request using GitHub Actions. label Oct 30, 2022
nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Oct 30, 2022

nodejs-github-bot commented Oct 30, 2022

Copy link
Copy Markdown
Collaborator
Commit Queue failed
- Loading data for nodejs/node/pull/45220
✔  Done loading data for nodejs/node/pull/45220
----------------------------------- PR info ------------------------------------
Title      test_runner: make sure tap subtest is reported in order (#45220)
Author     Moshe Atlow  (@MoLow)
Branch     MoLow:test_runner_fix_subtest -> nodejs:main
Labels     needs-ci, dont-land-on-v14.x, test_runner
Commits    3
 - test_runner: make sure tap subtest is reported by order
 - Update lib/internal/test_runner/test.js
 - fix
Committers 2
 - Moshe Atlow 
 - GitHub 
PR-URL: https://github.com/nodejs/node/pull/45220
Reviewed-By: Colin Ihrig 
Reviewed-By: Benjamin Gruenbaum 
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/45220
Reviewed-By: Colin Ihrig 
Reviewed-By: Benjamin Gruenbaum 
--------------------------------------------------------------------------------
   ⚠  Commits were pushed since the last review:
   ⚠  - Update lib/internal/test_runner/test.js
   ⚠  - fix
   ℹ  This PR was created on Fri, 28 Oct 2022 07:29:25 GMT
   ✔  Approvals: 2
   ✔  - Colin Ihrig (@cjihrig) (TSC): https://github.com/nodejs/node/pull/45220#pullrequestreview-1160009419
   ✔  - Benjamin Gruenbaum (@benjamingr): https://github.com/nodejs/node/pull/45220#pullrequestreview-1161016247
   ✔  Last GitHub CI successful
   ℹ  Last Full PR CI on 2022-10-29T18:39:28Z: https://ci.nodejs.org/job/node-test-pull-request/47559/
- Querying data for job/node-test-pull-request/47559/
   ✔  Last Jenkins CI successful
--------------------------------------------------------------------------------
   ✔  Aborted `git node land` session in /home/runner/work/node/node/.ncu
https://github.com/nodejs/node/actions/runs/3355445461

nodejs-github-bot added the commit-queue-failed An error occurred while landing this pull request using GitHub Actions. label Oct 30, 2022
MoLow removed the commit-queue-failed An error occurred while landing this pull request using GitHub Actions. label Oct 30, 2022
MoLow added a commit that referenced this pull request Oct 30, 2022
test_runner: report tap subtest in order
3e57891 
PR-URL: #45220
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>

MoLow commented Oct 30, 2022

Copy link
Copy Markdown
Member Author

Landed in 3e57891

MoLow closed this Oct 30, 2022
MoLow deleted the test_runner_fix_subtest branch October 30, 2022 13:36
RafaelGSS pushed a commit that referenced this pull request Nov 1, 2022
test_runner: report tap subtest in order
7c544bf 
PR-URL: #45220
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
RafaelGSS mentioned this pull request Nov 1, 2022
Merged
RafaelGSS pushed a commit that referenced this pull request Nov 10, 2022
test_runner: report tap subtest in order
0040030 
PR-URL: #45220
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
MoLow mentioned this pull request Nov 24, 2022
Closed
MoLow added a commit to MoLow/node that referenced this pull request Dec 9, 2022
test_runner: report tap subtest in order
e240525 
PR-URL: nodejs#45220
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
danielleadams mentioned this pull request Dec 30, 2022
Merged
danielleadams pushed a commit that referenced this pull request Dec 30, 2022
test_runner: report tap subtest in order
83992f4 
PR-URL: #45220
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
danielleadams pushed a commit that referenced this pull request Jan 3, 2023
test_runner: report tap subtest in order
a67da59 
PR-URL: #45220
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
MoLow added a commit to MoLow/node-core-test that referenced this pull request Feb 2, 2023
test_runner: report tap subtest in order
201df85 
PR-URL: nodejs/node#45220
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
(cherry picked from commit 3e57891ee2fde0971e18fc383c25acf8f90def05)
MoLow added a commit to MoLow/node-core-test that referenced this pull request Feb 2, 2023
fix: report tap subtest in order
72aa07c 
PR-URL: nodejs/node#45220
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
(cherry picked from commit 3e57891ee2fde0971e18fc383c25acf8f90def05)
MoLow added a commit to MoLow/node-core-test that referenced this pull request Feb 2, 2023
fix: report tap subtest in order
d18d92f 
PR-URL: nodejs/node#45220
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
(cherry picked from commit 3e57891ee2fde0971e18fc383c25acf8f90def05)
MoLow added a commit to MoLow/node-core-test that referenced this pull request Feb 2, 2023
fix: report tap subtest in order
08269c5 
PR-URL: nodejs/node#45220
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
(cherry picked from commit 3e57891ee2fde0971e18fc383c25acf8f90def05)
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

aduh95 aduh95 left review comments
benjamingr benjamingr approved these changes
cjihrig cjihrig approved these changes

Assignees

No one assigned

Labels

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.

5 participants

Back | FazBrowse Home | New Git URL