Skip to content

Commit

Permalink
test_runner: skip each hooks for skipped tests
Browse files Browse the repository at this point in the history
When a test is skipped, the corresponding beforeEach and afterEach
hooks should also be skipped.

Fixes: #52112
PR-URL: #52115
Reviewed-By: Raz Luvaton <[email protected]>
Reviewed-By: Chemi Atlow <[email protected]>
Reviewed-By: Moshe Atlow <[email protected]>
  • Loading branch information
cjihrig authored Mar 18, 2024
1 parent 8a191e4 commit 6f4d601
Show file tree
Hide file tree
Showing 6 changed files with 120 additions and 2 deletions.
4 changes: 2 additions & 2 deletions lib/internal/test_runner/test.js
Original file line number Diff line number Diff line change
Expand Up @@ -662,7 +662,7 @@ class Test extends AsyncResource {
}
};
const afterEach = runOnce(async () => {
if (this.parent?.hooks.afterEach.length > 0) {
if (this.parent?.hooks.afterEach.length > 0 && !this.skipped) {
await this.parent.runHook('afterEach', hookArgs);
}
});
Expand All @@ -674,7 +674,7 @@ class Test extends AsyncResource {
// This hook usually runs immediately, we need to wait for it to finish
await this.parent.runHook('before', this.parent.getRunArgs());
}
if (this.parent?.hooks.beforeEach.length > 0) {
if (this.parent?.hooks.beforeEach.length > 0 && !this.skipped) {
await this.parent.runHook('beforeEach', hookArgs);
}
stopPromise = stopTest(this.timeout, this.signal);
Expand Down
21 changes: 21 additions & 0 deletions test/fixtures/test-runner/output/skip-each-hooks.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
// Flags: --test-reporter=spec
'use strict';
const { after, afterEach, before, beforeEach, test } = require('node:test');

before(() => {
console.log('BEFORE');
});

beforeEach(() => {
console.log('BEFORE EACH');
});

after(() => {
console.log('AFTER');
});

afterEach(() => {
console.log('AFTER EACH');
});

test('skipped test', { skip: true });
11 changes: 11 additions & 0 deletions test/fixtures/test-runner/output/skip-each-hooks.snapshot
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
BEFORE
AFTER
﹣ skipped test (*ms) # SKIP
ℹ tests 1
ℹ suites 0
ℹ pass 0
ℹ fail 0
ℹ cancelled 0
ℹ skipped 1
ℹ todo 0
ℹ duration_ms *
60 changes: 60 additions & 0 deletions test/fixtures/test-runner/output/suite-skip-hooks.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
// Flags: --test-reporter=spec
'use strict';
const {
after,
afterEach,
before,
beforeEach,
describe,
it,
} = require('node:test');

describe('skip all hooks in this suite', { skip: true }, () => {
before(() => {
console.log('BEFORE 1');
});

beforeEach(() => {
console.log('BEFORE EACH 1');
});

after(() => {
console.log('AFTER 1');
});

afterEach(() => {
console.log('AFTER EACH 1');
});

it('should not run');
});

describe('suite runs with mixture of skipped tests', () => {
before(() => {
console.log('BEFORE 2');
});

beforeEach(() => {
console.log('BEFORE EACH 2');
});

after(() => {
console.log('AFTER 2');
});

afterEach(() => {
console.log('AFTER EACH 2');
});

it('should not run', { skip: true });

it('should run 1', () => {
console.log('should run 1');
});

it('should not run', { skip: true });

it('should run 2', () => {
console.log('should run 2');
});
});
24 changes: 24 additions & 0 deletions test/fixtures/test-runner/output/suite-skip-hooks.snapshot
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
BEFORE 2
BEFORE EACH 2
should run 1
AFTER EACH 2
BEFORE EACH 2
should run 2
AFTER EACH 2
AFTER 2
﹣ skip all hooks in this suite (*ms) # SKIP
▶ suite runs with mixture of skipped tests
﹣ should not run (*ms) # SKIP
✔ should run 1 (*ms)
﹣ should not run (*ms) # SKIP
✔ should run 2 (*ms)
▶ suite runs with mixture of skipped tests (*ms)

ℹ tests 4
ℹ suites 2
ℹ pass 2
ℹ fail 0
ℹ cancelled 0
ℹ skipped 2
ℹ todo 0
ℹ duration_ms *
2 changes: 2 additions & 0 deletions test/parallel/test-runner-output.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,8 @@ const tests = [
{ name: 'test-runner/output/eval_tap.js' },
{ name: 'test-runner/output/hooks.js' },
{ name: 'test-runner/output/hooks_spec_reporter.js', transform: specTransform },
{ name: 'test-runner/output/skip-each-hooks.js', transform: specTransform },
{ name: 'test-runner/output/suite-skip-hooks.js', transform: specTransform },
{ name: 'test-runner/output/timeout_in_before_each_should_not_affect_further_tests.js' },
{ name: 'test-runner/output/hooks-with-no-global-test.js' },
{ name: 'test-runner/output/global-hooks-with-no-tests.js' },
Expand Down

0 comments on commit 6f4d601

Please sign in to comment.