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

readme: add note about uncaught exceptions #918

Merged
merged 1 commit into from
Jun 19, 2016
Merged

Conversation

arve0
Copy link
Contributor

@arve0 arve0 commented Jun 14, 2016

fixes issue #917

@jamestalmage
Copy link
Contributor

I'm not sure this is the correct behavior. We may just want to run after.always on uncaught exceptions

@novemberborn
Copy link
Member

novemberborn commented Jun 19, 2016

I'm not sure this is the correct behavior. We may just want to run after.always on uncaught exceptions

Uncaught exceptions cause the worker to be torn down. That seems reasonable to me. The behavior is somewhat similar to --fail-fast, which we've previously decided does not cause after.always to run.

@@ -433,7 +433,7 @@ test.failing('demonstrate some bug', t => {

AVA lets you register hooks that are run before and after your tests. This allows you to run setup and/or teardown code.

`test.before()` registers a hook to be run before the first test in your test file. Similarly `test.after()` registers a hook to be run after the last test. Use `test.after.always()` to register a hook that will **always** run once your tests and other hooks complete. `.always()` hooks run regardless of whether there were earlier failures, so they are ideal for cleanup tasks.
`test.before()` registers a hook to be run before the first test in your test file. Similarly `test.after()` registers a hook to be run after the last test. Use `test.after.always()` to register a hook that will **always** run once your tests and other hooks complete. `.always()` hooks run regardless of whether there were earlier failures, so they are ideal for cleanup tasks. **Note**: In the case of uncaught exceptions, `test.after.always()` will not be called.
Copy link
Member

Choose a reason for hiding this comment

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

I would rephrase this though:

… they are ideal for cleanup tasks. There are two exceptions to this however. If you use --fail-fast AVA will stop testing as soon as a failure occurs, and it won't run any hooks including the .always() hooks. Uncaught exceptions will crash your tests, possibly preventing .always() hooks from running.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Uncaught exceptions will crash your tests, possibly preventing .always() hooks from running?

Copy link
Contributor Author

@arve0 arve0 Jun 19, 2016

Choose a reason for hiding this comment

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

I'm not familiar with the internals, but are there cases where uncaught exceptions will allow .after to run?

Copy link
Member

Choose a reason for hiding this comment

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

Yes, if they occur after .after has run. With asynchronous tests it's a race between a test completing and an uncaught exception occurring.

@arve0
Copy link
Contributor Author

arve0 commented Jun 19, 2016

Updated to new wording.

@novemberborn
Copy link
Member

LGTM 😉

@sindresorhus
Copy link
Member

LGTM, because it reflects reality.

Though, I'm not totally happy about .always not actually being always, but I think the earlier arguments were that you'd want the tests and fixtures to be in a consistent state so you can debug. Personally, I find it a bit annoying having to manually cleanup the leftovers. There's also the argument that cleanup should happen on startup, but then you get a dirty git repo. Feels like there's something we could do to ease this for the user.

@sindresorhus sindresorhus merged commit 9efcee0 into avajs:master Jun 19, 2016
@jamestalmage
Copy link
Contributor

There's also the argument that cleanup should happen on startup, but then you get a dirty git repo.

I usually get around that with .gitignore.

Feels like there's something we could do to ease this for the user.

What if we allowed a .cleanup modifier that ran before and after the tests?

Or what if we allowed an and modifier:

test.before.and.after(fn)
test.beforeEach.and.after.always(fn)

@sindresorhus
Copy link
Member

sindresorhus commented Jun 19, 2016

@jamestalmage Can you move the bottom part of your comment to a new issue?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants