-
-
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
Add eslint rules for ensuring methods on expect are called correctly #3426
Conversation
expectFunctionsWithoutArgs, | ||
); | ||
|
||
function checkExpectFunction(calleeName, node, context) { |
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.
https://github.com/facebook/jest/pull/3426/files?w=1 for this is obligatory 😄
9199211
to
3df5c01
Compare
@@ -28,6 +28,8 @@ type JasmineMatchersObject = {[id: string]: JasmineMatcher}; | |||
|
|||
module.exports = (config: {expand: boolean}) => { | |||
global.expect = expect; | |||
// setState is not documented |
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.
Should it be?
95ff89f
to
ddcd853
Compare
Codecov Report
@@ Coverage Diff @@
## master #3426 +/- ##
==========================================
+ Coverage 64.04% 64.33% +0.28%
==========================================
Files 177 177
Lines 6570 6626 +56
Branches 4 4
==========================================
+ Hits 4208 4263 +55
Misses 2361 2361
- Partials 1 2 +1
Continue to review full report at Codecov.
|
ddcd853
to
a5213d9
Compare
@SimenB Awesome! Thanks 😄 |
Hey @SimenB. I thought about this PR a bit over the past two days and have concluded that we shouldn't merge it. I think the maintenance cost is higher than the value it provides. For these functions, when they are invoked improperly, Jest will simply throw and crash the test. I think that is ok. The alternative here is to surface such mistakes earlier but at the cost of us having to maintain this API changes in two places. I'd prefer to not take on this cost right now but I'm happy to consider different viewpoints and may reopen if you feel strongly that we should do this. |
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
The rule only validated usage of matchers, not all the extra stuff
expect
can do./cc @anilreddykatta
Test plan
See new tests,
yarn jest