Order of test hooks seems wrong, and is different to the order of hooks for a describe suite

[philnash]

Version

20.1.0

Platform

No response

Subsystem

No response

What steps will reproduce the bug?

This test in the test runner fixtures is intended to test the order in which test hooks run. The expected output is:

  assert.deepStrictEqual(testArr, [
    'beforeEach 1', 'before test hooks', '1', 'afterEach 1',
    'beforeEach 2', '2', 'afterEach 2',
    'beforeEach nested',
    'beforeEach nested 1', 'nested beforeEach nested 1', 'nested1', 'afterEach nested 1', 'nested afterEach nested 1',
    'beforeEach nested 2', 'nested beforeEach nested 2', 'nested 2', 'afterEach nested 2', 'nested afterEach nested 2',
    'afterEach nested',
  ]);

In this case the before hook runs after the first beforeEach hook. And the after hook doesn't run at all as "after nested" isn't part of the array at all.

Comparing this to the order of hooks from "describe hooks":

    assert.deepStrictEqual(testArr, [
      'before describe hooks',
      'beforeEach 1', '1', 'afterEach 1',
      'beforeEach 2', '2', 'afterEach 2',
      'before nested',
      'beforeEach nested 1', '+beforeEach nested 1', 'nested 1', 'afterEach nested 1', '+afterEach nested 1',
      'beforeEach nested 2', '+beforeEach nested 2', 'nested 2', 'afterEach nested 2', '+afterEach nested 2',
      'after nested',
      'after describe hooks',
    ]);

This shows that the first before hook runs before any beforeEach hooks and that the outside after hook runs after all other hooks.

I would have thought that this behaviour should match between describe suites and tests with subtests. Otherwise it is likely to be confusing to the user.

How often does it reproduce? Is there a required condition?

This is reproduced in the test suite. I am questioning why this is the behaviour and if it should be different.

What is the expected behavior? Why is that the expected behavior?

I would expect the test for test hooks to look like this, and pass:

  assert.deepStrictEqual(testArr, [
    'before test hooks',
    'beforeEach 1', '1', 'afterEach 1',
    'beforeEach 2', '2', 'afterEach 2',
    'beforeEach nested',
    'beforeEach nested 1', 'nested beforeEach nested 1', 'nested1', 'afterEach nested 1', 'nested afterEach nested 1',
    'beforeEach nested 2', 'nested beforeEach nested 2', 'nested 2', 'afterEach nested 2', 'nested afterEach nested 2',
    'afterEach nested',
    'after test hooks',
  ]);

Additional information

If you agree that this is incorrect and should be fixed, I would love to be able to help out.

[mertcanaltin]

I am interested 🚀

[MoLow]

The behavior definitely seems wrong (seems like order of hooks should have been different in https://github.com/nodejs/node/pull/47586/files). @philnash if you are interested in helping out, feel free to submit a PR

[philnash]

Thanks @MoLow. I'll see what I can do with it!