Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

test_runner: run afterEach hooks in correct order #52239

Merged
merged 1 commit into from
Mar 30, 2024
Merged

test_runner: run afterEach hooks in correct order #52239

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 12 additions & 1 deletion lib/internal/test_runner/test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ const {
ArrayPrototypeShift,
ArrayPrototypeSlice,
ArrayPrototypeSome,
ArrayPrototypeSplice,
ArrayPrototypeUnshift,
FunctionPrototype,
MathMax,
Expand Down Expand Up @@ -267,6 +268,7 @@ class Test extends AsyncResource {
after: [],
beforeEach: [],
afterEach: [],
ownAfterEachCount: 0,
};
} else {
const nesting = parent.parent === null ? parent.nesting :
Expand All @@ -286,6 +288,7 @@ class Test extends AsyncResource {
after: [],
beforeEach: ArrayPrototypeSlice(parent.hooks.beforeEach),
afterEach: ArrayPrototypeSlice(parent.hooks.afterEach),
ownAfterEachCount: 0,
};
}

Expand Down Expand Up @@ -555,7 +558,15 @@ class Test extends AsyncResource {
}
});
}
ArrayPrototypePush(this.hooks[name], hook);
if (name === 'afterEach') {
// afterEach hooks for the current test should run in the order that they
// are created. However, the current test's afterEach hooks should run
// prior to any ancestor afterEach hooks.
ArrayPrototypeSplice(this.hooks[name], this.hooks.ownAfterEachCount, 0, hook);
this.hooks.ownAfterEachCount++;
Comment on lines +565 to +566
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
ArrayPrototypeSplice(this.hooks[name], this.hooks.ownAfterEachCount, 0, hook);
this.hooks.ownAfterEachCount++;
ArrayPrototypeSplice(this.hooks[name], this.hooks.ownAfterEachCount++, 0, hook);

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I personally find that a little less readable, only because the splice line is already a bit busy.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, works for me 🙂

} else {
ArrayPrototypePush(this.hooks[name], hook);
}
return hook;
}

Expand Down
4 changes: 2 additions & 2 deletions test/fixtures/test-runner/output/hooks-with-no-global-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,14 +26,14 @@ after(() => {
'describe beforeEach',
'describe nested beforeEach',
'describe nested it 1',
'describe afterEach',
'describe nested afterEach',
'describe afterEach',

'describe beforeEach',
'describe nested beforeEach',
'describe nested test 2',
'describe afterEach',
'describe nested afterEach',
'describe afterEach',

'describe nested after',
'describe after',
Expand Down
8 changes: 4 additions & 4 deletions test/fixtures/test-runner/output/hooks.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@ describe('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',
'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',
]);
Expand Down Expand Up @@ -139,8 +139,8 @@ test('test hooks', async (t) => {
'beforeEach 2', '2', 'afterEach 2',
'beforeEach nested',
'nested before 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',
'beforeEach nested 1', 'nested beforeEach nested 1', 'nested1', 'nested afterEach nested 1', 'afterEach nested 1',
'beforeEach nested 2', 'nested beforeEach nested 2', 'nested 2', 'nested afterEach nested 2', 'afterEach nested 2',
'afterEach nested',
'nested after nested',
'after test hooks',
Expand Down
Loading