From 68e241405335b5288a25b7b613cde45d884c90b2 Mon Sep 17 00:00:00 2001 From: Usman Khalid <2200617@gmail.com> Date: Thu, 28 Apr 2016 17:17:46 +0500 Subject: [PATCH 1/3] Use triggerHandler instead of trigger to bypass Karma's global error handler. Karma has a global event handler for events named "error" which expects the event data to be a string and so fails when this event is triggered. So we use triggerHandler which only invokes handlers attached via jQuery. --- .../lib/xmodule/xmodule/js/spec/video/video_bumper_spec.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/common/lib/xmodule/xmodule/js/spec/video/video_bumper_spec.js b/common/lib/xmodule/xmodule/js/spec/video/video_bumper_spec.js index 7e7436f74b..f6db02602d 100644 --- a/common/lib/xmodule/xmodule/js/spec/video/video_bumper_spec.js +++ b/common/lib/xmodule/xmodule/js/spec/video/video_bumper_spec.js @@ -1,6 +1,6 @@ (function (WAIT_TIMEOUT) { 'use strict'; - xdescribe('VideoBumper', function () { + describe('VideoBumper', function () { var state, oldOTBD, waitForPlaying; waitForPlaying = function (state, done) { @@ -36,7 +36,7 @@ }); it('can show the main video on error', function (done) { - state.el.trigger('error'); + state.el.triggerHandler('error'); jasmine.clock().tick(20); expect($('.is-bumper')).not.toExist(); waitForPlaying(state, done); @@ -85,7 +85,7 @@ it('can save appropriate states correctly on error', function () { var saveState = jasmine.createSpy('saveState'); state.bumperState.videoSaveStatePlugin.saveState = saveState; - state.el.trigger('error'); + state.el.triggerHandler('error'); expect(state.storage.getItem('isBumperShown')).toBeTruthy(); jasmine.clock().tick(20); expect(saveState).toHaveBeenCalledWith(true, { From 8fef4148a5f3866e3676c4daf836c195db33a83d Mon Sep 17 00:00:00 2001 From: Usman Khalid <2200617@gmail.com> Date: Mon, 2 May 2016 20:52:22 +0500 Subject: [PATCH 2/3] Re-enabled videolist_spec, message_manager_spec and file_uploader_spec. --- cms/static/coffee/spec/main.coffee | 6 +++--- .../js/spec/video/transcripts/file_uploader_spec.js | 12 ++++-------- .../spec/video/transcripts/message_manager_spec.js | 13 ++++++------- .../js/views/video/transcripts/file_uploader.js | 3 ++- 4 files changed, 15 insertions(+), 19 deletions(-) diff --git a/cms/static/coffee/spec/main.coffee b/cms/static/coffee/spec/main.coffee index d5d9980b47..f761fbb0e9 100644 --- a/cms/static/coffee/spec/main.coffee +++ b/cms/static/coffee/spec/main.coffee @@ -231,9 +231,9 @@ testFiles = [ "coffee/spec/views/upload_spec", "js/spec/video/transcripts/utils_spec", "js/spec/video/transcripts/editor_spec", -# "js/spec/video/transcripts/videolist_spec", -# "js/spec/video/transcripts/message_manager_spec", -# "js/spec/video/transcripts/file_uploader_spec", + "js/spec/video/transcripts/videolist_spec", + "js/spec/video/transcripts/message_manager_spec", + "js/spec/video/transcripts/file_uploader_spec", "js/spec/models/component_template_spec", "js/spec/models/explicit_url_spec", "js/spec/models/xblock_info_spec", diff --git a/cms/static/js/spec/video/transcripts/file_uploader_spec.js b/cms/static/js/spec/video/transcripts/file_uploader_spec.js index 41f159ad1a..fd899c76f8 100644 --- a/cms/static/js/spec/video/transcripts/file_uploader_spec.js +++ b/cms/static/js/spec/video/transcripts/file_uploader_spec.js @@ -5,8 +5,9 @@ define( "xmodule", "jquery.form" ], function ($, _, Utils, FileUploader) { - // TODO: fix TNL-559 Intermittent failures of Transcript FileUploader JS tests - xdescribe('Transcripts.FileUploader', function () { + 'use strict'; + + describe('Transcripts.FileUploader', function () { var videoListEntryTemplate = readFixtures( 'video/transcripts/metadata-videolist-entry.underscore' ), @@ -67,10 +68,9 @@ function ($, _, Utils, FileUploader) { it('Template doesn\'t exist', function () { spyOn(console, 'error'); view.uploadTpl = ''; - view.render(); - expect(console.error).toHaveBeenCalled(); expect(view.render).not.toThrow(); + expect(console.error).toHaveBeenCalled(); expect(_.template).not.toHaveBeenCalled(); }); @@ -78,8 +78,6 @@ function ($, _, Utils, FileUploader) { function () { $('.transcripts-file-uploader').remove(); - view.render(); - expect(view.render).not.toThrow(); expect(_.template).not.toHaveBeenCalled(); } @@ -93,8 +91,6 @@ function ($, _, Utils, FileUploader) { }).join(', '); view.validFileExtensions = validFileExtensions; - view.render(); - expect(view.render).not.toThrow(); expect(_.template).toHaveBeenCalled(); $.each(elList, function(index, el) { diff --git a/cms/static/js/spec/video/transcripts/message_manager_spec.js b/cms/static/js/spec/video/transcripts/message_manager_spec.js index 2324f6871f..4019a44aea 100644 --- a/cms/static/js/spec/video/transcripts/message_manager_spec.js +++ b/cms/static/js/spec/video/transcripts/message_manager_spec.js @@ -6,9 +6,9 @@ define( "xmodule" ], function ($, _, Utils, MessageManager, FileUploader, sinon) { + 'use strict'; - // TODO: fix TNL-559 Intermittent failures of Transcript FileUploader JS tests - xdescribe('Transcripts.MessageManager', function () { + describe('Transcripts.MessageManager', function () { var videoListEntryTemplate = readFixtures( 'video/transcripts/metadata-videolist-entry.underscore' ), @@ -46,7 +46,7 @@ function ($, _, Utils, MessageManager, FileUploader, sinon) { ); $container = $('#metadata-videolist-entry'); - spyOn(fileUploader, 'initialize'); + spyOn(fileUploader, 'initialize').and.callThrough(); spyOn(console, 'error'); spyOn(Utils.Storage, 'set'); @@ -66,12 +66,11 @@ function ($, _, Utils, MessageManager, FileUploader, sinon) { }); }); - // Disabled 2/6/14 after intermittent failure in master describe('Render', function () { beforeEach(function () { spyOn(_,'template').and.callThrough(); - spyOn(fileUploader, 'render'); + spyOn(view.fileUploader, 'render'); }); it('Template doesn\'t exist', function () { @@ -81,7 +80,7 @@ function ($, _, Utils, MessageManager, FileUploader, sinon) { expect(_.template).not.toHaveBeenCalled(); expect(view.$el.find('.transcripts-status')) .toHaveClass('is-invisible'); - expect(fileUploader.render).not.toHaveBeenCalled(); + expect(view.fileUploader.render).not.toHaveBeenCalled(); }); it('All works okay if correct data is passed', function () { @@ -90,7 +89,7 @@ function ($, _, Utils, MessageManager, FileUploader, sinon) { expect(console.error).not.toHaveBeenCalled(); expect(_.template).toHaveBeenCalled(); expect(view.$el).not.toHaveClass('is-invisible'); - expect(fileUploader.render).toHaveBeenCalled(); + expect(view.fileUploader.render).toHaveBeenCalled(); }); }); diff --git a/cms/static/js/views/video/transcripts/file_uploader.js b/cms/static/js/views/video/transcripts/file_uploader.js index 13f95dc039..e0e1842e04 100644 --- a/cms/static/js/views/video/transcripts/file_uploader.js +++ b/cms/static/js/views/video/transcripts/file_uploader.js @@ -19,7 +19,8 @@ function($, Backbone, _, Utils) { initialize: function (options) { _.bindAll(this, - 'changeHandler', 'clickHandler', 'xhrResetProgressBar', 'xhrProgressHandler', 'xhrCompleteHandler' + 'changeHandler', 'clickHandler', 'xhrResetProgressBar', 'xhrProgressHandler', 'xhrCompleteHandler', + 'render' ); this.options = _.extend({}, options); this.file = false; From 78af4100cd30372df197bd67b70dd362c264c305 Mon Sep 17 00:00:00 2001 From: Usman Khalid <2200617@gmail.com> Date: Mon, 2 May 2016 20:54:29 +0500 Subject: [PATCH 3/3] Fixes for course updates date validation. If the first date the user selects is invalid it is not set on the model. This is because jQuery datepicker's getDate returns the current date when an invalid value is put in the field and the model's initial value is also the current date. So always update the date. --- .../coffee/spec/views/course_info_spec.coffee | 12 +++------- cms/static/js/utils/date_utils.js | 22 ++++++------------- cms/static/js/views/course_info_update.js | 4 ++-- 3 files changed, 12 insertions(+), 26 deletions(-) diff --git a/cms/static/coffee/spec/views/course_info_spec.coffee b/cms/static/coffee/spec/views/course_info_spec.coffee index 344c95fb47..edcf9bf7a7 100644 --- a/cms/static/coffee/spec/views/course_info_spec.coffee +++ b/cms/static/coffee/spec/views/course_info_spec.coffee @@ -98,15 +98,9 @@ define ["js/views/course_info_handout", "js/views/course_info_update", "js/model @courseInfoEdit.onNew(@event) expect(@courseInfoEdit.$el.find('.save-button').hasClass("is-disabled")).toEqual(false) @courseInfoEdit.$el.find('input.date').val(value).trigger("change") - courseInfoEdit = @courseInfoEdit - jasmine.waitUntil(-> - courseInfoEdit.$el.find('.save-button').hasClass('is-disabled') == true - ).then -> - courseInfoEdit.$el.find('input.date').val('01/01/16').trigger 'change' - jasmine.waitUntil(-> - courseInfoEdit.$el.find('.save-button').hasClass('is-disabled') == false - ).always done - return + expect(@courseInfoEdit.$el.find('.save-button').hasClass("is-disabled")).toEqual(true) + @courseInfoEdit.$el.find('input.date').val("01/01/16").trigger("change") + expect(@courseInfoEdit.$el.find('.save-button').hasClass("is-disabled")).toEqual(false) cancelEditingUpdate = (update, modalCover, useCancelButton) -> if useCancelButton diff --git a/cms/static/js/utils/date_utils.js b/cms/static/js/utils/date_utils.js index 544ea32132..db2b9ff726 100644 --- a/cms/static/js/utils/date_utils.js +++ b/cms/static/js/utils/date_utils.js @@ -15,21 +15,13 @@ function($, date, TriggerChangeEventOnEnter) { var timefield = $(div).find("input.time"); var cacheview = view; var setfield = function (event) { - var newVal = getDate(datefield, timefield), - oldTime = new Date(cacheModel.get(fieldName)).getTime(); - if (newVal) { - if (!cacheModel.has(fieldName) || oldTime !== newVal.getTime()) { - cacheview.clearValidationErrors(); - cacheview.setAndValidate(fieldName, newVal, event); - } - } - else { - // Clear date (note that this clears the time as well, as date and time are linked). - // Note also that the validation logic prevents us from clearing the start date - // (start date is required by the back end). - cacheview.clearValidationErrors(); - cacheview.setAndValidate(fieldName, null, event); - } + var newVal = getDate(datefield, timefield); + + // Setting to null clears the time as well, as date and time are linked. + // Note also that the validation logic prevents us from clearing the start date + // (start date is required by the back end). + cacheview.clearValidationErrors(); + cacheview.setAndValidate(fieldName, (newVal || null), event); }; // instrument as date and time pickers diff --git a/cms/static/js/views/course_info_update.js b/cms/static/js/views/course_info_update.js index 7b5babc771..0a3c0277c0 100644 --- a/cms/static/js/views/course_info_update.js +++ b/cms/static/js/views/course_info_update.js @@ -71,7 +71,7 @@ define(["js/views/validation", "codemirror", "js/models/course_update", }, handleValidationError : function(model, error) { - var ele = this.$el.find('#course-update-list li[name=\"'+model.cid+'\"'); + var ele = this.$el.find('#course-update-list li[name=\"'+model.cid+'\"]'); $(ele).find('.message-error').remove(); for (var field in error) { if (error.hasOwnProperty(field)) { @@ -86,7 +86,7 @@ define(["js/views/validation", "codemirror", "js/models/course_update", validateModel: function(model) { if (model.isValid()) { - var ele = this.$el.find('#course-update-list li[name=\"' + model.cid + '\"'); + var ele = this.$el.find('#course-update-list li[name=\"' + model.cid + '\"]'); $(ele).find('.message-error').remove(); $(ele).find('.save-button').removeClass('is-disabled'); }