Repository: GoogleChrome/lighthouse
Pull request #5686 (2018-07-18 21:15:18)

misc(i18n): add assertion script
adds an assertion script to make sure we keep the strings up to date


Patch:
diff --git a/.travis.yml b/.travis.yml
index d8870d064..210f03e6e 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -41,6 +41,7 @@ script:
   - yarn test-extension
   - yarn test-viewer
   - yarn test-lantern
+  - yarn i18n:checks
   # _JAVA_OPTIONS is breaking parsing of compiler output. See #3338.
   - unset _JAVA_OPTIONS
   # FIXME(paulirish): re-enable after a roll of LH->CDT
diff --git a/lighthouse-core/audits/bootup-time.js b/lighthouse-core/audits/bootup-time.js
index 8768e56a2..51d80344b 100644
--- a/lighthouse-core/audits/bootup-time.js
+++ b/lighthouse-core/audits/bootup-time.js
@@ -23,7 +23,7 @@ class BootupTime extends Audit {
       description: 'Consider reducing the time spent parsing, compiling, and executing JS. ' +
         'You may find delivering smaller JS payloads helps with this. [Learn ' +
         'more](https://developers.google.com/web/tools/lighthouse/audits/bootup).',
-      requiredArtifacts: ['traces', 'LighthouseRunWarnings'],
+      requiredArtifacts: ['traces'],
     };
   }
 
