-
-
Notifications
You must be signed in to change notification settings - Fork 34.2k
test_runner: reland run global after() hook earlier #49116
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
This commit reverts the revert in bb52656. It also includes the fix for the issue that required the revert (nodejs#49059 (comment)) and an additional common.mustCall() in the added test. Refs: nodejs#49059 Refs: nodejs#49110
|
Review requested:
|
mcollina
left a comment
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
| const exitHandler = async () => { | ||
| await root.run(new ERR_TEST_FAILURE( | ||
| const exitHandler = () => { | ||
| root.postRun(new ERR_TEST_FAILURE( |
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.
why not await?
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.
@rluvaton postRun returns void (and not a Promise)
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.
I thought I changed that (or I'm thinking of another function there) 😅
|
Landed in b574075 |
This commit reverts the revert in bb52656. It also includes the fix for the issue that required the revert (nodejs#49059 (comment)) and an additional common.mustCall() in the added test. Refs: nodejs#49059 Refs: nodejs#49110 PR-URL: nodejs#49116 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Chemi Atlow <chemi@atlow.co.il> Reviewed-By: Moshe Atlow <moshe@atlow.co.il> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
This commit reverts the revert in bb52656. It also includes the fix for the issue that required the revert (#49059 (comment)) and an additional common.mustCall() in the added test. Refs: #49059 Refs: #49110 PR-URL: #49116 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Chemi Atlow <chemi@atlow.co.il> Reviewed-By: Moshe Atlow <moshe@atlow.co.il> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
This commit reverts the revert in bb52656. It also includes the fix for the issue that required the revert (nodejs#49059 (comment)) and an additional common.mustCall() in the added test. Refs: nodejs#49059 Refs: nodejs#49110 PR-URL: nodejs#49116 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Chemi Atlow <chemi@atlow.co.il> Reviewed-By: Moshe Atlow <moshe@atlow.co.il> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
This commit reverts the revert in bb52656. It also includes the fix for the issue that required the revert (nodejs#49059 (comment)) and an additional common.mustCall() in the added test. Refs: nodejs#49059 Refs: nodejs#49110 PR-URL: nodejs#49116 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Chemi Atlow <chemi@atlow.co.il> Reviewed-By: Moshe Atlow <moshe@atlow.co.il> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
This commit reverts the revert in bb52656. It also includes the fix for the issue that required the revert (#49059 (comment)) and an additional common.mustCall() in the added test. Refs: #49059 Refs: #49110 PR-URL: #49116 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Chemi Atlow <chemi@atlow.co.il> Reviewed-By: Moshe Atlow <moshe@atlow.co.il> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
|
Due to fact, #48877 didn't land cleanly on v20.x-staging. This PR somehow, depends on it. So we'll need a manual backport. Reference: https://github.com/nodejs/node/blob/main/doc/contributing/backporting-to-release-lines.md |
This commit reverts the revert in bb52656. It also includes the fix for the issue that required the revert (nodejs#49059 (comment)) and an additional common.mustCall() in the added test. Refs: nodejs#49059 Refs: nodejs#49110 PR-URL: nodejs#49116 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Chemi Atlow <chemi@atlow.co.il> Reviewed-By: Moshe Atlow <moshe@atlow.co.il> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
This commit reverts the revert in bb52656. It also includes the fix for the issue that required the revert (nodejs#49059 (comment)) and an additional common.mustCall() in the added test. Refs: nodejs#49059 Refs: nodejs#49110 PR-URL: nodejs#49116 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Chemi Atlow <chemi@atlow.co.il> Reviewed-By: Moshe Atlow <moshe@atlow.co.il> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
This commit reverts the revert in bb52656. It also includes the fix for the issue that required the revert (#49059 (comment)) and an additional common.mustCall() in the added test. Refs: #49059 Refs: #49110 PR-URL: #49116 Backport-PR-URL: #49225 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Chemi Atlow <chemi@atlow.co.il> Reviewed-By: Moshe Atlow <moshe@atlow.co.il> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
This commit reverts the revert in bb52656. It also includes the fix for the issue that required the revert (#49059 (comment)) and an additional common.mustCall() in the added test. Refs: #49059 Refs: #49110 PR-URL: #49116 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Chemi Atlow <chemi@atlow.co.il> Reviewed-By: Moshe Atlow <moshe@atlow.co.il> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
This commit reverts the revert in bb52656. It also includes the fix for the issue that required the revert (nodejs/node#49059 (comment)) and an additional common.mustCall() in the added test. Refs: nodejs/node#49059 Refs: nodejs/node#49110 PR-URL: nodejs/node#49116 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Chemi Atlow <chemi@atlow.co.il> Reviewed-By: Moshe Atlow <moshe@atlow.co.il> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
This commit reverts the revert in bb52656. It also includes the fix for the issue that required the revert (nodejs/node#49059 (comment)) and an additional common.mustCall() in the added test. Refs: nodejs/node#49059 Refs: nodejs/node#49110 PR-URL: nodejs/node#49116 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Chemi Atlow <chemi@atlow.co.il> Reviewed-By: Moshe Atlow <moshe@atlow.co.il> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
This commit reverts the revert in
bb52656. It also includes the fix for the issue that required the revert
(#49059 (comment)) and an additional
common.mustCall()in the added test.Refs: #49059
Refs: #49110