-
Notifications
You must be signed in to change notification settings - Fork 225
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
Replace fetch mock with jest fetch mock #11850
Replace fetch mock with jest fetch mock #11850
Conversation
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.
Note for reviewers: I have deleted this test because all of the tests were skipped (and I didn't want to have to fix the fetch-mock
issue if I didn't need to)
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.
Note for reviewers: I have deleted this test because all of the tests were skipped (and I didn't want to have to fix the fetch-mock
issue if I didn't need to)
@@ -362,78 +341,11 @@ describe('Routes', () => { | |||
).toBeInTheDocument(); | |||
}); | |||
|
|||
it('should route to and render a photo gallery page', async () => { |
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.
Note for reviewers: I have deleted this test because photo gallery pages are now routed to the article page
).toBeInTheDocument(); | ||
}); | ||
|
||
it('should route to and render a story page', async () => { |
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.
Note for reviewers: I have deleted this test because story pages are now routed to the article page
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.
Looks good 👍 Thanks for removing the older page type tests. We'll get around to deleting all of that code soon hopefully!
@amoore108 I did see a branch with all these deletions in it, so I hope this won't cause a merge conflict 😬 - I am sure we can figure it out then! |
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.
Looks good 👍
Overall changes
Replaces references of the
fetch-mock
library withjest-fetch-mock
asfetch-mock
requires ESM supportCode changes
Replace references of
fetch-mock
withjest-fetch-mock
and update tests accordinglyTesting
This affects unit tests only, so no manual testing required
Unit tests all passing as expected
Helpful Links
Add Links to useful resources related to this PR if applicable.
Coding Standards
Repository use guidelines