Use jsdom env. for testing and plugin-stats #1871
Conversation
openshift-ci-robot
requested review from
alecmerdler,
christianvogt,
rawagner and
spadgett
openshift-ci-robot
added
the
size/M
| }, | ||
| getItem(key) { | ||
| return _localStorage[key]; | ||
| return _localStorage[key] || null; |
There was a problem hiding this comment.
Note: these changes are made so that mock localStorage is as close to production as possible in terms of its API.
For example, in case of getItem method:
If the key does not exist,
nullis returned.
https://developer.mozilla.org/en-US/docs/Web/API/Storage/getItem
There was a problem hiding this comment.
You should only return null if the key doesn't exist. If the value is an empty string, this will incorrectly return null.
There was a problem hiding this comment.
You should only return null if the key doesn't exist. If the value is an empty string, this will incorrectly return
null.
Right, I've missed that. Will fix that in a separate PR.
There was a problem hiding this comment.
Will fix that in a separate PR.
| .first() | ||
| .props().filter, | ||
| ).toBe('url(blank#BaseNodeDropShadowFilterId)'); | ||
| ).toBe('url(/#BaseNodeDropShadowFilterId)'); |
There was a problem hiding this comment.
These changes are due to setting testURL value above.
| } | ||
| } | ||
| return activePerspective; | ||
| return activePerspective || undefined; |
There was a problem hiding this comment.
This is to coerce the expectation (undefined means no perspective) vs. underlying storage implementation (null means no such item in storage).
There was a problem hiding this comment.
Is something breaking without this?
I'd rather not distinguish between null and undefined here.
There was a problem hiding this comment.
Is something breaking without this?
I'd rather not distinguish between null and undefined here.
Without this, __tests__/reducers/ui.spec.ts fails on
expect(getDefaultPerspective()).toBeUndefined();(expected undefined but received null)
There was a problem hiding this comment.
A bad test. null and undefined should be equivalent for this assertion.
To reproduce, just put following code in import { Icon } from 'patternfly-react';
const c = <Icon type="fa" name="address-book" />; |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: rawagner, vojtechszocs The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
6 participants
This PR ensures that
plugin-statsscripthave access to relevant web APIs (notably DOM and HTML) provided via jsdom implementation.
This also fixes problems like Console plugin entry module directly or indirectly referencing such web APIs and
plugin-stats(ts-node) failing on errors likeReferenceError: Element is not defined.Notable changes:
__mocks__/localStorage.tssince it's not easy to mock jsdom's web storage__mocks__/matchMedia.jssince it's not provided by jsdom__mocks__/requestAnimationFrame.jssince it's already provided by jsdom/cc @spadgett @alecmerdler @rawagner @christianvogt