Skip to content

Commit fd1489a

Browse files
cjihrigmarco-ippolito
authored andcommittedMay 3, 2024
test_runner: skip each hooks for skipped tests
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>
1 parent 8d7d6ef commit fd1489a

File tree

6 files changed

+120
-2
lines changed

6 files changed

+120
-2
lines changed
 

‎lib/internal/test_runner/test.js

+2-2
Original file line numberDiff line numberDiff line change
@@ -609,7 +609,7 @@ class Test extends AsyncResource {
609609
}
610610
};
611611
const afterEach = runOnce(async () => {
612-
if (this.parent?.hooks.afterEach.length > 0) {
612+
if (this.parent?.hooks.afterEach.length > 0 && !this.skipped) {
613613
await this.parent.runHook('afterEach', hookArgs);
614614
}
615615
});
@@ -621,7 +621,7 @@ class Test extends AsyncResource {
621621
// This hook usually runs immediately, we need to wait for it to finish
622622
await this.parent.runHook('before', this.parent.getRunArgs());
623623
}
624-
if (this.parent?.hooks.beforeEach.length > 0) {
624+
if (this.parent?.hooks.beforeEach.length > 0 && !this.skipped) {
625625
await this.parent.runHook('beforeEach', hookArgs);
626626
}
627627
stopPromise = stopTest(this.timeout, this.signal);
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
// Flags: --test-reporter=spec
2+
'use strict';
3+
const { after, afterEach, before, beforeEach, test } = require('node:test');
4+
5+
before(() => {
6+
console.log('BEFORE');
7+
});
8+
9+
beforeEach(() => {
10+
console.log('BEFORE EACH');
11+
});
12+
13+
after(() => {
14+
console.log('AFTER');
15+
});
16+
17+
afterEach(() => {
18+
console.log('AFTER EACH');
19+
});
20+
21+
test('skipped test', { skip: true });
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
BEFORE
2+
AFTER
3+
﹣ skipped test (*ms) # SKIP
4+
ℹ tests 1
5+
ℹ suites 0
6+
ℹ pass 0
7+
ℹ fail 0
8+
ℹ cancelled 0
9+
ℹ skipped 1
10+
ℹ todo 0
11+
ℹ duration_ms *
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
// Flags: --test-reporter=spec
2+
'use strict';
3+
const {
4+
after,
5+
afterEach,
6+
before,
7+
beforeEach,
8+
describe,
9+
it,
10+
} = require('node:test');
11+
12+
describe('skip all hooks in this suite', { skip: true }, () => {
13+
before(() => {
14+
console.log('BEFORE 1');
15+
});
16+
17+
beforeEach(() => {
18+
console.log('BEFORE EACH 1');
19+
});
20+
21+
after(() => {
22+
console.log('AFTER 1');
23+
});
24+
25+
afterEach(() => {
26+
console.log('AFTER EACH 1');
27+
});
28+
29+
it('should not run');
30+
});
31+
32+
describe('suite runs with mixture of skipped tests', () => {
33+
before(() => {
34+
console.log('BEFORE 2');
35+
});
36+
37+
beforeEach(() => {
38+
console.log('BEFORE EACH 2');
39+
});
40+
41+
after(() => {
42+
console.log('AFTER 2');
43+
});
44+
45+
afterEach(() => {
46+
console.log('AFTER EACH 2');
47+
});
48+
49+
it('should not run', { skip: true });
50+
51+
it('should run 1', () => {
52+
console.log('should run 1');
53+
});
54+
55+
it('should not run', { skip: true });
56+
57+
it('should run 2', () => {
58+
console.log('should run 2');
59+
});
60+
});
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
BEFORE 2
2+
BEFORE EACH 2
3+
should run 1
4+
AFTER EACH 2
5+
BEFORE EACH 2
6+
should run 2
7+
AFTER EACH 2
8+
AFTER 2
9+
﹣ skip all hooks in this suite (*ms) # SKIP
10+
▶ suite runs with mixture of skipped tests
11+
﹣ should not run (*ms) # SKIP
12+
✔ should run 1 (*ms)
13+
﹣ should not run (*ms) # SKIP
14+
✔ should run 2 (*ms)
15+
▶ suite runs with mixture of skipped tests (*ms)
16+
17+
ℹ tests 4
18+
ℹ suites 2
19+
ℹ pass 2
20+
ℹ fail 0
21+
ℹ cancelled 0
22+
ℹ skipped 2
23+
ℹ todo 0
24+
ℹ duration_ms *

‎test/parallel/test-runner-output.mjs

+2
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,8 @@ const tests = [
9696
{ name: 'test-runner/output/eval_tap.js' },
9797
{ name: 'test-runner/output/hooks.js' },
9898
{ name: 'test-runner/output/hooks_spec_reporter.js', transform: specTransform },
99+
{ name: 'test-runner/output/skip-each-hooks.js', transform: specTransform },
100+
{ name: 'test-runner/output/suite-skip-hooks.js', transform: specTransform },
99101
{ name: 'test-runner/output/timeout_in_before_each_should_not_affect_further_tests.js' },
100102
{ name: 'test-runner/output/hooks-with-no-global-test.js' },
101103
{ name: 'test-runner/output/global-hooks-with-no-tests.js' },

0 commit comments

Comments
 (0)
Please sign in to comment.