-
Notifications
You must be signed in to change notification settings - Fork 822
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
Rename the workbox-google-analytics output file #1688
Conversation
df6b86f
to
bac27b0
Compare
I don't think that was intentional; you should be able to work around it and get all the tests running against the local build output by using something similar to workbox/test/workbox-sw/static/sw.js Lines 3 to 8 in 59e7d4f
|
Yeah, I can set a config for the loader, I just wanted to make sure the intent of this test wasn't to test that the CDN files were actually available. BTW, I'm confused as to how that code you referenced even works because from my reading of the code, workbox/packages/workbox-sw/controllers/WorkboxSW.mjs Lines 42 to 48 in 59e7d4f
|
That's... a good question. Okay, here's a more recently updated integration test that does the same thing in what's hopefully the correct way:
|
bac27b0
to
050f680
Compare
PR-Bot Size PluginChanged File Sizes
New Files
All File SizesView Table
Workbox Aggregate Size Plugin9.54KB gzip'ed (64% of limit) |
@jeffposnick, all the tests are passing now. I had to update some of the test server logic, but it looks like things are better tested now with these changes. |
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.
👍
R: @jeffposnick
Fixes #1646
As far as I can tell this is working with the
workbox-sw
loader (and all the tests pass), but I know our e2e tests manually reference individual dependency files, so it's not clear to me if the loader logic is actually tested against built filenames./cc @gauntface
EDIT: Actually, looks like tests are failing; looking into it now.