Skip to content

Conversation

@MoLow
Copy link
Member

No description provided.

@nodejs-github-botnodejs-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
return;
}
this.#reportedSubtest =true;
this.parent.reportSubtest();
Copy link
MemberAuthor

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.

@@ -0,0 +1,26 @@
TAP version 13
Copy link
MemberAuthor

Choose a reason for hiding this comment

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

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
Copy link
MemberAuthor

CC @nodejs/test_runner

@MoLowMoLow changed the title test_runner: make sure tap subtest is reported by ordertest_runner: make sure tap subtest is reported in orderOct 28, 2022
Copy link
Contributor

@cjihrigcjihrig left a comment

Choose a reason for hiding this comment

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

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

@MoLow
Copy link
MemberAuthor

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

@MoLowMoLow added the request-ci Add this label to start a Jenkins CI on a PR. label Oct 29, 2022
@github-actionsgithub-actionsbot removed the request-ci Add this label to start a Jenkins CI on a PR. label Oct 29, 2022
@nodejs-github-bot
Copy link
Collaborator

@MoLowMoLow added the commit-queue Add this label to land a pull request using GitHub Actions. label Oct 30, 2022
@nodejs-github-botnodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Oct 30, 2022
@nodejs-github-bot
Copy link
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-botnodejs-github-bot added the commit-queue-failed An error occurred while landing this pull request using GitHub Actions. label Oct 30, 2022
@MoLowMoLow 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
PR-URL: #45220 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]>
@MoLow
Copy link
MemberAuthor

Landed in 3e57891

@MoLowMoLow closed this Oct 30, 2022
@MoLowMoLow deleted the test_runner_fix_subtest branch October 30, 2022 13:36
RafaelGSS pushed a commit that referenced this pull request Nov 1, 2022
PR-URL: #45220 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]>
@RafaelGSSRafaelGSS mentioned this pull request Nov 1, 2022
RafaelGSS pushed a commit that referenced this pull request Nov 10, 2022
PR-URL: #45220 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]>
MoLow added a commit to MoLow/node that referenced this pull request Nov 23, 2022
PR-URL: nodejs#45220 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]>
MoLow added a commit to MoLow/node that referenced this pull request Nov 23, 2022
PR-URL: nodejs#45220 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]>
MoLow added a commit to MoLow/node that referenced this pull request Dec 9, 2022
PR-URL: nodejs#45220 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]>
danielleadams pushed a commit that referenced this pull request Dec 30, 2022
PR-URL: #45220 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]>
danielleadams pushed a commit that referenced this pull request Dec 30, 2022
PR-URL: #45220 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]>
danielleadams pushed a commit that referenced this pull request Jan 3, 2023
PR-URL: #45220 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]>
MoLow added a commit to MoLow/node-core-test that referenced this pull request Feb 2, 2023
PR-URL: nodejs/node#45220 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]> (cherry picked from commit 3e57891ee2fde0971e18fc383c25acf8f90def05)
MoLow added a commit to MoLow/node-core-test that referenced this pull request Feb 2, 2023
PR-URL: nodejs/node#45220 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]> (cherry picked from commit 3e57891ee2fde0971e18fc383c25acf8f90def05)
MoLow added a commit to MoLow/node-core-test that referenced this pull request Feb 2, 2023
PR-URL: nodejs/node#45220 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]> (cherry picked from commit 3e57891ee2fde0971e18fc383c25acf8f90def05)
MoLow added a commit to MoLow/node-core-test that referenced this pull request Feb 2, 2023
PR-URL: nodejs/node#45220 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]> (cherry picked from commit 3e57891ee2fde0971e18fc383c25acf8f90def05)
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-ciPRs that need a full CI run.test_runnerIssues and PRs related to the test runner subsystem.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@MoLow@nodejs-github-bot@benjamingr@cjihrig@aduh95