Repository: GoogleChrome/lighthouse
Pull request #6424 (2018-10-29 21:45:16)

misc(proto): reduce proto configSettings to minimum needed
**Summary**
Remove many of the configSettings from the proto definition.

**Related Issues/PRs**
moves #6202 along
fixes: #6309 (by deleting it from the proto)
closes: #6403 



Patch:
diff --git a/.github/ISSUE_TEMPLATE/Feature_request.md b/.github/ISSUE_TEMPLATE/Feature_request.md
index aaa6fcf6c..375b26ed5 100644
--- a/.github/ISSUE_TEMPLATE/Feature_request.md
+++ b/.github/ISSUE_TEMPLATE/Feature_request.md
@@ -16,5 +16,5 @@ about: Suggest an idea to us!
 **What is the motivation or use case for changing this?**
 
 
-**How is this beneficial to Ligthhouse?**
+**How is this beneficial to Lighthouse?**
 
diff --git a/lighthouse-core/lib/proto-preprocessor.js b/lighthouse-core/lib/proto-preprocessor.js
index 721a488d1..454037f73 100644
--- a/lighthouse-core/lib/proto-preprocessor.js
+++ b/lighthouse-core/lib/proto-preprocessor.js
@@ -23,13 +23,14 @@ function processForProto(result) {
   const reportJson = JSON.parse(result);
 
   // Clean up the configSettings
+  // Note: This is not strictly required for conversion if protobuf parsing is set to
+  // 'ignore unknown fields' in the language of conversion.
   if (reportJson.configSettings) {
-    // make sure the 'output' field is an array
-    if (reportJson.configSettings.output) {
-      if (!Array.isArray(reportJson.configSettings.output)) {
-        reportJson.configSettings.output = [reportJson.configSettings.output];
-      }
-    }
+    // The settings that are in both proto and LHR
+    const {emulatedFormFactor, locale, onlyCategories} = reportJson.configSettings;
+
+    // @ts-ignore - intentionally only a subset of settings.
+    reportJson.configSettings = {emulatedFormFactor, locale, onlyCategories};
   }
 
   // Remove runtimeError if it is NO_ERROR
diff --git a/proto/lighthouse-result.proto b/proto/lighthouse-result.proto
index e4e0f4b1a..befb32303 100644
--- a/proto/lighthouse-result.proto
+++ b/proto/lighthouse-result.proto
@@ -111,64 +111,6 @@ message LighthouseResult {
 
   // Message containing the configuration settings for the LH run
   message ConfigSettings {
-    // The output types the audit made (json, html...)
-    repeated string output = 1;
-
-    // The maximum amount of time to wait for loading in ms
-    google.protobuf.DoubleValue max_wait_for_load = 2;
-
-    // Network throttling options
-    enum ThrottlingMethod {
-      // Unknown method.  Should not be used
-      UNKNOWN_THROTTLING_METHOD = 0;
-
-      // Use devtools to throttle the request
-      devtools = 1;
-
-      // Use no additional throttling (only throttling provided by system
-      // itself)
-      provided = 2;
-
-      // Simulate throttling with Lantern
-      simulate = 3;
-    }
-
-    // The throttling method used during this audit
-    ThrottlingMethod throttling_method = 3;
-
-    // Message containing the throttling settings used in an LH configuration
-    message Throttling {
-      // The round trip time in ms
-      google.protobuf.DoubleValue rtt_ms = 1;
-
-      // The throughput in kilobytes per second
-      google.protobuf.DoubleValue throughput_kbps = 2;
-
-      // The request latency in milliseconds
-      google.protobuf.DoubleValue request_latency_ms = 3;
-
-      // The download throughput in kilobytes per second
-      google.protobuf.DoubleValue download_throughput_kbps = 4;
-
-      // The upload throughput in kilobytes per second
-      google.protobuf.DoubleValue upload_throughput_kbps = 5;
-
-      // The amount of slowdown to apply to the cpu
-      google.protobuf.DoubleValue cpu_slowdown_multiplier = 6;
-    }
-
-    // The throttling settings used
-    Throttling throttling = 4;
-
-    // Flag indicating this audit was gather only or not
-    google.protobuf.BoolValue gather_mode = 5;
-
-    // Flag disabling clearing the browser cache before runs
-    google.protobuf.BoolValue disable_storage_reset = 6;
-
-    // Flag indicating if device emulation was enabled or not
-    google.protobuf.BoolValue disable_device_emulation = 7;
-
     // The possible form factors an audit can be run in
     enum EmulatedFormFactor {
       // Unknown form factor.  This should not be used
@@ -185,36 +127,14 @@ message LighthouseResult {
     }
 
     // The form factor used in the audit
-    EmulatedFormFactor emulated_form_factor = 8;
+    EmulatedFormFactor emulated_form_factor = 1;
 
     // The locale that was active during the audit
-    string locale = 9;
-
-    // List of URL patterns that were blocked during this audit
-    // nullable list of strings
-    google.protobuf.Value blocked_url_patterns = 10;
-
-    // Comma delimited list of trace categories to capture
-    // nullable string
-    google.protobuf.Value additional_trace_categories = 11;
-
-    // Map of extra HTTP Headers to pass in with the request
-    google.protobuf.Value extra_headers = 12;
-
-    // List of the audits that were preformed, empty if all were run
-    // nullable list of strings
-    google.protobuf.Value only_audits = 13;
+    string locale = 2;
 
     // List of the categories that were run, empty if all were run
     // nullable list of strings
-    google.protobuf.Value only_categories = 14;
-
-    // List of the audits that were skipped, empty if all were run
-    // nullable list of strings
-    google.protobuf.Value skip_audits = 15;
-
-    // Flag indicating whether this Lighthouse run was audit-only
-    google.protobuf.BoolValue audit_mode = 16;
+    google.protobuf.Value only_categories = 3;
   }
 
   // The settings that were used to run this audit
diff --git a/proto/sample_v2_round_trip.json b/proto/sample_v2_round_trip.json
index 3d9910611..ba8e2c4b3 100644
--- a/proto/sample_v2_round_trip.json
+++ b/proto/sample_v2_round_trip.json
@@ -3245,30 +3245,9 @@
         }
     }, 
     "configSettings": {
-        "additionalTraceCategories": null, 
-        "blockedUrlPatterns": null, 
-        "disableDeviceEmulation": false, 
-        "disableStorageReset": false, 
         "emulatedFormFactor": "mobile", 
-        "extraHeaders": null, 
-        "gatherMode": false, 
         "locale": "en-US", 
-        "maxWaitForLoad": 45000.0, 
-        "onlyAudits": null, 
-        "onlyCategories": null, 
-        "output": [
-            "json"
-        ], 
-        "skipAudits": null, 
-        "throttling": {
-            "cpuSlowdownMultiplier": 4.0, 
-            "downloadThroughputKbps": 1474.5600000000002, 
-            "requestLatencyMs": 562.5, 
-            "rttMs": 150.0, 
-            "throughputKbps": 1638.4, 
-            "uploadThroughputKbps": 675.0
-        }, 
-        "throttlingMethod": "devtools"
+        "onlyCategories": null
     }, 
     "environment": {
         "benchmarkIndex": 1000.0, 


Test patch:
diff --git a/lighthouse-core/test/lib/proto-preprocessor-test.js b/lighthouse-core/test/lib/proto-preprocessor-test.js
index cadb0a455..3a76964d6 100644
--- a/lighthouse-core/test/lib/proto-preprocessor-test.js
+++ b/lighthouse-core/test/lib/proto-preprocessor-test.js
@@ -9,17 +9,40 @@ const processForProto = require('../../lib/proto-preprocessor').processForProto;
 
 /* eslint-env jest */
 describe('processing for proto', () => {
-  it('cleans up configSettings', () => {
+  it('keeps only necessary configSettings', () => {
     const input = {
       'configSettings': {
-        'output': 'json',
+        'output': [
+          'json',
+        ],
+        'maxWaitForLoad': 45000,
+        'throttlingMethod': 'devtools',
+        'throttling': {
+          'rttMs': 150,
+          'throughputKbps': 1638.4,
+          'requestLatencyMs': 562.5,
+          'downloadThroughputKbps': 1474.5600000000002,
+          'uploadThroughputKbps': 675,
+          'cpuSlowdownMultiplier': 4,
+        },
+        'gatherMode': false,
+        'disableStorageReset': false,
+        'disableDeviceEmulation': false,
+        'emulatedFormFactor': 'mobile',
+        'locale': 'en-US',
+        'blockedUrlPatterns': null,
+        'additionalTraceCategories': null,
+        'extraHeaders': null,
+        'onlyAudits': null,
+        'onlyCategories': null,
+        'skipAudits': null,
       },
     };
     const expectation = {
       'configSettings': {
-        'output': [
-          'json',
-        ],
+        'emulatedFormFactor': 'mobile',
+        'locale': 'en-US',
+        'onlyCategories': null,
       },
     };
     const output = processForProto(JSON.stringify(input));