Comments (7)
I think I have a weak preference in favor of jest.ToEqual
. I had to remind myself of the differences when looking at .toStrictEqual docs
:
- keys with undefined properties are checked, e.g.
{a: undefined, b: 2}
will not equal{b: 2}
; - undefined items are taken into account, e.g.
[2]
will not equal[2, undefined]
; - array sparseness is checked, e.g.
[, 1]
will not equal[undefined, 1]
; - object types are checked, e.g. a class instance with fields
a
andb
will not equal a literal object with fieldsa
andb
.
None of those seem deal-breaking to me, though I'd be interested to hear others' thoughts.
from core.js.
Thumbs up reaction (👍🏽) for
jest.ToEqual
, and I can apply the changes here:
👍🏼 from me, thank you @oscard0m for taking on this and other chore tasks 💛
from core.js.
I took a look into it.
The toStrictEqual
is doing some customTesters internally. The one giving problems is typeEquality
. Concretely, this comparison: a.constructor === b.constructor
.
When using fetch-mock
, the accepted type for response
parameter in getOnce(matcher, response)
are:
type MockResponse = Response | Promise<Response>
| number | Promise<number>
| string | Promise<string>
| {} | Promise<{}>
| MockResponseObject | Promise<MockResponseObject>;
According to this code comment:
- object: As long as the object is not a MockResponseObject it is
* converted into a json string and returned as the body of a 200 response
This means the constructor
equal comparison is lost at this point.12
Considering these are mocks controlled by ourselves, the alternatives I come up with are:
a) to use a jest.toEqual
b) to keep the hack of JSON.stringify
c) try if passing a MockResponseObject
or Response
gets the right constructor and toStrictEqual
works.
What do you think @octokit/extensibility-sdks @octokit/js-community ?
Footnotes
from core.js.
Thumbs up reaction (👍🏽) for jest.ToEqual
, and I can apply the changes here:
from core.js.
There is at least one case where I switched to toMatchObject()
Those could probably be updated as well
from core.js.
There is at least one case where I switched to
toMatchObject()
Those could probably be updated as well
In a quick search these are the ones I found:
The rest can be progressively updated. It's not a problem to have jest.toMatchObject()
.
from core.js.
Thank you all for the quick reviews. We are done with this. 🥳 🎆 🍾
from core.js.
Related Issues (20)
- [BUG]: Uncaught ReferenceError: global is not defined HOT 7
- [BUG]: octokit.graphql gives increasing number of duplicate HOT 1
- [BUG]: can not intercept requests with nock HOT 2
- [BUG]: Octokit version 4.2.2 has a breaking change dependency upgrade HOT 2
- https://github.com/ietf-tools/datatracker/commit/e68b9cf60c0f6771e40ed73a4f59aa47cacbb854
- Re-enable tests in `@octokit/core` using fetch dispatchers HOT 2
- Replace `toStrictEqual` with `JSON.stringify` Response assertions with `toEqual` in `core.js` HOT 1
- [BUG]: Unexpected token '{' in Safari pre 16.3 HOT 4
- [BUG]: Rest call to delete comment returns 404 HOT 3
- [MAINT]: Replace lolex with @sinon/fake-timers HOT 2
- [BUG]: OctokitOptions declared locally but not exported HOT 7
- [QUESTION] Is there an alternative version of this project written in c#? HOT 2
- [BUG]: The type of 'data' field in OctokitResponse type is an object but type hinting show it's an array HOT 2
- [BUG]: Commit not found when it exists HOT 2
- [BUG]: `No "exports" main defined` in version 6.0.1 HOT 5
- [BUG/ASK for HELP]: Issues with `@octokit/[email protected]+` when using `ncc` to bundle code HOT 6
- [BUG]: Error [ERR_PACKAGE_PATH_NOT_EXPORTED]: No "exports" main defined in HOT 24
- 👋 Hi! Thank you for this contribution! Just to let you know, our GitHub SDK team does a round of issue and PR reviews twice a week, every Monday and Friday! We have a [process in place](https://github.com/octokit/.github/blob/main/community/prioritization_response.md#overview) for prioritizing and responding to your input. Because you are a part of this community please feel free to comment, add to, or pick up any issues/PRs that are labled with `Status: Up for grabs`. You & others like you are the reason all of this works! So thank you & happy coding! 🚀 HOT 1
- [BUG]: Headers from API endpoint does not contain a link header HOT 2
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from core.js.