tests: remove protoc roundtrip check from update:sample-json (#10557)

.github/workflows/basics.yml

@@ -24,6 +24,20 @@ jobs:
       with:
         node-version: 10.x
 
+    - name: Setup protoc
+      uses: arduino/setup-protoc@master
+      with:
+        version: '3.7.1'
+
+    - name: Set up Python
+      uses: actions/setup-python@v1
+      with:
+        python-version: 2.7
+    - name: Install Python dependencies
+      run: |
+        python -m pip install --upgrade pip
+        pip install protobuf==3.7.1
+
     # Cache yarn deps. thx https://github.com/actions/cache/blob/master/examples.md#node---yarn
     - name: Get yarn cache directory path
       id: yarn-cache-dir-path
@@ -41,6 +55,7 @@ jobs:
     - run: yarn build-all
     - run: yarn diff:sample-json
     - run: yarn lint
+    - run: yarn test-proto # Run before unit-core because the roundtrip json is needed for proto tests.
     - run: yarn unit-core
     - run: yarn tsc -p .
 

.gitignore

@@ -53,3 +53,5 @@ proto/scripts/*_processed.json
 
 # require any lock file to be checked in explicitly
 yarn.lock
+
+proto/sample_v2_round_trip.json

lighthouse-core/test/report/html/renderer/psi-test.js

@@ -10,6 +10,7 @@ const fs = require('fs');
 
 const jsdom = require('jsdom');
 
+const testUtils = require('../../../test-utils.js');
 const prepareLabData = require('../../../../report/html/renderer/psi.js');
 const Util = require('../../../../report/html/renderer/util.js');
 const I18n = require('../../../../report/html/renderer/i18n.js');
@@ -19,11 +20,8 @@ const DetailsRenderer = require('../../../../report/html/renderer/details-render
 const CriticalRequestChainRenderer =
     require('../../../../report/html/renderer/crc-details-renderer.js');
 
+const {itIfProtoExists, sampleResultsRoundtripStr} = testUtils.getProtoRoundTrip();
 const sampleResultsStr = fs.readFileSync(__dirname + '/../../../results/sample_v2.json', 'utf-8');
-const sampleResultsRoundtripStr = fs.readFileSync(
-  __dirname + '/../../../../../proto/sample_v2_round_trip.json',
-  'utf-8'
-);
 
 const TEMPLATE_FILE = fs.readFileSync(
   __dirname + '/../../../../report/html/templates.html',
@@ -64,7 +62,7 @@ describe('DOM', () => {
 
   describe('psi prepareLabData helpers', () => {
     describe('prepareLabData', () => {
-      it('succeeds with LHResult object (roundtrip) input', () => {
+      itIfProtoExists('succeeds with LHResult object (roundtrip) input', () => {
         const roundTripLHResult = /** @type {LH.Result} */ JSON.parse(sampleResultsRoundtripStr);
         const result = prepareLabData(roundTripLHResult, document);
 

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

@@ -5,13 +5,16 @@
  */
 'use strict';
 
+const testUtils = require('../test-utils.js');
 const sampleJson = require('../results/sample_v2.json');
-const roundTripJson = require('../../../proto/sample_v2_round_trip.json');
 const preprocessor = require('../../lib/proto-preprocessor.js');
 
 /* eslint-env jest */
 
-describe('round trip JSON comparison subsets', () => {
+const {describeIfProtoExists, sampleResultsRoundtripStr} = testUtils.getProtoRoundTrip();
+const roundTripJson = sampleResultsRoundtripStr && JSON.parse(sampleResultsRoundtripStr);
+
+describeIfProtoExists('round trip JSON comparison subsets', () => {
   let processedLHR;
 
   beforeEach(() => {
@@ -51,7 +54,7 @@ describe('round trip JSON comparison subsets', () => {
   });
 });
 
-describe('round trip JSON comparison to everything', () => {
+describeIfProtoExists('round trip JSON comparison to everything', () => {
   let processedLHR;
 
   beforeEach(() => {

lighthouse-core/test/test-utils.js

@@ -7,6 +7,7 @@
 
 /* eslint-env jest */
 
+const fs = require('fs');
 const i18n = require('../lib/i18n/i18n.js');
 const {default: {toBeCloseTo}} = require('expect/build/matchers.js');
 
@@ -41,3 +42,42 @@ expect.extend({
     return toBeCloseTo.call(thisObj, ...args);
   },
 });
+
+/**
+ * Some tests use the result of a LHR processed by our proto serialization.
+ * Proto is an annoying dependency to setup, so we allows tests that use it
+ * to be skipped when run locally. This makes external contributions simpler.
+ *
+ * Along with the sample LHR, this function returns jest `it` and `describe`
+ * functions that will skip if the sample LHR could not be loaded.
+ */
+function getProtoRoundTrip() {
+  let sampleResultsRoundtripStr;
+  let describeIfProtoExists;
+  let itIfProtoExists;
+  try {
+    sampleResultsRoundtripStr =
+      fs.readFileSync(__dirname + '/../../proto/sample_v2_round_trip.json', 'utf-8');
+    describeIfProtoExists = describe;
+    itIfProtoExists = it;
+  } catch (err) {
+    if (process.env.GITHUB_ACTIONS) {
+      throw new Error('sample_v2_round_trip must be generated for CI proto test');
+    }
+    // Otherwise no proto roundtrip for tests, so skip them.
+    // This is fine for running the tests locally.
+
+    itIfProtoExists = it.skip;
+    describeIfProtoExists = describe.skip;
+  }
+
+  return {
+    itIfProtoExists,
+    describeIfProtoExists,
+    sampleResultsRoundtripStr,
+  };
+}
+
+module.exports = {
+  getProtoRoundTrip,
+};

package.json

@@ -35,6 +35,7 @@
     "test-lantern": "bash lighthouse-core/scripts/test-lantern.sh",
     "test-legacy-javascript": "bash lighthouse-core/scripts/test-legacy-javascript.sh",
     "test-docs": "yarn --cwd docs/recipes/auth && jest docs/recipes/integration-test && yarn --cwd docs/recipes/custom-gatherer-puppeteer test",
+    "test-proto": "yarn compile-proto && yarn build-proto-roundtrip",
     "unit-core": "jest \"lighthouse-core/\"",
     "unit-cli": "jest \"lighthouse-cli/\"",
     "unit-viewer": "jest \"lighthouse-viewer/.*-test.js\"",
@@ -65,7 +66,7 @@
     "i18n:collect-strings": "node lighthouse-core/scripts/i18n/collect-strings.js",
     "update:lantern-master": "node lighthouse-core/scripts/lantern/update-master-lantern-values.js",
     "update:sample-artifacts": "node lighthouse-core/scripts/update-report-fixtures.js",
-    "update:sample-json": "yarn i18n:collect-strings && node ./lighthouse-cli -A=./lighthouse-core/test/results/artifacts --config-path=./lighthouse-core/test/results/sample-config.js --output=json --output-path=./lighthouse-core/test/results/sample_v2.json && node lighthouse-core/scripts/cleanup-LHR-for-diff.js ./lighthouse-core/test/results/sample_v2.json --only-remove-timing && yarn compile-proto && yarn build-proto-roundtrip",
+    "update:sample-json": "yarn i18n:collect-strings && node ./lighthouse-cli -A=./lighthouse-core/test/results/artifacts --config-path=./lighthouse-core/test/results/sample-config.js --output=json --output-path=./lighthouse-core/test/results/sample_v2.json && node lighthouse-core/scripts/cleanup-LHR-for-diff.js ./lighthouse-core/test/results/sample_v2.json --only-remove-timing",
     "diff:sample-json": "yarn i18n:checks && bash lighthouse-core/scripts/assert-golden-lhr-unchanged.sh",
     "ultradumbBenchmark": "./lighthouse-core/scripts/benchmark.js",
     "mixed-content": "./lighthouse-cli/index.js --chrome-flags='--headless' --preset=mixed-content",