feat(testing): add tests for registerServiceWorker #3200
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jsjoeio
changed the title
jsjoeio
commented
Apr 23, 2021
| const options = getOptions() | ||
| const path = normalize(`${options.csStaticBase}/dist/serviceWorker.js`) | ||
| try { | ||
| await navigator.serviceWorker.register(path, { | ||
| scope: (options.base ?? "") + "/", | ||
| scope: options.base + "/", |
There was a problem hiding this comment.
TLDR: options.base will always return a string. No need to use nullish coalescing operator
See commit message for explanation: 67dbb36
Codecov Report
@@ Coverage Diff @@
## main #3200 +/- ##
==========================================
+ Coverage 46.77% 46.81% +0.04%
==========================================
Files 23 23
Lines 1193 1194 +1
Branches 237 237
==========================================
+ Hits 558 559 +1
Misses 451 451
Partials 184 184
Continue to review full report at Codecov.
|
Inside registerServiceWorker, we were originally using the nullash coalescing operator to check if options.base was null or undefined. However, I realized this check is not necessary. If you look at getOptions' return value, we return an object with a key "base" which is of type "string". We get that value by calling resolveBase which always returns a string. As a result, we didn't need to check if options.base was null or undefined because it never can be.
jsjoeio
force-pushed
the
jsjoeio/add-test-browser-register
branch
from
67dbb36 to
83746c8
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR adds two more tests to cover the last line for
src/browser/register.tswhich should be at 100% now.Interestingly - the output from Jest and what Codecov picks up are a little different 🤷♂️