processForProto took/returned a string, now takes/returns LH.Result.

lighthouse-core/lib/proto-preprocessor.js

@@ -16,11 +16,12 @@ const fs = require('fs');
  */
 
 /**
-  * @param {string} result
+  * Transform an LHR into a proto-friendly LHR
+  * @param {LH.Result} lhr
+  * @return {LH.Result}
   */
-function processForProto(result) {
-  /** @type {LH.Result} */
-  const reportJson = JSON.parse(result);
+function processForProto(lhr) {
+  const reportJson = JSON.parse(JSON.stringify(lhr));
 
   // Clean up the configSettings
   // Note: This is not strictly required for conversion if protobuf parsing is set to
@@ -95,7 +96,7 @@ function processForProto(result) {
 
   removeStrings(reportJson);
 
-  return JSON.stringify(reportJson);
+  return reportJson;
 }
 
 // @ts-ignore claims always false, but this checks if cli or module
@@ -113,9 +114,9 @@ if (require.main === module) {
 
   if (input && output) {
     // process the file
-    const report = processForProto(fs.readFileSync(input, 'utf-8'));
+    const report = processForProto(JSON.parse(fs.readFileSync(input, 'utf-8')));
     // write to output from argv
-    fs.writeFileSync(output, report, 'utf-8');
+    fs.writeFileSync(output, JSON.stringify(report), 'utf-8');
   }
 } else {
   module.exports = {

lighthouse-core/test/lib/proto-preprocessor-test.js

@@ -5,7 +5,7 @@
  */
 'use strict';
 
-const processForProto = require('../../lib/proto-preprocessor').processForProto;
+const {processForProto} = require('../../lib/proto-preprocessor');
 
 /* eslint-env jest */
 describe('processing for proto', () => {
@@ -44,9 +44,9 @@ describe('processing for proto', () => {
         'onlyCategories': null,
       },
     };
-    const output = processForProto(JSON.stringify(input));
+    const output = processForProto(input);
 
-    expect(JSON.parse(output)).toMatchObject(expectation);
+    expect(output).toMatchObject(expectation);
   });
 
   it('cleans up default runtimeErrors', () => {
@@ -56,9 +56,9 @@ describe('processing for proto', () => {
       },
     };
 
-    const output = processForProto(JSON.stringify(input));
+    const output = processForProto(input);
 
-    expect(JSON.parse(output)).not.toHaveProperty('runtimeError');
+    expect(output).not.toHaveProperty('runtimeError');
   });
 
   it('non-default runtimeErrors are untouched', () => {
@@ -68,9 +68,9 @@ describe('processing for proto', () => {
       },
     };
 
-    const output = processForProto(JSON.stringify(input));
+    const output = processForProto(input);
 
-    expect(JSON.parse(output)).toMatchObject(input);
+    expect(output).toMatchObject(input);
   });
 
   it('cleans up audits', () => {
@@ -91,9 +91,9 @@ describe('processing for proto', () => {
         },
       },
     };
-    const output = processForProto(JSON.stringify(input));
+    const output = processForProto(input);
 
-    expect(JSON.parse(output)).toMatchObject(expectation);
+    expect(output).toMatchObject(expectation);
   });
 
 
@@ -108,9 +108,9 @@ describe('processing for proto', () => {
     const expectation = {
       'i18n': {},
     };
-    const output = processForProto(JSON.stringify(input));
+    const output = processForProto(input);
 
-    expect(JSON.parse(output)).toMatchObject(expectation);
+    expect(output).toMatchObject(expectation);
   });
 
   it('removes empty strings', () => {
@@ -151,8 +151,8 @@ describe('processing for proto', () => {
         ],
       },
     };
-    const output = processForProto(JSON.stringify(input));
+    const output = processForProto(input);
 
-    expect(JSON.parse(output)).toMatchObject(expectation);
+    expect(output).toMatchObject(expectation);
   });
 });

lighthouse-core/test/report/proto-test.js

@@ -5,60 +5,57 @@
  */
 'use strict';
 
-const path = require('path');
-const fs = require('fs');
-
-const sample = fs.readFileSync(path.resolve(__dirname, '../results/sample_v2.json'));
+const sampleJson = require('../results/sample_v2.json');
 const roundTripJson = require('../../../proto/sample_v2_round_trip');
 const preprocessor = require('../../lib/proto-preprocessor.js');
 
 /* eslint-env jest */
 
 describe('round trip JSON comparison subsets', () => {
-  let sampleJson;
+  let processedLHR;
 
   beforeEach(() => {
-    sampleJson = JSON.parse(preprocessor.processForProto(sample));
+    processedLHR = preprocessor.processForProto(sampleJson);
   });
 
   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]);
+    for (const auditId of Object.keys(processedLHR.audits)) {
+      expect(roundTripJson.audits[auditId]).toEqual(processedLHR.audits[auditId]);
     }
   });
 
   it('has the same i18n rendererFormattedStrings', () => {
-    expect(roundTripJson.i18n).toMatchObject(sampleJson.i18n);
+    expect(roundTripJson.i18n).toMatchObject(processedLHR.i18n);
   });
 
   it('has the same top level values', () => {
     // Don't test all top level properties that are objects.
-    Object.keys(sampleJson).forEach(audit => {
-      if (typeof sampleJson[audit] === 'object' && !Array.isArray(sampleJson[audit])) {
-        delete sampleJson[audit];
+    Object.keys(processedLHR).forEach(audit => {
+      if (typeof processedLHR[audit] === 'object' && !Array.isArray(processedLHR[audit])) {
+        delete processedLHR[audit];
       }
     });
 
     // Properties set to their type's default value will be omitted in the roundTripJson.
     // For an explicit list of properties, remove sampleJson values if set to a default.
-    if (Array.isArray(sampleJson.stackPacks) && sampleJson.stackPacks.length === 0) {
-      delete sampleJson.stackPacks;
+    if (Array.isArray(processedLHR.stackPacks) && processedLHR.stackPacks.length === 0) {
+      delete processedLHR.stackPacks;
     }
 
-    expect(roundTripJson).toMatchObject(sampleJson);
+    expect(roundTripJson).toMatchObject(processedLHR);
   });
 
   it('has the same config values', () => {
     // Config settings from proto round trip should be a subset of the actual settings.
-    expect(sampleJson.configSettings).toMatchObject(roundTripJson.configSettings);
+    expect(processedLHR.configSettings).toMatchObject(roundTripJson.configSettings);
   });
 });
 
 describe('round trip JSON comparison to everything', () => {
-  let sampleJson;
+  let processedLHR;
 
   beforeEach(() => {
-    sampleJson = JSON.parse(preprocessor.processForProto(sample));
+    processedLHR = preprocessor.processForProto(sampleJson);
 
     // Proto conversion turns empty summaries into null. This is OK,
     // and is handled in the PSI roundtrip just fine, but messes up the easy
@@ -71,6 +68,6 @@ describe('round trip JSON comparison to everything', () => {
   });
 
   it('has the same JSON overall', () => {
-    expect(sampleJson).toMatchObject(roundTripJson);
+    expect(processedLHR).toMatchObject(roundTripJson);
   });
 });