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

MoLow · 2022-10-28T07:29:25Z

No description provided.

MoLow · 2022-10-28T07:29:40Z

+ return;
+ }
+ this.#reportedSubtest = true;
+ this.parent.reportSubtest();


this is the actual fix.

MoLow · 2022-10-28T07:30:16Z

@@ -0,0 +1,26 @@
+TAP version 13


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 · 2022-10-28T07:33:50Z

CC @nodejs/test_runner

cjihrig

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

Co-authored-by: Antoine du Hamel <duhamelantoine1995@gmail.com>

MoLow · 2022-10-29T18:16:06Z

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

nodejs-github-bot · 2022-10-29T18:39:28Z

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

nodejs-github-bot · 2022-10-30T12:25:37Z

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

PR-URL: #45220 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>

MoLow · 2022-10-30T13:36:54Z

Landed in 3e57891

PR-URL: #45220 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>

PR-URL: nodejs#45220 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>

PR-URL: #45220 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>

PR-URL: nodejs/node#45220 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> (cherry picked from commit 3e57891ee2fde0971e18fc383c25acf8f90def05)

test_runner: make sure tap subtest is reported by order
9b7fedd

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

MoLow changed the title ~~test_runner: make sure tap subtest is reported by order~~test_runner: make sure tap subtest is reported in orderOct 28, 2022

aduh95 reviewed Oct 28, 2022
View reviewed changes

Comment threadlib/internal/test_runner/test.js Outdated

cjihrig approved these changes Oct 28, 2022
View reviewed changes

benjamingr approved these changes Oct 29, 2022
View reviewed changes

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

fix
6c48216

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

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

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 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 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
v19.1.0 proposal #45269
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
[v16.x backport] backport multiple test runner features #45602
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
v18.13.0 release proposal #46025
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>

Navigation Menu

Uh oh!

Uh oh!

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

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

MoLow commented Oct 28, 2022

Uh oh!

MoLowOct 28, 2022

Uh oh!

MoLowOct 28, 2022

Uh oh!

MoLow commented Oct 28, 2022

Uh oh!

Uh oh!

cjihrig left a comment

Uh oh!

MoLow commented Oct 29, 2022

Uh oh!

nodejs-github-bot commented Oct 29, 2022

Uh oh!

nodejs-github-bot commented Oct 30, 2022

Uh oh!

MoLow commented Oct 30, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Uh oh!

Uh oh!

Conversation

MoLow commented Oct 28, 2022

Uh oh!

MoLowOct 28, 2022

Choose a reason for hiding this comment

Uh oh!

MoLowOct 28, 2022

Choose a reason for hiding this comment

Uh oh!

MoLow commented Oct 28, 2022

Uh oh!

Uh oh!

cjihrig left a comment

Choose a reason for hiding this comment

Uh oh!

MoLow commented Oct 29, 2022

Uh oh!

nodejs-github-bot commented Oct 29, 2022

Uh oh!

nodejs-github-bot commented Oct 30, 2022

Uh oh!

MoLow commented Oct 30, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants