From e50d8fe503d8b97f48fc07ca82cfc8a26363129e Mon Sep 17 00:00:00 2001 From: Greg Smith Date: Wed, 21 Feb 2018 11:43:10 -0500 Subject: [PATCH] test code review changes --- test/states/abstract/test.AdState.js | 10 +++++----- test/states/abstract/test.ContentState.js | 7 ++++--- test/states/test.BeforePreroll.js | 14 +++++++------- test/states/test.ContentPlayback.js | 22 +++++++++++----------- test/states/test.Postroll.js | 20 +++++++++++--------- test/states/test.Preroll.js | 22 ++++++++++++---------- 6 files changed, 50 insertions(+), 45 deletions(-) diff --git a/test/states/abstract/test.AdState.js b/test/states/abstract/test.AdState.js index 6e42dc65..1e9edf2f 100644 --- a/test/states/abstract/test.AdState.js +++ b/test/states/abstract/test.AdState.js @@ -14,7 +14,7 @@ QUnit.module('AdState', { this.adState = new AdState(this.player); this.adState.transitionTo = (newState) => { - this.transitionTo = newState.name; + this.newState = newState.name; }; } }); @@ -30,23 +30,23 @@ QUnit.test('is an ad state', function(assert) { QUnit.test('transitions to ContentPlayback on playing if content resuming', function(assert) { this.adState.contentResuming = true; this.adState.onPlaying(); - assert.equal(this.transitionTo, 'ContentPlayback'); + assert.equal(this.newState, 'ContentPlayback'); }); QUnit.test('doesn\'t transition on playing if content not resuming', function(assert) { this.adState.onPlaying(); - assert.equal(this.transitionTo, undefined, 'no transition'); + assert.equal(this.newState, undefined, 'no transition'); }); QUnit.test('transitions to ContentPlayback on contentresumed if content resuming', function(assert) { this.adState.contentResuming = true; this.adState.onContentResumed(); - assert.equal(this.transitionTo, 'ContentPlayback'); + assert.equal(this.newState, 'ContentPlayback'); }); QUnit.test('doesn\'t transition on contentresumed if content not resuming', function(assert) { this.adState.onContentResumed(); - assert.equal(this.transitionTo, undefined, 'no transition'); + assert.equal(this.newState, undefined, 'no transition'); }); QUnit.test('can check if content is resuming', function(assert) { diff --git a/test/states/abstract/test.ContentState.js b/test/states/abstract/test.ContentState.js index 8b2a3be4..7f0a18a7 100644 --- a/test/states/abstract/test.ContentState.js +++ b/test/states/abstract/test.ContentState.js @@ -16,7 +16,7 @@ QUnit.module('ContentState', { this.contentState = new ContentState(this.player); this.contentState.transitionTo = (newState) => { - this.transitionTo = newState.name; + this.newState = newState.name; }; } }); @@ -30,8 +30,9 @@ QUnit.test('handles content changed when not playing', function(assert) { this.player.pause = sinon.stub(); this.contentState.onContentChanged(this.player); - assert.equal(this.transitionTo, 'BeforePreroll'); + assert.equal(this.newState, 'BeforePreroll'); assert.equal(this.player.pause.callCount, 0, 'did not pause player'); + assert.ok(!this.player.ads._pausedOnContentupdate, 'did not set _pausedOnContentupdate'); }); QUnit.test('handles content changed when playing', function(assert) { @@ -39,7 +40,7 @@ QUnit.test('handles content changed when playing', function(assert) { this.player.pause = sinon.stub(); this.contentState.onContentChanged(this.player); - assert.equal(this.transitionTo, 'Preroll'); + assert.equal(this.newState, 'Preroll'); assert.equal(this.player.pause.callCount, 1, 'paused player'); assert.equal(this.player.ads._pausedOnContentupdate, true, 'set _pausedOnContentupdate'); }); diff --git a/test/states/test.BeforePreroll.js b/test/states/test.BeforePreroll.js index 18576ed6..5b10f6f9 100644 --- a/test/states/test.BeforePreroll.js +++ b/test/states/test.BeforePreroll.js @@ -22,7 +22,7 @@ QUnit.module('BeforePreroll', { this.beforePreroll = new BeforePreroll(this.player); this.beforePreroll.transitionTo = (newState, arg) => { - this.transitionTo = newState.name; + this.newState = newState.name; this.transitionArg = arg; }; @@ -40,7 +40,7 @@ QUnit.test('transitions to Preroll (adsready first)', function(assert) { this.beforePreroll.onAdsReady(this.player); assert.equal(this.beforePreroll.adsReady, true); this.beforePreroll.onPlay(this.player); - assert.equal(this.transitionTo, 'Preroll'); + assert.equal(this.newState, 'Preroll'); assert.equal(this.transitionArg, true); }); @@ -48,31 +48,31 @@ QUnit.test('transitions to Preroll (play first)', function(assert) { this.beforePreroll.init(); assert.equal(this.beforePreroll.adsReady, false); this.beforePreroll.onPlay(this.player); - assert.equal(this.transitionTo, 'Preroll'); + assert.equal(this.newState, 'Preroll'); assert.equal(this.transitionArg, false); }); QUnit.test('cancels ads', function(assert) { this.beforePreroll.init(); this.beforePreroll.onAdsCanceled(this.player); - assert.equal(this.transitionTo, 'ContentPlayback'); + assert.equal(this.newState, 'ContentPlayback'); }); QUnit.test('transitions to content playback on error', function(assert) { this.beforePreroll.init(); this.beforePreroll.onAdsError(this.player); - assert.equal(this.transitionTo, 'ContentPlayback'); + assert.equal(this.newState, 'ContentPlayback'); }); QUnit.test('skips the preroll', function(assert) { this.beforePreroll.init(); this.beforePreroll.skipLinearAdMode(); assert.equal(this.events[0], 'adskip'); - assert.equal(this.transitionTo, 'ContentPlayback'); + assert.equal(this.newState, 'ContentPlayback'); }); QUnit.test('does nothing on content change', function(assert) { this.beforePreroll.init(); this.beforePreroll.onContentChanged(this.player); - assert.equal(this.transitionTo, undefined); + assert.equal(this.newState, undefined); }); diff --git a/test/states/test.ContentPlayback.js b/test/states/test.ContentPlayback.js index 9e5c72ce..d8189bf6 100644 --- a/test/states/test.ContentPlayback.js +++ b/test/states/test.ContentPlayback.js @@ -8,12 +8,12 @@ import {ContentPlayback} from '../../src/states.js'; QUnit.module('ContentPlayback', { beforeEach: function() { this.events = []; - this.played = false; + this.playTriggered = false; this.player = { paused: () => false, play: () => { - this.played = true; + this.playTriggered = true; }, trigger: (event) => { this.events.push(event); @@ -25,7 +25,7 @@ QUnit.module('ContentPlayback', { this.contentPlayback = new ContentPlayback(this.player); this.contentPlayback.transitionTo = (newState) => { - this.transitionTo = newState.name; + this.newState = newState.name; }; } }); @@ -35,37 +35,37 @@ QUnit.test('only plays on init on correct conditions', function(assert) { this.player.ads._cancelledPlay = false; this.player.ads._pausedOnContentupdate = false; this.contentPlayback.init(this.player); - assert.equal(this.played, false); + assert.equal(this.playTriggered, false); this.player.paused = () => true; this.player.ads._cancelledPlay = false; this.player.ads._pausedOnContentupdate = false; this.contentPlayback.init(this.player); - assert.equal(this.played, false); + assert.equal(this.playTriggered, false); this.player.paused = () => false; this.player.ads._cancelledPlay = true; this.player.ads._pausedOnContentupdate = false; this.contentPlayback.init(this.player); - assert.equal(this.played, false); + assert.equal(this.playTriggered, false); this.player.paused = () => false; this.player.ads._cancelledPlay = false; this.player.ads._pausedOnContentupdate = true; this.contentPlayback.init(this.player); - assert.equal(this.played, false); + assert.equal(this.playTriggered, false); this.player.paused = () => true; this.player.ads._cancelledPlay = true; this.player.ads._pausedOnContentupdate = false; this.contentPlayback.init(this.player); - assert.equal(this.played, true); + assert.equal(this.playTriggered, true); this.player.paused = () => true; this.player.ads._cancelledPlay = false; this.player.ads._pausedOnContentupdate = true; this.contentPlayback.init(this.player); - assert.equal(this.played, true); + assert.equal(this.playTriggered, true); }); QUnit.test('adsready triggers readyforpreroll', function(assert) { @@ -84,11 +84,11 @@ QUnit.test('no readyforpreroll if _nopreroll', function(assert) { QUnit.test('transitions to Postroll on contentended', function(assert) { this.contentPlayback.init(this.player, false); this.contentPlayback.onContentEnded(this.player); - assert.equal(this.transitionTo, 'Postroll', 'transitioned to Postroll'); + assert.equal(this.newState, 'Postroll', 'transitioned to Postroll'); }); QUnit.test('transitions to Midroll on startlinearadmode', function(assert) { this.contentPlayback.init(this.player, false); this.contentPlayback.startLinearAdMode(); - assert.equal(this.transitionTo, 'Midroll', 'transitioned to Midroll'); + assert.equal(this.newState, 'Midroll', 'transitioned to Midroll'); }); diff --git a/test/states/test.Postroll.js b/test/states/test.Postroll.js index 6c8afab8..a10270e2 100644 --- a/test/states/test.Postroll.js +++ b/test/states/test.Postroll.js @@ -29,7 +29,7 @@ QUnit.module('Postroll', { this.postroll = new Postroll(this.player); this.postroll.transitionTo = (newState) => { - this.transitionTo = newState.name; + this.newState = newState.name; }; this.adBreakStartStub = sinon.stub(adBreak, 'start'); @@ -51,6 +51,7 @@ QUnit.test('startLinearAdMode starts ad break', function(assert) { this.postroll.init(this.player); this.postroll.startLinearAdMode(); assert.equal(this.adBreakStartStub.callCount, 1, 'ad break started'); + assert.equal(this.player.ads.adType, 'postroll', 'ad type is postroll'); }); QUnit.test('removes ad loading class on ad started', function(assert) { @@ -77,59 +78,60 @@ QUnit.test('no endLinearAdMode on adserror if not in ad break', function(assert) this.player.ads.inAdBreak = () => false; this.postroll.onAdsError(this.player); assert.equal(this.player.ads.endLinearAdMode.callCount, 0, 'linear ad mode ended'); + assert.equal(this.events[0], 'ended', 'saw ended event'); }); QUnit.test('does not transition to AdsDone unless content resuming', function(assert) { this.postroll.init(this.player); this.postroll.onEnded(this.player); - assert.equal(this.transitionTo, undefined, 'no transition'); + assert.equal(this.newState, undefined, 'no transition'); }); QUnit.test('transitions to AdsDone on ended', function(assert) { this.postroll.isContentResuming = () => true; this.postroll.init(this.player); this.postroll.onEnded(this.player); - assert.equal(this.transitionTo, 'AdsDone'); + assert.equal(this.newState, 'AdsDone'); }); QUnit.test('transitions to BeforePreroll on content changed after ad break', function(assert) { this.postroll.isContentResuming = () => true; this.postroll.init(this.player); this.postroll.onContentChanged(this.player); - assert.equal(this.transitionTo, 'BeforePreroll'); + assert.equal(this.newState, 'BeforePreroll'); }); QUnit.test('transitions to Preroll on content changed before ad break', function(assert) { this.postroll.init(this.player); this.postroll.onContentChanged(this.player); - assert.equal(this.transitionTo, 'Preroll'); + assert.equal(this.newState, 'Preroll'); }); QUnit.test('doesn\'t transition on content changed during ad break', function(assert) { this.postroll.inAdBreak = () => true; this.postroll.init(this.player); this.postroll.onContentChanged(this.player); - assert.equal(this.transitionTo, undefined, 'no transition'); + assert.equal(this.newState, undefined, 'no transition'); }); QUnit.test('transitions to AdsDone on nopostroll before ad break', function(assert) { this.postroll.init(this.player); this.postroll.onNoPostroll(this.player); - assert.equal(this.transitionTo, 'AdsDone'); + assert.equal(this.newState, 'AdsDone'); }); QUnit.test('no transition on nopostroll during ad break', function(assert) { this.postroll.inAdBreak = () => true; this.postroll.init(this.player); this.postroll.onNoPostroll(this.player); - assert.equal(this.transitionTo, undefined, 'no transition'); + assert.equal(this.newState, undefined, 'no transition'); }); QUnit.test('no transition on nopostroll after ad break', function(assert) { this.postroll.isContentResuming = () => true; this.postroll.init(this.player); this.postroll.onNoPostroll(this.player); - assert.equal(this.transitionTo, undefined, 'no transition'); + assert.equal(this.newState, undefined, 'no transition'); }); QUnit.test('can abort', function(assert) { diff --git a/test/states/test.Preroll.js b/test/states/test.Preroll.js index 74967516..b5631513 100644 --- a/test/states/test.Preroll.js +++ b/test/states/test.Preroll.js @@ -31,7 +31,7 @@ QUnit.module('Preroll', { this.preroll = new Preroll(this.player); this.preroll.transitionTo = (newState, arg) => { - this.transitionTo = newState.name; + this.newState = newState.name; this.transitionArg = arg; }; @@ -56,7 +56,8 @@ QUnit.test('plays a preroll (adsready true)', function(assert) { assert.equal(this.preroll.inAdBreak(), false, 'not in ad break'); this.preroll.startLinearAdMode(); - this.player.ads._inLinearAdMode = true; // Because adBreak.start is mocked. + // Because adBreak.start is mocked. + this.player.ads._inLinearAdMode = true; assert.equal(this.adBreakStartStub.callCount, 1, 'ad break started'); assert.equal(this.player.ads.adType, 'preroll', 'adType is preroll'); assert.equal(this.preroll.isContentResuming(), false, 'content not resuming'); @@ -67,7 +68,7 @@ QUnit.test('plays a preroll (adsready true)', function(assert) { assert.equal(this.preroll.isContentResuming(), true, 'content resuming'); this.preroll.onPlaying(); - assert.equal(this.transitionTo, 'ContentPlayback', 'transitioned to ContentPlayback'); + assert.equal(this.newState, 'ContentPlayback', 'transitioned to ContentPlayback'); }); QUnit.test('plays a preroll (adsready false)', function(assert) { @@ -80,7 +81,8 @@ QUnit.test('plays a preroll (adsready false)', function(assert) { assert.equal(this.preroll.inAdBreak(), false, 'not in ad break'); this.preroll.startLinearAdMode(); - this.player.ads._inLinearAdMode = true; // Because adBreak.start is mocked. + // Because adBreak.start is mocked. + this.player.ads._inLinearAdMode = true; assert.equal(this.adBreakStartStub.callCount, 1, 'ad break started'); assert.equal(this.player.ads.adType, 'preroll', 'adType is preroll'); assert.equal(this.preroll.isContentResuming(), false, 'content not resuming'); @@ -91,37 +93,37 @@ QUnit.test('plays a preroll (adsready false)', function(assert) { assert.equal(this.preroll.isContentResuming(), true, 'content resuming'); this.preroll.onPlaying(); - assert.equal(this.transitionTo, 'ContentPlayback', 'transitioned to ContentPlayback'); + assert.equal(this.newState, 'ContentPlayback', 'transitioned to ContentPlayback'); }); QUnit.test('can handle nopreroll event', function(assert) { this.preroll.init(this.player, false); this.preroll.onNoPreroll(this.player); - assert.equal(this.transitionTo, 'ContentPlayback', 'transitioned to ContentPlayback'); + assert.equal(this.newState, 'ContentPlayback', 'transitioned to ContentPlayback'); }); QUnit.test('can handle adscanceled', function(assert) { this.preroll.init(this.player, false); this.preroll.onAdsCanceled(this.player); - assert.equal(this.transitionTo, 'ContentPlayback', 'transitioned to ContentPlayback'); + assert.equal(this.newState, 'ContentPlayback', 'transitioned to ContentPlayback'); }); QUnit.test('can handle adserror', function(assert) { this.preroll.init(this.player, false); this.preroll.onAdsError(this.player); - assert.equal(this.transitionTo, 'ContentPlayback', 'transitioned to ContentPlayback'); + assert.equal(this.newState, 'ContentPlayback', 'transitioned to ContentPlayback'); }); QUnit.test('can skip linear ad mode', function(assert) { this.preroll.init(this.player, false); this.preroll.skipLinearAdMode(); - assert.equal(this.transitionTo, 'ContentPlayback', 'transitioned to ContentPlayback'); + assert.equal(this.newState, 'ContentPlayback', 'transitioned to ContentPlayback'); }); QUnit.test('plays content after ad timeout', function(assert) { this.preroll.init(this.player, false); this.preroll.onAdTimeout(this.player); - assert.equal(this.transitionTo, 'ContentPlayback', 'transitioned to ContentPlayback'); + assert.equal(this.newState, 'ContentPlayback', 'transitioned to ContentPlayback'); }); QUnit.test('removes ad loading class on ads started', function(assert) {