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

Revert "test_runner: do not invoke after hook when test is empty" by cjihrig · Pull Request #51998 · nodejs/node · GitHub

/ node Public

Revert "test_runner: do not invoke after hook when test is empty" - #51998

Closed
cjihrig wants to merge 2 commits into
nodejs:mainfrom
cjihrig:revert
ConversationCommits (2)ChecksFiles changed
Closed

Revert "test_runner: do not invoke after hook when test is empty"#51998
cjihrig wants to merge 2 commits into
nodejs:mainfrom
cjihrig:revert

Conversation

cjihrig commented Mar 7, 2024

Copy link
Copy Markdown
Contributor

Fixes: #51997

cjihrig added 2 commits March 6, 2024 21:13
Revert "test_runner: do not invoke after hook when test is empty"
c39a545 
This reverts commit a53fd95.

This caused a regression because the original issue this commit
was attempting to fix is not a bug. The after() hook should
always run.

Fixes: nodejs#51997
test: add regression test for test_runner after hook
cfccb99 
Refs: nodejs#51997

nodejs-github-bot commented Mar 7, 2024

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/test_runner

nodejs-github-bot added needs-ci PRs that need a full CI run. test_runner Issues and PRs related to the test runner subsystem. labels Mar 7, 2024
cjihrig mentioned this pull request Mar 7, 2024
Closed
cjihrig changed the title https://github.com/nodejs/node/commit/c39a545bd96873873f2f5f4aff299194438964b8 Revert "test_runner: do not invoke after hook when test is empty" Mar 7, 2024
richardlau mentioned this pull request Mar 7, 2024
Merged
richardlau added the request-ci Add this label to start a Jenkins CI on a PR. label Mar 7, 2024
github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Mar 7, 2024

nodejs-github-bot commented Mar 7, 2024

Copy link
Copy Markdown
Collaborator

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

nodejs-github-bot commented Mar 7, 2024

Copy link
Copy Markdown
Collaborator

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

aduh95 added author ready PRs that have at least one approval, no outstanding review comments, and a CI started. commit-queue Add this label to land a pull request using GitHub Actions. labels Mar 7, 2024
mcollina approved these changes Mar 7, 2024
View reviewed changes

mcollina left a comment

Copy link
Copy Markdown
Member

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

mcollina added the commit-queue-rebase Add this label to allow the Commit Queue to land a PR in several commits. label Mar 7, 2024
metcoder95 mentioned this pull request Mar 7, 2024
Closed

targos commented Mar 7, 2024

Copy link
Copy Markdown
Member

Should we make a quick release to fix this regression?

mcollina commented Mar 7, 2024

Copy link
Copy Markdown
Member

That'd be really useful, thanks.

mcollina added the fast-track PRs that do not need to wait for 48 hours to land. label Mar 7, 2024

github-actions Bot commented Mar 7, 2024

Copy link
Copy Markdown
Contributor

Fast-track has been requested by @mcollina. Please 👍 to approve.

nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Mar 7, 2024

nodejs-github-bot commented Mar 7, 2024

Copy link
Copy Markdown
Collaborator

Landed in 4f3cf4e...3aa0658

nodejs-github-bot pushed a commit that referenced this pull request Mar 7, 2024
Revert "test_runner: do not invoke after hook when test is empty"
0e8070d 
This reverts commit a53fd95.

This caused a regression because the original issue this commit
was attempting to fix is not a bug. The after() hook should
always run.

Fixes: #51997
PR-URL: #51998
Reviewed-By: Matthew Aitken <maitken033380023@gmail.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
nodejs-github-bot pushed a commit that referenced this pull request Mar 7, 2024
test: add regression test for test_runner after hook
3aa0658 
Refs: #51997
PR-URL: #51998
Fixes: #51997
Reviewed-By: Matthew Aitken <maitken033380023@gmail.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
targos pushed a commit that referenced this pull request Mar 7, 2024
Revert "test_runner: do not invoke after hook when test is empty"
85aa6ca 
This reverts commit a53fd95.

This caused a regression because the original issue this commit
was attempting to fix is not a bug. The after() hook should
always run.

Fixes: #51997
PR-URL: #51998
Reviewed-By: Matthew Aitken <maitken033380023@gmail.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
targos pushed a commit that referenced this pull request Mar 7, 2024
test: add regression test for test_runner after hook
bee3b36 
Refs: #51997
PR-URL: #51998
Fixes: #51997
Reviewed-By: Matthew Aitken <maitken033380023@gmail.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
This was referenced Mar 7, 2024
Closed
Merged

marco-ippolito commented Mar 7, 2024

Copy link
Copy Markdown
Member

Sorry for the regression 😢🙏🏻

cjihrig deleted the revert branch March 7, 2024 14:01
climba03003 mentioned this pull request Mar 8, 2024
Closed
richardlau pushed a commit that referenced this pull request Mar 25, 2024
test: add regression test for test_runner after hook
905c48f 
Refs: #51997
PR-URL: #51998
Fixes: #51997
Reviewed-By: Matthew Aitken <maitken033380023@gmail.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
richardlau mentioned this pull request Mar 25, 2024
Merged
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

mcollina mcollina approved these changes
richardlau richardlau approved these changes
MoLow MoLow approved these changes
marco-ippolito marco-ippolito approved these changes
KhafraDev KhafraDev approved these changes
+1 more reviewer
targos targos approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

author ready PRs that have at least one approval, no outstanding review comments, and a CI started. commit-queue-rebase Add this label to allow the Commit Queue to land a PR in several commits. fast-track PRs that do not need to wait for 48 hours to land. 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.

test_runner: t.after is never called

9 participants

Back | FazBrowse Home | New Git URL