test_runner: skip each hooks for skipped tests

cjihrig · marco-ippolito · commit fd1489a6231c · 2024-05-03T08:30:15.000+02:00
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>
diff --git a/lib/internal/test_runner/test.js b/lib/internal/test_runner/test.js
@@ -609,7 +609,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);
       }
     });
@@ -621,7 +621,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);
diff --git a/test/fixtures/test-runner/output/skip-each-hooks.js b/test/fixtures/test-runner/output/skip-each-hooks.js
@@ -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 });
diff --git a/test/fixtures/test-runner/output/skip-each-hooks.snapshot b/test/fixtures/test-runner/output/skip-each-hooks.snapshot
@@ -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 *
diff --git a/test/fixtures/test-runner/output/suite-skip-hooks.js b/test/fixtures/test-runner/output/suite-skip-hooks.js
@@ -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');
+  });
+});
diff --git a/test/fixtures/test-runner/output/suite-skip-hooks.snapshot b/test/fixtures/test-runner/output/suite-skip-hooks.snapshot
@@ -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 *
diff --git a/test/parallel/test-runner-output.mjs b/test/parallel/test-runner-output.mjs
@@ -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' },
