Merge pull request #104 from MailOnline/fix/QA_fixes

fix: 🐜 destroy video ad container on run finish
MailOnline · Nov 27, 2018 · 8c9d5b3 · 8c9d5b3
2 parents ff1cff7 + 7ffc9a0
commit 8c9d5b3
Show file tree

Hide file tree

Showing 2 changed files with 29 additions and 0 deletions.
diff --git a/src/runner/__tests__/run.spec.js b/src/runner/__tests__/run.spec.js
@@ -66,6 +66,31 @@ describe('run', () => {
     expect(startVideoAd).toHaveBeenCalledWith(vastAdChain, adContainer, options);
   });
 
+  test('must destroy the ad container on adUnit finish', async () => {
+    adContainer.destroy = jest.fn();
+
+    expect(await run(vastAdChain, placeholder, options)).toBe(adUnit);
+    expect(adContainer.destroy).toHaveBeenCalledTimes(0);
+
+    adUnit.cancel();
+    expect(adContainer.destroy).toHaveBeenCalledTimes(1);
+  });
+
+  test('must propagate adUnit creation errors', async () => {
+    expect.assertions(1);
+    const testError = new Error('Ad container creation error');
+
+    createVideoAdContainer.mockImplementation(() => {
+      throw testError;
+    });
+
+    try {
+      await run(vastAdChain, placeholder, options);
+    } catch (error) {
+      expect(error).toBe(testError);
+    }
+  });
+
   test('must destroy the adContainer if there is a problem starting the adUnit', async () => {
     const adUnitError = new Error('boom');
 

diff --git a/src/runner/run.js b/src/runner/run.js
@@ -69,6 +69,10 @@ const run = async (vastChain, placeholder, options) => {
 
     const adUnit = await adUnitPromise;
 
+    adUnit.onFinish(() => {
+      videoAdContainer.destroy();
+    });
+
     return adUnit;
   } catch (error) {
     if (videoAdContainer) {