-
-
Notifications
You must be signed in to change notification settings - Fork 6.5k
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
Allow eslint-plugin-jest to recognize more disabled tests #4533
Conversation
Codecov Report
@@ Coverage Diff @@
## master #4533 +/- ##
=========================================
+ Coverage 55.98% 56.1% +0.12%
=========================================
Files 185 185
Lines 6236 6254 +18
Branches 3 3
=========================================
+ Hits 3491 3509 +18
Misses 2744 2744
Partials 1 1
Continue to review full report at Codecov.
|
@SimenB mind reviewing this one? |
@borilla Could you rename the file/rule in a separate commit to make reviewing easier? 🙂 First glance looks good, but according to github everything changed |
I've renamed the file back to the original so it's easier to see the diffs |
Already recognizes: * tests that append `skip` (e.g. `it.skip(...)`) * tests that prepend `x` (e.g. `xit(...)`) This change adds: * tests that don't contain a function body * tests that contain a call to `pending()` https://jasmine.github.io/2.0/introduction.html#section-Pending_Specs
Great, thank you! Makes reading the new test cases way easier 🙂 The change LGTM. 👍 Feel free to rename the file back |
@@ -37,6 +37,7 @@ module.exports = { | |||
it: false, | |||
jasmine: false, | |||
jest: false, | |||
pending: false, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I honestly didn't realize we exposed this global…
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have to admit I wasn't even aware of it myself until I checked out the docs for Jasmine
Thanks for doing this! |
Already recognizes: * tests that append `skip` (e.g. `it.skip(...)`) * tests that prepend `x` (e.g. `xit(...)`) This change adds: * tests that don't contain a function body * tests that contain a call to `pending()` https://jasmine.github.io/2.0/introduction.html#section-Pending_Specs
This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
Summary
This change affects the
jest/no-disabled-tests
rule of the Jest eslint pluginThis update allows the rule to recognize more disabled/skipped/pending tests, in line with information for Pending Specs in Jasmine
The existing rule already recognizes:
it.skip(...)
)xit(...)
)This change adds:
it('foo')"
)it('foo', () => { pending(); })
)Test plan
Additional test cases for the rule changes have been added to
__tests__/no_disabled_tests.test.js
[This file was renamed from
__tests__/no_skipped_tests.test.js
to better match both the rule-name and the source file name it is testing]