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: skip beforeEach and afterEach hooks if test is skipped #52112

Closed
ArktinenKarpalo opened this issue Mar 16, 2024 · 1 comment · Fixed by #52115
Closed

test_runner: skip beforeEach and afterEach hooks if test is skipped #52112

ArktinenKarpalo opened this issue Mar 16, 2024 · 1 comment · Fixed by #52115
Labels
confirmed-bug Issues with confirmed bugs. test_runner Issues and PRs related to the test runner subsystem.

Comments

@ArktinenKarpalo
Copy link

What is the problem this feature will solve?

Currently beforeEach and afterEach hooks are run even for tests that are skipped.
This becomes problematic when there are many hooks that are expensive to run and one wants to run only a few tests.

Consider case:

beforeEach(() => {
    expensiveSetup();
});
afterEach(() => {
    expensiveTeardown();
});
test('test1', () => {
    expensiveTest1();
});
test('test2', () => {
    expensiveTest2();
});

Now --test-name-pattern test1 will run both hooks twice or even if no test is run (both skipped) they will still be run twice.

It would be logical to assume that if the test is skipped, then before/afterEach hook is also skipped for that test. This is in fact the behavior in other popular js test runners such as mocha and jest.

Also, current documentation says that:

The --test-name-pattern command-line option can be used to only run tests whose name matches the provided pattern. (...)
For each test that is executed, any corresponding test hooks, such as beforeEach(), are also run.

Even though it is not explicitly said that beforeEach() would be skipped if test is not executed, I think this implies that, because otherwise that statement would be useless, if test hooks are ran regardless of whether test is filtered in or out.

What is the feature you are proposing to solve the problem?

If test case is skipped, beforeEach and afterEach hooks for that case should also be skipped.

What alternatives have you considered?

No response

@ArktinenKarpalo ArktinenKarpalo added the feature request Issues that request new features to be added to Node.js. label Mar 16, 2024
cjihrig added a commit to cjihrig/node that referenced this issue Mar 16, 2024
When a test is skipped, the corresponding beforeEach and afterEach
hooks should also be skipped.

Fixes: nodejs#52112
@cjihrig
Copy link
Contributor

cjihrig commented Mar 16, 2024

Thanks for the report. See #52115

@MoLow MoLow added confirmed-bug Issues with confirmed bugs. test_runner Issues and PRs related to the test runner subsystem. and removed feature request Issues that request new features to be added to Node.js. labels Mar 17, 2024
nodejs-github-bot pushed a commit that referenced this issue Mar 18, 2024
When a test is skipped, the corresponding beforeEach and afterEach
hooks should also be skipped.

Fixes: #52112
PR-URL: #52115
Reviewed-By: Raz Luvaton <rluvaton@gmail.com>
Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
rdw-msft pushed a commit to rdw-msft/node that referenced this issue Mar 26, 2024
When a test is skipped, the corresponding beforeEach and afterEach
hooks should also be skipped.

Fixes: nodejs#52112
PR-URL: nodejs#52115
Reviewed-By: Raz Luvaton <rluvaton@gmail.com>
Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
marco-ippolito pushed a commit that referenced this issue May 2, 2024
When a test is skipped, the corresponding beforeEach and afterEach
hooks should also be skipped.

Fixes: #52112
PR-URL: #52115
Reviewed-By: Raz Luvaton <rluvaton@gmail.com>
Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
marco-ippolito pushed a commit that referenced this issue May 3, 2024
When a test is skipped, the corresponding beforeEach and afterEach
hooks should also be skipped.

Fixes: #52112
PR-URL: #52115
Reviewed-By: Raz Luvaton <rluvaton@gmail.com>
Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
confirmed-bug Issues with confirmed bugs. test_runner Issues and PRs related to the test runner subsystem.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants