Add no-failing-hooks rule

[jamestalmage]

While the failing modifier could technically be used for a hook (avajs/ava#831 (comment)), it's probably a mistake in most cases.

Rather than outlawing it in AVA, I think it would be best to make a rule here that defaults to an error if they use the failing modifier on any hooks (before, beforeEach, after, etc.). Users can then disable with a eslint comment if they really meant to do it.

Optionally, we could add an allowedHooks config option.

Perhaps it's not even worth doing this, since the intent is to have AVA print the failing tests in red eventually (which should be warning enough, even if it is in a hook).

[sindresorhus]

Why would anyone want to have a failing hook? I kinda think we should just disallow it in AVA until someone complains with a good use-case. We should strive to keep AVA as sensible as possible. ESLint rules are purely for situations where we can't.

[jamestalmage]

My thought was some an afterEach hook that ran some common assertion (i.e. "always returns the db connection to the pool when it's done").

As the PR stands now, it allows any use. If someone comes up with a valid reason to use it in a beforeEach, it will just work. A lint rule gives their build a failure for that odd usage, but in a way they can override with a comment. It's basically a way to signal, "Let me do this, I know what I'm doing". Building that flexibility into AVA wouldn't be easy.

It's niche enough, I'm willing to let it go though.

[sindresorhus]

If someone comes up with a valid reason to use it in a beforeEach, it will just work.

If someone does, we could easily change to allow it in AVA too, but allowing it without any real-world use-cases we end up having a linter rule for something I see no use-case for. I might be wrong. I just don't like doing stuff for hypothetical reasons.

@novemberborn @vdemedes @sotojuan Thoughts?

[jamestalmage]

Well, we will probably do this linter rule regardless of what we do in AVA right? For early feedback in atom?

[sindresorhus]

Right, yeah.

[novemberborn]

Happy to disallow in AVA too.

[sindresorhus]

Closing this in favor of #186, which will restrict modifier chaining for many other things too. In the latest AVA 1.0.0 beta, test.before.failing('foo', () => {}); is a syntax error.