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

Add no-before-after rule #101

Closed
wants to merge 1 commit into from
Closed

Conversation

lencioni
Copy link

@lencioni lencioni commented Sep 6, 2016

before() and after() run once before and after all examples. In
practice, this can end up causing confusion regarding the order that
things are run, and test pollution. To improve the readability and
accuracy of your Mocha tests, use beforeEach() and afterEach()
instead, wherever possible.

I had this rule sitting around in an internal ESLint plugin and figured
I would contribute it to this plugin in case others find it useful.

`before()` and `after()` run once before and after all examples. In
practice, this can end up causing confusion regarding the order that
things are run, and test pollution. To improve the readability and
accuracy of your Mocha tests, use `beforeEach()` and `afterEach()`
instead, wherever possible.
@lencioni
Copy link
Author

lencioni commented Sep 6, 2016

Looking through the rules here again and I spotted no-hooks. I wonder if this would be better as an option on that rule instead of its own rule. Thoughts?

@jfmengels
Copy link
Collaborator

This might be a better option on no-hooks yeah, maybe a allow option.

Just FYI, when I only use beforeEach, it's usually to have a fresh clean slate for each test. Now I prefer to do that by calling explicitly a function which gives me the fixtures that I need to use in the test. But this gets harder to do when you use stubs/mocks/spies and that you wish to reset correctly after the test (so an afterEach is nice in that case).

@lencioni
Copy link
Author

lencioni commented Sep 7, 2016

In my rule here, I took some care to ensure that this is only flagged on uses within context or describe. I noticed that the rules here don't do that. Perhaps that's not important, but if you think it is useful, should we add that type of thing to the rules here?

@lo1tuma
Copy link
Owner

lo1tuma commented Sep 8, 2016

Thanks for this PR. I would also agree that this feature could be implemented as an option of no-hooks. It is basically the same option we already have for no-hooks-for-single-case.

In my rule here, I took some care to ensure that this is only flagged on uses within context or describe

Is there a specific reason why top-level/global hooks should be excluded from this rule?

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.

3 participants