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

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

No description provided.

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

lib/internal/test_runner/test.js

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


this is the actual fix.

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

test/message/test_runner_describe_nested.out

@@ -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

lib/internal/test_runner/test.js

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 labels Oct 28, 2022

MoLow commented Oct 28, 2022

View reviewed changes

lib/internal/test_runner/test.js

return;

}

this.#reportedSubtest = true;

this.parent.reportSubtest();

Copy link

Member Author

MoLow Oct 28, 2022

Choose a reason for hiding this comment

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

this is the actual fix.

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 order Oct 28, 2022

aduh95 reviewed Oct 28, 2022

View reviewed changes

lib/internal/test_runner/test.js Outdated Show resolved Hide resolved

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-actions bot 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 added a commit to MoLow/node that referenced this pull request Nov 23, 2022

test_runner: report tap subtest in order

176ecd8

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

MoLow added a commit to MoLow/node that referenced this pull request Nov 23, 2022

test_runner: report tap subtest in order

7396ad5

PR-URL: nodejs#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 pushed a commit that referenced this pull request Dec 30, 2022

test_runner: report tap subtest in order

bbf85b4

PR-URL: #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>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

MoLow commented Oct 28, 2022

MoLow Oct 28, 2022

MoLow Oct 28, 2022

MoLow commented Oct 28, 2022

cjihrig left a comment

MoLow commented Oct 29, 2022

nodejs-github-bot commented Oct 29, 2022

nodejs-github-bot commented Oct 30, 2022

MoLow commented Oct 30, 2022

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

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

Conversation

MoLow commented Oct 28, 2022

MoLow Oct 28, 2022

Choose a reason for hiding this comment

MoLow Oct 28, 2022

Choose a reason for hiding this comment

MoLow commented Oct 28, 2022

cjihrig left a comment

Choose a reason for hiding this comment

MoLow commented Oct 29, 2022

nodejs-github-bot commented Oct 29, 2022

nodejs-github-bot commented Oct 30, 2022

MoLow commented Oct 30, 2022