From 1c081c9ab8d5ffa610eb7854c75bc923461b5f6b Mon Sep 17 00:00:00 2001 From: Paul Irish Date: Mon, 10 Oct 2016 19:22:41 -0700 Subject: [PATCH 1/2] fix(cli): Don't report scores on nonscored items --- lighthouse-cli/printer.js | 3 ++- lighthouse-core/aggregator/aggregate.js | 16 ++++++++++------ 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/lighthouse-cli/printer.js b/lighthouse-cli/printer.js index 498bbf8e1e36..536ab51b1def 100644 --- a/lighthouse-cli/printer.js +++ b/lighthouse-cli/printer.js @@ -80,6 +80,7 @@ function formatScore(score, suffix) { if (typeof score !== 'number') { return `${purple}${score}${reset}`; } + let colorChoice = red; if (score > 45) { colorChoice = yellow; @@ -123,7 +124,7 @@ function createOutput(results, outputMode) { let score = (item.overall * 100).toFixed(0); if (item.name) { - output += `${bold}${item.name}${reset}: ${formatScore(Number(score), '%')}\n`; + output += `${bold}${item.name}${reset}: ${item.scored ? formatScore(score, '%') : ''}\n`; } item.subItems.forEach(subitem => { diff --git a/lighthouse-core/aggregator/aggregate.js b/lighthouse-core/aggregator/aggregate.js index 45536100b954..91a78fb13199 100644 --- a/lighthouse-core/aggregator/aggregate.js +++ b/lighthouse-core/aggregator/aggregate.js @@ -37,12 +37,11 @@ class Aggregate { */ static _getTotalWeight(expected) { const expectedNames = Object.keys(expected); - let weight = expectedNames.reduce((last, e) => last + expected[e].weight, 0); - if (weight === 0) { - weight = 1; + let totalWeight = expectedNames.reduce((last, e) => last + (expected[e].weight || 0), 0); + if (totalWeight === 0) { + totalWeight = 1; } - - return weight; + return totalWeight; } /** @@ -158,9 +157,10 @@ class Aggregate { Aggregate._remapResultsByName( Aggregate._filterResultsByAuditNames(results, item.audits) ); - const maxScore = Aggregate._getTotalWeight(item.audits); + const subItems = []; let overallScore = 0; + let maxScore = 0; // Step through each item in the expected results, and add them // to the overall score and add each to the subItems list. @@ -197,6 +197,10 @@ class Aggregate { e); }); + if (aggregationIsScored) { + maxScore = Aggregate._getTotalWeight(item.audits); + } + return { overall: (overallScore / maxScore), name: item.name, From 1082ed3d1d7a987e4de291deb3e120a01cf6fccd Mon Sep 17 00:00:00 2001 From: Paul Irish Date: Mon, 10 Oct 2016 19:59:54 -0700 Subject: [PATCH 2/2] add test for nonscored items on CLI printer --- lighthouse-cli/test/cli/printer-test.js | 5 +++ lighthouse-cli/test/fixtures/sample.json | 34 +++++++++++++++++++ lighthouse-core/aggregator/aggregate.js | 5 +-- .../test/aggregator/aggregate-test.js | 13 +------ 4 files changed, 41 insertions(+), 16 deletions(-) diff --git a/lighthouse-cli/test/cli/printer-test.js b/lighthouse-cli/test/cli/printer-test.js index e180c71d38d6..b0651de4e566 100644 --- a/lighthouse-cli/test/cli/printer-test.js +++ b/lighthouse-cli/test/cli/printer-test.js @@ -57,6 +57,11 @@ describe('Printer', () => { // Just check there's no HTML / JSON there. assert.throws(_ => JSON.parse(prettyOutput)); assert.equal(/ { diff --git a/lighthouse-cli/test/fixtures/sample.json b/lighthouse-cli/test/fixtures/sample.json index 20730e4640c5..d144b1bdbb05 100644 --- a/lighthouse-cli/test/fixtures/sample.json +++ b/lighthouse-cli/test/fixtures/sample.json @@ -60,6 +60,24 @@ "category": "JavaScript", "description": "Page contains some content when its scripts are not available" }, + "appcache-manifest": { + "score": true, + "displayValue": "", + "rawValue": true, + "name": "appcache-manifest", + "category": "Offline", + "description": "Site is not using Application Cache", + "helpText": "Application Cache has been deprecated by Service Workers. Consider implementing an offline solution using the Cache Storage API." + }, + "no-websql": { + "score": true, + "displayValue": "", + "rawValue": true, + "name": "no-websql", + "category": "Offline", + "description": "Site is not using WebSQL DB.", + "helpText": "Web SQL Database is deprecated. Consider implementing an offline solution using IndexedDB." + }, "first-meaningful-paint": { "score": 100, "displayValue": "615.8ms", @@ -610,6 +628,22 @@ ] } ] + }, + { + "name": "Do Better Web", + "description": "We've compiled some recommendations for modernizing your web app and avoiding performance pitfalls.", + "scored": false, + "categorizable": true, + "score": [ + { + "overall": null, + "name": "Using modern offline features", + "subItems": [ + "appcache-manifest", + "no-websql" + ] + } + ] } ] } diff --git a/lighthouse-core/aggregator/aggregate.js b/lighthouse-core/aggregator/aggregate.js index 91a78fb13199..145653d4b441 100644 --- a/lighthouse-core/aggregator/aggregate.js +++ b/lighthouse-core/aggregator/aggregate.js @@ -38,9 +38,6 @@ class Aggregate { static _getTotalWeight(expected) { const expectedNames = Object.keys(expected); let totalWeight = expectedNames.reduce((last, e) => last + (expected[e].weight || 0), 0); - if (totalWeight === 0) { - totalWeight = 1; - } return totalWeight; } @@ -160,7 +157,7 @@ class Aggregate { const subItems = []; let overallScore = 0; - let maxScore = 0; + let maxScore = 1; // Step through each item in the expected results, and add them // to the overall score and add each to the subItems list. diff --git a/lighthouse-core/test/aggregator/aggregate-test.js b/lighthouse-core/test/aggregator/aggregate-test.js index 853d7c1fdd51..35682eae05b5 100644 --- a/lighthouse-core/test/aggregator/aggregate-test.js +++ b/lighthouse-core/test/aggregator/aggregate-test.js @@ -54,18 +54,7 @@ describe('Aggregate', () => { const a = {}; const weight = Aggregate._getTotalWeight(a); - return assert.equal(weight, 1); - }); - - it('returns a weight of at least 1', () => { - const a = { - x: { - weight: 0 - } - }; - - const weight = Aggregate._getTotalWeight(a); - return assert.equal(weight, 1); + return assert.equal(weight, 0); }); it('generates the correct total weight', () => {