Merge branch 'master' into feat/dialog-default-action

.travis.yml

@@ -23,6 +23,11 @@ install:
   # Noisy output, but useful for debugging npm package dependency version issues:
   #- npm ls
 
+after_script:
+  # Prevent Travis job logs from getting truncated early.
+  # See https://github.com/travis-ci/travis-ci/issues/6421#issuecomment-238137337
+  - sleep 5
+
 matrix:
   include:
 

packages/mdc-dialog/_variables.scss

@@ -35,3 +35,5 @@ $mdc-dialog-max-width: 560px;
 $mdc-dialog-margin: 16px;
 
 $mdc-dialog-corner-radius: 4px;
+
+$mdc-dialog-z-index: 7;

packages/mdc-dialog/mdc-dialog.scss

@@ -56,7 +56,7 @@
 
   // Use `display: none` instead of `visibility: hidden` to avoid recalculating layout when the dialog is closed.
   display: none;
-  z-index: 5;
+  z-index: $mdc-dialog-z-index;
 }
 
 .mdc-dialog__scrim {

scripts/travis-env-vars.sh

@@ -92,5 +92,5 @@ fi
 
 if [[ "$TEST_SUITE" == 'screenshot' ]]; then
   # Only run screenshot tests if package JS/Sass files, non-Markdown screenshot test files, or image files changed.
-  check_for_testable_files '^packages/.+\.(js|css|scss)$' '^test/screenshot/.+[^m][^d]$' '\.(png|jpg|jpeg|gif|svg)$'
+  check_for_testable_files '^packages/.+\.(js|css|scss)$' '^test/screenshot/' '\.(png|jpg|jpeg|gif|svg)$'
 fi

test/screenshot/golden.json

@@ -368,6 +368,15 @@
       "desktop_windows_ie@11": "https://storage.googleapis.com/mdc-web-screenshot-tests/rlfriedman/2018/09/13/20_54_11_470/spec/mdc-chips/mixins/trailing-icon-size.html.windows_ie_11.png"
     }
   },
