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

Use of onCleanup() when test returns a promise #765

Closed
devinivy opened this issue Oct 6, 2017 · 5 comments · Fixed by #766
Closed

Use of onCleanup() when test returns a promise #765

devinivy opened this issue Oct 6, 2017 · 5 comments · Fixed by #766
Assignees
Labels
breaking changes Change that can breaking existing code feature New functionality or improvement
Milestone

Comments

@devinivy
Copy link
Member

devinivy commented Oct 6, 2017

Currently the API to register a cleanup routine for a test is tangled with the use of callback-style tests (done()). That's because onCleanup is passed as the second argument to each test, and this check forbids the test to both return a promise and receive any arguments.

It would be very useful to be able to register a cleanup routine and use the promise API, especially with hapi now embracing async/await.

I do not know the ideal API for this, but here's something to get started with.

it('works great.', async function () {

    this.onCleanup(async () => rimraf('some/file.txt'));

    await otherTestActivities();
});

Another option is to loosen the restriction, and only complain if done() is actually called in a test that returns a promise. Then it could be ignored as the first argument, and onCleanup could still be utilized.

@geek
Copy link
Member

geek commented Oct 6, 2017

@devinivy I'm moving onCleanup to the async/await style and will come on a single flags object that will be passed in... let me know what you think:

it('works great.', async (flags) {

    flags.onCleanup(async () => rimraf('some/file.txt'));

    await otherTestActivities();
});

@geek geek added the request label Oct 6, 2017
@devinivy
Copy link
Member Author

devinivy commented Oct 6, 2017

Looks good to me! What else might go on flags?

@geek
Copy link
Member

geek commented Oct 12, 2017

@devinivy flags will also have notes and in the future mustCall

@geek geek added this to the 15.0.0 milestone Oct 12, 2017
@geek geek added breaking changes Change that can breaking existing code feature New functionality or improvement and removed request labels Oct 12, 2017
@geek geek self-assigned this Oct 12, 2017
@AdriVanHoudt
Copy link
Contributor

how about ?

it('works great.', async (flags) {

    await otherTestActivities();
}, async () => rimraf('some/file.txt'));

@lock
Copy link

lock bot commented Jan 9, 2020

This thread has been automatically locked due to inactivity. Please open a new issue for related bugs or questions following the new issue template instructions.

@lock lock bot locked as resolved and limited conversation to collaborators Jan 9, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
breaking changes Change that can breaking existing code feature New functionality or improvement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants