tests(legacy-javascript): cache variants, check in lockfile, … #10568
Conversation
|
@brendankenny bump :) kinda-sorta a blocker on #10564 |
| * summary-sizes.txt | ||
| * summary-signals.json | ||
|
|
||
| `summary-sizes.txt` lists each of the variants (grouped by type) and sorted by their byte size. This is mostly a diagnostic tool and changes in this can be ignored. This is checked in because whenever the lockfile is updated, `summary-sizes.txt` tells us things like if transforms are getting worse. And it is a quick reference for the relative weight of each of these transform/polyfills. |
There was a problem hiding this comment.
I don't think variant is defined anywhere, so it might be good to add that to the readme as well
| * summary-sizes.txt | ||
| * summary-signals.json | ||
|
|
||
| `summary-sizes.txt` lists each of the variants (grouped by type) and sorted by their byte size. This is mostly a diagnostic tool and changes in this can be ignored. This is checked in because whenever the lockfile is updated, `summary-sizes.txt` tells us things like if transforms are getting worse. And it is a quick reference for the relative weight of each of these transform/polyfills. |
There was a problem hiding this comment.
mostly a diagnostic tool and changes in this can be ignored
This is checked in because whenever the lockfile is updated,summary-sizes.txttells us things like if transforms are getting worse
seem somewhat contradictory, but is this saying this file is an indicator of transforms (out in the developer ecosystem) getting better/worse so the file is checked in so that can be seen in the github UI, but any changes aren't important (they're changes in the transforms, not in lighthouse) so commit freely if there is a diff?
There was a problem hiding this comment.
yeah, I tried capturing that better with a rewording
| 4774 variants/core-js-2-only-polyfill/es6-array-species/main.bundle.min.js | ||
| 3410 variants/core-js-2-only-polyfill/es6-function-name/main.bundle.min.js | ||
| core-js-2-only-polyfill | ||
| 47738 es6-typed-uint8-clamped-array/main.bundle.min.js |
There was a problem hiding this comment.
maybe worth putting a header somewhere? bytes and whatever the other thing is :) polyfilled bundle of something?
| removeCoreJs(); // (in case the script was canceled halfway - there shouldn't be a core-js dep checked in.) | ||
|
|
||
| const hash = crypto | ||
| .createHash('sha256') |
| @@ -258,6 +272,36 @@ function makeSummary() { | |||
| }; | |||
| } | |||
|
|
|||
| function createSummarySizes() { | |||
There was a problem hiding this comment.
| function createSummarySizes() { | |
| function createSizesSummary() { |
?
:)
There was a problem hiding this comment.
ehhhhh I wanted symmetry with summary-signals, i'll just leave it as is
| lines.push(path.relative(VARIANT_DIR, variantGroupFolder)); | ||
|
|
||
| const variants = []; | ||
| for (const variantFolder of glob.sync(`${variantGroupFolder}/**/main.bundle.min.js `)) { |
There was a problem hiding this comment.
variantFile or variantBundle?
| // Create variants in a directory named-cached by contents of this script and the lockfile. | ||
| // This folder is in the CI cache, so that the time consuming part of this test only runs if | ||
| // the output would change. | ||
| removeCoreJs(); // (in case the script was canceled halfway - there shouldn't be a core-js dep checked in.) |
There was a problem hiding this comment.
it does seem a little odd to control this here (both running here and not in main() and yarn commands inside this file in general), but I haven't reviewed the full control flow so I'll assume it makes sense :)
Co-Authored-By: Brendan Kenny <[email protected]>
connorjclark
changed the title
Fixes #10566