Skip to content
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

test_runner: do not invoke after hook when test is empty #51389

Merged
merged 1 commit into from
Jan 8, 2024

Conversation

marco-ippolito
Copy link
Member

@marco-ippolito marco-ippolito commented Jan 5, 2024

Fixes: #51371
(recreated because apparently if you force push on a closed PR it cannot be reopened)

@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/test_runner

@nodejs-github-bot 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 Jan 5, 2024
Comment on lines +6 to +10
test('test', async (t) => {
t.after(common.mustNotCall(() => {
t.fail('should not run');
}));
});
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
test('test', async (t) => {
t.after(common.mustNotCall(() => {
t.fail('should not run');
}));
});
test('test', (t) => {
t.after(common.mustNotCall());
});

@lpinca lpinca added the request-ci Add this label to start a Jenkins CI on a PR. label Jan 6, 2024
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jan 6, 2024
@nodejs-github-bot
Copy link
Collaborator

@marco-ippolito marco-ippolito added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Jan 7, 2024
@marco-ippolito marco-ippolito added the commit-queue Add this label to land a pull request using GitHub Actions. label Jan 8, 2024
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Jan 8, 2024
@nodejs-github-bot nodejs-github-bot merged commit a53fd95 into nodejs:main Jan 8, 2024
63 checks passed
@nodejs-github-bot
Copy link
Collaborator

Landed in a53fd95

marco-ippolito added a commit to marco-ippolito/node that referenced this pull request Jan 12, 2024
Medhansh404 pushed a commit to Medhansh404/node that referenced this pull request Jan 19, 2024
targos pushed a commit that referenced this pull request Feb 15, 2024
PR-URL: #51389
Fixes: #51371
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Chemi Atlow <[email protected]>
@marco-ippolito marco-ippolito mentioned this pull request Mar 1, 2024
@richardlau richardlau added dont-land-on-v18.x PRs that should not land on the v18.x-staging branch and should not be released in v18.x. dont-land-on-v20.x PRs that should not land on the v20.x-staging branch and should not be released in v20.x. labels Mar 7, 2024
@richardlau
Copy link
Member

This is being reverted in #51998

targos added a commit that referenced this pull request Mar 7, 2024
Notable changes:

This release reverts #51389,
which landed in Node.js 21.7.0. It is a documented feature that
`t.after()` hooks are run even if a test has no subtests. The hook can
be used to clean up the test itself.

PR-URL: #52002
@targos targos mentioned this pull request Mar 7, 2024
targos added a commit that referenced this pull request Mar 8, 2024
Notable changes:

This release reverts #51389,
which landed in Node.js 21.7.0. It is a documented feature that
`t.after()` hooks are run even if a test has no subtests. The hook can
be used to clean up the test itself.

PR-URL: #52002
targos added a commit that referenced this pull request Mar 8, 2024
Notable changes:

This release reverts #51389,
which landed in Node.js 21.7.0. It is a documented feature that
`t.after()` hooks are run even if a test has no subtests. The hook can
be used to clean up the test itself.

PR-URL: #52002
targos added a commit that referenced this pull request Mar 8, 2024
Notable changes:

This release reverts #51389,
which landed in Node.js 21.7.0. It is a documented feature that
`t.after()` hooks are run even if a test has no subtests. The hook can
be used to clean up the test itself.

PR-URL: #52002
rdw-msft pushed a commit to rdw-msft/node that referenced this pull request Mar 26, 2024
Notable changes:

This release reverts nodejs#51389,
which landed in Node.js 21.7.0. It is a documented feature that
`t.after()` hooks are run even if a test has no subtests. The hook can
be used to clean up the test itself.

PR-URL: nodejs#52002
jcbhmr pushed a commit to jcbhmr/node that referenced this pull request May 15, 2024
Notable changes:

This release reverts nodejs#51389,
which landed in Node.js 21.7.0. It is a documented feature that
`t.after()` hooks are run even if a test has no subtests. The hook can
be used to clean up the test itself.

PR-URL: nodejs#52002
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. dont-land-on-v18.x PRs that should not land on the v18.x-staging branch and should not be released in v18.x. dont-land-on-v20.x PRs that should not land on the v20.x-staging branch and should not be released in v20.x. needs-ci PRs that need a full CI run. test_runner Issues and PRs related to the test runner subsystem.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

test_runner: after() hook is called for no subtests
5 participants