ci: save perf trace to S3 on failure #2051
Conversation
.travis.yml
Outdated
| - npm run lint | ||
| - npm run unit | ||
| - npm run closure | ||
| - npm run smoke |
There was a problem hiding this comment.
this formatting was all done by travis on travis encrypt --add I can remove if we care
There was a problem hiding this comment.
I vote for reverting where it's unchanged. We just used git blame on .travis.yml in filing #2048 :)
There was a problem hiding this comment.
yah ++ on not adjusting whitespace in this PR.
(the last commit didn't revert it fwiw)
edit: all good!
There was a problem hiding this comment.
(the last commit didn't revert it fwiw)
yeah the noisiness referred to not uploading traces that failed unrelated to screenshots failures not the travis noise fix until the next PR :)
| @@ -244,7 +254,8 @@ const cli = yargs | |||
| .help('help') | |||
| .describe({ | |||
| 'config-path': 'The path to the config JSON file', | |||
| 'expectations-path': 'The path to the expected audit results file' | |||
| 'expectations-path': 'The path to the expected audit results file', | |||
| 'save-assets-to': 'The path to save the assets to', | |||
There was a problem hiding this comment.
'save-assets-path'? description could be "Saves run assets to this path" or something to be clear that it's not saved usually
| @@ -89,6 +95,10 @@ function runLighthouse(url, configPath) { | |||
| process.exit(runResults.status); | |||
| } | |||
|
|
|||
| if (saveAssetsPath) { | |||
| return require(resolveLocalOrCwd(saveAssetsPath)); | |||
There was a problem hiding this comment.
This is confusing because of the output-path magic. Add a comment?
.travis.yml
Outdated
| - npm run lint | ||
| - npm run unit | ||
| - npm run closure | ||
| - npm run smoke |
There was a problem hiding this comment.
I vote for reverting where it's unchanged. We just used git blame on .travis.yml in filing #2048 :)
| - node_modules | ||
| - chrome-linux | ||
| - lighthouse-extension/node_modules | ||
| - lighthouse-viewer/node_modules |
There was a problem hiding this comment.
@brendankenny this I'm keeping because it's actually misaligned :P
patrickhulce
force-pushed
the
travis_artifacts
branch
from
e6036a0
to
e59288b
Compare
.travis.yml
Outdated
| - npm run lint | ||
| - npm run unit | ||
| - npm run closure | ||
| - npm run smoke |
There was a problem hiding this comment.
yah ++ on not adjusting whitespace in this PR.
(the last commit didn't revert it fwiw)
edit: all good!
| @@ -259,7 +270,7 @@ let passingCount = 0; | |||
| let failingCount = 0; | |||
| expectations.forEach(expected => { | |||
| console.log(`Checking '${expected.initialUrl}'...`); | |||
| const results = runLighthouse(expected.initialUrl, configPath); | |||
| const results = runLighthouse(expected.initialUrl, configPath, cli['save-assets-to']); | |||
There was a problem hiding this comment.
it should be both now that the bug is fixed but we were using the kebab case versions above
.travis.yml
Outdated
| - npm run coveralls | ||
| - npm run coveralls | ||
| after_failure: | ||
| - grep 'No screenshots' perf.json && travis-artifacts upload --path perf-0.trace.json |
There was a problem hiding this comment.
- smokehouse runs a bunch of sites, thus making a number of traces, right? why does it look like we're only uploading a single json file?
- i presume this isn't actually overwriting the one file in the bucket with this name? what does it look like in the bucket?
There was a problem hiding this comment.
@paulirish perf smokehouse run is the only one that actually exercises an audit that needs screenshots, travis-artifacts creates a directory for every job and sticks it in there
fo sho, send me your AWS emails or user IDs |
|
PTAL :) |
|
@brendankenny any more objections? |
|
|
||
| npm run -s smokehouse -- --config-path=$config --expectations-path=$expectations | ||
| if [[ "$CI" = true ]]; then | ||
| save_assets="--save-assets-path=perf.json" |
There was a problem hiding this comment.
can you add a comment here and in .travis.yml why this is being done. e.g. "Save assets so that traces of test failures can be examined" or something
* test: save perf trace to S3 on failure * test: fix tests and reduce noisiness * feedback * add comment
saves perf trace to an S3 bucket (GCS isn't supported by
travis-artifacts)