GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — test_runner: make sure tap subtest is reported in order by MoLow · Pull Request #45220 · nodejs/node

Conversation

@MoLow

@MoLow MoLow commented Oct 28, 2022

Copy link
Copy Markdown
Member

No description provided.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
bricss and tonivj5 reacted with thumbs up emoji
All reactions
  • 👍 2 reactions
@MoLow
test_runner: make sure tap subtest is reported by order
9b7fedd
@nodejs-github-bot 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
MoLow commented Oct 28, 2022
View reviewed changes
Comment thread lib/internal/test_runner/test.js
return;
}
this.#reportedSubtest = true;
this.parent.reportSubtest();

@MoLow MoLow Oct 28, 2022

Copy link
Copy Markdown
Member Author

There was a problem hiding this comment.

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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
MoLow
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 MoLow Oct 28, 2022

Copy link
Copy Markdown
Member Author

There was a problem hiding this comment.

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@MoLow

MoLow commented Oct 28, 2022

Copy link
Copy Markdown
Member Author

CC @nodejs/test_runner
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@MoLow 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
aduh95 reviewed Oct 28, 2022
View reviewed changes
Comment thread lib/internal/test_runner/test.js Resolved

Uh oh!

There was an error while loading. Please reload this page.
cjihrig
cjihrig approved these changes Oct 28, 2022
View reviewed changes

@cjihrig cjihrig left a comment

Copy link
Copy Markdown
Contributor

There was a problem hiding this 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?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
benjamingr
benjamingr approved these changes Oct 29, 2022
View reviewed changes
@MoLow @aduh95
Update lib/internal/test_runner/test.js
7fa3e01 
Co-authored-by: Antoine du Hamel <[email protected]>
@MoLow

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
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@MoLow
fix
6c48216
@MoLow MoLow added the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Oct 29, 2022
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Oct 29, 2022
@nodejs-github-bot

nodejs-github-bot commented Oct 29, 2022

Copy link
Copy Markdown
Collaborator

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@MoLow MoLow added the commit-queue PRs queued for automated landing through the Commit Queue. label Oct 30, 2022
@nodejs-github-bot nodejs-github-bot removed the commit-queue PRs queued for automated landing through the Commit Queue. label Oct 30, 2022
@nodejs-github-bot

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
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@nodejs-github-bot nodejs-github-bot added the commit-queue-failed PRs whose Commit Queue landing failed and need manual intervention before retrying. label Oct 30, 2022
@MoLow MoLow removed the commit-queue-failed PRs whose Commit Queue landing failed and need manual intervention before retrying. label Oct 30, 2022
MoLow added a commit that referenced this pull request Oct 30, 2022
@MoLow
test_runner: report tap subtest in order
3e57891 
PR-URL: #45220
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
@MoLow

MoLow commented Oct 30, 2022

Copy link
Copy Markdown
Member Author

Landed in 3e57891
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@MoLow MoLow closed this Oct 30, 2022
@MoLow
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
@MoLow @RafaelGSS
test_runner: report tap subtest in order
7c544bf 
PR-URL: #45220
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
@RafaelGSS 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
@MoLow @RafaelGSS
test_runner: report tap subtest in order
0040030 
PR-URL: #45220
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
@MoLow 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
@MoLow
test_runner: report tap subtest in order
e240525 
PR-URL: nodejs#45220
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
@danielleadams 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
@MoLow @danielleadams
test_runner: report tap subtest in order
83992f4 
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
@MoLow @danielleadams
test_runner: report tap subtest in order
a67da59 
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
@MoLow
test_runner: report tap subtest in order
201df85 
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
@MoLow
fix: report tap subtest in order
72aa07c 
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
@MoLow
fix: report tap subtest in order
d18d92f 
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
@MoLow
fix: report tap subtest in order
08269c5 
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 free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

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

Uh oh!

There was an error while loading. Please reload this page.

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.

Uh oh!

There was an error while loading. Please reload this page.

5 participants

@MoLow @nodejs-github-bot @benjamingr @cjihrig @aduh95
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.