Skip to content

always return formatted results from runner#636

Merged
paulirish merged 2 commits intomasterfrom
noaggs
Sep 9, 2016
Merged

always return formatted results from runner#636
paulirish merged 2 commits intomasterfrom
noaggs

Conversation

@brendankenny
Copy link
Copy Markdown
Contributor

@brendankenny brendankenny commented Sep 8, 2016

Besides just being more consistent, this also allows printed --output json results of the raw audit results, even if no aggregations are given in the config.

Also a good time to delete lighthouse-core/aggregations/index.js since that was an entire file for a map statement :)

this also allows printed json results of the raw audits, even if no aggregations are given
// JSON report.
if (outputMode === 'json') {
return JSON.stringify({audits: results.audits, aggregations: results.aggregations}, null, 2);
return JSON.stringify(results, null, 2);
Copy link
Copy Markdown
Contributor Author

@brendankenny brendankenny Sep 8, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

json print full results, including url and initialUrl

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm on that.

can you confirm we have test coverage for the structure of the JSON output?
does the coverage extend to include the CLI/printer code?

@paulirish
Copy link
Copy Markdown
Member

seems good. In the case of no aggregations, we were just returning nothing?

@brendankenny
Copy link
Copy Markdown
Contributor Author

In the case of no aggregations, we were just returning nothing?

the array of audit results were being returned directly, and the json printer was printing {audits: results.audits, aggregations: results.aggregations}, so it was printing just an empty {}

@paulirish
Copy link
Copy Markdown
Member

lgtm pending the question about test coverage.

@brendankenny
Copy link
Copy Markdown
Contributor Author

sweet json output test added, fully end-to-end (of module, not cli)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants