Disable headless mode, fix tests broken on Chrome 71 (#19658)

* Add TODO to remove BindEvents. * Revert DRE upgrade and use Justin's polyfill for FIE. * Disable headless mode. * Only force non-headless for integration tests on Travis. * Also run non-headless for single pass test.
ampproject · Dec 6, 2018 · e7957bb · e7957bb
1 parent f7a6b60
commit e7957bb
Show file tree

Hide file tree

Showing 7 changed files with 30 additions and 18 deletions.
diff --git a/build-system/pr-check.js b/build-system/pr-check.js
@@ -375,7 +375,9 @@ const command = {
     }
     if (process.env.TRAVIS) {
       if (coverage) {
-        timedExecOrDie(cmd + ' --headless --coverage');
+        // TODO(choumx, #19658): --headless disabled for integration tests on
+        // Travis until Chrome 72.
+        timedExecOrDie(cmd + ' --coverage');
       } else {
         startSauceConnect();
         timedExecOrDie(cmd + ' --saucelabs');
@@ -388,7 +390,9 @@ const command = {
   runSinglePassCompiledIntegrationTests: function() {
     timedExecOrDie('rm -R dist');
     timedExecOrDie('gulp dist --fortesting --single_pass --pseudo_names');
-    timedExecOrDie('gulp test --integration --nobuild --headless '
+    // TODO(choumx, #19658): --headless disabled for integration tests on
+    // Travis until Chrome 72.
+    timedExecOrDie('gulp test --integration --nobuild '
         + '--compiled --single_pass');
     timedExecOrDie('rm -R dist');
   },

diff --git a/build-system/tasks/karma.conf.js b/build-system/tasks/karma.conf.js
@@ -143,7 +143,9 @@ module.exports = {
     },
     Chrome_no_extensions_headless: {
       base: 'ChromeHeadless',
-      flags: ['--no-sandbox'].concat(COMMON_CHROME_FLAGS),
+      // https://developers.google.com/web/updates/2017/04/headless-chrome#frontend
+      flags: ['--no-sandbox --remote-debugging-port=9222']
+          .concat(COMMON_CHROME_FLAGS),
     },
     // SauceLabs configurations.
     // New configurations can be created here:

diff --git a/extensions/amp-bind/0.1/bind-events.js b/extensions/amp-bind/0.1/bind-events.js
@@ -15,10 +15,9 @@
  */
 
 /**
-* Enum used to specify Events for the Amp Bind extension
-*
-* @enum {string}
-*/
+ * TODO(choumx, #19657): Remove/replace with DOM polling in integration tests.
+ * @enum {string}
+ */
 export const BindEvents = {
   INITIALIZE: 'amp:bind:initialize',
   RESCAN_TEMPLATE: 'amp:bind:rescan-template',

diff --git a/extensions/amp-bind/0.1/test/integration/test-bind-impl.js b/extensions/amp-bind/0.1/test/integration/test-bind-impl.js
@@ -129,8 +129,7 @@ function waitForEvent(env, name) {
   });
 }
 
-// TODO(#19647): Unskip tests
-describe.configure().ifNewChrome().skip('Bind', function() {
+describe.configure().ifNewChrome().run('Bind', function() {
   // Give more than default 2000ms timeout for local testing.
   const TIMEOUT = Math.max(window.ampTestRuntimeConfig.mochaTimeout, 4000);
   this.timeout(TIMEOUT);

diff --git a/extensions/amp-list/0.1/test/integration/test-amp-list.js b/extensions/amp-list/0.1/test/integration/test-amp-list.js
@@ -103,7 +103,7 @@ describe('amp-list', function() {
       '</template>' +
     '</amp-list>';
 
-  // TODO(#19647): Unskip tests
+  // TODO(cathyxz, #19647): Fix test on Chrome 71.
   describes.integration.skip('with bindable is-layout-container', {
     body: body3, extensions,
     experiments: ['amp-list-resizable-children']},

diff --git a/src/service/extensions-impl.js b/src/service/extensions-impl.js
@@ -689,7 +689,7 @@ export function stubLegacyElements(win) {
 function installPolyfillsInChildWindow(parentWin, childWin) {
   installDocContains(childWin);
   installDOMTokenListToggle(childWin);
-  if (isExperimentOn(parentWin, 'custom-elements-v1')) {
+  if (isExperimentOn(parentWin, 'custom-elements-v1') || getMode().test) {
     installCustomElements(childWin);
   } else {
     installRegisterElement(childWin, 'auto');

diff --git a/test/integration/test-amp-bind.js b/test/integration/test-amp-bind.js
@@ -17,10 +17,9 @@ import {AmpEvents} from '../../src/amp-events';
 import {BindEvents} from '../../extensions/amp-bind/0.1/bind-events';
 import {FormEvents} from '../../extensions/amp-form/0.1/form-events';
 import {Services} from '../../src/services';
-import {createFixtureIframe} from '../../testing/iframe';
+import {createFixtureIframe, poll} from '../../testing/iframe';
 
-// TODO(#19647): Unskip tests
-describe.configure().ifNewChrome().skip('amp-bind', function() {
+describe.configure().ifNewChrome().run('amp-bind', function() {
   // Give more than default 2000ms timeout for local testing.
   const TIMEOUT = Math.max(window.ampTestRuntimeConfig.mochaTimeout, 4000);
   this.timeout(TIMEOUT);
@@ -50,12 +49,21 @@ describe.configure().ifNewChrome().skip('amp-bind', function() {
     return createFixtureIframe(fixtureLocation).then(f => {
       fixture = f;
       // Most fixtures have a single AMP element that will be laid out.
-      const loadStartsToExpect =
+      const numberOfAmpComponents =
           (opt_numberOfAmpElements === undefined) ? 1 : opt_numberOfAmpElements;
-      return Promise.all([
+      const promises = [
         fixture.awaitEvent(BindEvents.INITIALIZE, 1),
-        fixture.awaitEvent(AmpEvents.LOAD_START, loadStartsToExpect),
-      ]);
+      ];
+      if (numberOfAmpComponents > 0) {
+        promises.push(
+            poll('All AMP components are laid out', () => {
+              const laidOutElements =
+                  fixture.doc.querySelectorAll('.i-amphtml-layout').length;
+              return laidOutElements === numberOfAmpComponents;
+            })
+        );
+      }
+      return Promise.all(promises);
     });
   }