tests(fixtures): introduce script to update the report fixtures #4793
Conversation
paulirish
changed the title
paulirish
changed the title
patrickhulce
left a comment
patrickhulce
left a comment
There was a problem hiding this comment.
maybe I'm being blinded, but not entirely what this gets us over a shell script with lighthouse -GA=./lighthouse-cli/test/sample_artifacts --output=json --output-path=./lighthouse-core/test/results/sample_v2.json 😄
| @@ -0,0 +1,53 @@ | |||
| /** | |||
| * @license Copyright 2016 Google Inc. All Rights Reserved. | |||
| const cli = require('../../lighthouse-cli/run'); | ||
|
|
||
| const {server} = require( | ||
| '../../lighthouse-cli/test/fixtures/static-server'); |
There was a problem hiding this comment.
yikes is this really over the line length!?
sure sg. for the -A side of things, I've updated the npm script to use this direct approach. The -G side of things still needs some special handling because of the web server. |
.gitignore
Outdated
| *.report.pretty | ||
| *.artifacts.log | ||
|
|
||
| !lighthouse-cli/test/sample_artifacts/*.trace.json |
There was a problem hiding this comment.
should these live somewhere with sample_v2.json?
There was a problem hiding this comment.
sure!
how about lighthouse-core/test/results/artifacts/?
|
|
||
| (async function() { | ||
| await update(); | ||
| })(); |
There was a problem hiding this comment.
nit: any reason to wrap update()?
package.json
Outdated
| "plots-smoke": "bash plots/test/smoke.sh", | ||
| "changelog": "conventional-changelog --config ./build/changelog-generator/index.js --infile changelog.md --same-file", | ||
| "type-check": "tsc -p .", | ||
| "update:artifacts": "node lighthouse-core/scripts/update-report-fixtures.js -G", |
There was a problem hiding this comment.
Need something more specific. Maybe update:sample-artifacts or something? (same with json below)
| "lighthouse-core/lib/dependency-graph/**/*.js", | ||
| "lighthouse-core/gather/connections/**/*.js", | ||
| "lighthouse-core/gather/gatherers/gatherer.js", | ||
| "lighthouse-core/scripts/*.js", |
lighthouse-core/runner.js
Outdated
| } | ||
|
|
||
| module.exports = Runner; | ||
|
|
brendankenny
left a comment
brendankenny
left a comment
There was a problem hiding this comment.
LGTM!
I wonder if we should start printing devtoolsLog in the style of traces so we can get better diffs on them in artifacts
good idea. |
4 participants
This includes my previous two PRs. sorry. Compare link without those PRs in the mix.
Additions / changes:
yarn update:json: refreshes the sample_v2.json from static artifacts. Great as it's the exact same load, so minimal json changes.yarn update:artifacts: needed less often, but necessary when changes/additions are made to gatherersThis does check in 3.9MB of artifacts to the repo. But I think it's worth it.