-
Notifications
You must be signed in to change notification settings - Fork 685
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
Tests for Service Worker #1965
Tests for Service Worker #1965
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.
Great mocking and great tests - thank you!
The biggest change request is restoring mocks when they're modified outside of a beforeAll
or similar (that will restore for you).
packages/venia-concept/src/ServiceWorker/Utilities/__tests__/htmlHandler.spec.js
Outdated
Show resolved
Hide resolved
packages/venia-concept/src/ServiceWorker/Utilities/__tests__/htmlHandler.spec.js
Show resolved
Hide resolved
packages/venia-concept/src/ServiceWorker/Utilities/__tests__/htmlHandler.spec.js
Outdated
Show resolved
Hide resolved
packages/venia-concept/src/ServiceWorker/Utilities/__tests__/htmlHandler.spec.js
Show resolved
Hide resolved
packages/venia-concept/src/ServiceWorker/Utilities/__tests__/htmlHandler.spec.js
Show resolved
Hide resolved
packages/venia-concept/src/ServiceWorker/Utilities/__tests__/routeHandler.spec.js
Show resolved
Hide resolved
packages/venia-concept/src/ServiceWorker/Utilities/messageHandler.js
Outdated
Show resolved
Hide resolved
packages/venia-concept/src/ServiceWorker/__tests__/registerRoutes.spec.js
Show resolved
Hide resolved
packages/venia-concept/src/ServiceWorker/__tests__/registerRoutes.spec.js
Show resolved
Hide resolved
Changes look good. Deferring to @jimbo on our Unit Testing Best Practices™️ (see #1965 (comment)). |
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.
Approved. We can always refactor these tests later if we standardize on best practices.
Description
Added tests for the Service Worker.
Related Issue
Closes #1789
Verification Steps
yarn test
andyarn test:ci
. You should not see any errors.Screenshots / Screen Captures (if appropriate)
Checklist
None