search

vercel / style-guide

Vercel's engineering style guide
Mozilla Public License 2.0
1.28k stars 37 forks source link

`it.only()` or `describe.only()` should fail lint #9

Closed styfle closed 2 years ago

styfle commented 2 years ago

Sometimes developers will use it.only() or describe.only() to run a single jest test locally but accidentally commit it to a PR and thus skip all tests except the one they're working on.

We should probably disable it.skip() as well for the same reason.

styfle commented 2 years ago

Here's an example of how to achieve this https://github.com/vercel/vercel/pull/4229

adrianbw commented 2 years ago

Yes, 100%.

codybrouwers commented 2 years ago

Good idea!

ernestd commented 2 years ago

+1 for .skip too

mrmckeb commented 2 years ago

Thanks for raising this @styfle! Can I confirm that this doesn't work with this style guide today?

We extend plugin:jest/recommended, which enables: https://github.com/jest-community/eslint-plugin-jest/blob/HEAD/docs/rules/no-disabled-tests.md

There's an example showing how to set up the jest config here: https://github.com/vercel/style-guide#scoped-configuration-with-overrides

styfle commented 2 years ago

@mrmckeb I created this issue because I was reviewing a PR and it used describe.only() yet CI was passing.

mrmckeb commented 2 years ago

Ah, that repo didn't have the jest config. I've now added it.

CC @dglsparsons who was leading the charge on that part of our internal rollout.