fix: Support class instances in .toHaveProperty() matcher#5367
fix: Support class instances in .toHaveProperty() matcher#5367cpojer merged 2 commits intojestjs:masterfrom
Conversation
Codecov Report
@@ Coverage Diff @@
## master #5367 +/- ##
=======================================
Coverage 61.33% 61.33%
=======================================
Files 205 205
Lines 6924 6924
Branches 4 3 -1
=======================================
Hits 4247 4247
Misses 2676 2676
Partials 1 1Continue to review full report at Codecov.
|
|
|
||
| * `[jest]` Add `import-local` to `jest` package. | ||
| ([#5353](https://github.com/facebook/jest/pull/5353)) | ||
| * `[expect]` Support class instances in `.toHaveProperty()` matcher. |
There was a problem hiding this comment.
This also affected .toMatchObject() as far as I can tell.
There was a problem hiding this comment.
In a very good way. It basically enables .toMatchObject() to check if a given class implements an interface.
But in my opinion this should be included in the changelog regardless of whether one finds the change good or bad.
There was a problem hiding this comment.
It was more in the vein of "is this a regression we have to fix or not?". Good that it's not!
PR welcome to expand the changelog entry 🙂
There was a problem hiding this comment.
Should I include some tests just to make sure I identified that behavior correctly?
- jestjs/jest#5367 added support for "interface" matching with getters
|
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. |
6 participants
Summary
Checking for object own property with
Object.prototype.hasOwnProperty.call(object, value)won't work on class instance getters. To support that, we can fall back to checkingobject.constructor.prototype, because we knowobjectis an Object so we can actually get its class prototype which stores the getter.Also refactored the code a bit for perf (remove unnecessary
delete) and readability (flattened if/else branching, use?w=1to review)Resolves #5339
Test plan
Added a test or two.