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.

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

Execute onTestFinished handlers in the reverse order they are scheduled in #5529

Closed
4 tasks done
nvie opened this issue Apr 12, 2024 · 3 comments · Fixed by #5598
Closed
4 tasks done

Execute onTestFinished handlers in the reverse order they are scheduled in #5529

nvie opened this issue Apr 12, 2024 · 3 comments · Fixed by #5598
Labels
p3-minor-bug An edge case that only affects very specific usage (priority)

Comments

@nvie
Copy link

nvie commented Apr 12, 2024

Clear and concise description of the problem

Currently, it looks like onTestFinished runs as a FIFO queue:

onTestFinished(() => console.log("foo"));
onTestFinished(() => console.log("bar"));

Output after test:

foo
bar

However, it seems to be more useful if those registrations are run in the reverse order they were scheduled in, so you can colocate setup and teardown code and never have to worry about the order of execution here:

// Suppose this code lives in a `getDb()` helper
const db = setupDb();
onTestFinished(() => db.teardown());

// And this code lives in a `getTable()` helper
const table = db.createTable();
onTestFinished(() => db.dropTable(table.name));

Right now, db.teardown() gets called before db.dropTable(), which causes an issue.

Suggested solution

If the order the teardown functions are run in is reversed, this class of problems will disappear.

Alternative

No response

Additional context

No response

Validations

@sheremet-va
Copy link
Member

I think they are actually called in parallel.

@sheremet-va
Copy link
Member

Ideally, they should follow sequence.hooks option. So this is a bug at the very least.

@sheremet-va sheremet-va added p3-minor-bug An edge case that only affects very specific usage (priority) and removed enhancement: pending triage labels Apr 12, 2024
@nvie
Copy link
Author

nvie commented Apr 12, 2024

Oh yeah, if sequence.hooks: "stack" could be used, that'd be great 👍

@github-actions github-actions bot locked and limited conversation to collaborators May 9, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
p3-minor-bug An edge case that only affects very specific usage (priority)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants