From a24a218452695f28ec7875a4205b22c36bc153ef Mon Sep 17 00:00:00 2001 From: Anton Stupak Date: Tue, 27 Aug 2013 18:10:07 +0300 Subject: [PATCH 1/4] Add hearbeats logging. --- .../xmodule/js/src/video/01_initialize.js | 25 ++++++++++--------- .../xmodule/js/src/video/03_video_player.js | 22 ++++++++++++++++ 2 files changed, 35 insertions(+), 12 deletions(-) diff --git a/common/lib/xmodule/xmodule/js/src/video/01_initialize.js b/common/lib/xmodule/xmodule/js/src/video/01_initialize.js index 79bc16dbdad7..41c08e3fd13d 100644 --- a/common/lib/xmodule/xmodule/js/src/video/01_initialize.js +++ b/common/lib/xmodule/xmodule/js/src/video/01_initialize.js @@ -214,22 +214,23 @@ function (VideoPlayer) { this.config = { element: element, - start: this.el.data('start'), - end: this.el.data('end'), + start: this.el.data('start'), + end: this.el.data('end'), - caption_data_dir: this.el.data('caption-data-dir'), - caption_asset_path: this.el.data('caption-asset-path'), - show_captions: (this.el.data('show-captions').toString().toLowerCase() === 'true'), - youtubeStreams: this.el.data('streams'), + caption_data_dir: this.el.data('caption-data-dir'), + caption_asset_path: this.el.data('caption-asset-path'), + show_captions: (this.el.data('show-captions').toString().toLowerCase() === 'true'), + youtubeStreams: this.el.data('streams'), - sub: this.el.data('sub'), - mp4Source: this.el.data('mp4-source'), - webmSource: this.el.data('webm-source'), - oggSource: this.el.data('ogg-source'), + sub: this.el.data('sub'), + mp4Source: this.el.data('mp4-source'), + webmSource: this.el.data('webm-source'), + oggSource: this.el.data('ogg-source'), - fadeOutTimeout: 1400, + fadeOutTimeout: 1400, - availableQualities: ['hd720', 'hd1080', 'highres'] + availableQualities: ['hd720', 'hd1080', 'highres'], + heartbeatsLoggingDelay: 30*1000 }; if (!(_parseYouTubeIDs(this))) { diff --git a/common/lib/xmodule/xmodule/js/src/video/03_video_player.js b/common/lib/xmodule/xmodule/js/src/video/03_video_player.js index 25617283f59a..2fbb100f72b5 100644 --- a/common/lib/xmodule/xmodule/js/src/video/03_video_player.js +++ b/common/lib/xmodule/xmodule/js/src/video/03_video_player.js @@ -154,6 +154,24 @@ function (HTML5Video) { }); } + function _startHeartbeatLogging(state) { + if (!state.videoPlayer.heartbeatsInterval) { + state.videoPlayer.heartbeatsInterval = window.setInterval( + _.bind(state.videoPlayer.log, state, 'is_video_playing'), + state.config.heartbeatsLoggingDelay + ); + } + } + + function _stopHeartbeatLogging(state) { + window.clearInterval(state.videoPlayer.heartbeatsInterval); + } + + function _restartHeartbeatLogging(state) { + _stopHeartbeatLogging(); + _startHeartbeatLogging(); + } + // *************************************************************** // Public functions start here. // These are available via the 'state' object. Their context ('this' keyword) is the 'state' object. @@ -244,6 +262,7 @@ function (HTML5Video) { if (this.videoPlayer.isPlaying()) { clearInterval(this.videoPlayer.updateInterval); this.videoPlayer.updateInterval = setInterval(this.videoPlayer.update, 200); + _restartHeartbeatLogging(this); } else { this.videoPlayer.currentTime = params.time; } @@ -267,6 +286,7 @@ function (HTML5Video) { } ); + _stopHeartbeatLogging(this); clearInterval(this.videoPlayer.updateInterval); delete this.videoPlayer.updateInterval; @@ -285,6 +305,8 @@ function (HTML5Video) { } ); + _startHeartbeatLogging(this); + if (!this.videoPlayer.updateInterval) { this.videoPlayer.updateInterval = setInterval(this.videoPlayer.update, 200); } From 5337cf72b95bb7e591f75eb5e564ba6c453fc90d Mon Sep 17 00:00:00 2001 From: Anton Stupak Date: Wed, 28 Aug 2013 10:01:33 +0300 Subject: [PATCH 2/4] Add tests. --- .../js/spec/video/video_player_spec.js | 53 +++++++++++++++++++ 1 file changed, 53 insertions(+) diff --git a/common/lib/xmodule/xmodule/js/spec/video/video_player_spec.js b/common/lib/xmodule/xmodule/js/spec/video/video_player_spec.js index 7beb2957da9b..4cfb1ee042a0 100644 --- a/common/lib/xmodule/xmodule/js/spec/video/video_player_spec.js +++ b/common/lib/xmodule/xmodule/js/spec/video/video_player_spec.js @@ -179,6 +179,59 @@ }); }); + describe('Check heartbeats logging', function() { + beforeEach(function() { + initialize(); + spyOn(videoPlayer, 'log').andCallThrough(); + state.config.heartbeatsLoggingDelay = 1000; + + videoPlayer.onStateChange({ + data: YT.PlayerState.PLAYING + }); + }); + + function initHeartbets(expectation, waitsForTime) { + var runsTimer = waitsForTime - 100, + flag; + + runs(function() { + flag = false; + var timer = window.setTimeout(function(){ + flag = true; + }, runsTimer); + }); + + waitsFor(function() { + return flag; + }, "The Flag should be True", waitsForTime); + + runs(expectation); + + } + + it('heartbeat log called twice', function() { + initHeartbets(function() { + var heartbeatsLogsArray = videoPlayer.log.calls.slice(-2); + + $.each(heartbeatsLogsArray, function(index, log){ + expect(log.args[0]).toEqual('is_video_playing'); + }); + }, 2200); + }); + + it('heartbeat log did\'t called after pausing', function() { + videoPlayer.onStateChange({ + data: YT.PlayerState.PAUSED + }); + + initHeartbets(function() { + var log = videoPlayer.log.mostRecentCall.args[0]; + expect(log).not.toEqual('is_video_playing'); + }, 1200); + }); + + }); + describe('when the video is playing', function() { var oldState; From c633223c85c19df34ff9c70003edf349599f64e5 Mon Sep 17 00:00:00 2001 From: Anton Stupak Date: Wed, 28 Aug 2013 10:06:07 +0300 Subject: [PATCH 3/4] Fix setTimeout in tests. --- .../combinedopenended/display_spec.coffee | 24 ++++++++++++++----- .../js/spec/video/video_caption_spec.js | 5 ++-- .../spec/video/video_progress_slider_spec.js | 1 + 3 files changed, 21 insertions(+), 9 deletions(-) diff --git a/common/lib/xmodule/xmodule/js/spec/combinedopenended/display_spec.coffee b/common/lib/xmodule/xmodule/js/spec/combinedopenended/display_spec.coffee index 293d6405ada2..6547182ef51c 100644 --- a/common/lib/xmodule/xmodule/js/spec/combinedopenended/display_spec.coffee +++ b/common/lib/xmodule/xmodule/js/spec/combinedopenended/display_spec.coffee @@ -48,17 +48,23 @@ describe 'CombinedOpenEnded', -> expect(@combined.task_count).toEqual 2 expect(@combined.task_number).toEqual 1 - it 'subelements are made collapsible', -> + it 'subelements are made collapsible', -> expect(Collapsible.setCollapsibles).toHaveBeenCalled() describe 'poll', -> + nativeSetTimeout = null + beforeEach => + nativeSetTimeout = window.setTimeout # setup the spies @combined = new CombinedOpenEnded @element spyOn(@combined, 'reload').andCallFake -> return 0 window.setTimeout = jasmine.createSpy().andCallFake (callback, timeout) -> return 5 + afterEach => + window.setTimeout = nativeSetTimeout + it 'polls at the correct intervals', => fakeResponseContinue = state: 'not done' spyOn($, 'postWithPrefix').andCallFake (url, callback) -> callback(fakeResponseContinue) @@ -67,19 +73,25 @@ describe 'CombinedOpenEnded', -> expect(window.queuePollerID).toBe(5) it 'polling stops properly', => - fakeResponseDone = state: "done" + fakeResponseDone = state: "done" spyOn($, 'postWithPrefix').andCallFake (url, callback) -> callback(fakeResponseDone) @combined.poll() expect(window.queuePollerID).toBeUndefined() expect(window.setTimeout).not.toHaveBeenCalled() describe 'rebind', -> + nativeSetTimeout = null + beforeEach -> + nativeSetTimeout = window.setTimeout @combined = new CombinedOpenEnded @element spyOn(@combined, 'queueing').andCallFake -> return 0 spyOn(@combined, 'skip_post_assessment').andCallFake -> return 0 window.setTimeout = jasmine.createSpy().andCallFake (callback, timeout) -> return 5 + afterEach => + window.setTimeout = nativeSetTimeout + it 'when our child is in an assessing state', -> @combined.child_state = 'assessing' @combined.rebind() @@ -87,19 +99,19 @@ describe 'CombinedOpenEnded', -> expect(@combined.submit_button.val()).toBe("Submit assessment") expect(@combined.queueing).toHaveBeenCalled() - it 'when our child state is initial', -> + it 'when our child state is initial', -> @combined.child_state = 'initial' @combined.rebind() expect(@combined.answer_area.attr("disabled")).toBeUndefined() expect(@combined.submit_button.val()).toBe("Submit") - it 'when our child state is post_assessment', -> + it 'when our child state is post_assessment', -> @combined.child_state = 'post_assessment' @combined.rebind() expect(@combined.answer_area.attr("disabled")).toBe("disabled") expect(@combined.submit_button.val()).toBe("Submit post-assessment") - it 'when our child state is done', -> + it 'when our child state is done', -> spyOn(@combined, 'next_problem').andCallFake -> @combined.child_state = 'done' @combined.rebind() @@ -112,7 +124,7 @@ describe 'CombinedOpenEnded', -> @combined.child_state = 'done' it 'handling a successful call', -> - fakeResponse = + fakeResponse = success: true html: "dummy html" allow_reset: false diff --git a/common/lib/xmodule/xmodule/js/spec/video/video_caption_spec.js b/common/lib/xmodule/xmodule/js/spec/video/video_caption_spec.js index 9458b483dabc..b9f1330d9316 100644 --- a/common/lib/xmodule/xmodule/js/spec/video/video_caption_spec.js +++ b/common/lib/xmodule/xmodule/js/spec/video/video_caption_spec.js @@ -13,6 +13,7 @@ beforeEach(function() { oldOTBD = window.onTouchBasedDevice; + spyOn(window, 'setTimeout').andReturn(100); window.onTouchBasedDevice = jasmine.createSpy('onTouchBasedDevice').andReturn(false); initialize(); }); @@ -130,8 +131,6 @@ describe('mouse movement', function() { beforeEach(function() { - window.setTimeout = jasmine.createSpy().andCallFake(function(callback, timeout) { return 5; }) - window.setTimeout.andReturn(100); spyOn(window, 'clearTimeout'); }); @@ -446,7 +445,7 @@ }); // Temporarily disabled due to intermittent failures - // Fails with error: "InvalidStateError: An attempt was made to + // Fails with error: "InvalidStateError: An attempt was made to // use an object that is not, or is no longer, usable // Expected 0 to equal 14.91." // on Firefox diff --git a/common/lib/xmodule/xmodule/js/spec/video/video_progress_slider_spec.js b/common/lib/xmodule/xmodule/js/spec/video/video_progress_slider_spec.js index 09a74e8e2564..2a3f45151088 100644 --- a/common/lib/xmodule/xmodule/js/spec/video/video_progress_slider_spec.js +++ b/common/lib/xmodule/xmodule/js/spec/video/video_progress_slider_spec.js @@ -146,6 +146,7 @@ describe('onStop', function() { beforeEach(function() { + spyOn(window, 'setTimeout'); initialize(); spyOn(videoPlayer, 'onSlideSeek').andCallThrough(); videoProgressSlider.onStop({}, { From 69a6c28a29aaacc0e9663284004949fa2306e0d5 Mon Sep 17 00:00:00 2001 From: Anton Stupak Date: Wed, 28 Aug 2013 15:42:26 +0300 Subject: [PATCH 4/4] Add comments. --- .../xmodule/xmodule/js/src/video/03_video_player.js | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/common/lib/xmodule/xmodule/js/src/video/03_video_player.js b/common/lib/xmodule/xmodule/js/src/video/03_video_player.js index 2fbb100f72b5..159901fc5bd0 100644 --- a/common/lib/xmodule/xmodule/js/src/video/03_video_player.js +++ b/common/lib/xmodule/xmodule/js/src/video/03_video_player.js @@ -154,6 +154,11 @@ function (HTML5Video) { }); } + // Start heartbeats logging. + // Send logs to the server each state.config.heartbeatsLoggingDelay seconds + // and provide an indication of whether students still have a video playing + // on their screen so that we have a better idea of whether they are getting + // the educational benefit of watching the lecture. function _startHeartbeatLogging(state) { if (!state.videoPlayer.heartbeatsInterval) { state.videoPlayer.heartbeatsInterval = window.setInterval( @@ -163,15 +168,11 @@ function (HTML5Video) { } } + // Stop heartbeats logging. function _stopHeartbeatLogging(state) { window.clearInterval(state.videoPlayer.heartbeatsInterval); } - function _restartHeartbeatLogging(state) { - _stopHeartbeatLogging(); - _startHeartbeatLogging(); - } - // *************************************************************** // Public functions start here. // These are available via the 'state' object. Their context ('this' keyword) is the 'state' object. @@ -262,7 +263,6 @@ function (HTML5Video) { if (this.videoPlayer.isPlaying()) { clearInterval(this.videoPlayer.updateInterval); this.videoPlayer.updateInterval = setInterval(this.videoPlayer.update, 200); - _restartHeartbeatLogging(this); } else { this.videoPlayer.currentTime = params.time; }