@@ -85,7 +85,6 @@ class BootupTime extends Audit {
    * @return {Promise<LH.Audit.Product>}
    */
   static async audit(artifacts, context) {
-    const runWarnings = artifacts.LighthouseRunWarnings;
     const settings = context.settings || {};
     const trace = artifacts.traces[BootupTime.DEFAULT_PASS];
     const devtoolsLog = artifacts.devtoolsLogs[BootupTime.DEFAULT_PASS];
@@ -133,8 +132,8 @@ class BootupTime extends Audit {
 
     // TODO: consider moving this to core gathering so you don't need to run the audit for warning
     if (hadExcessiveChromeExtension) {
-      runWarnings.push('Chrome extensions negatively affected this page\'s load performance. ' +
-        'Try auditing the page in incognito mode or from a clean Chrome profile.');
+      context.LighthouseRunWarnings.push('Chrome extensions negatively affected this page\'s load' +
+        ' performance. Try auditing the page in incognito mode or from a clean Chrome profile.');
     }
 
     const summary = {wastedMs: totalBootupTime};
diff --git a/lighthouse-core/gather/gatherers/dobetterweb/optimized-images.js b/lighthouse-core/gather/gatherers/dobetterweb/optimized-images.js
index b593ae9c1..80ee69be4 100644
--- a/lighthouse-core/gather/gatherers/dobetterweb/optimized-images.js
+++ b/lighthouse-core/gather/gatherers/dobetterweb/optimized-images.js
@@ -21,6 +21,8 @@ const WEBP_QUALITY = 0.85;
 
 const MINIMUM_IMAGE_SIZE = 4096; // savings of <4 KB will be ignored in the audit anyway
 
+const IMAGE_REGEX = /^image\/((x|ms|x-ms)-)?(png|bmp|jpeg)$/;
+
 /** @typedef {{isSameOrigin: boolean, isBase64DataUri: boolean, requestId: string, url: string, mimeType: string, resourceSize: number}} SimplifiedNetworkRecord */
 
 /* global document, Image, atob */
@@ -87,7 +89,7 @@ class OptimizedImages extends Gatherer {
 
       seenUrls.add(record.url);
       const isOptimizableImage = record.resourceType === NetworkRequest.TYPES.Image &&
-        /image\/(png|bmp|jpeg)/.test(record.mimeType);
+        IMAGE_REGEX.test(record.mimeType);
       const isSameOrigin = URL.originsMatch(pageUrl, record.url);
       const isBase64DataUri = /^data:.{2,40}base64\s*,/.test(record.url);
 
diff --git a/lighthouse-core/runner.js b/lighthouse-core/runner.js
index 77717cab6..e84410700 100644
--- a/lighthouse-core/runner.js
+++ b/lighthouse-core/runner.js
@@ -94,7 +94,8 @@ class Runner {
       if (!opts.config.audits) {
         throw new Error('No audits to evaluate.');
       }
-      const auditResults = await Runner._runAudits(settings, opts.config.audits, artifacts);
+      const auditResults = await Runner._runAudits(settings, opts.config.audits, artifacts,
+          lighthouseRunWarnings);
 
       // LHR construction phase
       log.log('status', 'Generating results...');
@@ -166,13 +167,14 @@ class Runner {
   }
 
   /**
-   * Save collected artifacts to disk
+   * Run all audits with specified settings and artifacts.
    * @param {LH.Config.Settings} settings
    * @param {Array<LH.Config.AuditDefn>} audits
    * @param {LH.Artifacts} artifacts
+   * @param {Array<string>} runWarnings
    * @return {Promise<Array<LH.Audit.Result>>}
    */
-  static async _runAudits(settings, audits, artifacts) {
+  static async _runAudits(settings, audits, artifacts, runWarnings) {
     log.log('status', 'Analyzing and running audits...');
     artifacts = Object.assign({}, Runner.instantiateComputedArtifacts(), artifacts);
 
@@ -190,7 +192,7 @@ class Runner {
     // Run each audit sequentially
     const auditResults = [];
     for (const auditDefn of audits) {
-      const auditResult = await Runner._runAudit(auditDefn, artifacts, settings);
+      const auditResult = await Runner._runAudit(auditDefn, artifacts, settings, runWarnings);
       auditResults.push(auditResult);
     }
 
@@ -203,10 +205,11 @@ class Runner {
    * @param {LH.Config.AuditDefn} auditDefn
    * @param {LH.Artifacts} artifacts
    * @param {LH.Config.Settings} settings
+   * @param {Array<string>} runWarnings
    * @return {Promise<LH.Audit.Result>}
    * @private
    */
-  static async _runAudit(auditDefn, artifacts, settings) {
+  static async _runAudit(auditDefn, artifacts, settings, runWarnings) {
     const audit = auditDefn.implementation;
     const status = `Evaluating: ${audit.meta.title}`;
 
@@ -254,7 +257,13 @@ class Runner {
 
       // all required artifacts are in good shape, so we proceed
       const auditOptions = Object.assign({}, audit.defaultOptions, auditDefn.options);
-      const product = await audit.audit(artifacts, {options: auditOptions, settings: settings});
+      const auditContext = {
+        options: auditOptions,
+        settings,
+        LighthouseRunWarnings: runWarnings,
+      };
+
+      const product = await audit.audit(artifacts, auditContext);
       auditResult = Audit.generateAuditResult(audit, product);
     } catch (err) {
       log.warn(audit.meta.id, `Caught exception: ${err.message}`);
diff --git a/lighthouse-core/scripts/i18n/assert-strings-collected.sh b/lighthouse-core/scripts/i18n/assert-strings-collected.sh
new file mode 100755
index 000000000..c5d6a9ae6
--- /dev/null
+++ b/lighthouse-core/scripts/i18n/assert-strings-collected.sh
@@ -0,0 +1,50 @@
+#!/usr/bin/env bash
+
+##
+# @license Copyright 2018 Google Inc. All Rights Reserved.
+# Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. You may obtain a copy of the License at http://www.apache.org/licenses/LICENSE-2.0
+# Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License.
+##
+
+pwd="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )"
+lhroot_path="$pwd/../../.."
+lh_tmp_path="$lhroot_path/.tmp"
+mkdir -p "$lh_tmp_path"
+
+purple='\033[1;35m'
+red='\033[1;31m'
+green='\033[1;32m'
+colorText() {
+  printf "\\n$2$1%b\\n" '\033[0m'
+}
+
+collectedstringsPath="$lhroot_path/lighthouse-core/lib/locales/en-US.json";
+currentstringsPath="$lh_tmp_path/current_strings.json";
+freshstringsPath="$lh_tmp_path/fresh_strings.json";
+
+# always run this before exiting
+function teardown { rm -f "$currentstringsPath" "$freshstringsPath"; }
+trap teardown EXIT
+
+cp "$collectedstringsPath" "$currentstringsPath"
+
+colorText "Collecting strings..." "$purple"
+set -x
+node "$lhroot_path/lighthouse-core/scripts/i18n/collect-strings.js"
+set +x
+
+cp "$collectedstringsPath" "$freshstringsPath"
+
+colorText "Diff'ing golden strings against the fresh strings" "$purple"
+git --no-pager diff --color=always --no-index "$currentstringsPath" "$freshstringsPath"
+
+# Use the return value from last command
+retVal=$?
+
+if [ $retVal -eq 0 ]; then
+  colorText "✅  PASS. All strings have been collected." "$green"
+else
+  colorText "❌  FAIL. Strings have changed." "$red"
+  echo "Commit the changes to lighthouse-core/lib/locales/ update the strings."
+fi
+exit $retVal
diff --git a/lighthouse-core/scripts/i18n/collect-strings.js b/lighthouse-core/scripts/i18n/collect-strings.js
index 72bbae1c2..582d81971 100644
--- a/lighthouse-core/scripts/i18n/collect-strings.js
+++ b/lighthouse-core/scripts/i18n/collect-strings.js
@@ -91,7 +91,7 @@ function writeStringsToLocaleFormat(locale, strings) {
     output[key] = {message};
   }
 
-  fs.writeFileSync(fullPath, JSON.stringify(output, null, 2));
+  fs.writeFileSync(fullPath, JSON.stringify(output, null, 2) + '\n');
 }
 
 const strings = collectAllStringsInDir(path.join(LH_ROOT, 'lighthouse-core'));
diff --git a/package.json b/package.json
index f15e5ab79..d19015d41 100644
--- a/package.json
+++ b/package.json
@@ -62,6 +62,8 @@
     "plots-smoke": "bash plots/test/smoke.sh",
     "changelog": "conventional-changelog --config ./build/changelog-generator/index.js --infile changelog.md --same-file",
     "type-check": "tsc -p . && cd ./lighthouse-viewer && yarn type-check",
+    "i18n:checks": "./lighthouse-core/scripts/i18n/assert-strings-collected.sh",
+    "i18n:collect-strings": "node lighthouse-core/scripts/i18n/collect-strings.js",
     "update:sample-artifacts": "node lighthouse-core/scripts/update-report-fixtures.js -G",
     "update:sample-json": "node ./lighthouse-cli -A=./lighthouse-core/test/results/artifacts --throttling-method=devtools --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": "bash lighthouse-core/scripts/assert-golden-lhr-unchanged.sh",
diff --git a/typings/audit.d.ts b/typings/audit.d.ts
index 66e84e482..15527db54 100644
--- a/typings/audit.d.ts
+++ b/typings/audit.d.ts
@@ -7,8 +7,11 @@
 declare global {
   module LH.Audit {
     export interface Context {
-      options: Record<string, any>; // audit options
+      /** audit options */
+      options: Record<string, any>;
       settings: Config.Settings;
+      /** Push to this array to add top-level warnings to the LHR. */
+      LighthouseRunWarnings: Array<string>;
     }
 
     export interface ScoreOptions {
diff --git a/typings/gatherer.d.ts b/typings/gatherer.d.ts
index 7d679db96..db4755b36 100644
--- a/typings/gatherer.d.ts
+++ b/typings/gatherer.d.ts
@@ -18,7 +18,7 @@ declare global {
       passConfig: Config.Pass
       settings: Config.Settings;
       options?: object;
-      /** Push to this array to add top-level warnings to the LHR.  */
+      /** Push to this array to add top-level warnings to the LHR. */
       LighthouseRunWarnings: Array<string>;
     }
 


Test patch:
diff --git a/lighthouse-core/test/audits/dobetterweb/doctype-test.js b/lighthouse-core/test/audits/dobetterweb/doctype-test.js
index 937aa568a..b77e49bf1 100644
--- a/lighthouse-core/test/audits/dobetterweb/doctype-test.js
+++ b/lighthouse-core/test/audits/dobetterweb/doctype-test.js
@@ -8,7 +8,7 @@
 const Audit = require('../../../audits/dobetterweb/doctype.js');
 const assert = require('assert');
 
-/* eslint-env mocha */
+/* eslint-env jest */
 
 describe('DOBETTERWEB: doctype audit', () => {
   it('fails when document does not contain a doctype', () => {
diff --git a/lighthouse-core/test/gather/gatherers/dobetterweb/optimized-images-test.js b/lighthouse-core/test/gather/gatherers/dobetterweb/optimized-images-test.js
index 68dfe839c..d017f0387 100644
--- a/lighthouse-core/test/gather/gatherers/dobetterweb/optimized-images-test.js
+++ b/lighthouse-core/test/gather/gatherers/dobetterweb/optimized-images-test.js
@@ -5,7 +5,7 @@
  */
 'use strict';
 
-/* eslint-env mocha */
+/* eslint-env jest */
 
 const OptimizedImages =
     require('../../../../gather/gatherers/dobetterweb/optimized-images');
@@ -185,4 +185,23 @@ describe('Optimized images', () => {
       assert.ok(/gmail.*image.jpg/.test(artifact[3].url));
     });
   });
+
+  it('handles non-standard mime types too', async () => {
+    const traceData = {
+      networkRecords: [
+        {
+          requestId: '1',
+          url: 'http://google.com/image.bmp?x-ms',
+          mimeType: 'image/x-ms-bmp',
+          resourceSize: 12000,
+          transferSize: 20000,
+          resourceType: 'Image',
+          finished: true,
+        },
+      ],
+    };
+
+    const artifact = await optimizedImages.afterPass(options, traceData);
+    expect(artifact).toHaveLength(1);
+  });
 });
diff --git a/lighthouse-core/test/runner-test.js b/lighthouse-core/test/runner-test.js
index 4f765d65c..f266ed7f3 100644
--- a/lighthouse-core/test/runner-test.js
+++ b/lighthouse-core/test/runner-test.js
@@ -36,6 +36,13 @@ describe('Runner', () => {
     resetSpies();
   });
 
+  const basicAuditMeta = {
+    id: 'test-audit',
+    title: 'A test audit',
+    description: 'An audit for testing',
+    requiredArtifacts: [],
+  };
+
   describe('Gather Mode & Audit Mode', () => {
     const url = 'https://example.com';
     const generateConfig = settings => new Config({
@@ -546,6 +553,58 @@ describe('Runner', () => {
     });
   });
 
+  it('includes any LighthouseRunWarnings from audits in LHR', () => {
+    const warningString = 'Really important audit warning!';
+
+    const config = new Config({
+      settings: {
+        auditMode: __dirname + '/fixtures/artifacts/empty-artifacts/',
+      },
+      audits: [
+        class WarningAudit extends Audit {
+          static get meta() {
+            return basicAuditMeta;
+          }
+          static audit(artifacts, context) {
+            context.LighthouseRunWarnings.push(warningString);
+            return {
+              rawValue: 5,
+            };
+          }
+        },
+      ],
+    });
+
+    return Runner.run(null, {config, driverMock}).then(results => {
+      assert.deepStrictEqual(results.lhr.runWarnings, [warningString]);
+    });
+  });
+
+  it('includes any LighthouseRunWarnings from errored audits in LHR', () => {
+    const warningString = 'Audit warning just before a terrible error!';
+
+    const config = new Config({
+      settings: {
+        auditMode: __dirname + '/fixtures/artifacts/empty-artifacts/',
+      },
+      audits: [
+        class WarningAudit extends Audit {
+          static get meta() {
+            return basicAuditMeta;
+          }
+          static audit(artifacts, context) {
+            context.LighthouseRunWarnings.push(warningString);
+            throw new Error('Terrible.');
+          }
+        },
+      ],
+    });
+
+    return Runner.run(null, {config, driverMock}).then(results => {
+      assert.deepStrictEqual(results.lhr.runWarnings, [warningString]);
+    });
+  });
+
   it('can handle array of outputs', async () => {
     const url = 'https://example.com';
     const config = new Config({