`t.skip` does not invoke `test.afterEach`

[jsumners-nr]

'use strict'

const test = require('node:test')

test.afterEach(() => {
  process._rawDebug('!!! afterEach')
})

test('one', (t) => {
  t.assert.ok('one is good')
})

test('two', (t) => {
  if (process.env.TEST_SKIP === 'false') {
    t.assert.ok('two is good')
  } else {
    t.skip('two gets skipped after some logic')
  }
})

I expect that !!! afterEach will be printed twice when this test suite is run. However, on Node.js 20.20.0 and 24.13.0, that is not the case. Instead, the result is:

❯ node index.js
!!! afterEach
✔ one (1.120209ms)
﹣ two (0.08625ms) # two gets skipped after some logic
ℹ tests 2
ℹ suites 0
ℹ pass 1
ℹ fail 0
ℹ cancelled 0
ℹ skipped 1
ℹ todo 0
ℹ duration_ms 5.285417

This is a problem when the test being skipped depends upon logic internal to the test. As an example, we may have a test setup like:

test.beforeEach((ctx) => {
  ctx.testNamespace = {
    foo: 'bar'
    // other more complicated bootstrapping
  }
})

test.afterEach((ctx) => {
  cleanUp(ctx.testNamespace.somethingThatNeedsCleaningAfterEachTest)
})

test('something', (t) => {
  const { foo } = t.testNamespace
  if (foo !== 'bar') return t.skip('reason')
  t.assert.ok('whatever')
})

[mattskel]

@jsumners-nr I’m seeing the same behaviour. Looking at the test runner, test.afterEach is only invoked when this.skipped is false.

@cjihrig and @MoLow — is this the intended behaviour? I see you’ve both worked on the test runner.

If this is expected, it may be worth clarifying the docs. They currently state that afterEach() runs even if a test fails, but don’t mention that it does not run when a test is skipped.

Alternatively, should afterEach run when skip() is called from within the test body, and only be skipped when skip is provided as a test option?

Happy to work on this once the intended behaviour is clear.

[jsumners-nr]

Alternatively, should afterEach run when skip() is called from within the test body, and only be skipped when skip is provided as a test option?

That would make more sense to me given:

'use strict'

const test = require('node:test')

test.beforeEach((t) => {
  process._rawDebug('!!! beforeEach:', t.name)
})

test.afterEach(() => {
  process._rawDebug('!!! afterEach')
})

test('one', (t) => {
  t.assert.ok('one is good')
})

test('two', (t) => {
  if (process.env.TEST_SKIP === 'false') {
    t.assert.ok('two is good')
  } else {
    t.skip('two gets skipped after some logic')
  }
})

test('three', { skip: true }, () => {})

// ❯ node index.js
// !!! beforeEach: one
// !!! afterEach
// !!! beforeEach: two
// ✔ one (1.0415ms)
// ﹣ two (0.132708ms) # two gets skipped after some logic
// ﹣ three (0.045167ms) # SKIP
// ℹ tests 3
// ℹ suites 0
// ℹ pass 1
// ℹ fail 0
// ℹ cancelled 0
// ℹ skipped 2
// ℹ todo 0
// ℹ duration_ms 4.874333

Since { skip: true } means that the test.beforeEach is not invoked, it is most likely that no clean up needs to happen in test.afterEach. But with t.skip, the test.beforeEach is invoked and therefore has a high likelihood of setting up stuff that needs to be cleaned up after the test. In my opinion, this is a bug.

[ljharb]

Regardless, t.skip shouldn't be skipping the containing test - it's just a skip comment for an assertion. The only way to skip a test in my experience is usually to invoke it with .skip() or skip: true in the first place.

[jsumners-nr]

Regardless, t.skip shouldn't be skipping the containing test - it's just a skip comment for an assertion. The only way to skip a test in my experience is usually to invoke it with .skip() or skip: true in the first place.

Correct: https://nodejs.org/docs/latest/api/test.html#contextskipmessage

This issue is:

1. the test is being invoked
2. the test issues context.skip to show the test has been skipped internally
3. the test.afterEach is not invoked because context.skip was invoked

I assert that this is wrong because once the test function has been invoked any beforeEach setup should have its associated afterEach invoked.

[ljharb]

This is 100% true; if a before callback ran, the after callback MUST be run to clean things up.

[jsumners-nr]

Thank you for resolving this @igor-shevelenkov.