From 4f0aee3f2d68fcac405a5e9578fc1472f438c45d Mon Sep 17 00:00:00 2001 From: Brian Jacobel Date: Mon, 2 May 2016 12:32:11 -0400 Subject: [PATCH] Fix unit and acceptance tests broken in upgrade (squashed) Fix syntax error in selectors .attr() now returns a string (though it can still be passed an integer) Fixes checkbox test failures Remove remaining references to jquery.min (in wrong folder) $.ajax now returns 422 if type is json and body is not JSON, e.g. '' Substitute prop for attr Remove references to jquery.min, add jquery.migrate (again) "Fix" jquery karma config This wasn't suppoed to survive the merge This throws an error when called with an 'undefined' error Fix Karma warning about [re|un]loading the window Fix path for jquery in cms-squire tests Move jasmine.clock.uninstall() to afterEach so it runs even on failure Fix test failing due to timezone issues Do the timeout before the window scrolling (so handler will not be _.throttled) Fix an alert() triggered by window.onBeforeUnload while testing in Chrome --- cms/static/coffee/spec/main_spec.coffee | 6 +++--- cms/static/coffee/spec/models/section_spec.coffee | 4 ++-- .../spec/views/certificate_preview_spec.js | 3 +-- cms/static/js/spec/views/settings/main_spec.js | 6 +++--- cms/static/karma_cms.conf.js | 5 +++++ cms/static/karma_cms_squire.conf.js | 5 +++++ common/djangoapps/pipeline_mako/__init__.py | 4 ++-- common/lib/xmodule/xmodule/js/karma_xmodule.conf.js | 3 ++- .../xmodule/xmodule/js/spec/capa/display_spec.coffee | 4 ++-- .../xmodule/js/spec/video/video_caption_spec.js | 6 +++--- .../js/spec/video/video_focus_grabber_spec.js | 4 ++-- .../js/spec/video/video_play_placeholder_spec.js | 4 ++-- common/static/karma_common.conf.js | 3 ++- .../src/instructor_dashboard/data_download.coffee | 10 +++++----- .../js/spec/courseware/updates_visibility_spec.js | 6 +++--- .../js/spec/discovery/discovery_factory_spec.js | 12 ++++++++---- .../student_profile/learner_profile_fields_spec.js | 6 ++++++ lms/static/js/student_account/views/RegisterView.js | 3 ++- lms/static/karma_lms.conf.js | 2 ++ lms/static/karma_lms_coffee.conf.js | 3 ++- 20 files changed, 62 insertions(+), 37 deletions(-) diff --git a/cms/static/coffee/spec/main_spec.coffee b/cms/static/coffee/spec/main_spec.coffee index 39098691ea..ed6b6b6d6e 100644 --- a/cms/static/coffee/spec/main_spec.coffee +++ b/cms/static/coffee/spec/main_spec.coffee @@ -33,7 +33,7 @@ require ["jquery", "backbone", "coffee/src/main", "common/js/spec_helpers/ajax_h server && server.restore() it "successful AJAX request does not pop an error notification", -> - server = AjaxHelpers.server([200, {}, '']) + server = AjaxHelpers.server([200, {"Content-Type": "application/json"}, "{}"]) expect($("#page-notification")).toBeEmpty() $.ajax("/test") @@ -42,7 +42,7 @@ require ["jquery", "backbone", "coffee/src/main", "common/js/spec_helpers/ajax_h expect($("#page-notification")).toBeEmpty() it "AJAX request with error should pop an error notification", -> - server = AjaxHelpers.server([500, {}, '']) + server = AjaxHelpers.server([500, {"Content-Type": "application/json"}, "{}"]) $.ajax("/test") server.respond() @@ -50,7 +50,7 @@ require ["jquery", "backbone", "coffee/src/main", "common/js/spec_helpers/ajax_h expect($("#page-notification")).toContainElement('div.wrapper-notification-error') it "can override AJAX request with error so it does not pop an error notification", -> - server = AjaxHelpers.server([500, {}, '']) + server = AjaxHelpers.server([500, {"Content-Type": "application/json"}, "{}"]) $.ajax url: "/test" diff --git a/cms/static/coffee/spec/models/section_spec.coffee b/cms/static/coffee/spec/models/section_spec.coffee index 82fd1c9b3f..8e7438f5ba 100644 --- a/cms/static/coffee/spec/models/section_spec.coffee +++ b/cms/static/coffee/spec/models/section_spec.coffee @@ -34,7 +34,7 @@ define ["js/models/section", "common/js/spec_helpers/ajax_helpers", "js/utils/mo }) it "show/hide a notification when it saves to the server", -> - server = AjaxHelpers.server([200, {}, '']) + server = AjaxHelpers.server([200, {"Content-Type": "application/json"}, "{}"]) @model.save() expect(Section.prototype.showNotification).toHaveBeenCalled() @@ -43,7 +43,7 @@ define ["js/models/section", "common/js/spec_helpers/ajax_helpers", "js/utils/mo it "don't hide notification when saving fails", -> # this is handled by the global AJAX error handler - server = AjaxHelpers.server([500, {}, '']) + server = AjaxHelpers.server([500, {"Content-Type": "application/json"}, "{}"]) @model.save() server.respond() diff --git a/cms/static/js/certificates/spec/views/certificate_preview_spec.js b/cms/static/js/certificates/spec/views/certificate_preview_spec.js index 5818e0ea01..205b5fdf1c 100644 --- a/cms/static/js/certificates/spec/views/certificate_preview_spec.js +++ b/cms/static/js/certificates/spec/views/certificate_preview_spec.js @@ -7,8 +7,7 @@ define([ // jshint ignore:line 'js/certificates/views/certificate_preview', 'common/js/spec_helpers/template_helpers', 'common/js/spec_helpers/view_helpers', - 'common/js/spec_helpers/ajax_helpers', - 'jasmine-stealth' + 'common/js/spec_helpers/ajax_helpers' ], function(_, $, Course, CertificatePreview, TemplateHelpers, ViewHelpers, AjaxHelpers) { 'use strict'; diff --git a/cms/static/js/spec/views/settings/main_spec.js b/cms/static/js/spec/views/settings/main_spec.js index ff5a7ce788..b9c1818e5d 100644 --- a/cms/static/js/spec/views/settings/main_spec.js +++ b/cms/static/js/spec/views/settings/main_spec.js @@ -144,7 +144,7 @@ define([ // select the entrance-exam-enabled checkbox. grade requirement section should be visible. entrance_exam_enabled_field - .attr('checked', 'true') + .prop('checked', true) .trigger('change'); this.view.render(); @@ -152,7 +152,7 @@ define([ // deselect the entrance-exam-enabled checkbox. grade requirement section should be hidden. entrance_exam_enabled_field - .removeAttr('checked') + .prop('checked', false) .trigger('change'); expect(this.view.$(SELECTORS.grade_requirement_div)).toBeHidden(); @@ -173,7 +173,7 @@ define([ // select the entrance-exam-enabled checkbox. entrance_exam_enabled_field - .attr('checked', 'true') + .prop('checked', true) .trigger('change'); // input a valid value for entrance exam minimum score. diff --git a/cms/static/karma_cms.conf.js b/cms/static/karma_cms.conf.js index b2048623aa..edfb65d9f4 100644 --- a/cms/static/karma_cms.conf.js +++ b/cms/static/karma_cms.conf.js @@ -13,6 +13,11 @@ var options = { libraryFiles: [], + libraryFilesToInclude: [ + {pattern: 'common/js/vendor/jquery.js', included: true}, + {pattern: 'common/js/vendor/jquery-migrate.js', included: true} + ], + // Make sure the patterns in sourceFiles and specFiles do not match the same file. // Otherwise Istanbul which is used for coverage tracking will cause tests to not run. sourceFiles: [ diff --git a/cms/static/karma_cms_squire.conf.js b/cms/static/karma_cms_squire.conf.js index 469a3891e8..d58bdc16dd 100644 --- a/cms/static/karma_cms_squire.conf.js +++ b/cms/static/karma_cms_squire.conf.js @@ -13,6 +13,11 @@ var options = { libraryFiles: [], + libraryFilesToInclude: [ + {pattern: 'xmodule_js/common_static/common/js/vendor/jquery.js', included: true}, + {pattern: 'xmodule_js/common_static/common/js/vendor/jquery-migrate.js', included: true} + ], + // Make sure the patterns in sourceFiles and specFiles do not match the same file. // Otherwise Istanbul which is used for coverage tracking will cause tests to not run. sourceFiles: [ diff --git a/common/djangoapps/pipeline_mako/__init__.py b/common/djangoapps/pipeline_mako/__init__.py index d8987550a4..754976d509 100644 --- a/common/djangoapps/pipeline_mako/__init__.py +++ b/common/djangoapps/pipeline_mako/__init__.py @@ -93,12 +93,12 @@ def render_require_js_path_overrides(path_overrides): # pylint: disable=invalid For example: - "js/vendor/jquery.min.js" --> "js/vendor/jquery.min.abcd1234" + "js/vendor/jquery.js" --> "js/vendor/jquery.abcd1234" To achive this we will add overrided paths in requirejs config at runtime. So that any reference to 'jquery' in a JavaScript module - will cause RequireJS to load '/static/js/vendor/jquery.min.abcd1234.js' + will cause RequireJS to load '/static/js/vendor/jquery.abcd1234.js' If running in DEBUG mode (as in devstack), the resolved JavaScript URLs won't contain hashes, so the new paths will match the original paths. diff --git a/common/lib/xmodule/xmodule/js/karma_xmodule.conf.js b/common/lib/xmodule/xmodule/js/karma_xmodule.conf.js index 6ff01d6856..5ee38962a7 100644 --- a/common/lib/xmodule/xmodule/js/karma_xmodule.conf.js +++ b/common/lib/xmodule/xmodule/js/karma_xmodule.conf.js @@ -28,7 +28,8 @@ var options = { {pattern: 'common_static/edx-ui-toolkit/js/utils/global-loader.js', included: true}, {pattern: 'common_static/js/vendor/CodeMirror/codemirror.js', included: true}, {pattern: 'common_static/js/vendor/draggabilly.js'}, - {pattern: 'common_static/js/vendor/jquery.min.js', included: true}, + {pattern: 'common_static/common/js/vendor/jquery.js', included: true}, + {pattern: 'common_static/common/js/vendor/jquery-migrate.js', included: true}, {pattern: 'common_static/js/vendor/jquery.cookie.js', included: true}, {pattern: 'common_static/js/vendor/jquery.leanModal.js', included: true}, {pattern: 'common_static/js/vendor/jquery.timeago.js', included: true}, diff --git a/common/lib/xmodule/xmodule/js/spec/capa/display_spec.coffee b/common/lib/xmodule/xmodule/js/spec/capa/display_spec.coffee index 9c6a396cc8..0e0b29f776 100644 --- a/common/lib/xmodule/xmodule/js/spec/capa/display_spec.coffee +++ b/common/lib/xmodule/xmodule/js/spec/capa/display_spec.coffee @@ -289,9 +289,9 @@ describe 'Problem', -> $('#input_example_1').replaceWith(html) @problem.checkAnswersAndCheckButton true @checkDisabled true - $('#input_1_1_1').attr('checked', true).trigger('click') + $('#input_1_1_1').click() @checkDisabled false - $('#input_1_1_1').attr('checked', false).trigger('click') + $('#input_1_1_1').click() @checkDisabled true it 'should become enabled after a radiobutton is checked', -> 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 f3293cf0e4..b8cf3065b1 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 @@ -343,7 +343,7 @@ expect(parseIntAttribute(item, 'data-index')).toEqual(index); expect(parseIntAttribute(item, 'data-start')).toEqual(captionsData.start[index]); - expect(item.attr('tabindex')).toEqual(0); + expect(item.attr('tabindex')).toEqual('0'); expect(item.text().trim()).toEqual(captionsData.text[index]); }); }); @@ -432,7 +432,7 @@ expect(parseIntAttribute(item, 'data-index')).toEqual(index); expect(parseIntAttribute(item, 'data-start')).toEqual(captionsData.start[index]); - expect(item.attr('tabindex')).toEqual(0); + expect(item.attr('tabindex')).toEqual('0'); expect(item.text().trim()).toEqual(text); }); }).always(done); @@ -842,7 +842,7 @@ function (index, item) { expect(parseIntAttribute($(item), 'data-index')).toEqual(index); expect(parseIntAttribute($(item), 'data-start')).toEqual(captionsData.start[index]); - expect($(item).attr('tabindex')).toEqual(0); + expect($(item).attr('tabindex')).toEqual('0'); expect($(item).text().trim()).toEqual(captionsData.text[index]); }); }); diff --git a/common/lib/xmodule/xmodule/js/spec/video/video_focus_grabber_spec.js b/common/lib/xmodule/xmodule/js/spec/video/video_focus_grabber_spec.js index 8dd9bceb40..8d465b15a9 100644 --- a/common/lib/xmodule/xmodule/js/spec/video/video_focus_grabber_spec.js +++ b/common/lib/xmodule/xmodule/js/spec/video/video_focus_grabber_spec.js @@ -49,8 +49,8 @@ }); it('from the start, focus grabbers are disabled', function () { - expect(state.focusGrabber.elFirst.attr('tabindex')).toBe(-1); - expect(state.focusGrabber.elLast.attr('tabindex')).toBe(-1); + expect(state.focusGrabber.elFirst.attr('tabindex')).toBe('-1'); + expect(state.focusGrabber.elLast.attr('tabindex')).toBe('-1'); }); it( diff --git a/common/lib/xmodule/xmodule/js/spec/video/video_play_placeholder_spec.js b/common/lib/xmodule/xmodule/js/spec/video/video_play_placeholder_spec.js index b7119dd9b4..4532742dee 100644 --- a/common/lib/xmodule/xmodule/js/spec/video/video_play_placeholder_spec.js +++ b/common/lib/xmodule/xmodule/js/spec/video/video_play_placeholder_spec.js @@ -55,7 +55,7 @@ expect(btnPlay).not.toHaveClass('is-hidden'); expect(btnPlay).toHaveAttrs({ 'aria-hidden': 'false', - 'tabindex': 0 + 'tabindex': '0' }); state.videoPlayPlaceholder.hide(); @@ -63,7 +63,7 @@ expect(btnPlay).toHaveClass('is-hidden'); expect(btnPlay).toHaveAttrs({ 'aria-hidden': 'true', - 'tabindex': -1 + 'tabindex': '-1' }); }); diff --git a/common/static/karma_common.conf.js b/common/static/karma_common.conf.js index d592819644..cf19a73fd7 100644 --- a/common/static/karma_common.conf.js +++ b/common/static/karma_common.conf.js @@ -19,7 +19,8 @@ var options = { libraryFilesToInclude: [ {pattern: 'coffee/src/ajax_prefix.js', included: true}, {pattern: 'js/vendor/draggabilly.js', included: true}, - {pattern: 'js/vendor/jquery.min.js', included: true}, + {pattern: 'common/js/vendor/jquery.js', included: true}, + {pattern: 'common/js/vendor/jquery-migrate.js', included: true}, {pattern: 'coffee/src/jquery.immediateDescendents.js', included: true}, {pattern: 'js/vendor/jquery.leanModal.js', included: true}, {pattern: 'js/vendor/jquery.timeago.js', included: true}, diff --git a/lms/static/coffee/src/instructor_dashboard/data_download.coffee b/lms/static/coffee/src/instructor_dashboard/data_download.coffee index 8de7cce86e..7d2ab57af6 100644 --- a/lms/static/coffee/src/instructor_dashboard/data_download.coffee +++ b/lms/static/coffee/src/instructor_dashboard/data_download.coffee @@ -79,11 +79,11 @@ class DataDownload @$list_may_enroll_csv_btn = @$section.find("input[name='list-may-enroll-csv']") @$list_problem_responses_csv_input = @$section.find("input[name='problem-location']") @$list_problem_responses_csv_btn = @$section.find("input[name='list-problem-responses-csv']") - @$list_anon_btn = @$section.find("input[name='list-anon-ids']'") - @$grade_config_btn = @$section.find("input[name='dump-gradeconf']'") - @$calculate_grades_csv_btn = @$section.find("input[name='calculate-grades-csv']'") - @$problem_grade_report_csv_btn = @$section.find("input[name='problem-grade-report']'") - @$async_report_btn = @$section.find("input[class='async-report-btn']'") + @$list_anon_btn = @$section.find("input[name='list-anon-ids']") + @$grade_config_btn = @$section.find("input[name='dump-gradeconf']") + @$calculate_grades_csv_btn = @$section.find("input[name='calculate-grades-csv']") + @$problem_grade_report_csv_btn = @$section.find("input[name='problem-grade-report']") + @$async_report_btn = @$section.find("input[class='async-report-btn']") # response areas @$download = @$section.find '.data-download-container' diff --git a/lms/static/js/spec/courseware/updates_visibility_spec.js b/lms/static/js/spec/courseware/updates_visibility_spec.js index f8523f6974..f5505b90b9 100644 --- a/lms/static/js/spec/courseware/updates_visibility_spec.js +++ b/lms/static/js/spec/courseware/updates_visibility_spec.js @@ -1,5 +1,5 @@ -define(['jquery', 'logger', 'js/courseware/toggle_element_visibility'], - function ($, Logger, ToggleElementVisibility) { +define(['jquery', 'logger', 'js/courseware/toggle_element_visibility', 'moment'], + function ($, Logger, ToggleElementVisibility, moment) { 'use strict'; describe('show/hide with mouse click', function () { @@ -43,7 +43,7 @@ define(['jquery', 'logger', 'js/courseware/toggle_element_visibility'], $update.siblings('.toggle-visibility-button').trigger('click'); expect(Logger.log).toHaveBeenCalledWith('edx.course.home.course_update.toggled', { action: 'hide', - publish_date: '2015-12-01T00:00:00+00:00' + publish_date: moment('December 1, 2015', 'MMM DD, YYYY').format() }); }); }); diff --git a/lms/static/js/spec/discovery/discovery_factory_spec.js b/lms/static/js/spec/discovery/discovery_factory_spec.js index 2395895946..5c29af81b1 100644 --- a/lms/static/js/spec/discovery/discovery_factory_spec.js +++ b/lms/static/js/spec/discovery/discovery_factory_spec.js @@ -107,6 +107,12 @@ define([ 'templates/discovery/filter_bar' ]); DiscoveryFactory(MEANINGS); + + jasmine.clock().install(); + }); + + afterEach(function () { + jasmine.clock().uninstall(); }); it('does search', function () { @@ -121,22 +127,20 @@ define([ it('loads more', function () { var requests = AjaxHelpers.requests(this); - jasmine.clock().install(); + $('.discovery-input').val('test'); $('.discovery-submit').trigger('click'); AjaxHelpers.respondWithJson(requests, JSON_RESPONSE); expect($('.courses-listing article').length).toEqual(1); expect($('.courses-listing .course-title')).toContainHtml('edX Demonstration Course'); + jasmine.clock().tick(500); window.scroll(0, $(document).height()); $(window).trigger('scroll'); - jasmine.clock().tick(500); // TODO: determine why the search API is invoked twice AjaxHelpers.respondWithJson(requests, JSON_RESPONSE); AjaxHelpers.respondWithJson(requests, JSON_RESPONSE); expect($('.courses-listing article').length).toEqual(2); - - jasmine.clock().uninstall(); }); it('displays not found message', function () { diff --git a/lms/static/js/spec/student_profile/learner_profile_fields_spec.js b/lms/static/js/spec/student_profile/learner_profile_fields_spec.js index 694171335c..dbd9fec9bf 100644 --- a/lms/static/js/spec/student_profile/learner_profile_fields_spec.js +++ b/lms/static/js/spec/student_profile/learner_profile_fields_spec.js @@ -54,6 +54,11 @@ define(['backbone', 'jquery', 'underscore', 'common/js/spec_helpers/ajax_helpers TemplateHelpers.installTemplate("templates/fields/message_banner"); }); + afterEach(function () { + // image_field.js's window.onBeforeUnload breaks Karma in Chrome, clean it up after each test + $(window).off('beforeunload'); + }); + var createFakeImageFile = function (size) { var fileFakeData = 'i63ljc6giwoskyb9x5sw0169bdcmcxr3cdz8boqv0lik971972cmd6yknvcxr5sw0nvc169bdcmcxsdf'; return new Blob( @@ -260,6 +265,7 @@ define(['backbone', 'jquery', 'underscore', 'common/js/spec_helpers/ajax_helpers // Verify image upload progress message verifyImageUploadButtonMessage(imageView, true); + window.onbeforeunload = null; $(window).trigger('beforeunload'); expect(imageView.onBeforeUnload).toHaveBeenCalled(); }); diff --git a/lms/static/js/student_account/views/RegisterView.js b/lms/static/js/student_account/views/RegisterView.js index 416b1c088f..96d43a78f3 100644 --- a/lms/static/js/student_account/views/RegisterView.js +++ b/lms/static/js/student_account/views/RegisterView.js @@ -78,7 +78,8 @@ $(this.el).show(); // Show in case the form was hidden for auto-submission this.errors = _.flatten( _.map( - JSON.parse(error.responseText), + // Something is passing this 'undefined'. Protect against this. + JSON.parse(error.responseText || "[]"), function(error_list) { return _.map( error_list, diff --git a/lms/static/karma_lms.conf.js b/lms/static/karma_lms.conf.js index 5acfe8b93f..c0a17a5305 100644 --- a/lms/static/karma_lms.conf.js +++ b/lms/static/karma_lms.conf.js @@ -13,6 +13,8 @@ var options = { // Avoid adding files to this list. Use RequireJS. libraryFilesToInclude: [ + {pattern: 'common/js/vendor/jquery.js', included: true}, + {pattern: 'common/js/vendor/jquery-migrate.js', included: true}, {pattern: 'xmodule_js/common_static/js/vendor/jquery.event.drag-2.2.js', included: true}, {pattern: 'xmodule_js/common_static/js/vendor/slick.core.js', included: true}, {pattern: 'xmodule_js/common_static/js/vendor/slick.grid.js', included: true} diff --git a/lms/static/karma_lms_coffee.conf.js b/lms/static/karma_lms_coffee.conf.js index 29efa73a85..6050f154df 100644 --- a/lms/static/karma_lms_coffee.conf.js +++ b/lms/static/karma_lms_coffee.conf.js @@ -25,7 +25,8 @@ var options = { {pattern: 'xmodule_js/common_static/js/src/logger.js', included: true}, {pattern: 'xmodule_js/common_static/js/test/i18n.js', included: true}, {pattern: 'xmodule_js/common_static/js/vendor/CodeMirror/codemirror.js', included: true}, - {pattern: 'xmodule_js/common_static/js/vendor/jquery.min.js', included: true}, + {pattern: 'common/js/vendor/jquery.js', included: true}, + {pattern: 'common/js/vendor/jquery-migrate.js', included: true}, {pattern: 'xmodule_js/common_static/js/vendor/jquery.cookie.js', included: true}, {pattern: 'xmodule_js/common_static/js/vendor/flot/jquery.flot.js', included: true}, {pattern: 'xmodule_js/common_static/coffee/src/jquery.immediateDescendents.js', included: true},