node icon indicating copy to clipboard operation
node copied to clipboard

Order of nested vs outer `afterEach` hooks is unexpected and inconsistent with `after`

Open bendemboski opened this issue 1 year ago • 2 comments

Version

21.6.1

Platform

darwin

Subsystem

test runner

What steps will reproduce the bug?

Create a file called hooks.mjs containing:

import { describe, afterEach, after, it } from 'node:test';

describe('parent', () => {
  afterEach(() => console.log('parent after each'));
  after(() => console.log('parent after'));

  describe('child', () => {
    afterEach(() => console.log('child after each'));
    after(() => console.log('child after'));

    it('works', () => {});
  });
});

and run node --test hooks.mjs.

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

Always

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

child after each
parent after each
child after
parent after
<test results>

As with mocha, qunit, jest, vitest, and probably others, a child suite's afterEach hooks should be called before those of parent suites, not after them. Also, the execution order of parent/child afterEach hooks should match the order of parent/child after hooks.

Currently, after hooks correctly run child-first, but afterEach hooks run incorrectly parent-first.

What do you see instead?

parent after each
child after each
child after
parent after
<test results>

Additional information

This also reproduces with the test()/subtest API -- a test's afterEach hook will run before a subtest's afterEach hook.

bendemboski avatar Feb 05 '24 21:02 bendemboski

I agree subtest after and afterEach should be called before parent @nodejs/test_runner. I couldnt reproduce with testthough.

This works fine:

'use strict';
require('../common');
const assert = require('node:assert');
const { test, afterEach, after } = require('node:test');


let afterEachRacing = true;
let afterRacing = true;

test('parent', () => {
    afterEach(() => afterEachRacing = false);
    after(() => afterRacing = false);

    test('child', () => {
        afterEach(() => assert.ok(afterEachRacing));
        after(() => assert.ok(afterRacing));
        test(() => { });
    });
});

this fails:

'use strict';
require('../common');
const assert = require('node:assert');
const { describe, it,  afterEach, after } = require('node:test');


let afterEachRacing = true;
let afterRacing = true;

describe('parent', () => {
    afterEach(() => afterEachRacing = false);
    after(() => afterRacing = false);

    describe('child', () => {
        afterEach(() => assert.ok(afterEachRacing));
        after(() => assert.ok(afterRacing));
        it(() => { });
    });
});

marco-ippolito avatar Feb 07 '24 10:02 marco-ippolito

I meant that

test('parent', async (t) => {
  t.afterEach((c) => console.log('parent after each', c.name));
  t.after((c) => console.log('parent after', c.name));

  await t.test('child', async (t) => {
    t.afterEach((c) => console.log('child after each', c.name));
    t.after((c) => console.log('child after', c.name));
  
    await t.test('works', () => {});
  });
});

produces

parent after each works
child after each works
parent after each child
child after child
parent after parent

instead of

child after each works
parent after each works
parent after each child
child after child
parent after parent

so the parent's afterEach for works runs before the child's.

bendemboski avatar Feb 07 '24 16:02 bendemboski