deps(angular): update minor version of angular fixture #11043
Conversation
Co-authored-by: Patrick Hulce <patrick.hulce@gmail.com>
| @@ -10,6 +10,7 @@ const assert = require('assert').strict; | |||
| const {computeCSSTokenLength, computeJSTokenLength} = require('../../lib/minification-estimator.js'); // eslint-disable-line max-len | |||
|
|
|||
| const angularFullScript = fs.readFileSync(require.resolve('angular/angular.js'), 'utf8'); | |||
| const zoneMinifiedScript = fs.readFileSync(`${__dirname}/../../../lighthouse-cli/test/fixtures/dobetterweb/third_party/aggressive-promise-polyfill.js`, 'utf8'); // eslint-disable-line max-len | |||
There was a problem hiding this comment.
can we use this as motivation to move aggressive-promise-polyfill.js to the root third-party/ where it probably should be? :)
There was a problem hiding this comment.
i looked into it, but
makes that pretty challenging. ideas?
There was a problem hiding this comment.
we could do another of these
lighthouse/lighthouse-cli/test/fixtures/static-server.js
Lines 161 to 164 in 1330133
🤮
There was a problem hiding this comment.
plz no this breaks smoke rider
There was a problem hiding this comment.
plz no this breaks smoke rider
Can we talk solutions, then? How did smokerider work when we used to serve it out of node_modules/?
There was a problem hiding this comment.
Can we talk solutions, then? How did smokerider work when we used to serve it out of node_modules/?
we skip urls that use external resources, so I guess we skipped that one too when it served a file from node_modules. search for usesExternalResources in google3 smokerider.
can we check it into fixtures/third_party instead, so there is no serving rewiring needed?
EDIT: ah, I see that is how it is now but brendan is suggesting moving to root. so my take is to keep it where it is. but I'd prefer fixtures/third_party as a central place for all possible 3p smoke assets
There was a problem hiding this comment.
so my take is to keep it where it is. but I'd prefer
fixtures/third_partyas a central place for all possible 3p smoke assets
well, those are equally wrong places to put third-party code, so I'm equally fine with them :P
Honestly we should probably delete this file and replace it with a considerably simpler promise stomper (we have a lot more confidence in our isolated context and cached globals than we did when we added this test), but adding it to the unit test is probably going to make it so that never happens :)
There was a problem hiding this comment.
devtools frontend does this (/third_party and /front_end/third_party), so it's not without prior art
There was a problem hiding this comment.
resolution: we handwrite a promisepolyfill for that test.
we get rid of this big one.
maybe use some other minified js for this estimator test.
@paulirish to do
|
same as #10086 just 7 months later.
this gets rid of the last security advisory we have.
also added one more minification-estimator test.... to make sure it doesn't overestimate pre-minified savings.