-
Notifications
You must be signed in to change notification settings - Fork 29.7k
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 each hooks for skipped tests #52115
Conversation
When a test is skipped, the corresponding beforeEach and afterEach hooks should also be skipped. Fixes: nodejs#52112
Review requested:
|
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.
What do you think about adding a test that it's not skipped for regular tests when having both regular and skipped + adding test that not running before each when entire describe is skipped
Added more tests. |
Co-authored-by: Chemi Atlow <chemi@atlow.co.il>
Commit Queue failed- Loading data for nodejs/node/pull/52115 ✔ Done loading data for nodejs/node/pull/52115 ----------------------------------- PR info ------------------------------------ Title test_runner: skip each hooks for skipped tests (#52115) Author Colin Ihrig (@cjihrig) Branch cjihrig:each-hooks -> nodejs:main Labels author ready, needs-ci, test_runner Commits 3 - test_runner: skip each hooks for skipped tests - additional tests - Update test/fixtures/test-runner/output/suite-skip-hooks.js Committers 2 - cjihrig - GitHub PR-URL: https://github.com/nodejs/node/pull/52115 Fixes: https://github.com/nodejs/node/issues/52112 Reviewed-By: Raz Luvaton Reviewed-By: Chemi Atlow Reviewed-By: Moshe Atlow ------------------------------ Generated metadata ------------------------------ PR-URL: https://github.com/nodejs/node/pull/52115 Fixes: https://github.com/nodejs/node/issues/52112 Reviewed-By: Raz Luvaton Reviewed-By: Chemi Atlow Reviewed-By: Moshe Atlow -------------------------------------------------------------------------------- ℹ This PR was created on Sat, 16 Mar 2024 16:13:58 GMT ✔ Approvals: 3 ✔ - Raz Luvaton (@rluvaton): https://github.com/nodejs/node/pull/52115#pullrequestreview-1941189165 ✔ - Chemi Atlow (@atlowChemi): https://github.com/nodejs/node/pull/52115#pullrequestreview-1941388256 ✔ - Moshe Atlow (@MoLow) (TSC): https://github.com/nodejs/node/pull/52115#pullrequestreview-1941414647 ✔ Last GitHub CI successful ℹ Last Full PR CI on 2024-03-17T07:13:58Z: https://ci.nodejs.org/job/node-test-pull-request/57786/ - Querying data for job/node-test-pull-request/57786/ ✔ Last Jenkins CI successful -------------------------------------------------------------------------------- ✔ No git cherry-pick in progress ✔ No git am in progress ✔ No git rebase in progress -------------------------------------------------------------------------------- - Bringing origin/main up to date... From https://github.com/nodejs/node * branch main -> FETCH_HEAD ✔ origin/main is now up-to-date - Downloading patch for 52115 From https://github.com/nodejs/node * branch refs/pull/52115/merge -> FETCH_HEAD ✔ Fetched commits as 8a191e4e6a39..3d5934220ed7 -------------------------------------------------------------------------------- Auto-merging lib/internal/test_runner/test.js [main 3d705cbf9b] test_runner: skip each hooks for skipped tests Author: cjihrig Date: Sat Mar 16 12:12:44 2024 -0400 4 files changed, 35 insertions(+), 2 deletions(-) create mode 100644 test/fixtures/test-runner/output/skip-each-hooks.js create mode 100644 test/fixtures/test-runner/output/skip-each-hooks.snapshot [main a567bd199f] additional tests Author: cjihrig Date: Sat Mar 16 13:32:22 2024 -0400 3 files changed, 101 insertions(+) create mode 100644 test/fixtures/test-runner/output/suite-skip-hooks.js create mode 100644 test/fixtures/test-runner/output/suite-skip-hooks.snapshot [main 8c4534e860] Update test/fixtures/test-runner/output/suite-skip-hooks.js Author: Colin Ihrig Date: Sat Mar 16 19:13:57 2024 -0400 1 file changed, 16 deletions(-) ✔ Patches applied There are 3 commits in the PR. Attempting autorebase. Rebasing (2/6)https://github.com/nodejs/node/actions/runs/8329948090 |
Landed in 6f4d601 |
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>
When a test is skipped, the corresponding beforeEach and afterEach hooks should also be skipped.
Fixes: #52112