Repository: GoogleChrome/lighthouse
Pull request #8867 (2019-05-05 20:06:32)

misc(proto): require protobuf 3.7.1, add stricter audit details test
follow up to #8863 and https://github.com/GoogleChrome/lighthouse/pull/8857#discussion_r281034929

Something was fixed between protobuf 3.6.1 and 3.7.1 that handled the `details.summary` type when the value was just an empty `{}`. 3.6.1 round tripped it as `null`, 3.7.1 correctly round trips it as `{}`.

The round trip test wasn't catching the issue because `{}` was the `summary` expectation and `{}` matches basically everything with `toMatchObject`. The round trip is good enough for audit results now that we can just do the deep, strict `toEqual` instead.


Patch:
diff --git a/docs/throttling.md b/docs/throttling.md
index 761c660ae..c54b162cd 100644
--- a/docs/throttling.md
+++ b/docs/throttling.md
@@ -7,22 +7,22 @@ This is the standard recommendation for mobile throttling:
 - Throughput: 1.6Mbps down / 750 Kbps up.
 - Packet loss: none.
 
-These exact figures are used as [Lighthouse's throttling default](https://github.com/GoogleChrome/lighthouse/blob/8f500e00243e07ef0a80b39334bedcc8ddc8d3d0/lighthouse-core/config/constants.js#L19-L26) and represent roughly the bottom 25% of 4G connections and top 25% of 3G connections. They are identical to the [WebPageTest "Mobile 3G - Fast" preset](https://github.com/WPO-Foundation/webpagetest/blob/master/www/settings/connectivity.ini.sample) and, due to a lower latency, slightly faster for some pages than the [WebPageTest "4G" preset](https://github.com/WPO-Foundation/webpagetest/blob/master/www/settings/connectivity.ini.sample).
+These exact figures are used as [Lighthouse's throttling default](https://github.com/GoogleChrome/lighthouse/blob/8f500e00243e07ef0a80b39334bedcc8ddc8d3d0/lighthouse-core/config/constants.js#L19-L26) and represent roughly the bottom 25% of 4G connections and top 25% of 3G connections (In Lighthouse it is sometimes called "Slow 4G" used to be labeled as "Fast 3G"). This preset is identical to the [WebPageTest's "Mobile 3G - Fast"](https://github.com/WPO-Foundation/webpagetest/blob/master/www/settings/connectivity.ini.sample) and, due to a lower latency, slightly faster for some pages than the [WebPageTest "4G" preset](https://github.com/WPO-Foundation/webpagetest/blob/master/www/settings/connectivity.ini.sample).
 
-## Throttling basics
+## Types of throttling
 
-1. Simulated throttling, which Lighthouse uses by default, is computed throttling _after a trace has been recorded_ which makes it very fast and deterministic. However, due to the imperfect nature of predicting alternate execution paths, there is inherent inaccuracy that is summarized in this doc: [Lighthouse Metric Variability and Accuracy](https://docs.google.com/document/d/1BqtL-nG53rxWOI5RO0pItSRPowZVnYJ_gBEQCJ5EeUE/edit). The TLDR: while it's roughly as accurate or better than DevTools throttling for most sites, it suffers from edge cases and a deep investigation to performance should use _Packet-level_ throttling tools.
-1. The DevTools network throttling, which Lighthouse uses for `throttlingMethod='devtools'`, is implemented within Chrome at the _request-level_. As a result, it has some downsides that are now summarized in this doc: [Network Throttling & Chrome - status](https://docs.google.com/document/d/1TwWLaLAfnBfbk5_ZzpGXegPapCIfyzT4MWuZgspKUAQ/edit). The TLDR: while it's a [decent approximation](https://docs.google.com/document/d/10lfVdS1iDWCRKQXPfbxEn4Or99D64mvNlugP1AQuFlE/edit), it's not a sufficient model of a slow connection. The [multipliers used in Lighthouse](https://github.com/GoogleChrome/lighthouse/blob/3be483287a530fb560c843b7299ef9cfe91ce1cc/lighthouse-core/lib/emulation.js#L33-L39) attempt to correct for the differences.
-1. _Proxy-level_ throttling tools do not affect UDP data, so they're not recommended.
-1. _Packet-level_ throttling tools are able to make the most accurate network simulation.
+Within web performance testing, there are four typical styles of throttling:
 
-## How do I get high-quality packet-level throttling?
+1. **_Simulated throttling_**, which Lighthouse uses by **default**, uses a simulation of a page load, based on the data observed in the initial unthrottled load. This approach which makes it both very fast and deterministic. However, due to the imperfect nature of predicting alternate execution paths, there is inherent inaccuracy that is summarized in this doc: [Lighthouse Metric Variability and Accuracy](https://docs.google.com/document/d/1BqtL-nG53rxWOI5RO0pItSRPowZVnYJ_gBEQCJ5EeUE/edit). The TLDR: while it's roughly as accurate or better than DevTools throttling for most sites, it suffers from edge cases and a deep investigation to performance should use _Packet-level_ throttling tools.
+1. **_Request-level_** throttling, also referred to as _DevTools throttling_ or in the Audits Panel as _Applied throttling_, is how throttling is implemented with Chrome DevTools. In real mobile connectivity, latency affects things at the packet level rather than the request level. As a result, this throttling isn't highly accurate. It also has a few more downsides that are summarized in [Network Throttling & Chrome - status](https://docs.google.com/document/d/1TwWLaLAfnBfbk5_ZzpGXegPapCIfyzT4MWuZgspKUAQ/edit). The TLDR: while it's a [decent approximation](https://docs.google.com/document/d/10lfVdS1iDWCRKQXPfbxEn4Or99D64mvNlugP1AQuFlE/edit), it's not a sufficient model of a slow connection. The [multipliers used in Lighthouse](https://github.com/GoogleChrome/lighthouse/blob/3be483287a530fb560c843b7299ef9cfe91ce1cc/lighthouse-core/lib/emulation.js#L33-L39) attempt to correct for the differences.
+1. **_Proxy-level_** throttling tools do not affect UDP data, so they're decent, but not ideal.
+1. **_Packet-level_** throttling tools are able to make the most accurate network simulation. While this approach can model real network conditions most effectively, it also can introduce [more variance](https://docs.google.com/document/d/1BqtL-nG53rxWOI5RO0pItSRPowZVnYJ_gBEQCJ5EeUE/edit) than request-level or simulated throttling. [WebPageTest uses](https://github.com/WPO-Foundation/wptagent/blob/master/docs/remote_trafficshaping.md) packet-level throttling.
 
-If you want to use more accurate throttling, read on.
+Lighthouse, by default, uses simulated throttling as it provides both quick evaluation and minimized variance. However, some may want to experiment with more accurate throttling...
 
-This Performance Calendar article, [Testing with Realistic Networking Conditions](https://calendar.perfplanet.com/2016/testing-with-realistic-networking-conditions/), has a good explanation of packet-level traffic shaping (which applies across TCP/UDP/ICMP) and recommendations.
+## How do I get packet-level throttling?
 
-### Using `comcast` for network throttling
+This Performance Calendar article, [Testing with Realistic Networking Conditions](https://calendar.perfplanet.com/2016/testing-with-realistic-networking-conditions/), has a good explanation of packet-level traffic shaping (which applies across TCP/UDP/ICMP) and recommendations.
 
 The `comcast` Go package appears to be the most usable Mac/Linux commandline app for managing your network connection. Important to note: it changes your **entire** machine's network interface. Also, **`comcast` requires `sudo`** (as all packet-level shapers do).
 
diff --git a/lighthouse-core/report/html/report-styles.css b/lighthouse-core/report/html/report-styles.css
index 996e03478..2522e0e23 100644
--- a/lighthouse-core/report/html/report-styles.css
+++ b/lighthouse-core/report/html/report-styles.css
@@ -85,6 +85,7 @@
   --color-black-600: #757575;
   --color-black-800: #424242;
   --color-black-900: #212121;
+  --off-black: #111111;
   --color-black: #000000;
   --color-blue: #2962FF;
   --color-green-700: #018642;
@@ -135,6 +136,7 @@
   --topbar-height: 36px;
   --topbar-icon-size: 24px;
   --topbar-padding: 0 8px;
+  --metrics-toggle-color: var(--color-black-200);
 
   --color-average-secondary: var(--color-orange-700);
   --color-average: var(--color-orange);
@@ -180,7 +182,8 @@
   --topbar-bg: var(--color-black);
   --plugin-badge-bg: var(--color-black-800);
   --header-bg-color: var(--color-black-900);
-  --env-item-bg: rgba(0, 0, 0, 0.5);
+  --env-item-bg: var(--color-black);
+  --report-secondary-border-color: var(--color-black-200);
 
   --body-background-color: var(--color-black-900);
   --body-text-color: var(--color-black-100);
@@ -643,6 +646,11 @@
 .lh-metrics-toggle__lines {
   fill: var(--color-metric-toggle-lines);
 }
+
+.lh-metrics-toggle label {
+  background-color: var(--metrics-toggle-color);
+}
+
 .lh-metrics-toggle label .lh-metrics-toggle__icon--less {
   padding-left: 8px;
 }
diff --git a/package.json b/package.json
index 2014e4714..5b92e60b3 100644
--- a/package.json
+++ b/package.json
@@ -59,7 +59,7 @@
     "ultradumbBenchmark": "./lighthouse-core/scripts/benchmark.js",
     "mixed-content": "./lighthouse-cli/index.js --chrome-flags='--headless' --preset=mixed-content",
     "minify-latest-run": "./lighthouse-core/scripts/lantern/minify-trace.js ./latest-run/defaultPass.trace.json ./latest-run/defaultPass.trace.min.json && ./lighthouse-core/scripts/lantern/minify-devtoolslog.js ./latest-run/defaultPass.devtoolslog.json ./latest-run/defaultPass.devtoolslog.min.json",
-    "compile-proto": "[ \"$(protoc --version)\" == 'libprotoc 3.6.1' ] && protoc --python_out=./ ./proto/lighthouse-result.proto && mv ./proto/*_pb2.py ./proto/scripts || (echo \"❌ Install protobuf to compile the proto file.\" && false)",
+    "compile-proto": "protoc --python_out=./ ./proto/lighthouse-result.proto && mv ./proto/*_pb2.py ./proto/scripts || (echo \"❌ Install protobuf ≥ 3.7.1 to compile the proto file.\" && false)",
     "build-proto-roundtrip": "cd proto/scripts && PROTOCOL_BUFFERS_PYTHON_IMPLEMENTATION=cpp PROTOCOL_BUFFERS_PYTHON_IMPLEMENTATION_VERSION=2 python json_roundtrip_via_proto.py"
   },
   "devDependencies": {


Test patch:
diff --git a/lighthouse-core/test/report/proto-test.js b/lighthouse-core/test/report/proto-test.js
index 8282ffbf3..2c13b20ef 100644
--- a/lighthouse-core/test/report/proto-test.js
+++ b/lighthouse-core/test/report/proto-test.js
@@ -21,23 +21,10 @@ describe('round trip JSON comparison subsets', () => {
     sampleJson = JSON.parse(preprocessor.processForProto(sample));
   });
 
-  it('has the same audit results sans details', () => {
-    Object.keys(sampleJson.audits).forEach(audit => {
-      delete sampleJson.audits[audit].details;
-    });
-
-    expect(roundTripJson.audits).toMatchObject(sampleJson.audits);
-  });
-
-  it('has the same audit results & details if applicable', () => {
-    Object.keys(sampleJson.audits).forEach(auditId => {
-      expect(roundTripJson.audits[auditId]).toMatchObject(sampleJson.audits[auditId]);
-
-      if ('details' in sampleJson.audits[auditId]) {
-        expect(roundTripJson.audits[auditId].details)
-          .toMatchObject(sampleJson.audits[auditId].details);
-      }
-    });
+  it('has the same audit results and details (if applicable)', () => {
+    for (const auditId of Object.keys(sampleJson.audits)) {
+      expect(roundTripJson.audits[auditId]).toEqual(sampleJson.audits[auditId]);
+    }
   });
 
   it('has the same i18n rendererFormattedStrings', () => {