From 07bfe59818e54b29007364944208c0172a54dd26 Mon Sep 17 00:00:00 2001 From: Adam Raine Date: Wed, 6 Oct 2021 17:57:05 -0500 Subject: [PATCH 01/11] report(flow): refine snapshot and timespan performance --- flow-report/assets/styles.css | 2 +- flow-report/src/summary/category.tsx | 14 +++++- flow-report/test/summary/category-test.tsx | 39 +++++++++++---- .../renderer/performance-category-renderer.js | 42 ++++++++-------- report/renderer/util.js | 19 +++++-- .../performance-category-renderer-test.js | 12 +++++ report/test/renderer/util-test.js | 50 +++++++++++++++++-- 7 files changed, 139 insertions(+), 39 deletions(-) diff --git a/flow-report/assets/styles.css b/flow-report/assets/styles.css index bf628376a5bd..5eac8df8cf90 100644 --- a/flow-report/assets/styles.css +++ b/flow-report/assets/styles.css @@ -369,7 +369,7 @@ width: max-content; background-color: var(--report-background-color); border: 1px solid var(--color-gray-900); - border-radius: 5px; + border-radius: 3px; padding: var(--base-spacing); right: 0; box-shadow: 0px 4px 4px var(--summary-tooltip-box-shadow-color); diff --git a/flow-report/src/summary/category.tsx b/flow-report/src/summary/category.tsx index 99e7bf1b5563..88c1fb70d7c8 100644 --- a/flow-report/src/summary/category.tsx +++ b/flow-report/src/summary/category.tsx @@ -35,7 +35,12 @@ export const SummaryTooltip: FunctionComponent<{ gatherMode: LH.Result.GatherMode }> = ({category, gatherMode}) => { const strings = useUIStrings(); - const {numPassed, numAudits, totalWeight} = Util.calculateCategoryFraction(category); + const { + numPassed, + numAudits, + numInformative, + totalWeight, + } = Util.calculateCategoryFraction(category); const displayAsFraction = Util.shouldDisplayAsFraction(gatherMode); const rating = displayAsFraction ? @@ -66,6 +71,13 @@ export const SummaryTooltip: FunctionComponent<{
{`${numPassed} audits passed / ${numAudits} audits run`}
+ { + numInformative ? +
+ {`${numInformative} informative audits`} +
: + null + } ); }; diff --git a/flow-report/test/summary/category-test.tsx b/flow-report/test/summary/category-test.tsx index 6b8df3a42669..1ae78afc4053 100644 --- a/flow-report/test/summary/category-test.tsx +++ b/flow-report/test/summary/category-test.tsx @@ -30,9 +30,9 @@ describe('SummaryTooltip', () => { const category: any = { score: 1, auditRefs: [ - {result: {score: 1, scoreDisplayMode: 'binary'}, weight: 1}, - {result: {score: 1, scoreDisplayMode: 'binary'}, weight: 1}, - {result: {score: 0, scoreDisplayMode: 'binary'}, weight: 1}, + {result: {score: 1, scoreDisplayMode: 'binary'}, weight: 1, group: 'diagnostics'}, + {result: {score: 1, scoreDisplayMode: 'binary'}, weight: 1, group: 'diagnostics'}, + {result: {score: 0, scoreDisplayMode: 'binary'}, weight: 1, group: 'diagnostics'}, ], }; @@ -50,9 +50,9 @@ describe('SummaryTooltip', () => { const category: any = { score: 1, auditRefs: [ - {result: {score: 1, scoreDisplayMode: 'binary'}, weight: 0}, - {result: {score: 1, scoreDisplayMode: 'binary'}, weight: 0}, - {result: {score: 0, scoreDisplayMode: 'binary'}, weight: 0}, + {result: {score: 1, scoreDisplayMode: 'binary'}, weight: 0, group: 'diagnostics'}, + {result: {score: 1, scoreDisplayMode: 'binary'}, weight: 0, group: 'diagnostics'}, + {result: {score: 0, scoreDisplayMode: 'binary'}, weight: 0, group: 'diagnostics'}, ], }; @@ -70,9 +70,9 @@ describe('SummaryTooltip', () => { const category: any = { score: 1, auditRefs: [ - {result: {score: 1, scoreDisplayMode: 'binary'}, weight: 1}, - {result: {score: 1, scoreDisplayMode: 'binary'}, weight: 1}, - {result: {score: 0, scoreDisplayMode: 'binary'}, weight: 1}, + {result: {score: 1, scoreDisplayMode: 'binary'}, weight: 1, group: 'diagnostics'}, + {result: {score: 1, scoreDisplayMode: 'binary'}, weight: 1, group: 'diagnostics'}, + {result: {score: 0, scoreDisplayMode: 'binary'}, weight: 1, group: 'diagnostics'}, ], }; @@ -85,4 +85,25 @@ describe('SummaryTooltip', () => { expect(root.getByText('100')).toBeTruthy(); expect(root.getByText('2 audits passed / 3 audits run')).toBeTruthy(); }); + + it('renders informative audit count if any', async () => { + const category: any = { + score: 1, + auditRefs: [ + {result: {score: 1, scoreDisplayMode: 'binary'}, weight: 1, group: 'diagnostics'}, + {result: {score: 1, scoreDisplayMode: 'binary'}, weight: 1, group: 'diagnostics'}, + {result: {score: 0, scoreDisplayMode: 'informative'}, weight: 1, group: 'diagnostics'}, + ], + }; + + const root = render( + , + {wrapper} + ); + + expect(root.getByText('Good')).toBeTruthy(); + expect(root.getByText('100')).toBeTruthy(); + expect(root.getByText('2 audits passed / 2 audits run')).toBeTruthy(); + expect(root.getByText('1 informative audits')).toBeTruthy(); + }); }); diff --git a/report/renderer/performance-category-renderer.js b/report/renderer/performance-category-renderer.js index c0f1235aec3b..a0df6cb19b6e 100644 --- a/report/renderer/performance-category-renderer.js +++ b/report/renderer/performance-category-renderer.js @@ -172,33 +172,35 @@ export class PerformanceCategoryRenderer extends CategoryRenderer { } // Metrics. - const metricAuditsEl = this.renderAuditGroup(groups.metrics); + const metricAudits = category.auditRefs.filter(audit => audit.group === 'metrics'); + if (metricAudits.length) { + const metricAuditsEl = this.renderAuditGroup(groups.metrics); - // Metric descriptions toggle. - const toggleTmpl = this.dom.createComponent('metricsToggle'); - const _toggleEl = this.dom.find('.lh-metrics-toggle', toggleTmpl); - metricAuditsEl.append(..._toggleEl.childNodes); + // Metric descriptions toggle. + const toggleTmpl = this.dom.createComponent('metricsToggle'); + const _toggleEl = this.dom.find('.lh-metrics-toggle', toggleTmpl); + metricAuditsEl.append(..._toggleEl.childNodes); - const metricAudits = category.auditRefs.filter(audit => audit.group === 'metrics'); - const metricsBoxesEl = this.dom.createChildOf(metricAuditsEl, 'div', 'lh-metrics-container'); + const metricsBoxesEl = this.dom.createChildOf(metricAuditsEl, 'div', 'lh-metrics-container'); - metricAudits.forEach(item => { - metricsBoxesEl.appendChild(this._renderMetric(item)); - }); + metricAudits.forEach(item => { + metricsBoxesEl.appendChild(this._renderMetric(item)); + }); - const estValuesEl = this.dom.createChildOf(metricAuditsEl, 'div', 'lh-metrics__disclaimer'); - const disclaimerEl = this.dom.convertMarkdownLinkSnippets(strings.varianceDisclaimer); - estValuesEl.appendChild(disclaimerEl); + const estValuesEl = this.dom.createChildOf(metricAuditsEl, 'div', 'lh-metrics__disclaimer'); + const disclaimerEl = this.dom.convertMarkdownLinkSnippets(strings.varianceDisclaimer); + estValuesEl.appendChild(disclaimerEl); - // Add link to score calculator. - const calculatorLink = this.dom.createChildOf(estValuesEl, 'a', 'lh-calclink'); - calculatorLink.target = '_blank'; - calculatorLink.textContent = strings.calculatorLink; - this.dom.safelySetHref(calculatorLink, this._getScoringCalculatorHref(category.auditRefs)); + // Add link to score calculator. + const calculatorLink = this.dom.createChildOf(estValuesEl, 'a', 'lh-calclink'); + calculatorLink.target = '_blank'; + calculatorLink.textContent = strings.calculatorLink; + this.dom.safelySetHref(calculatorLink, this._getScoringCalculatorHref(category.auditRefs)); - metricAuditsEl.classList.add('lh-audit-group--metrics'); - element.appendChild(metricAuditsEl); + metricAuditsEl.classList.add('lh-audit-group--metrics'); + element.appendChild(metricAuditsEl); + } // Filmstrip const timelineEl = this.dom.createChildOf(element, 'div', 'lh-filmstrip-container'); diff --git a/report/renderer/util.js b/report/renderer/util.js index f4e09046db93..da4a79e6d288 100644 --- a/report/renderer/util.js +++ b/report/renderer/util.js @@ -518,15 +518,26 @@ export class Util { * @param {LH.ReportResult.Category} category */ static calculateCategoryFraction(category) { - const numAudits = category.auditRefs.length; - + let numAudits = 0; let numPassed = 0; + let numInformative = 0; let totalWeight = 0; for (const auditRef of category.auditRefs) { + const auditPassed = Util.showAsPassed(auditRef.result); + + // Don't count the audit if it's manual or isn't displayed. + if (!auditRef.group || auditRef.result.scoreDisplayMode === 'manual') { + continue; + } else if (auditRef.result.scoreDisplayMode === 'informative' && !auditPassed) { + ++numInformative; + continue; + } + + ++numAudits; totalWeight += auditRef.weight; - if (Util.showAsPassed(auditRef.result)) numPassed++; + if (auditPassed) numPassed++; } - return {numPassed, numAudits, totalWeight}; + return {numPassed, numAudits, numInformative, totalWeight}; } } diff --git a/report/test/renderer/performance-category-renderer-test.js b/report/test/renderer/performance-category-renderer-test.js index d356a6dfadfa..e04ad360f309 100644 --- a/report/test/renderer/performance-category-renderer-test.js +++ b/report/test/renderer/performance-category-renderer-test.js @@ -69,6 +69,18 @@ describe('PerfCategoryRenderer', () => { assert.equal(timelineElements.length + nontimelineElements.length, metricAudits.length); }); + it('does not render metrics section if no metric group audits', () => { + // Remove metrics from category + const newCategory = JSON.parse(JSON.stringify(category)); + newCategory.auditRefs = category.auditRefs.filter(audit => audit.group !== 'metrics'); + + const categoryDOM = renderer.render(newCategory, sampleResults.categoryGroups); + const sections = categoryDOM.querySelectorAll('.lh-category > .lh-audit-group'); + const metricSection = categoryDOM.querySelector('.lh-audit-group--metrics'); + assert.ok(!metricSection); + assert.equal(sections.length, 4); + }); + it('renders the metrics variance disclaimer as markdown', () => { const categoryDOM = renderer.render(category, sampleResults.categoryGroups); const disclaimerEl = diff --git a/report/test/renderer/util-test.js b/report/test/renderer/util-test.js index b55a4d99143d..ee30f50135fe 100644 --- a/report/test/renderer/util-test.js +++ b/report/test/renderer/util-test.js @@ -400,16 +400,58 @@ describe('util helpers', () => { it('returns passed audits and total audits', () => { const category = { auditRefs: [ - {weight: 3, result: {score: 1, scoreDisplayMode: 'binary'}}, - {weight: 2, result: {score: 1, scoreDisplayMode: 'binary'}}, - {weight: 0, result: {score: 1, scoreDisplayMode: 'binary'}}, - {weight: 1, result: {score: 0, scoreDisplayMode: 'binary'}}, + {weight: 3, result: {score: 1, scoreDisplayMode: 'binary'}, group: 'diagnostics'}, + {weight: 2, result: {score: 1, scoreDisplayMode: 'binary'}, group: 'diagnostics'}, + {weight: 0, result: {score: 1, scoreDisplayMode: 'binary'}, group: 'diagnostics'}, + {weight: 1, result: {score: 0, scoreDisplayMode: 'binary'}, group: 'diagnostics'}, ], }; const {numPassed, numAudits, totalWeight} = Util.calculateCategoryFraction(category); expect(numPassed).toEqual(3); expect(numAudits).toEqual(4); expect(totalWeight).toEqual(6); + const fraction = Util.calculateCategoryFraction(category); + expect(fraction).toEqual({ + numAudits: 4, + numPassed: 3, + numInformative: 0, + totalWeight: 6, + }); + }); + + it('ignores manual audits and audits with no group', () => { + const category = { + auditRefs: [ + {weight: 1, result: {score: 1, scoreDisplayMode: 'binary'}, group: 'diagnostics'}, + {weight: 1, result: {score: 1, scoreDisplayMode: 'binary'}}, + {weight: 1, result: {score: 0, scoreDisplayMode: 'manual'}, group: 'diagnostics'}, + ], + }; + const fraction = Util.calculateCategoryFraction(category); + expect(fraction).toEqual({ + numAudits: 1, + numPassed: 1, + numInformative: 0, + totalWeight: 1, + }); + }); + + it('tracks informative audits separately', () => { + const category = { + auditRefs: [ + {weight: 1, result: {score: 1, scoreDisplayMode: 'binary'}, group: 'diagnostics'}, + {weight: 1, result: {score: 1, scoreDisplayMode: 'binary'}, group: 'diagnostics'}, + {weight: 0, result: {score: 1, scoreDisplayMode: 'informative'}, group: 'diagnostics'}, + {weight: 1, result: {score: 0, scoreDisplayMode: 'informative'}, group: 'diagnostics'}, + ], + }; + const fraction = Util.calculateCategoryFraction(category); + expect(fraction).toEqual({ + numAudits: 2, + numPassed: 2, + numInformative: 2, + totalWeight: 2, + }); }); }); }); From df98a92b546f60545a7ee2c3bad218a486f9218e Mon Sep 17 00:00:00 2001 From: Adam Raine Date: Wed, 6 Oct 2021 18:19:34 -0500 Subject: [PATCH 02/11] fix --- report/renderer/util.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/report/renderer/util.js b/report/renderer/util.js index da4a79e6d288..69de4852ba47 100644 --- a/report/renderer/util.js +++ b/report/renderer/util.js @@ -528,8 +528,10 @@ export class Util { // Don't count the audit if it's manual or isn't displayed. if (!auditRef.group || auditRef.result.scoreDisplayMode === 'manual') { continue; - } else if (auditRef.result.scoreDisplayMode === 'informative' && !auditPassed) { - ++numInformative; + } else if (auditRef.result.scoreDisplayMode === 'informative') { + if (!auditPassed) { + ++numInformative; + } continue; } From ed72d907393f87774a6764afa7561e2bf232dc8b Mon Sep 17 00:00:00 2001 From: Adam Raine Date: Thu, 7 Oct 2021 12:35:25 -0500 Subject: [PATCH 03/11] todo --- flow-report/src/summary/category.tsx | 6 +++++- flow-report/src/summary/summary.tsx | 1 + 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/flow-report/src/summary/category.tsx b/flow-report/src/summary/category.tsx index 88c1fb70d7c8..9ef2f48adc0c 100644 --- a/flow-report/src/summary/category.tsx +++ b/flow-report/src/summary/category.tsx @@ -69,9 +69,13 @@ export const SummaryTooltip: FunctionComponent<{ }
- {`${numPassed} audits passed / ${numAudits} audits run`} + { + // TODO(FLOW-I18N): Placeholder format. + `${numPassed} audits passed / ${numAudits} audits run` + }
{ + // TODO(FLOW-I18N): Placeholder format. numInformative ?
{`${numInformative} informative audits`} diff --git a/flow-report/src/summary/summary.tsx b/flow-report/src/summary/summary.tsx index d890e723a041..82b783ff468b 100644 --- a/flow-report/src/summary/summary.tsx +++ b/flow-report/src/summary/summary.tsx @@ -127,6 +127,7 @@ export const SummaryHeader: FunctionComponent = () => { } } + // TODO(FLOW-I18N): Placeholder format. const subtitleCounts = []; if (numNavigation) subtitleCounts.push(`${numNavigation} navigation reports`); if (numTimespan) subtitleCounts.push(`${numTimespan} timespan reports`); From 4ff01a96081779409df523b0a887e8ab9a382eb0 Mon Sep 17 00:00:00 2001 From: Adam Raine Date: Thu, 7 Oct 2021 12:43:55 -0500 Subject: [PATCH 04/11] rn --- flow-report/src/summary/category.tsx | 6 +++--- report/renderer/category-renderer.js | 6 +++--- report/renderer/util.js | 6 +++--- report/test/renderer/util-test.js | 10 +++------- 4 files changed, 12 insertions(+), 16 deletions(-) diff --git a/flow-report/src/summary/category.tsx b/flow-report/src/summary/category.tsx index 9ef2f48adc0c..6d68a5a7400d 100644 --- a/flow-report/src/summary/category.tsx +++ b/flow-report/src/summary/category.tsx @@ -37,14 +37,14 @@ export const SummaryTooltip: FunctionComponent<{ const strings = useUIStrings(); const { numPassed, - numAudits, + numPassableAudits, numInformative, totalWeight, } = Util.calculateCategoryFraction(category); const displayAsFraction = Util.shouldDisplayAsFraction(gatherMode); const rating = displayAsFraction ? - Util.calculateRating(numPassed / numAudits) : + Util.calculateRating(numPassed / numPassableAudits) : Util.calculateRating(category.score); return ( @@ -71,7 +71,7 @@ export const SummaryTooltip: FunctionComponent<{
{ // TODO(FLOW-I18N): Placeholder format. - `${numPassed} audits passed / ${numAudits} audits run` + `${numPassed} audits passed / ${numPassableAudits} audits run` }
{ diff --git a/report/renderer/category-renderer.js b/report/renderer/category-renderer.js index c3ec8e3f0e83..32ec1226fc6c 100644 --- a/report/renderer/category-renderer.js +++ b/report/renderer/category-renderer.js @@ -375,12 +375,12 @@ export class CategoryRenderer { const wrapper = this.dom.find('a.lh-fraction__wrapper', tmpl); this.dom.safelySetHref(wrapper, `#${category.id}`); - const {numPassed, numAudits, totalWeight} = Util.calculateCategoryFraction(category); + const {numPassed, numPassableAudits, totalWeight} = Util.calculateCategoryFraction(category); - const fraction = numPassed / numAudits; + const fraction = numPassed / numPassableAudits; const content = this.dom.find('.lh-fraction__content', tmpl); const text = this.dom.createElement('span'); - text.textContent = `${numPassed}/${numAudits}`; + text.textContent = `${numPassed}/${numPassableAudits}`; content.appendChild(text); let rating = Util.calculateRating(fraction); diff --git a/report/renderer/util.js b/report/renderer/util.js index 69de4852ba47..75e2a76d4d90 100644 --- a/report/renderer/util.js +++ b/report/renderer/util.js @@ -518,7 +518,7 @@ export class Util { * @param {LH.ReportResult.Category} category */ static calculateCategoryFraction(category) { - let numAudits = 0; + let numPassableAudits = 0; let numPassed = 0; let numInformative = 0; let totalWeight = 0; @@ -535,11 +535,11 @@ export class Util { continue; } - ++numAudits; + ++numPassableAudits; totalWeight += auditRef.weight; if (auditPassed) numPassed++; } - return {numPassed, numAudits, numInformative, totalWeight}; + return {numPassed, numPassableAudits, numInformative, totalWeight}; } } diff --git a/report/test/renderer/util-test.js b/report/test/renderer/util-test.js index ee30f50135fe..0a6ff8caf5cb 100644 --- a/report/test/renderer/util-test.js +++ b/report/test/renderer/util-test.js @@ -406,13 +406,9 @@ describe('util helpers', () => { {weight: 1, result: {score: 0, scoreDisplayMode: 'binary'}, group: 'diagnostics'}, ], }; - const {numPassed, numAudits, totalWeight} = Util.calculateCategoryFraction(category); - expect(numPassed).toEqual(3); - expect(numAudits).toEqual(4); - expect(totalWeight).toEqual(6); const fraction = Util.calculateCategoryFraction(category); expect(fraction).toEqual({ - numAudits: 4, + numPassableAudits: 4, numPassed: 3, numInformative: 0, totalWeight: 6, @@ -429,7 +425,7 @@ describe('util helpers', () => { }; const fraction = Util.calculateCategoryFraction(category); expect(fraction).toEqual({ - numAudits: 1, + numPassableAudits: 1, numPassed: 1, numInformative: 0, totalWeight: 1, @@ -447,7 +443,7 @@ describe('util helpers', () => { }; const fraction = Util.calculateCategoryFraction(category); expect(fraction).toEqual({ - numAudits: 2, + numPassableAudits: 2, numPassed: 2, numInformative: 2, totalWeight: 2, From b8bc3a4e1fe1fb0f8bad26defe21dd4a19ca395d Mon Sep 17 00:00:00 2001 From: Adam Raine Date: Thu, 7 Oct 2021 12:46:37 -0500 Subject: [PATCH 05/11] cat renderer --- report/test/renderer/category-renderer-test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/report/test/renderer/category-renderer-test.js b/report/test/renderer/category-renderer-test.js index f655d570cf21..68246caeb2d9 100644 --- a/report/test/renderer/category-renderer-test.js +++ b/report/test/renderer/category-renderer-test.js @@ -297,7 +297,7 @@ describe('CategoryRenderer', () => { ); const gauge = categoryDOM.querySelector('.lh-fraction__content'); - assert.equal(gauge.textContent.trim(), '49/54', 'fraction is included'); + assert.equal(gauge.textContent.trim(), '39/44', 'fraction is included'); const score = categoryDOM.querySelector('.lh-category-header'); const title = score.querySelector('.lh-fraction__label'); From 448dfac735f9232f4f472e1c01c42e559f2abd1d Mon Sep 17 00:00:00 2001 From: Adam Raine Date: Thu, 7 Oct 2021 12:49:09 -0500 Subject: [PATCH 06/11] cjs --- lighthouse-core/util-commonjs.js | 21 +++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/lighthouse-core/util-commonjs.js b/lighthouse-core/util-commonjs.js index 4ce534006e24..b0bcba655d7d 100644 --- a/lighthouse-core/util-commonjs.js +++ b/lighthouse-core/util-commonjs.js @@ -521,15 +521,28 @@ class Util { * @param {LH.ReportResult.Category} category */ static calculateCategoryFraction(category) { - const numAudits = category.auditRefs.length; - + let numPassableAudits = 0; let numPassed = 0; + let numInformative = 0; let totalWeight = 0; for (const auditRef of category.auditRefs) { + const auditPassed = Util.showAsPassed(auditRef.result); + + // Don't count the audit if it's manual or isn't displayed. + if (!auditRef.group || auditRef.result.scoreDisplayMode === 'manual') { + continue; + } else if (auditRef.result.scoreDisplayMode === 'informative') { + if (!auditPassed) { + ++numInformative; + } + continue; + } + + ++numPassableAudits; totalWeight += auditRef.weight; - if (Util.showAsPassed(auditRef.result)) numPassed++; + if (auditPassed) numPassed++; } - return {numPassed, numAudits, totalWeight}; + return {numPassed, numPassableAudits, numInformative, totalWeight}; } } From 339fa13fab818dadcf4213e3fc7bdaf80cf4d9f1 Mon Sep 17 00:00:00 2001 From: Adam Raine Date: Thu, 7 Oct 2021 13:04:36 -0500 Subject: [PATCH 07/11] only performance --- lighthouse-core/util-commonjs.js | 3 ++- report/renderer/util.js | 3 ++- report/test/renderer/util-test.js | 23 ++++++++++++++++++++++- 3 files changed, 26 insertions(+), 3 deletions(-) diff --git a/lighthouse-core/util-commonjs.js b/lighthouse-core/util-commonjs.js index b0bcba655d7d..b05ffb960932 100644 --- a/lighthouse-core/util-commonjs.js +++ b/lighthouse-core/util-commonjs.js @@ -527,9 +527,10 @@ class Util { let totalWeight = 0; for (const auditRef of category.auditRefs) { const auditPassed = Util.showAsPassed(auditRef.result); + const notDisplayed = !auditRef.group && category.id === 'performance'; // Don't count the audit if it's manual or isn't displayed. - if (!auditRef.group || auditRef.result.scoreDisplayMode === 'manual') { + if (notDisplayed || auditRef.result.scoreDisplayMode === 'manual') { continue; } else if (auditRef.result.scoreDisplayMode === 'informative') { if (!auditPassed) { diff --git a/report/renderer/util.js b/report/renderer/util.js index 75e2a76d4d90..a8af082bba4b 100644 --- a/report/renderer/util.js +++ b/report/renderer/util.js @@ -524,9 +524,10 @@ export class Util { let totalWeight = 0; for (const auditRef of category.auditRefs) { const auditPassed = Util.showAsPassed(auditRef.result); + const notDisplayed = !auditRef.group && category.id === 'performance'; // Don't count the audit if it's manual or isn't displayed. - if (!auditRef.group || auditRef.result.scoreDisplayMode === 'manual') { + if (notDisplayed || auditRef.result.scoreDisplayMode === 'manual') { continue; } else if (auditRef.result.scoreDisplayMode === 'informative') { if (!auditPassed) { diff --git a/report/test/renderer/util-test.js b/report/test/renderer/util-test.js index 0a6ff8caf5cb..17efc5142050 100644 --- a/report/test/renderer/util-test.js +++ b/report/test/renderer/util-test.js @@ -399,6 +399,7 @@ describe('util helpers', () => { describe('#calculateCategoryFraction', () => { it('returns passed audits and total audits', () => { const category = { + id: 'performance', auditRefs: [ {weight: 3, result: {score: 1, scoreDisplayMode: 'binary'}, group: 'diagnostics'}, {weight: 2, result: {score: 1, scoreDisplayMode: 'binary'}, group: 'diagnostics'}, @@ -415,8 +416,9 @@ describe('util helpers', () => { }); }); - it('ignores manual audits and audits with no group', () => { + it('ignores manual audits and performance audits with no group', () => { const category = { + id: 'performance', auditRefs: [ {weight: 1, result: {score: 1, scoreDisplayMode: 'binary'}, group: 'diagnostics'}, {weight: 1, result: {score: 1, scoreDisplayMode: 'binary'}}, @@ -432,8 +434,27 @@ describe('util helpers', () => { }); }); + it('does not ignore audits with no group in non-performance category', () => { + const category = { + id: 'seo', + auditRefs: [ + {weight: 1, result: {score: 1, scoreDisplayMode: 'binary'}, group: 'diagnostics'}, + {weight: 1, result: {score: 1, scoreDisplayMode: 'binary'}}, + {weight: 1, result: {score: 0, scoreDisplayMode: 'manual'}, group: 'diagnostics'}, + ], + }; + const fraction = Util.calculateCategoryFraction(category); + expect(fraction).toEqual({ + numPassableAudits: 2, + numPassed: 2, + numInformative: 0, + totalWeight: 2, + }); + }); + it('tracks informative audits separately', () => { const category = { + id: 'performance', auditRefs: [ {weight: 1, result: {score: 1, scoreDisplayMode: 'binary'}, group: 'diagnostics'}, {weight: 1, result: {score: 1, scoreDisplayMode: 'binary'}, group: 'diagnostics'}, From 5d67b33b5e41ce17fc566093f2e59138b23151f8 Mon Sep 17 00:00:00 2001 From: Adam Raine Date: Thu, 7 Oct 2021 16:39:15 -0500 Subject: [PATCH 08/11] ignore n/a --- flow-report/src/summary/category.tsx | 2 +- report/renderer/util.js | 6 ++++-- report/test/renderer/category-renderer-test.js | 2 +- report/test/renderer/util-test.js | 3 ++- 4 files changed, 8 insertions(+), 5 deletions(-) diff --git a/flow-report/src/summary/category.tsx b/flow-report/src/summary/category.tsx index 6d68a5a7400d..27ffcc33c36f 100644 --- a/flow-report/src/summary/category.tsx +++ b/flow-report/src/summary/category.tsx @@ -71,7 +71,7 @@ export const SummaryTooltip: FunctionComponent<{
{ // TODO(FLOW-I18N): Placeholder format. - `${numPassed} audits passed / ${numPassableAudits} audits run` + `${numPassed} audits passed / ${numPassableAudits} passable audits` }
{ diff --git a/report/renderer/util.js b/report/renderer/util.js index a8af082bba4b..4934ebd713e3 100644 --- a/report/renderer/util.js +++ b/report/renderer/util.js @@ -526,8 +526,10 @@ export class Util { const auditPassed = Util.showAsPassed(auditRef.result); const notDisplayed = !auditRef.group && category.id === 'performance'; - // Don't count the audit if it's manual or isn't displayed. - if (notDisplayed || auditRef.result.scoreDisplayMode === 'manual') { + // Don't count the audit if it's manual, N/A, or isn't displayed. + if (notDisplayed + || auditRef.result.scoreDisplayMode === 'manual' + || auditRef.result.scoreDisplayMode === 'notApplicable') { continue; } else if (auditRef.result.scoreDisplayMode === 'informative') { if (!auditPassed) { diff --git a/report/test/renderer/category-renderer-test.js b/report/test/renderer/category-renderer-test.js index 68246caeb2d9..4b1a70975b11 100644 --- a/report/test/renderer/category-renderer-test.js +++ b/report/test/renderer/category-renderer-test.js @@ -297,7 +297,7 @@ describe('CategoryRenderer', () => { ); const gauge = categoryDOM.querySelector('.lh-fraction__content'); - assert.equal(gauge.textContent.trim(), '39/44', 'fraction is included'); + assert.equal(gauge.textContent.trim(), '13/18', 'fraction is included'); const score = categoryDOM.querySelector('.lh-category-header'); const title = score.querySelector('.lh-fraction__label'); diff --git a/report/test/renderer/util-test.js b/report/test/renderer/util-test.js index 17efc5142050..63640dce6b20 100644 --- a/report/test/renderer/util-test.js +++ b/report/test/renderer/util-test.js @@ -416,13 +416,14 @@ describe('util helpers', () => { }); }); - it('ignores manual audits and performance audits with no group', () => { + it('ignores manual audits, N/A audits, and performance audits with no group', () => { const category = { id: 'performance', auditRefs: [ {weight: 1, result: {score: 1, scoreDisplayMode: 'binary'}, group: 'diagnostics'}, {weight: 1, result: {score: 1, scoreDisplayMode: 'binary'}}, {weight: 1, result: {score: 0, scoreDisplayMode: 'manual'}, group: 'diagnostics'}, + {weight: 1, result: {score: 0, scoreDisplayMode: 'notApplicable'}, group: 'diagnostics'}, ], }; const fraction = Util.calculateCategoryFraction(category); From 696442bd7f4b76708a0b62252a6cbb2178bed0a6 Mon Sep 17 00:00:00 2001 From: Adam Raine Date: Thu, 7 Oct 2021 16:53:35 -0500 Subject: [PATCH 09/11] commonjs --- lighthouse-core/util-commonjs.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/lighthouse-core/util-commonjs.js b/lighthouse-core/util-commonjs.js index b05ffb960932..285b266535e6 100644 --- a/lighthouse-core/util-commonjs.js +++ b/lighthouse-core/util-commonjs.js @@ -529,8 +529,10 @@ class Util { const auditPassed = Util.showAsPassed(auditRef.result); const notDisplayed = !auditRef.group && category.id === 'performance'; - // Don't count the audit if it's manual or isn't displayed. - if (notDisplayed || auditRef.result.scoreDisplayMode === 'manual') { + // Don't count the audit if it's manual, N/A, or isn't displayed. + if (notDisplayed + || auditRef.result.scoreDisplayMode === 'manual' + || auditRef.result.scoreDisplayMode === 'notApplicable') { continue; } else if (auditRef.result.scoreDisplayMode === 'informative') { if (!auditPassed) { From 3d663991ee9bf271174bc0b6ac89be44d9ac5a6b Mon Sep 17 00:00:00 2001 From: Adam Raine Date: Thu, 7 Oct 2021 16:56:02 -0500 Subject: [PATCH 10/11] fix tests --- flow-report/test/summary/category-test.tsx | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/flow-report/test/summary/category-test.tsx b/flow-report/test/summary/category-test.tsx index 1ae78afc4053..ffe3b4bb9570 100644 --- a/flow-report/test/summary/category-test.tsx +++ b/flow-report/test/summary/category-test.tsx @@ -28,6 +28,7 @@ beforeEach(() => { describe('SummaryTooltip', () => { it('renders tooltip with rating', async () => { const category: any = { + id: 'performance', score: 1, auditRefs: [ {result: {score: 1, scoreDisplayMode: 'binary'}, weight: 1, group: 'diagnostics'}, @@ -43,11 +44,12 @@ describe('SummaryTooltip', () => { expect(root.getByText('Average')).toBeTruthy(); expect(() => root.getByText(/^[0-9]+$/)).toThrow(); - expect(root.getByText('2 audits passed / 3 audits run')).toBeTruthy(); + expect(root.getByText('2 audits passed / 3 passable audits')).toBeTruthy(); }); it('renders tooltip without rating', async () => { const category: any = { + id: 'performance', score: 1, auditRefs: [ {result: {score: 1, scoreDisplayMode: 'binary'}, weight: 0, group: 'diagnostics'}, @@ -63,11 +65,12 @@ describe('SummaryTooltip', () => { expect(() => root.getByText(/^(Average|Good|Poor)$/)).toThrow(); expect(() => root.getByText(/^[0-9]+$/)).toThrow(); - expect(root.getByText('2 audits passed / 3 audits run')).toBeTruthy(); + expect(root.getByText('2 audits passed / 3 passable audits')).toBeTruthy(); }); it('renders scored category tooltip with score', async () => { const category: any = { + id: 'performance', score: 1, auditRefs: [ {result: {score: 1, scoreDisplayMode: 'binary'}, weight: 1, group: 'diagnostics'}, @@ -83,11 +86,12 @@ describe('SummaryTooltip', () => { expect(root.getByText('Good')).toBeTruthy(); expect(root.getByText('100')).toBeTruthy(); - expect(root.getByText('2 audits passed / 3 audits run')).toBeTruthy(); + expect(root.getByText('2 audits passed / 3 passable audits')).toBeTruthy(); }); it('renders informative audit count if any', async () => { const category: any = { + id: 'performance', score: 1, auditRefs: [ {result: {score: 1, scoreDisplayMode: 'binary'}, weight: 1, group: 'diagnostics'}, @@ -103,7 +107,7 @@ describe('SummaryTooltip', () => { expect(root.getByText('Good')).toBeTruthy(); expect(root.getByText('100')).toBeTruthy(); - expect(root.getByText('2 audits passed / 2 audits run')).toBeTruthy(); + expect(root.getByText('2 audits passed / 2 passable audits')).toBeTruthy(); expect(root.getByText('1 informative audits')).toBeTruthy(); }); }); From a34283fc0e0ecc4212d55e43a5f2bd8da78888f5 Mon Sep 17 00:00:00 2001 From: Adam Raine Date: Fri, 8 Oct 2021 10:44:10 -0500 Subject: [PATCH 11/11] neat --- lighthouse-core/util-commonjs.js | 6 +++--- report/renderer/util.js | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/lighthouse-core/util-commonjs.js b/lighthouse-core/util-commonjs.js index 285b266535e6..1859136ba834 100644 --- a/lighthouse-core/util-commonjs.js +++ b/lighthouse-core/util-commonjs.js @@ -530,9 +530,9 @@ class Util { const notDisplayed = !auditRef.group && category.id === 'performance'; // Don't count the audit if it's manual, N/A, or isn't displayed. - if (notDisplayed - || auditRef.result.scoreDisplayMode === 'manual' - || auditRef.result.scoreDisplayMode === 'notApplicable') { + if (notDisplayed || + auditRef.result.scoreDisplayMode === 'manual' || + auditRef.result.scoreDisplayMode === 'notApplicable') { continue; } else if (auditRef.result.scoreDisplayMode === 'informative') { if (!auditPassed) { diff --git a/report/renderer/util.js b/report/renderer/util.js index 4934ebd713e3..488911b3d283 100644 --- a/report/renderer/util.js +++ b/report/renderer/util.js @@ -527,9 +527,9 @@ export class Util { const notDisplayed = !auditRef.group && category.id === 'performance'; // Don't count the audit if it's manual, N/A, or isn't displayed. - if (notDisplayed - || auditRef.result.scoreDisplayMode === 'manual' - || auditRef.result.scoreDisplayMode === 'notApplicable') { + if (notDisplayed || + auditRef.result.scoreDisplayMode === 'manual' || + auditRef.result.scoreDisplayMode === 'notApplicable') { continue; } else if (auditRef.result.scoreDisplayMode === 'informative') { if (!auditPassed) {