[BUG] mocha-globals beforeEach hooks run for describes too when it should only for it tests

[Qard]

Is there an existing issue for this?

• I have searched the existing issues

Have you read the CONTRIBUTING guide on posting bugs, and CODE_OF_CONDUCT?

• yes I read the things

This issue exists in the latest tap version

• I am using the latest tap

Description

In previous versions, nested describe scopes would trigger outer beforeEach/afterEach hooks only around the inner it tests. In 18 however it seems to also trigger around the describe calls itself, which results in some very strange issues when used with libraries like sinon where you would typically apply a stub in the beforeEach and restore it in the afterEach. The way it works now though, the hook will run an additional time on nested tests and then crash because it's trying to reapply patches that are already applied.

Reproduction

require('tap')
require('@tapjs/mocha-globals').globalize()

describe('foo', () => {
  beforeEach(() => process._rawDebug('beforeEach'))
  afterEach(() => process._rawDebug('afterEach'))

  describe('bar', () => {
    describe('baz', () => {
      it('1', () => {})
      it('2', () => {})
    })
  })
})

Expected output:

beforeEach
afterEach
beforeEach
afterEach

Actual Output:

beforeEach
beforeEach
beforeEach
afterEach
beforeEach
afterEach
afterEach
afterEach

Environment

./node_modules/.bin/tap
dd-trace@6.0.0-pre /Users/stephen.belanger/Code/dd-trace-js
└── tap@18.7.0

tap: 18.7.0
"@tapjs/config": 2.4.15
"@tapjs/core": 1.5.0
"@tapjs/run": 1.5.0
"@tapjs/stack": 1.2.7
"@tapjs/test": 1.4.0
tap-parser: 15.3.1
tap-yaml: 2.2.1
tcompare: 6.4.5
plugins:
  "@tapjs/after": 1.1.18
  "@tapjs/after-each": 1.1.18
  "@tapjs/asserts": 1.1.18
  "@tapjs/before": 1.1.18
  "@tapjs/before-each": 1.1.18
  "@tapjs/filter": 1.2.18
  "@tapjs/fixture": 1.2.18
  "@tapjs/intercept": 1.2.18
  "@tapjs/mocha-globals": 1.1.18
  "@tapjs/mock": 1.3.0
  "@tapjs/node-serialize": 1.3.0
  "@tapjs/snapshot": 1.2.18
  "@tapjs/spawn": 1.1.18
  "@tapjs/stdin": 1.1.18
  "@tapjs/typescript": 1.4.0
  "@tapjs/worker": 1.1.18
# vim: set filetype=yaml :

# from .taprc
allow-incomplete-coverage: true
color: true
disable-coverage: true
node-arg:
  - --expose-gc
plugin:
  - "@tapjs/mocha-globals"

# env, cli, and defaults
coverage-report:
  - text
exclude:
  - "**/@(fixture*(s)|dist)/**"
include:
  - "**/@(test?(s)|__test?(s)__)/**/*.@(js|cjs|mjs|tap|cts|jsx|mts|ts|tsx)"
  - "**/*.@(test?(s)|spec).@(js|cjs|mjs|tap|cts|jsx|mts|ts|tsx)"
  - "**/test?(s).@(js|cjs|mjs|tap|cts|jsx|mts|ts|tsx)"
jobs: 8
mocha-globals: true
reporter: base
snapshot-clean-cwd: true
timeout: 30
@tapjs/after
@tapjs/after-each
@tapjs/asserts
@tapjs/before
@tapjs/before-each
@tapjs/filter
@tapjs/fixture
@tapjs/intercept
@tapjs/mock
@tapjs/node-serialize
@tapjs/snapshot
@tapjs/spawn
@tapjs/stdin
@tapjs/typescript
@tapjs/worker
@tapjs/mocha-globals
Darwin COMP-JX471G9FQQ 23.3.0 Darwin Kernel Version 23.3.0: Wed Dec 20 21:30:44 PST 2023; root:xnu-10002.81.5~7/RELEASE_ARM64_T6000 arm64 arm Darwin

[isaacs]

Ah, that's annoying, yes.

The tricky bit here is that describe() and it() are really both just aliases for t.test(...). And t.beforeEach runs before each test, not only certain ones.