Merge pull request #7031 from edx/anton/video-fix-flaky-js-tests

[TNL-439]: Fix flaky jasmine tests.

common/lib/xmodule/xmodule/js/spec/video/video_context_menu_spec.js

@@ -217,43 +217,30 @@
                 expect(menuSubmenuItem).not.toHaveClass('is-opened');
             });
 
-            // Flaky-test resulting in timeout errors. Disabled 09/18/2014
-            // See TNL-439
-            xit('mouse left/right-clicking behaves as expected on play/pause menu item', function () {
+            it('mouse left/right-clicking behaves as expected on play/pause menu item', function () {
                 var menuItem = menuItems.first();
-                runs(function () {
-                    // Left-click on play
-                    menuItem.click();
+                spyOn(state.videoPlayer, 'play').andCallFake(function () {
+                    state.videoControl.isPlaying = true;
+                    state.el.trigger('play');
                 });
-
-                waitsFor(function () {
-                    return state.videoPlayer.isPlaying();
-                }, 'video to start playing', 200);
-
-                runs(function () {
-                    expect(menuItem).toHaveText('Pause');
-                    openMenu();
-                    // Left-click on pause
-                    menuItem.click();
-                });
-
-                waitsFor(function () {
-                    return !state.videoPlayer.isPlaying();
-                }, 'video to start playing', 200);
-
-                runs(function () {
-                    expect(menuItem).toHaveText('Play');
-                    // Right-click on play
-                    menuItem.trigger('contextmenu');
-                });
-
-                waitsFor(function () {
-                    return state.videoPlayer.isPlaying();
-                }, 'video to start playing', 200);
-
-                runs(function () {
-                    expect(menuItem).toHaveText('Pause');
+                spyOn(state.videoPlayer, 'pause').andCallFake(function () {
+                    state.videoControl.isPlaying = false;
+                    state.el.trigger('pause');
                 });
+                // Left-click on play
+                menuItem.click();
+                expect(state.videoPlayer.play).toHaveBeenCalled();
+                expect(menuItem).toHaveText('Pause');
+                openMenu();
+                // Left-click on pause
+                menuItem.click();
+                expect(state.videoPlayer.pause).toHaveBeenCalled();
+                expect(menuItem).toHaveText('Play');
+                state.videoPlayer.play.reset();
+                // Right-click on play
+                menuItem.trigger('contextmenu');
+                expect(state.videoPlayer.play).toHaveBeenCalled();
+                expect(menuItem).toHaveText('Pause');
             });
 
             it('mouse left/right-clicking behaves as expected on mute/unmute menu item', function () {

common/lib/xmodule/xmodule/js/spec/video/video_player_spec.js

@@ -367,6 +367,8 @@ function (VideoPlayer) {
             beforeEach(function () {
                 state = jasmine.initializePlayer();
                 state.videoEl = $('video, iframe');
+                jasmine.Clock.useMock();
+                spyOn(state.videoPlayer, 'duration').andReturn(120);
             });
 
             describe('when the video is playing', function () {
@@ -376,9 +378,7 @@ function (VideoPlayer) {
                     });
 
                     waitsFor(function () {
-                        var duration = state.videoPlayer.duration();
-
-                        return duration > 0 && state.videoPlayer.isPlaying();
+                        return state.videoPlayer.isPlaying();
                     }, 'video didn\'t start playing', WAIT_TIMEOUT);
                 });
 
@@ -388,148 +388,103 @@ function (VideoPlayer) {
                         spyOn(state.videoPlayer, 'stopTimer');
                         spyOn(state.videoPlayer, 'runTimer');
                         state.videoPlayer.seekTo(10);
-                    });
-
-                    waitsFor(function () {
-                        return state.videoPlayer.currentTime >= 10;
-                    }, 'currentTime is less than 10 seconds', WAIT_TIMEOUT);
-
-                    runs(function () {
-                        expect(state.videoPlayer.stopTimer)
-                            .toHaveBeenCalled();
-                        expect(state.videoPlayer.runTimer)
-                            .toHaveBeenCalled();
+                        // Video player uses _.debounce (with a wait time in 300 ms) for seeking.
+                        // That's why we have to do this tick(300).
+                        jasmine.Clock.tick(300);
+                        expect(state.videoPlayer.currentTime).toBe(10);
+                        expect(state.videoPlayer.stopTimer).toHaveBeenCalled();
+                        expect(state.videoPlayer.runTimer).toHaveBeenCalled();
                     });
                 });
 
-                // as per TNL-439 this test is deemed flaky and needs to be fixed.
-                // disabled 09/18/2014
-                xit('slider event causes log update', function () {
+                it('slider event causes log update', function () {
                     runs(function () {
                         spyOn(state.videoPlayer, 'log');
                         state.videoProgressSlider.onSlide(
                             jQuery.Event('slide'), { value: 2 }
                         );
-                    });
-
-                    waitsFor(function () {
-                        return state.videoPlayer.currentTime >= 2;
-                    }, 'currentTime is less than 2 seconds', WAIT_TIMEOUT);
-
-                    runs(function () {
-                        // Depending on the browser, the object of arrays may list the
-                        // arrays in a different order. Find the array that is relevent
-                        // to onSeek. Fail if that is not found.
-                        var seekVideoArgIndex
-                        for(var i = 0; i < state.videoPlayer.log.calls.length; i++){
-                            if (state.videoPlayer.log.calls[i].args[0] == 'seek_video') {
-                                seekVideoArgIndex = i
-                                break;
-                            }
-                        }
-
-                        expect(seekVideoArgIndex).toBeDefined;
-
-                        var args = state.videoPlayer.log.calls[seekVideoArgIndex].args;
-
-                        expect(args[1].old_time).toBeLessThan(2);
-                        expect(args[1].new_time).toBe(2);
-                        expect(args[1].type).toBe('onSlideSeek');
+                        // Video player uses _.debounce (with a wait time in 300 ms) for seeking.
+                        // That's why we have to do this tick(300).
+                        jasmine.Clock.tick(300);
+                        expect(state.videoPlayer.currentTime).toBe(2);
+
+                        expect(state.videoPlayer.log).toHaveBeenCalledWith('seek_video', {
+                            old_time: jasmine.any(Number),
+                            new_time: 2,
+                            type: 'onSlideSeek'
+                        });
                     });
                 });
 
-                // as per TNL-439 this test is deemed flaky and needs to be fixed.
-                // disabled 09/18/2014
-                xit('seek the player', function () {
+                it('seek the player', function () {
                     runs(function () {
-                        spyOn(state.videoPlayer.player, 'seekTo')
-                            .andCallThrough();
+                        spyOn(state.videoPlayer.player, 'seekTo').andCallThrough();
                         state.videoProgressSlider.onSlide(
                             jQuery.Event('slide'), { value: 30 }
                         );
-                    });
-
-                    waitsFor(function () {
-                        return state.videoPlayer.currentTime >= 30;
-                    }, 'currentTime is less than 30 seconds', WAIT_TIMEOUT);
-
-                    runs(function () {
-                        expect(state.videoPlayer.player.seekTo)
-                            .toHaveBeenCalledWith(30, true);
+                        // Video player uses _.debounce (with a wait time in 300 ms) for seeking.
+                        // That's why we have to do this tick(300).
+                        jasmine.Clock.tick(300);
+                        expect(state.videoPlayer.currentTime).toBe(30);
+                        expect(state.videoPlayer.player.seekTo).toHaveBeenCalledWith(30, true);
                     });
                 });
 
-                // as per TNL-439 this test is deemed flaky and needs to be fixed.
-                // disabled 09/18/2014
-                xit('call updatePlayTime on player', function () {
+                it('call updatePlayTime on player', function () {
                     runs(function () {
-                        spyOn(state.videoPlayer, 'updatePlayTime')
-                            .andCallThrough();
+                        spyOn(state.videoPlayer, 'updatePlayTime').andCallThrough();
                         state.videoProgressSlider.onSlide(
                             jQuery.Event('slide'), { value: 30 }
                         );
-                    });
-
-                    waitsFor(function () {
-                        return state.videoPlayer.currentTime >= 30;
-                    }, 'currentTime is less than 30 seconds', WAIT_TIMEOUT);
-
-                    runs(function () {
-                        expect(state.videoPlayer.updatePlayTime)
-                            .toHaveBeenCalledWith(30, true);
+                        // Video player uses _.debounce (with a wait time in 300 ms) for seeking.
+                        // That's why we have to do this tick(300).
+                        jasmine.Clock.tick(300);
+                        expect(state.videoPlayer.currentTime).toBe(30);
+                        expect(state.videoPlayer.updatePlayTime).toHaveBeenCalledWith(30, true);
                     });
                 });
             });
 
-            // Disabled 10/25/13 due to flakiness in master
-            xit(
-                'when the player is not playing: set the current time',
-                function ()
-            {
-                runs(function () {
-                    state.videoProgressSlider.onSlide(
-                        jQuery.Event('slide'), { value: 20 }
-                    );
-                    state.videoPlayer.pause();
-                    state.videoProgressSlider.onSlide(
-                        jQuery.Event('slide'), { value: 10 }
-                    );
-
-                    waitsFor(function () {
-                        return Math.round(state.videoPlayer.currentTime) === 10;
-                    }, 'currentTime got updated', 10000);
-                });
+            it('when the player is not playing: set the current time', function () {
+                state.videoProgressSlider.onSlide(
+                    jQuery.Event('slide'), { value: 20 }
+                );
+                // Video player uses _.debounce (with a wait time in 300 ms) for seeking.
+                // That's why we have to do this tick(300).
+                jasmine.Clock.tick(300);
+                state.videoPlayer.pause();
+                expect(state.videoPlayer.currentTime).toBe(20);
+                state.videoProgressSlider.onSlide(
+                    jQuery.Event('slide'), { value: 10 }
+                );
+                // Video player uses _.debounce (with a wait time in 300 ms) for seeking.
+                // That's why we have to do this tick(300).
+                jasmine.Clock.tick(300);
+                expect(state.videoPlayer.currentTime).toBe(10);
             });
 
-            // as per TNL-439 these tests are deemed flaky and needs to be fixed.
-            // disabled 09/18/2014
-            xdescribe('when the video is not playing', function () {
+            describe('when the video is not playing', function () {
                 beforeEach(function () {
                     spyOn(state.videoPlayer, 'setPlaybackRate')
                         .andCallThrough();
                 });
 
                 it('slider event causes log update', function () {
-                    runs(function () {
-                        spyOn(state.videoPlayer, 'log');
-                        state.videoProgressSlider.onSlide(
-                            jQuery.Event('slide'), { value: 2 }
-                        );
-                    });
-
-                    waitsFor(function () {
-                        return state.videoPlayer.currentTime >= 2;
-                    }, 'currentTime is less than 2 seconds', WAIT_TIMEOUT);
-
-                    runs(function () {
-                        expect(state.videoPlayer.log).toHaveBeenCalledWith(
-                            'seek_video', {
-                                old_time: 0,
-                                new_time: 2,
-                                type: 'onSlideSeek'
-                            }
-                        );
-                    });
+                    spyOn(state.videoPlayer, 'log');
+                    state.videoProgressSlider.onSlide(
+                        jQuery.Event('slide'), { value: 2 }
+                    );
+                    // Video player uses _.debounce (with a wait time in 300 ms) for seeking.
+                    // That's why we have to do this tick(300).
+                    jasmine.Clock.tick(300);
+                    expect(state.videoPlayer.currentTime).toBe(2);
+                    expect(state.videoPlayer.log).toHaveBeenCalledWith(
+                        'seek_video', {
+                            old_time: 0,
+                            new_time: 2,
+                            type: 'onSlideSeek'
+                        }
+                    );
                 });
 
                 it('video has a correct speed', function () {