+  "spec/mdc-dialog/classes/baseline-alert-above-drawer.html": {
+    "public_url": "https://storage.googleapis.com/mdc-web-screenshot-tests/travis/2018/09/20/18_53_58_822/spec/mdc-dialog/classes/baseline-alert-above-drawer.html?utm_source=golden_json",
+    "screenshots": {
+      "desktop_windows_chrome@latest": "https://storage.googleapis.com/mdc-web-screenshot-tests/travis/2018/09/20/18_53_58_822/spec/mdc-dialog/classes/baseline-alert-above-drawer.html.windows_chrome_69.png",
+      "desktop_windows_edge@latest": "https://storage.googleapis.com/mdc-web-screenshot-tests/travis/2018/09/20/18_53_58_822/spec/mdc-dialog/classes/baseline-alert-above-drawer.html.windows_edge_17.png",
+      "desktop_windows_firefox@latest": "https://storage.googleapis.com/mdc-web-screenshot-tests/travis/2018/09/20/18_53_58_822/spec/mdc-dialog/classes/baseline-alert-above-drawer.html.windows_firefox_62.png",
+      "desktop_windows_ie@11": "https://storage.googleapis.com/mdc-web-screenshot-tests/travis/2018/09/20/18_53_58_822/spec/mdc-dialog/classes/baseline-alert-above-drawer.html.windows_ie_11.png"
+    }
+  },
   "spec/mdc-dialog/classes/baseline-alert-with-title.html": {
     "public_url": "https://storage.googleapis.com/mdc-web-screenshot-tests/travis/2018/09/18/18_18_22_548/spec/mdc-dialog/classes/baseline-alert-with-title.html?utm_source=golden_json",
     "screenshots": {
@@ -422,11 +431,11 @@
     }
   },
   "spec/mdc-dialog/classes/overflow-bottom.html": {
-    "public_url": "https://storage.googleapis.com/mdc-web-screenshot-tests/travis/2018/09/18/18_18_22_548/spec/mdc-dialog/classes/overflow-bottom.html?utm_source=golden_json",
+    "public_url": "https://storage.googleapis.com/mdc-web-screenshot-tests/travis/2018/09/21/11_20_28_699/spec/mdc-dialog/classes/overflow-bottom.html?utm_source=golden_json",
     "screenshots": {
       "desktop_windows_chrome@latest": "https://storage.googleapis.com/mdc-web-screenshot-tests/travis/2018/09/18/18_18_22_548/spec/mdc-dialog/classes/overflow-bottom.html.windows_chrome_69.png",
       "desktop_windows_edge@latest": "https://storage.googleapis.com/mdc-web-screenshot-tests/travis/2018/09/17/15_33_59_538/spec/mdc-dialog/classes/overflow-bottom.html.windows_edge_17.png",
-      "desktop_windows_firefox@latest": "https://storage.googleapis.com/mdc-web-screenshot-tests/travis/2018/09/18/18_18_22_548/spec/mdc-dialog/classes/overflow-bottom.html.windows_firefox_62.png"
+      "desktop_windows_firefox@latest": "https://storage.googleapis.com/mdc-web-screenshot-tests/travis/2018/09/21/11_20_28_699/spec/mdc-dialog/classes/overflow-bottom.html.windows_firefox_62.png"
     }
   },
   "spec/mdc-dialog/classes/overflow-top.html": {

test/screenshot/infra/commands/test.js

@@ -26,7 +26,7 @@
 const VError = require('verror');
 
 const mdcProto = require('../proto/mdc.pb').mdc.proto;
-// const GitRevision = mdcProto.GitRevision;
+const GitRevision = mdcProto.GitRevision;
 const InclusionType = mdcProto.Screenshot.InclusionType;
 const ReportData = mdcProto.ReportData;
 
@@ -67,9 +67,9 @@ class TestCommand {
 
     /** @type {!mdc.proto.DiffBase} */
     const snapshotDiffBase = await this.diffBaseParser_.parseGoldenDiffBase();
-    // const snapshotGitRev = snapshotDiffBase.git_revision;
+    const snapshotGitRev = snapshotDiffBase.git_revision;
 
-    // const isTravisPr = snapshotGitRev && snapshotGitRev.type === GitRevision.Type.TRAVIS_PR;
+    const isTravisPr = snapshotGitRev && snapshotGitRev.type === GitRevision.Type.TRAVIS_PR;
     const shouldExit = process.env.HAS_TESTABLE_FILES === 'false';
 
     if (shouldExit) {
@@ -86,15 +86,14 @@ class TestCommand {
       return localExitCode;
     }
 
-    // Temporarily disabled, see https://github.com/material-components/material-components-web/issues/3555
-    // if (isTravisPr) {
-    //   /** @type {!mdc.proto.ReportData} */
-    //   const masterReportData = await this.diffAgainstMaster_({localReportData, snapshotGitRev});
-    //   this.logTestResults_(localReportData);
-    //   this.logTestResults_(masterReportData);
-    // } else {
-    this.logTestResults_(localReportData);
-    // }
+    if (isTravisPr) {
+      /** @type {!mdc.proto.ReportData} */
+      const masterReportData = await this.diffAgainstMaster_({localReportData, snapshotGitRev});
+      this.logTestResults_(localReportData);
+      this.logTestResults_(masterReportData);
+    } else {
+      this.logTestResults_(localReportData);
+    }
 
     // Diffs against master shouldn't fail the Travis job.
     return ExitCode.OK;
@@ -223,7 +222,7 @@ class TestCommand {
     const plural = num === 1 ? '' : 's';
     this.logger_.foldStart('screenshot.compare_master', `Comparing ${num} screenshot${plural} to master`);
 
-    const promises = [];
+    const comparisonFunctions = [];
     const masterScreenshotSets = masterReportData.screenshots;
     const masterScreenshotList = masterScreenshotSets.actual_screenshot_list;
 
@@ -240,7 +239,7 @@ class TestCommand {
           continue;
         }
 
-        promises.push(new Promise(async (resolve) => {
+        comparisonFunctions.push(async (resolve) => {
           masterScreenshot.actual_html_file = capturedScreenshot.actual_html_file;
           masterScreenshot.actual_image_file = capturedScreenshot.actual_image_file;
           masterScreenshot.capture_state = capturedScreenshot.capture_state;
@@ -268,17 +267,28 @@ class TestCommand {
           }
 
           resolve();
-        }));
+        });
       }
     }
 
-    await Promise.all(promises);
+    // When there are a lot of images (~400 or more), comparing all of them in parallel causes timeouts and OOMs.
+    // To avoid this, we compare images in batches of 50 at a time.
+    // See https://github.com/material-components/material-components-web/issues/3555
+    let startIndex = 0;
+    while (comparisonFunctions.length > 0) {
+      const promises = comparisonFunctions.splice(0, 50).map((comparisonFunction) => new Promise(comparisonFunction));
+      const endIndex = startIndex + promises.length - 1;
+      this.logger_.debug(`Comparing screenshots ${startIndex + 1}–${endIndex + 1}...`);
+      await Promise.all(promises);
+      startIndex += promises.length;
+    }
 
     const endTimeIsoUtc = new Date().toISOString();
     masterReportData.meta.start_time_iso_utc = startTimeIsoUtc;
     masterReportData.meta.end_time_iso_utc = endTimeIsoUtc;
     masterReportData.meta.duration_ms = Duration.elapsed(startTimeIsoUtc, endTimeIsoUtc).toMillis();
 
+    this.logger_.log('');
     this.logger_.foldEnd('screenshot.compare_master');
   }
 

test/screenshot/infra/lib/file-cache.js

@@ -21,6 +21,7 @@
  * THE SOFTWARE.
  */
 
+const VError = require('verror');
 const mkdirp = require('mkdirp');
 const os = require('os');
 const path = require('path');
@@ -49,12 +50,22 @@ class FileCache {
     this.localStorage_ = new LocalStorage();
   }
 
+  /**
+   * @param {string} uri
+   * @return {string}
+   */
+  getAbsolutePath(uri) {
+    const fileName = this.getFilename_(uri);
+    return path.resolve(this.tempDirPath_, fileName);
+  }
+
   /**
    * @param {string} uri Public URI or local file path.
    * @param {?string=} encoding 'utf8' for text, or `null` for binary data.
+   * @param {boolean=} download
    * @return {!Promise<!mdc.proto.TestFile>} Local copy of the file pointed to by `uri`.
    */
-  async downloadUrlToDisk(uri, encoding = null) {
+  async getFile({uri, encoding = null, download = true}) {
     mkdirp.sync(this.tempDirPath_);
 
     // TODO(acdvorak): Document this hack
@@ -68,8 +79,7 @@ class FileCache {
       });
     }
 
-    const fileName = this.getFilename_(uri);
-    const filePath = path.resolve(this.tempDirPath_, fileName);
+    const filePath = this.getAbsolutePath(uri);
     if (await this.localStorage_.exists(filePath)) {
       return TestFile.create({
         absolute_path: filePath,
@@ -78,15 +88,23 @@ class FileCache {
       });
     }
 
-    const buffer = await request({uri, encoding});
-    await this.localStorage_.writeBinaryFile(filePath, buffer, encoding)
-      .catch(async (err) => {
-        console.error(`downloadUrlToDisk("${uri}"):`);
-        console.error(err);
-        if (await this.localStorage_.exists(filePath)) {
-          await this.localStorage_.delete(filePath);
-        }
-      });
+    if (download) {
+      let buffer;
+      try {
+        buffer = await request({uri, encoding});
+      } catch (err) {
+        throw new VError(err, `Failed to download ${uri} with encoding ${encoding}!`);
+      }
+
+      await this.localStorage_.writeBinaryFile(filePath, buffer, encoding)
+        .catch(async (err) => {
+          console.error(`downloadUrlToDisk("${uri}"):`);
+          console.error(err);
+          if (await this.localStorage_.exists(filePath)) {
+            await this.localStorage_.delete(filePath);
+          }
+        });
+    }
 
     return TestFile.create({
       absolute_path: filePath,
@@ -100,9 +118,9 @@ class FileCache {
    * @param {?string=} encoding 'utf8' for text, or `null` for binary data.
    * @return {!Promise<!Buffer>} Buffer containing the contents of the file pointed to by `uri`.
    */
-  async downloadFileToBuffer(uri, encoding = null) {
+  async getBuffer({uri, encoding = null}) {
     /** @type {!mdc.proto.TestFile} */
-    const file = await this.downloadUrlToDisk(uri, encoding);
+    const file = await this.getFile({uri, encoding});
     return this.localStorage_.readBinaryFile(file.absolute_path, encoding);
   }
 

test/screenshot/infra/lib/report-builder.js

@@ -128,7 +128,7 @@ class ReportBuilder {
    */
   async initForApproval({runReportJsonUrl}) {
     /** @type {!mdc.proto.TestFile} */
-    const runReportJsonFile = await this.fileCache_.downloadUrlToDisk(runReportJsonUrl, 'utf8');
+    const runReportJsonFile = await this.fileCache_.getFile({uri: runReportJsonUrl, encoding: 'utf8'});
     /** @type {!mdc.proto.ReportData} */
     const reportData = ReportData.fromObject(require(runReportJsonFile.absolute_path));
     reportData.approvals = Approvals.create();
@@ -357,7 +357,7 @@ class ReportBuilder {
     await Promise.all(
       urls
         .filter((url) => Boolean(url))
-        .map((url) => this.fileCache_.downloadUrlToDisk(url))
+        .map((url) => this.fileCache_.getFile({uri: url}))
     );
   }
 
@@ -659,10 +659,6 @@ class ReportBuilder {
    * @private
    */
   async getExpectedScreenshots_(goldenFile) {
-    const downloadFileAndGetPath = async (url) => {
-      return (await this.fileCache_.downloadUrlToDisk(url)).absolute_path;
-    };
-
     /** @type {!Array<!mdc.proto.Screenshot>} */
     const expectedScreenshots = [];
     const goldenScreenshots = goldenFile.getGoldenScreenshots();
@@ -673,12 +669,12 @@ class ReportBuilder {
         expected_html_file: TestFile.create({
           public_url: goldenScreenshot.html_file_url,
           relative_path: goldenScreenshot.html_file_path,
-          absolute_path: await downloadFileAndGetPath(goldenScreenshot.html_file_url),
+          absolute_path: this.fileCache_.getAbsolutePath(goldenScreenshot.html_file_url),
         }),
         expected_image_file: TestFile.create({
           public_url: goldenScreenshot.screenshot_image_url,
           relative_path: goldenScreenshot.screenshot_image_path,
-          absolute_path: await downloadFileAndGetPath(goldenScreenshot.screenshot_image_url),
+          absolute_path: this.fileCache_.getAbsolutePath(goldenScreenshot.screenshot_image_url),
         }),
         user_agent: await this.userAgentStore_.getUserAgent(goldenScreenshot.user_agent_alias),
       });
@@ -723,7 +719,7 @@ class ReportBuilder {
         /** @type {?mdc.proto.TestFile} */
         const expectedImageFile =
           expectedScreenshotImageUrl
-            ? await this.fileCache_.downloadUrlToDisk(expectedScreenshotImageUrl)
+            ? await this.fileCache_.getFile({uri: expectedScreenshotImageUrl, download: false})
             : null;
 
         allScreenshots.push(Screenshot.create({
@@ -863,7 +859,7 @@ class ReportBuilder {
 
       if (!actualScreenshot) {
         const expectedImageUrl = expectedScreenshot.expected_image_file.public_url;
-        const expectedJimpImage = await Jimp.read(await this.fileCache_.downloadFileToBuffer(expectedImageUrl));
+        const expectedJimpImage = await Jimp.read(await this.fileCache_.getBuffer({uri: expectedImageUrl}));
         const {width, height} = expectedJimpImage.bitmap;
         expectedScreenshot.diff_image_result = DiffImageResult.create({
           expected_image_dimensions: Dimensions.create({width, height}),

test/screenshot/infra/lib/selenium-api.js

@@ -782,7 +782,8 @@ class SeleniumApi {
     const fontLoadTimeoutMs = isOnline ? flakeConfig.font_face_observer_timeout_ms : 500;
 
     await driver.get(urlWithQsParams);
-    await driver.wait(until.elementLocated(By.css('[data-fonts-loaded]')), fontLoadTimeoutMs).catch(() => 0);
+    const domReadyCssSelector = '[data-fonts-loaded][data-animations-settled][data-dom-ready]';
+    await driver.wait(until.elementLocated(By.css(domReadyCssSelector)), fontLoadTimeoutMs).catch(() => {});
 
     const delayMs = flakeConfig.retry_delay_ms;
     if (delayMs > 0) {