Repository: GoogleChrome/lighthouse
Pull request #6980 (2019-01-10 20:58:20)

core(tsc): add type checking to pwmetrics-events
My brain was tired yesterday afternoon so I did some non-essential zen type checking on one of the few remaining `// @ts-nocheck` files (only four more after this :).

This file is only ever called from `asset-saver.js` and the interface remains unchanged. Removed some of the defensiveness that was needed before #5101 made the `metrics.js` output flat and more dependable (that's what the TODO was referencing).


Patch:
diff --git a/lighthouse-core/lib/traces/pwmetrics-events.js b/lighthouse-core/lib/traces/pwmetrics-events.js
index 3fc0aa9fa..33fccecc6 100644
--- a/lighthouse-core/lib/traces/pwmetrics-events.js
+++ b/lighthouse-core/lib/traces/pwmetrics-events.js
@@ -3,24 +3,28 @@
  * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. You may obtain a copy of the License at http://www.apache.org/licenses/LICENSE-2.0
  * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License.
  */
-// @ts-nocheck
 'use strict';
 
 const log = require('lighthouse-logger');
 
-// TODO: rework this file to not need this function
-// see https://github.com/GoogleChrome/lighthouse/pull/5101/files#r186168840
-function findValueInMetricsAuditFn(metricName) {
-  return auditResults => {
-    const metricsAudit = auditResults.metrics;
-    if (!metricsAudit || !metricsAudit.details || !metricsAudit.details.items) return;
+/** @typedef {import('../../audits/metrics.js').UberMetricsItem} UberMetricsItem */
 
-    const values = metricsAudit.details.items[0];
-    return values && values[metricName];
-  };
+/**
+ * @param {LH.Audit.Results} auditResults
+ * @return {UberMetricsItem|undefined}
+ */
+function getUberMetrics(auditResults) {
+  const metricsAudit = auditResults.metrics;
+  if (!metricsAudit || !metricsAudit.details || !metricsAudit.details.items) return;
+
+  return metricsAudit.details.items[0];
 }
 
 class Metrics {
+  /**
+   * @param {Array<LH.TraceEvent>} traceEvents
+   * @param {LH.Audit.Results} auditResults
+   */
   constructor(traceEvents, auditResults) {
     this._traceEvents = traceEvents;
     this._auditResults = auditResults;
@@ -28,109 +32,107 @@ class Metrics {
 
   /**
    * Returns simplified representation of all metrics
-   * @return {!Array<{getTs: Function, id: string, name: string}>} metrics to consider
+   * @return {Array<{id: string, name: string, tsKey: keyof UberMetricsItem}>} metrics to consider
    */
   static get metricsDefinitions() {
     return [
       {
         name: 'Navigation Start',
         id: 'navstart',
-        getTs: findValueInMetricsAuditFn('observedNavigationStartTs'),
-        getTiming: findValueInMetricsAuditFn('observedNavigationStart'),
+        tsKey: 'observedNavigationStartTs',
       },
       {
         name: 'First Contentful Paint',
         id: 'ttfcp',
-        getTs: findValueInMetricsAuditFn('observedFirstContentfulPaintTs'),
-        getTiming: findValueInMetricsAuditFn('observedFirstContentfulPaint'),
+        tsKey: 'observedFirstContentfulPaintTs',
       },
       {
         name: 'First Meaningful Paint',
         id: 'ttfmp',
-        getTs: findValueInMetricsAuditFn('observedFirstMeaningfulPaintTs'),
-        getTiming: findValueInMetricsAuditFn('observedFirstMeaningfulPaint'),
+        tsKey: 'observedFirstMeaningfulPaintTs',
       },
       {
         name: 'Speed Index',
         id: 'si',
-        getTs: findValueInMetricsAuditFn('observedSpeedIndexTs'),
-        getTiming: findValueInMetricsAuditFn('observedSpeedIndex'),
+        tsKey: 'observedSpeedIndexTs',
       },
       {
         name: 'First Visual Change',
         id: 'fv',
-        getTs: findValueInMetricsAuditFn('observedFirstVisualChangeTs'),
-        getTiming: findValueInMetricsAuditFn('observedFirstVisualChange'),
+        tsKey: 'observedFirstVisualChangeTs',
       },
       {
         name: 'Visually Complete 100%',
         id: 'vc100',
-        getTs: findValueInMetricsAuditFn('observedLastVisualChangeTs'),
-        getTiming: findValueInMetricsAuditFn('observedLastVisualChange'),
+        tsKey: 'observedLastVisualChangeTs',
       },
       {
         name: 'First CPU Idle',
         id: 'ttfi',
-        getTs: findValueInMetricsAuditFn('firstCPUIdleTs'),
-        getTiming: findValueInMetricsAuditFn('firstCPUIdle'),
+        tsKey: 'firstCPUIdleTs',
       },
       {
         name: 'Interactive',
         id: 'tti',
-        getTs: findValueInMetricsAuditFn('interactiveTs'),
-        getTiming: findValueInMetricsAuditFn('interactive'),
+        tsKey: 'interactiveTs',
       },
       {
         name: 'End of Trace',
         id: 'eot',
-        getTs: findValueInMetricsAuditFn('observedTraceEndTs'),
-        getTiming: findValueInMetricsAuditFn('observedTraceEnd'),
+        tsKey: 'observedTraceEndTs',
       },
       {
         name: 'On Load',
         id: 'onload',
-        getTs: findValueInMetricsAuditFn('observedLoadTs'),
-        getTiming: findValueInMetricsAuditFn('observedLoad'),
+        tsKey: 'observedLoadTs',
       },
       {
         name: 'DOM Content Loaded',
         id: 'dcl',
-        getTs: findValueInMetricsAuditFn('observedDomContentLoadedTs'),
-        getTiming: findValueInMetricsAuditFn('observedDomContentLoaded'),
+        tsKey: 'observedDomContentLoadedTs',
       },
     ];
   }
 
   /**
    * Returns simplified representation of all metrics' timestamps from monotonic clock
-   * @return {!Array<{ts: number, id: string, name: string}>} metrics to consider
+   * @return {Array<{ts: number, id: string, name: string}>} metrics to consider
    */
   gatherMetrics() {
-    const metricDfns = Metrics.metricsDefinitions;
+    const uberMetrics = getUberMetrics(this._auditResults);
+    if (!uberMetrics) {
+      return [];
+    }
+
+    /** @type {Array<{ts: number, id: string, name: string}>} */
     const resolvedMetrics = [];
-    metricDfns.forEach(metric => {
-      // try/catch in case auditResults is missing a particular audit result
-      try {
-        resolvedMetrics.push({
-          id: metric.id,
-          name: metric.name,
-          ts: metric.getTs(this._auditResults),
-        });
-      } catch (e) {
-        log.error('pwmetrics-events', `${metric.name} timestamp not found: ${e.message}`);
+    Metrics.metricsDefinitions.forEach(metric => {
+      // Skip if auditResults is missing a particular audit result
+      const ts = uberMetrics[metric.tsKey];
+      if (ts === undefined) {
+        log.error('pwmetrics-events', `${metric.name} timestamp not found`);
+        return;
       }
+
+      resolvedMetrics.push({
+        id: metric.id,
+        name: metric.name,
+        ts,
+      });
     });
+
     return resolvedMetrics;
   }
 
   /**
    * Get the full trace event for our navigationStart
-   * @param {!Array<{ts: number, id: string, name: string}>} metrics
+   * @param {Array<{ts: number, id: string, name: string}>} metrics
+   * @return {LH.TraceEvent|undefined}
    */
-  identifyNavigationStartEvt(metrics) {
+  getNavigationStartEvt(metrics) {
     const navStartMetric = metrics.find(e => e.id === 'navstart');
     if (!navStartMetric) return;
-    this._navigationStartEvt = this._traceEvents.find(
+    return this._traceEvents.find(
       e => e.name === 'navigationStart' && e.ts === navStartMetric.ts
     );
   }
@@ -140,13 +142,14 @@ class Metrics {
    *     { "pid": 89922,"tid":1295,"ts":77176783452,"ph":"b","cat":"blink.user_timing","name":"innermeasure","args":{},"tts":1257886,"id":"0xe66c67"}
    *     { "pid": 89922,"tid":1295,"ts":77176882592,"ph":"e","cat":"blink.user_timing","name":"innermeasure","args":{},"tts":1257898,"id":"0xe66c67"}
    * @param {{ts: number, id: string, name: string}} metric
-   * @return {!Array} Pair of trace events (start/end)
+   * @param {LH.TraceEvent} navigationStartEvt
+   * @return {Array<LH.TraceEvent>} Pair of trace events (start/end)
    */
-  synthesizeEventPair(metric) {
+  synthesizeEventPair(metric, navigationStartEvt) {
     // We'll masquerade our fake events to look mostly like navigationStart
     const eventBase = {
-      pid: this._navigationStartEvt.pid,
-      tid: this._navigationStartEvt.tid,
+      pid: navigationStartEvt.pid,
+      tid: navigationStartEvt.tid,
       cat: 'blink.user_timing',
       name: metric.name,
       args: {},
@@ -154,33 +157,34 @@ class Metrics {
       id: `0x${((Math.random() * 1000000) | 0).toString(16)}`,
     };
     const fakeMeasureStartEvent = Object.assign({}, eventBase, {
-      ts: this._navigationStartEvt.ts,
+      ts: navigationStartEvt.ts,
       ph: 'b',
     });
     const fakeMeasureEndEvent = Object.assign({}, eventBase, {
       ts: metric.ts,
       ph: 'e',
     });
-    return [fakeMeasureStartEvent, fakeMeasureEndEvent];
+    return /** @type {Array<LH.TraceEvent>} */ ([fakeMeasureStartEvent, fakeMeasureEndEvent]);
   }
 
   /**
-   * @returns {Array<LH.TraceEvent>} User timing raw trace event pairs
+   * @return {Array<LH.TraceEvent>} User timing raw trace event pairs
    */
   generateFakeEvents() {
-    const fakeEvents = [];
     const metrics = this.gatherMetrics();
     if (metrics.length === 0) {
       log.error('metrics-events', 'Metrics collection had errors, not synthetizing trace events');
       return [];
     }
 
-    this.identifyNavigationStartEvt(metrics);
-    if (!this._navigationStartEvt) {
+    const navigationStartEvt = this.getNavigationStartEvt(metrics);
+    if (!navigationStartEvt) {
       log.error('pwmetrics-events', 'Reference navigationStart not found');
       return [];
     }
 
+    /** @type {Array<LH.TraceEvent>} */
+    const fakeEvents = [];
     metrics.forEach(metric => {
       if (metric.id === 'navstart') {
         return;
@@ -190,7 +194,7 @@ class Metrics {
         return;
       }
       log.verbose('pwmetrics-events', `Sythesizing trace events for ${metric.name}`);
-      fakeEvents.push(...this.synthesizeEventPair(metric));
+      fakeEvents.push(...this.synthesizeEventPair(metric, navigationStartEvt));
     });
     return fakeEvents;
   }
diff --git a/lighthouse-core/report/html/renderer/category-renderer.js b/lighthouse-core/report/html/renderer/category-renderer.js
index 25976cce4..0fa8be23b 100644
--- a/lighthouse-core/report/html/renderer/category-renderer.js
+++ b/lighthouse-core/report/html/renderer/category-renderer.js
@@ -43,23 +43,11 @@ class CategoryRenderer {
   /**
    * Display info per top-level clump. Define on class to avoid race with Util init.
    */
-  get _clumpDisplayInfo() {
+  get _clumpTitles() {
     return {
-      failed: {
-        className: 'lh-clump--failed',
-      },
-      manual: {
-        title: Util.UIStrings.manualAuditsGroupTitle,
-        className: 'lh-clump--manual',
-      },
-      passed: {
-        title: Util.UIStrings.passedAuditsGroupTitle,
-        className: 'lh-clump--passed',
-      },
-      notApplicable: {
-        title: Util.UIStrings.notApplicableAuditsGroupTitle,
-        className: 'lh-clump--notapplicable',
-      },
+      manual: Util.UIStrings.manualAuditsGroupTitle,
+      passed: Util.UIStrings.passedAuditsGroupTitle,
+      notApplicable: Util.UIStrings.notApplicableAuditsGroupTitle,
     };
   }
 
@@ -183,20 +171,13 @@ class CategoryRenderer {
    * Renders the group container for a group of audits. Individual audit elements can be added
    * directly to the returned element.
    * @param {LH.Result.ReportGroup} group
-   * @param {{expandable: boolean, itemCount?: number}} opts
    * @return {Element}
    */
-  renderAuditGroup(group, opts) {
-    const expandable = opts.expandable;
-    const groupEl = this.dom.createElement(expandable ? 'details' : 'div', 'lh-audit-group');
-    const summaryEl = this.dom.createChildOf(groupEl, expandable ? 'summary' : 'div');
+  renderAuditGroup(group) {
+    const groupEl = this.dom.createElement('div', 'lh-audit-group');
+    const summaryEl = this.dom.createChildOf(groupEl, 'div');
     const summaryInnerEl = this.dom.createChildOf(summaryEl, 'div', 'lh-audit-group__summary');
     const headerEl = this.dom.createChildOf(summaryInnerEl, 'div', 'lh-audit-group__header');
-    const itemCountEl = this.dom.createChildOf(summaryInnerEl, 'div', 'lh-audit-group__itemcount');
-    if (expandable) {
-      const chevronEl = summaryInnerEl.appendChild(this._createChevron());
-      chevronEl.title = Util.UIStrings.auditGroupExpandTooltip;
-    }
 
     if (group.description) {
       const auditGroupDescription = this.dom.createElement('div', 'lh-audit-group__description');
@@ -205,10 +186,6 @@ class CategoryRenderer {
     }
     headerEl.textContent = group.title;
 
-    if (opts.itemCount) {
-      // TODO(i18n): support multiple locales here
-      itemCountEl.textContent = `${opts.itemCount} audits`;
-    }
     return groupEl;
   }
 
@@ -217,10 +194,9 @@ class CategoryRenderer {
    * array of audit and audit-group elements.
    * @param {Array<LH.ReportResult.AuditRef>} auditRefs
    * @param {Object<string, LH.Result.ReportGroup>} groupDefinitions
-   * @param {{expandable: boolean}} opts
    * @return {Array<Element>}
    */
-  _renderGroupedAudits(auditRefs, groupDefinitions, opts) {
+  _renderGroupedAudits(auditRefs, groupDefinitions) {
     // Audits grouped by their group (or under notAGroup).
     /** @type {Map<string, Array<LH.ReportResult.AuditRef>>} */
     const grouped = new Map();
@@ -252,7 +228,7 @@ class CategoryRenderer {
 
       // Push grouped audits as a group.
       const groupDef = groupDefinitions[groupId];
-      const auditGroupElem = this.renderAuditGroup(groupDef, opts);
+      const auditGroupElem = this.renderAuditGroup(groupDef);
       for (const auditRef of groupAuditRefs) {
         auditGroupElem.appendChild(this.renderAudit(auditRef, index++));
       }
@@ -272,52 +248,46 @@ class CategoryRenderer {
    */
   renderUnexpandableClump(auditRefs, groupDefinitions) {
     const clumpElement = this.dom.createElement('div');
-    const elements = this._renderGroupedAudits(auditRefs, groupDefinitions, {expandable: false});
+    const elements = this._renderGroupedAudits(auditRefs, groupDefinitions);
     elements.forEach(elem => clumpElement.appendChild(elem));
     return clumpElement;
   }
 
   /**
-   * Renders a clump (a grouping of groups), under a status of failed, manual,
-   * passed, or notApplicable. The result ends up something like:
-   *
-   * clump (e.g. 'failed')
-   *   ├── audit 1 (w/o group)
-   *   ├── audit 2 (w/o group)
-   *   ├── audit group
-   *   |  ├── audit 3
-   *   |  └── audit 4
-   *   └── audit group
-   *      ├── audit 5
-   *      └── audit 6
-   * clump (e.g. 'manual')
-   *   ├── …
-   *   ⋮
-   * @param {TopLevelClumpId} clumpId
-   * @param {{auditRefs: Array<LH.ReportResult.AuditRef>, groupDefinitions: Object<string, LH.Result.ReportGroup>, description?: string}} clumpOpts
+   * Take a set of audits and render in a top-level, expandable clump that starts
+   * in a collapsed state.
+   * @param {Exclude<TopLevelClumpId, 'failed'>} clumpId
+   * @param {{auditRefs: Array<LH.ReportResult.AuditRef>, description?: string}} clumpOpts
    * @return {Element}
    */
-  renderClump(clumpId, {auditRefs, groupDefinitions, description}) {
-    if (clumpId === 'failed') {
-      // Failed audit clump is always expanded and not nested in an lh-audit-group.
-      const failedElem = this.renderUnexpandableClump(auditRefs, groupDefinitions);
-      failedElem.classList.add('lh-clump', this._clumpDisplayInfo.failed.className);
-      return failedElem;
+  renderClump(clumpId, {auditRefs, description}) {
+    const clumpTmpl = this.dom.cloneTemplate('#tmpl-lh-clump', this.templateContext);
+    const clumpElement = this.dom.find('.lh-clump', clumpTmpl);
+
+    const summaryInnerEl = this.dom.find('.lh-audit-group__summary', clumpElement);
+    const chevronEl = summaryInnerEl.appendChild(this._createChevron());
+    chevronEl.title = Util.UIStrings.auditGroupExpandTooltip;
+
+    const headerEl = this.dom.find('.lh-audit-group__header', clumpElement);
+    const title = this._clumpTitles[clumpId];
+    headerEl.textContent = title;
+    if (description) {
+      const markdownDescriptionEl = this.dom.convertMarkdownLinkSnippets(description);
+      const auditGroupDescription = this.dom.createElement('div', 'lh-audit-group__description');
+      auditGroupDescription.appendChild(markdownDescriptionEl);
+      clumpElement.appendChild(auditGroupDescription);
     }
 
-    const expandable = true;
-    const elements = this._renderGroupedAudits(auditRefs, groupDefinitions, {expandable});
-
-    const clumpInfo = this._clumpDisplayInfo[clumpId];
-    // TODO: renderAuditGroup shouldn't be used to render a clump (since it *contains* audit groups).
-    const groupDef = {title: clumpInfo.title, description};
-    const opts = {expandable, itemCount: auditRefs.length};
-    const clumpElem = this.renderAuditGroup(groupDef, opts);
-    clumpElem.classList.add('lh-clump', clumpInfo.className);
+    const itemCountEl = this.dom.find('.lh-audit-group__itemcount', clumpElement);
+    // TODO(i18n): support multiple locales here
+    itemCountEl.textContent = `${auditRefs.length} audits`;
 
-    elements.forEach(elem => clumpElem.appendChild(elem));
+    // Add all audit results to the clump.
+    const auditElements = auditRefs.map(this.renderAudit.bind(this));
+    clumpElement.append(...auditElements);
 
-    return clumpElem;
+    clumpElement.classList.add(`lh-clump--${clumpId.toLowerCase()}`);
+    return clumpElement;
   }
 
   /**
@@ -382,6 +352,23 @@ class CategoryRenderer {
   }
 
   /**
+   * Renders a set of top level sections (clumps), under a status of failed, manual,
+   * passed, or notApplicable. The result ends up something like:
+   *
+   * failed clump
+   *   ├── audit 1 (w/o group)
+   *   ├── audit 2 (w/o group)
+   *   ├── audit group
+   *   |  ├── audit 3
+   *   |  └── audit 4
+   *   └── audit group
+   *      ├── audit 5
+   *      └── audit 6
+   * other clump (e.g. 'manual')
+   *   ├── audit 1
+   *   ├── audit 2
+   *   ├── …
+   *   ⋮
    * @param {LH.ReportResult.Category} category
    * @param {Object<string, LH.Result.ReportGroup>} [groupDefinitions]
    * @return {Element}
@@ -408,12 +395,18 @@ class CategoryRenderer {
     }
 
     // Render each clump.
-    for (const [clumpId, clumpRefs] of clumps) {
-      if (clumpRefs.length === 0) continue;
+    for (const [clumpId, auditRefs] of clumps) {
+      if (auditRefs.length === 0) continue;
+
+      if (clumpId === 'failed') {
+        const clumpElem = this.renderUnexpandableClump(auditRefs, groupDefinitions);
+        clumpElem.classList.add(`lh-clump--failed`);
+        element.appendChild(clumpElem);
+        continue;
+      }
 
       const description = clumpId === 'manual' ? category.manualDescription : undefined;
-      const clumpElem = this.renderClump(clumpId, {auditRefs: clumpRefs, groupDefinitions,
-        description});
+      const clumpElem = this.renderClump(clumpId, {auditRefs, description});
       element.appendChild(clumpElem);
     }
 
diff --git a/lighthouse-core/report/html/renderer/performance-category-renderer.js b/lighthouse-core/report/html/renderer/performance-category-renderer.js
index 252b66701..870bdfc4e 100644
--- a/lighthouse-core/report/html/renderer/performance-category-renderer.js
+++ b/lighthouse-core/report/html/renderer/performance-category-renderer.js
@@ -129,7 +129,7 @@ class PerformanceCategoryRenderer extends CategoryRenderer {
 
     // Metrics
     const metricAudits = category.auditRefs.filter(audit => audit.group === 'metrics');
-    const metricAuditsEl = this.renderAuditGroup(groups.metrics, {expandable: false});
+    const metricAuditsEl = this.renderAuditGroup(groups.metrics);
 
     const keyMetrics = metricAudits.filter(a => a.weight >= 3);
     const otherMetrics = metricAudits.filter(a => a.weight < 3);
@@ -176,7 +176,7 @@ class PerformanceCategoryRenderer extends CategoryRenderer {
       const wastedMsValues = opportunityAudits.map(audit => this._getWastedMs(audit));
       const maxWaste = Math.max(...wastedMsValues);
       const scale = Math.max(Math.ceil(maxWaste / 1000) * 1000, minimumScale);
-      const groupEl = this.renderAuditGroup(groups['load-opportunities'], {expandable: false});
+      const groupEl = this.renderAuditGroup(groups['load-opportunities']);
       const tmpl = this.dom.cloneTemplate('#tmpl-lh-opportunity-header', this.templateContext);
 
       this.dom.find('.lh-load-opportunity__col--one', tmpl).textContent =
@@ -202,7 +202,7 @@ class PerformanceCategoryRenderer extends CategoryRenderer {
         });
 
     if (diagnosticAudits.length) {
-      const groupEl = this.renderAuditGroup(groups['diagnostics'], {expandable: false});
+      const groupEl = this.renderAuditGroup(groups['diagnostics']);
       diagnosticAudits.forEach((item, i) => groupEl.appendChild(this.renderAudit(item, i)));
       groupEl.classList.add('lh-audit-group--diagnostics');
       element.appendChild(groupEl);
diff --git a/lighthouse-core/report/html/renderer/pwa-category-renderer.js b/lighthouse-core/report/html/renderer/pwa-category-renderer.js
index 690e8da89..7e07206f3 100644
--- a/lighthouse-core/report/html/renderer/pwa-category-renderer.js
+++ b/lighthouse-core/report/html/renderer/pwa-category-renderer.js
@@ -40,7 +40,7 @@ class PwaCategoryRenderer extends CategoryRenderer {
     // Manual audits are still in a manual clump.
     const manualAuditRefs = auditRefs.filter(ref => ref.result.scoreDisplayMode === 'manual');
     const manualElem = this.renderClump('manual',
-      {auditRefs: manualAuditRefs, groupDefinitions, description: category.manualDescription});
+      {auditRefs: manualAuditRefs, description: category.manualDescription});
     categoryElem.appendChild(manualElem);
 
     return categoryElem;
diff --git a/lighthouse-core/report/html/templates.html b/lighthouse-core/report/html/templates.html
index 0971ccd1f..b7ac00986 100644
--- a/lighthouse-core/report/html/templates.html
+++ b/lighthouse-core/report/html/templates.html
@@ -52,6 +52,20 @@ limitations under the License.
   </div>
 </template>
 
+<!-- Lighthouse clump -->
+<template id="tmpl-lh-clump">
+  <!-- TODO: group classes shouldn't be reused for clumps. -->
+  <details class="lh-clump lh-audit-group">
+    <summary>
+      <div class="lh-audit-group__summary">
+        <div class="lh-audit-group__header"></div>
+        <div class="lh-audit-group__itemcount"></div>
+        <div class=""></div>
+      </div>
+    </summary>
+  </details>
+</template>
+
 <!-- Lighthouse audit -->
 <template id="tmpl-lh-audit">
   <div class="lh-audit">


Test patch:
diff --git a/lighthouse-core/test/report/html/renderer/category-renderer-test.js b/lighthouse-core/test/report/html/renderer/category-renderer-test.js
index 8be1099a4..bbb554754 100644
--- a/lighthouse-core/test/report/html/renderer/category-renderer-test.js
+++ b/lighthouse-core/test/report/html/renderer/category-renderer-test.js
@@ -227,14 +227,16 @@ describe('CategoryRenderer', () => {
       assert.equal(failedAuditGroups.length, failedAuditTags.size);
     });
 
-    it('renders the passed audits grouped by group', () => {
+    it('renders the passed audits ungrouped', () => {
       const categoryDOM = renderer.render(category, sampleResults.categoryGroups);
       const passedAudits = category.auditRefs.filter(audit =>
           audit.result.scoreDisplayMode !== 'notApplicable' && audit.result.score === 1);
-      const passedAuditTags = new Set(passedAudits.map(audit => audit.group));
 
       const passedAuditGroups = categoryDOM.querySelectorAll('.lh-clump--passed .lh-audit-group');
-      assert.equal(passedAuditGroups.length, passedAuditTags.size);
+      const passedAuditsElems = categoryDOM.querySelectorAll('.lh-clump--passed .lh-audit');
+
+      assert.equal(passedAuditGroups.length, 0);
+      assert.equal(passedAuditsElems.length, passedAudits.length);
     });
 
     it('renders all the audits', () => {
@@ -293,15 +295,20 @@ describe('CategoryRenderer', () => {
     });
 
     it('gives each group a selectable class', () => {
+      const categoryClone = JSON.parse(JSON.stringify(category));
+      // Force all results to be Failed for accurate counting of groups.
+      categoryClone.auditRefs.forEach(ref => {
+        ref.result.score = 0;
+        ref.result.scoreDisplayMode = 'binary';
+      });
       const categoryGroupIds = new Set(category.auditRefs.filter(a => a.group).map(a => a.group));
       assert.ok(categoryGroupIds.size > 6); // Ensure there's something to test.
 
-      const categoryElem = renderer.render(category, sampleResults.categoryGroups);
+      const categoryElem = renderer.render(categoryClone, sampleResults.categoryGroups);
 
       categoryGroupIds.forEach(groupId => {
         const selector = `.lh-audit-group--${groupId}`;
-        // Could be multiple results (e.g. found in both passed and failed clumps).
-        assert.ok(categoryElem.querySelectorAll(selector).length >= 1,
+        assert.equal(categoryElem.querySelectorAll(selector).length, 1,
           `could not find '${selector}'`);
       });
     });