eslint-plugin-ava icon indicating copy to clipboard operation
eslint-plugin-ava copied to clipboard

Change `no-skip-test` to warning in the recommended preset?

Open sindresorhus opened this issue 9 years ago • 9 comments

Does it really make sense to make it error by default in the recommended preset? Usually, I skip tests that doesn't work for some reason and I don't have time to fix right now. It's weird having to also add a ESLint ignore comment.

I propose we make it a warning.

sindresorhus avatar Apr 25 '16 17:04 sindresorhus

I'm 👍 on warning. I feel skip is a little bit the same as a TODO comment. Eventually you want them out, but they can be darn handy to not forget something in the future.

SamVerschueren avatar Apr 25 '16 17:04 SamVerschueren

:+1:

jamestalmage avatar Apr 25 '16 17:04 jamestalmage

:-1:

In my day job, my colleagues and I often use .skip and only to temporarily run tests only on what we want to fix/add. At the end of the day, we remove them, commit and push. But we had to add additional tools (we use Mocha for most projects) to abort the push if they remained somewhere because we often forgot to remove them.

We're used to: "If it passes all tests, it's all good", and warnings would be ignored in the buttload of text generated by all the different test phases. I have the same mentality when developing modules (though there is a lot less text, so I could maybe see it in the console). Also: a CI wouldn't catch those, and then who knows how long it would take before someone notices it.

I don't mind re-enforcing it in my projects, but to me, it makes sense to keep it as an error in quite some cases.

jfmengels avatar Apr 26 '16 09:04 jfmengels

You can always overwrite it in the config rules. It's more about "what's the best default for this rule".

SamVerschueren avatar Apr 26 '16 09:04 SamVerschueren

You can always overwrite it in the config rules. It's more about "what's the best default for this rule".

I agree, but I think that allowing omitted tests is a rarer case than not allowing omitted tests. If it's an error, it can be a life-saver at best, a nuisance at worst?

jfmengels avatar Apr 26 '16 09:04 jfmengels

Actually, https://github.com/sindresorhus/ava/issues/673 might make this discussion moot, as I would rather use it than a skip test.

sindresorhus avatar Apr 26 '16 09:04 sindresorhus

#673 is a good alternative to this.

jamestalmage avatar Apr 26 '16 10:04 jamestalmage

Ok, let's defer this discussion until after https://github.com/sindresorhus/ava/issues/673 is resolved.

sindresorhus avatar Apr 26 '16 11:04 sindresorhus

I guess it depends whether you lint before or after running tests. If before then it should be configured to be a warning, else you can't run your tests to begin with.

novemberborn avatar Apr 26 '16 15:04 novemberborn