✅ video-player-* unit tests: don't wait for iframe to fully load (#17316

)
ampproject · Aug 6, 2018 · 70e9cbb · 70e9cbb
1 parent 03b8e02
commit 70e9cbb
Show file tree

Hide file tree

Showing 7 changed files with 23 additions and 17 deletions.
diff --git a/extensions/amp-3q-player/0.1/test/test-amp-3q-player.js b/extensions/amp-3q-player/0.1/test/test-amp-3q-player.js
@@ -42,13 +42,12 @@ describes.realWin('amp-3q-player', {
     }
     doc.body.appendChild(player);
     return player.build().then(() => {
-      const layoutPromise = player.layoutCallback();
+      player.layoutCallback();
       const iframe = player.querySelector('iframe');
       player.implementation_.sdnBridge_({
         source: iframe.contentWindow,
         data: JSON.stringify({data: 'ready'}),
       });
-      return layoutPromise;
     }).then(() => {
       return player;
     });

diff --git a/extensions/amp-brid-player/0.1/test/test-amp-brid-player.js b/extensions/amp-brid-player/0.1/test/test-amp-brid-player.js
@@ -57,7 +57,7 @@ describes.realWin('amp-brid-player', {
       });
     });
     doc.body.appendChild(bc);
-    return bc.build().then(() => bc.layoutCallback()).then(() => bc);
+    return bc.build().then(() => { bc.layoutCallback(); }).then(() => bc);
   }
 
   it('renders', () => {

diff --git a/extensions/amp-dailymotion/0.1/test/test-amp-dailymotion.js b/extensions/amp-dailymotion/0.1/test/test-amp-dailymotion.js
@@ -43,7 +43,7 @@ describes.realWin('amp-dailymotion', {
     }
     doc.body.appendChild(dailymotion);
     return dailymotion.build().then(() => {
-      return dailymotion.layoutCallback();
+      dailymotion.layoutCallback();
     }).then(() => dailymotion);
   }
 

diff --git a/extensions/amp-nexxtv-player/0.1/test/test-amp-nexxtv-player.js b/extensions/amp-nexxtv-player/0.1/test/test-amp-nexxtv-player.js
@@ -44,7 +44,7 @@ describes.realWin('amp-nexxtv-player', {
     // see yt test implementation
     doc.body.appendChild(nexxtv);
     return nexxtv.build().then(() => {
-      return nexxtv.layoutCallback();
+      nexxtv.layoutCallback();
     }).then(() => {
       const nexxTimerIframe = nexxtv.querySelector('iframe');
       nexxtv.implementation_.handleNexxMessage_({

diff --git a/extensions/amp-reach-player/0.1/test/test-amp-reach-player.js b/extensions/amp-reach-player/0.1/test/test-amp-reach-player.js
@@ -40,7 +40,8 @@ describes.realWin('amp-reach-player', {
       reach.setAttribute('layout', 'responsive');
     }
     doc.body.appendChild(reach);
-    return reach.build().then(() => reach.layoutCallback()).then(() => reach);
+    return reach.build().then(() => { reach.layoutCallback(); })
+        .then(() => reach);
   }
 
   it('renders', () => {

diff --git a/extensions/amp-springboard-player/0.1/amp-springboard-player.js b/extensions/amp-springboard-player/0.1/amp-springboard-player.js
@@ -32,6 +32,12 @@ class AmpSpringboardPlayer extends AMP.BaseElement {
     /** @private {string} */
     this.domain_ = '';
 
+    /** @private {string} */
+    this.siteId_ = '';
+
+    /** @private {string} */
+    this.playerId_ = '';
+
     /** @private {?HTMLIFrameElement} */
     this.iframe_ = null;
   }
@@ -65,31 +71,31 @@ class AmpSpringboardPlayer extends AMP.BaseElement {
         this.element.getAttribute('data-domain'),
         'The data-domain attribute is required for <amp-springboard-player> %s',
         this.element);
-  }
-
-  /** @override */
-  layoutCallback() {
-    const iframe = this.element.ownerDocument.createElement('iframe');
-    const siteId = user().assert(
+    this.siteId_ = user().assert(
         this.element.getAttribute('data-site-id'),
         'The data-site-id attribute is required for' +
         '<amp-springboard-player> %s',
         this.element);
-    const playerId = user().assert(
+    this.playerId_ = user().assert(
         this.element.getAttribute('data-player-id'),
         'The data-player-id attribute is required for' +
         '<amp-springboard-player> %s',
         this.element);
+  }
+
+  /** @override */
+  layoutCallback() {
+    const iframe = this.element.ownerDocument.createElement('iframe');
     const items = this.element.getAttribute('data-items') || '10';
 
     iframe.setAttribute('frameborder', '0');
     iframe.setAttribute('allowfullscreen', 'true');
-    iframe.id = playerId + '_' + this.contentId_;
+    iframe.id = this.playerId_ + '_' + this.contentId_;
     iframe.src = 'https://cms.springboardplatform.com/embed_iframe/' +
-        encodeURIComponent(siteId) + '/' +
+        encodeURIComponent(this.siteId_) + '/' +
         encodeURIComponent(this.mode_) +
         '/' + encodeURIComponent(this.contentId_) + '/' +
-        encodeURIComponent(playerId) + '/' +
+        encodeURIComponent(this.playerId_) + '/' +
         encodeURIComponent(this.domain_) +
         '/' + encodeURIComponent(items);
     this.applyFillContent(iframe);

diff --git a/extensions/amp-springboard-player/0.1/test/test-amp-springboard-player.js b/extensions/amp-springboard-player/0.1/test/test-amp-springboard-player.js
@@ -38,7 +38,7 @@ describes.realWin('amp-springboard-player', {
     sp.setAttribute('height', '270');
     sp.setAttribute('layout', 'responsive');
     doc.body.appendChild(sp);
-    return sp.build().then(() => sp.layoutCallback()).then(() => sp);
+    return sp.build().then(() => { sp.layoutCallback(); }).then(() => sp);
   }
 
   it('renders', () => {