From 3af497738e58e77b43784fc84f78f7468279c6df Mon Sep 17 00:00:00 2001 From: Shane Exterkamp Date: Mon, 29 Oct 2018 14:36:53 -0700 Subject: [PATCH 1/3] Removed some proto config settings from proto. --- lighthouse-core/lib/proto-preprocessor.js | 21 +++-- .../test/lib/proto-preprocessor-test.js | 33 +++++-- proto/lighthouse-result.proto | 86 +------------------ proto/sample_v2_round_trip.json | 23 +---- 4 files changed, 47 insertions(+), 116 deletions(-) diff --git a/lighthouse-core/lib/proto-preprocessor.js b/lighthouse-core/lib/proto-preprocessor.js index 721a488d1d34..918059b33b97 100644 --- a/lighthouse-core/lib/proto-preprocessor.js +++ b/lighthouse-core/lib/proto-preprocessor.js @@ -23,13 +23,22 @@ 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]; - } - } + // Filter out unwanted fields + const valid = ['emulatedFormFactor', 'locale', 'onlyCategories']; + + const minifiedSettings = Object.keys(reportJson.configSettings) + .filter(key => valid.includes(key)) + .reduce((obj, key) => { + // @ts-ignore + obj[key] = reportJson.configSettings[key]; + return obj; + }, {}); + + // @ts-ignore + reportJson.configSettings = minifiedSettings; } // Remove runtimeError if it is NO_ERROR diff --git a/lighthouse-core/test/lib/proto-preprocessor-test.js b/lighthouse-core/test/lib/proto-preprocessor-test.js index cadb0a455545..3a76964d68e5 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)); diff --git a/proto/lighthouse-result.proto b/proto/lighthouse-result.proto index 8c93bdc201e1..dc0930e2dbf1 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 f6c43f781f17..e9d73315342c 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, From bfd5d702aa881e87c028ea6f265e12389a8a0502 Mon Sep 17 00:00:00 2001 From: Shane Exterkamp Date: Mon, 29 Oct 2018 16:16:18 -0700 Subject: [PATCH 2/3] Tricky ts partial settings workaround. --- lighthouse-core/lib/proto-preprocessor.js | 18 +++++------------- 1 file changed, 5 insertions(+), 13 deletions(-) diff --git a/lighthouse-core/lib/proto-preprocessor.js b/lighthouse-core/lib/proto-preprocessor.js index 918059b33b97..c77f298c7e8b 100644 --- a/lighthouse-core/lib/proto-preprocessor.js +++ b/lighthouse-core/lib/proto-preprocessor.js @@ -26,19 +26,11 @@ function processForProto(result) { // 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) { - // Filter out unwanted fields - const valid = ['emulatedFormFactor', 'locale', 'onlyCategories']; - - const minifiedSettings = Object.keys(reportJson.configSettings) - .filter(key => valid.includes(key)) - .reduce((obj, key) => { - // @ts-ignore - obj[key] = reportJson.configSettings[key]; - return obj; - }, {}); - - // @ts-ignore - reportJson.configSettings = minifiedSettings; + // 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 From d9413cd6ef4a720f82a76992c464e05447d77c02 Mon Sep 17 00:00:00 2001 From: Shane Exterkamp Date: Tue, 30 Oct 2018 10:57:55 -0700 Subject: [PATCH 3/3] Removing extra comment space --- lighthouse-core/lib/proto-preprocessor.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lighthouse-core/lib/proto-preprocessor.js b/lighthouse-core/lib/proto-preprocessor.js index c77f298c7e8b..454037f73c9b 100644 --- a/lighthouse-core/lib/proto-preprocessor.js +++ b/lighthouse-core/lib/proto-preprocessor.js @@ -24,7 +24,7 @@ function processForProto(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. + // 'ignore unknown fields' in the language of conversion. if (reportJson.configSettings) { // The settings that are in both proto and LHR const {emulatedFormFactor, locale, onlyCategories} = reportJson.configSettings;