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

[sindresorhus]

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.

[SamVerschueren]

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.

[jamestalmage]

👍

[jfmengels]

👎

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.

[SamVerschueren]

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

[jfmengels]

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?

[sindresorhus]

Actually, avajs/ava#673 might make this discussion moot, as I would rather use it than a skip test.

[jamestalmage]

#673 is a good alternative to this.

[sindresorhus]

Ok, let's defer this discussion until after avajs/ava#673 is resolved.

[novemberborn]

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.