From 76c95393bb1e7f8fbcaf9e7924f5150c0a1f685e Mon Sep 17 00:00:00 2001 From: Tyoma Deev Date: Tue, 4 Jun 2019 22:27:37 +0300 Subject: [PATCH 1/3] Add import statement for removeElement function --- extensions/amp-insticator/0.1/amp-insticator.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/extensions/amp-insticator/0.1/amp-insticator.js b/extensions/amp-insticator/0.1/amp-insticator.js index 3f83be80a107..4e6a1b652544 100644 --- a/extensions/amp-insticator/0.1/amp-insticator.js +++ b/extensions/amp-insticator/0.1/amp-insticator.js @@ -19,8 +19,7 @@ /* ++++++++++ --------------- IMPORTS --------------- ++++++++++ */ import { Layout } from '../../../src/layout'; import { setStyles } from '../../../src/style'; - - +import {removeElement} from '../../../src/dom'; /* ++++++++++ --------------- EXPORT AMP-INSTICATOR --------------- ++++++++++ */ export class AmpInsticator extends AMP.BaseElement { From bf4ddd365a4c7a341ddc0741b0622070b99262f0 Mon Sep 17 00:00:00 2001 From: Tyoma Deev Date: Tue, 4 Jun 2019 22:28:57 +0300 Subject: [PATCH 2/3] Add unit tests for amp-insticator component --- .../0.1/test/test-amp-insticator.js | 55 ++++++++++++++++--- 1 file changed, 47 insertions(+), 8 deletions(-) diff --git a/extensions/amp-insticator/0.1/test/test-amp-insticator.js b/extensions/amp-insticator/0.1/test/test-amp-insticator.js index ba8ac7024721..8be95b6b6159 100644 --- a/extensions/amp-insticator/0.1/test/test-amp-insticator.js +++ b/extensions/amp-insticator/0.1/test/test-amp-insticator.js @@ -14,7 +14,7 @@ * limitations under the License. */ -import '../amp-insticator'; +import {AmpInsticator} from '../amp-insticator'; describes.realWin('amp-insticator', { amp: { @@ -23,16 +23,55 @@ describes.realWin('amp-insticator', { }, env => { let win; - let element; + let doc; + let ampInsticator; + let layoutCallbackSpy; + let getRequestSpy; - beforeEach(() => { + function getAmpInsticatior() { + const ampInsticator = doc.createElement('amp-insticator'); + ampInsticator.setAttribute('data-embed-id', '5bf0548f-a332-4934-9d35-c9ca9e93d4ff') + doc.body.appendChild(ampInsticator); + + //Seting up spys + getRequestSpy = env.sandbox.spy(ampInsticator.implementation_, 'getRequest'); + + return ampInsticator.build() + .then( () => ampInsticator.layoutCallback() ) + .then( () => ampInsticator ); + } + + beforeEach(async function() { win = env.win; - element = win.document.createElement('amp-insticator'); - win.document.body.appendChild(element); + doc = win.document; + ampInsticator = await getAmpInsticatior(); + }); + + it('renders', function() { + expect(ampInsticator).to.not.be.undefined; + expect(ampInsticator).to.not.be.null; + const iframe = ampInsticator.querySelector('iframe'); + console.log(iframe.contentWindow.document.head ? 'there is a head' : 'there is nothing in the iframe'); + expect(iframe).to.not.be.undefined; + expect(iframe).to.not.be.null; + }); + + it('makes a request for ads data was made correctly', function() { + //we'll read the url property from the real instance + const obj = ampInsticator.implementation_; + expect(getRequestSpy).to.be.calledOnce; + const argumentsPassedToGetRequest = getRequestSpy.args[0]; + const requestedUrl = argumentsPassedToGetRequest[0]; + expect(requestedUrl).to.include(obj.url.ads); + expect(argumentsPassedToGetRequest[1]).to.be.a('function'); }); - it('should have hello world when built', () => { - element.build(); - expect(element.querySelector('div').textContent).to.equal('hello world'); + it('removes iframe on unlayoutCallback()', function() { + const iframe = ampInsticator.querySelector('iframe'); + expect(iframe).to.not.be.undefined; + expect(iframe).to.not.be.null; + const obj = ampInsticator.implementation_; + obj.unlayoutCallback(); + expect(ampInsticator.querySelector('iframe')).to.be.null; }); }); From 018bd395d9c080281c78de9c3c88e3fb11d6ada8 Mon Sep 17 00:00:00 2001 From: Tyoma Deev Date: Wed, 5 Jun 2019 12:58:50 +0300 Subject: [PATCH 3/3] Rewrite beforEach, render and getRequest tests * Now the AMP class instance is created in beforeEach, no need to set it up in each test * The 'renders' is now a separate block with 3 tests * Inspection of spy's args in getRequest()'s test conforms sinon's best practices --- .../0.1/test/test-amp-insticator.js | 42 ++++++++++++------- 1 file changed, 26 insertions(+), 16 deletions(-) diff --git a/extensions/amp-insticator/0.1/test/test-amp-insticator.js b/extensions/amp-insticator/0.1/test/test-amp-insticator.js index 8be95b6b6159..9ebcea452a18 100644 --- a/extensions/amp-insticator/0.1/test/test-amp-insticator.js +++ b/extensions/amp-insticator/0.1/test/test-amp-insticator.js @@ -25,6 +25,7 @@ describes.realWin('amp-insticator', { let win; let doc; let ampInsticator; + let obj; let layoutCallbackSpy; let getRequestSpy; @@ -33,7 +34,7 @@ describes.realWin('amp-insticator', { ampInsticator.setAttribute('data-embed-id', '5bf0548f-a332-4934-9d35-c9ca9e93d4ff') doc.body.appendChild(ampInsticator); - //Seting up spys + //Seting up spys and fakes getRequestSpy = env.sandbox.spy(ampInsticator.implementation_, 'getRequest'); return ampInsticator.build() @@ -45,32 +46,41 @@ describes.realWin('amp-insticator', { win = env.win; doc = win.document; ampInsticator = await getAmpInsticatior(); + obj = ampInsticator.implementation_; }); - it('renders', function() { - expect(ampInsticator).to.not.be.undefined; - expect(ampInsticator).to.not.be.null; - const iframe = ampInsticator.querySelector('iframe'); - console.log(iframe.contentWindow.document.head ? 'there is a head' : 'there is nothing in the iframe'); - expect(iframe).to.not.be.undefined; - expect(iframe).to.not.be.null; + describe('renders', function renders(){ + it('renders an iframe', function() { + expect(ampInsticator).to.not.be.undefined; + expect(ampInsticator).to.not.be.null; + const iframe = ampInsticator.querySelector('iframe'); + expect(iframe).to.not.be.undefined; + expect(iframe).to.not.be.null; + }); + it("appends Insticator's script to iframe", function(){ + const iframe = ampInsticator.querySelector('iframe'); + const scriptTagInIframe = iframe.contentWindow.document.querySelector('script'); + expect(scriptTagInIframe).to.not.be.undefined; + expect(scriptTagInIframe).to.not.be.null; + expect(scriptTagInIframe.src).to.include(obj.url.embed); + }); + it("appends div#app to iframe", function(){ + const iframe = ampInsticator.querySelector('iframe'); + const divApp = iframe.contentWindow.document.getElementById('app'); + expect(divApp).to.not.be.undefined; + expect(divApp).to.not.be.null; + }); }); - it('makes a request for ads data was made correctly', function() { - //we'll read the url property from the real instance - const obj = ampInsticator.implementation_; + it('makes a request for ads data correctly', function() { expect(getRequestSpy).to.be.calledOnce; - const argumentsPassedToGetRequest = getRequestSpy.args[0]; - const requestedUrl = argumentsPassedToGetRequest[0]; - expect(requestedUrl).to.include(obj.url.ads); - expect(argumentsPassedToGetRequest[1]).to.be.a('function'); + expect(getRequestSpy.calledWith(sinon.match(obj.url.ads), sinon.match.func)).to.be.true; }); it('removes iframe on unlayoutCallback()', function() { const iframe = ampInsticator.querySelector('iframe'); expect(iframe).to.not.be.undefined; expect(iframe).to.not.be.null; - const obj = ampInsticator.implementation_; obj.unlayoutCallback(); expect(ampInsticator.querySelector('iframe')).to.be.null; });