From 4d228199d1adbff577e9d55e219185fe8d6c72a7 Mon Sep 17 00:00:00 2001 From: Usman Khalid <2200617@gmail.com> Date: Mon, 25 Apr 2016 21:17:07 +0500 Subject: [PATCH 1/3] When using RequireJS load spec files one by one. Jasmine has a global stack for creating a tree of specs. We need to load spec files one by one, otherwise some end up getting nested under others. --- cms/static/coffee/spec/main.coffee | 4 +++- cms/static/coffee/spec/main_squire.coffee | 5 +++- cms/static/karma_cms.conf.js | 4 +++- cms/static/karma_cms_squire.conf.js | 3 ++- .../static/common/js/spec/main_requirejs.js | 2 +- .../static/common/js/utils/require-serial.js | 23 +++++++++++++++++++ common/static/karma_common_requirejs.conf.js | 3 ++- lms/static/js/spec/main.js | 4 +++- lms/static/karma_lms.conf.js | 3 ++- 9 files changed, 43 insertions(+), 8 deletions(-) create mode 100644 common/static/common/js/utils/require-serial.js diff --git a/cms/static/coffee/spec/main.coffee b/cms/static/coffee/spec/main.coffee index a3b8eb237c..d5d9980b47 100644 --- a/cms/static/coffee/spec/main.coffee +++ b/cms/static/coffee/spec/main.coffee @@ -283,6 +283,8 @@ while i < testFiles.length testFiles[i] = '/base/' + testFiles[i] + '.js' i++ -require testFiles, -> +# Jasmine has a global stack for creating a tree of specs. We need to load +# spec files one by one, otherwise some end up getting nested under others. +requireSerial testFiles, -> # start test run, once Require.js is done window.__karma__.start() diff --git a/cms/static/coffee/spec/main_squire.coffee b/cms/static/coffee/spec/main_squire.coffee index b155a0e667..2fd193996c 100644 --- a/cms/static/coffee/spec/main_squire.coffee +++ b/cms/static/coffee/spec/main_squire.coffee @@ -202,6 +202,9 @@ i = 0 while i < testFiles.length testFiles[i] = '/base/' + testFiles[i] + '.js' i++ -require testFiles, -> + +# Jasmine has a global stack for creating a tree of specs. We need to load +# spec files one by one, otherwise some end up getting nested under others. +requireSerial testFiles, -> # start test run, once Require.js is done window.__karma__.start() diff --git a/cms/static/karma_cms.conf.js b/cms/static/karma_cms.conf.js index f86da7cc51..f00ba06281 100644 --- a/cms/static/karma_cms.conf.js +++ b/cms/static/karma_cms.conf.js @@ -82,7 +82,9 @@ var libraryFiles = [ {pattern: 'edx-pattern-library/js/afontgarde.js', included: false}, {pattern: 'edx-pattern-library/js/edx-icons.js', included: false}, {pattern: 'edx-pattern-library/js/**/*.js', included: false}, - {pattern: 'edx-ui-toolkit/js/**/*.js', included: false} + {pattern: 'edx-ui-toolkit/js/**/*.js', included: false}, + + {pattern: 'common/js/utils/require-serial.js', included: true} ]; // Paths to source JavaScript files diff --git a/cms/static/karma_cms_squire.conf.js b/cms/static/karma_cms_squire.conf.js index 7e23b748a9..01c036480b 100644 --- a/cms/static/karma_cms_squire.conf.js +++ b/cms/static/karma_cms_squire.conf.js @@ -68,7 +68,8 @@ var libraryFiles = [ pattern: 'xmodule_js/common_static/js/vendor/jQuery-File-Upload/js/jquery.fileupload-validate.js', included: false }, - {pattern: 'xmodule_js/common_static/js/vendor/requirejs/text.js', included: false} + {pattern: 'xmodule_js/common_static/js/vendor/requirejs/text.js', included: false}, + {pattern: 'common/js/utils/require-serial.js', included: true} ]; // Paths to source JavaScript files diff --git a/common/static/common/js/spec/main_requirejs.js b/common/static/common/js/spec/main_requirejs.js index c6fb079f91..4a9c064fda 100644 --- a/common/static/common/js/spec/main_requirejs.js +++ b/common/static/common/js/spec/main_requirejs.js @@ -178,7 +178,7 @@ testFiles[i] = '/base/' + testFiles[i]; } - require(testFiles, function () { + window.requireSerial(testFiles, function () { // start test run, once Require.js is done window.__karma__.start(); }); diff --git a/common/static/common/js/utils/require-serial.js b/common/static/common/js/utils/require-serial.js new file mode 100644 index 0000000000..c55b6bd628 --- /dev/null +++ b/common/static/common/js/utils/require-serial.js @@ -0,0 +1,23 @@ +/* +Helper function used to require files serially instead of concurrently. + */ +(function (window, require) { + 'use strict'; + + var requireModules = function (paths, callback, modules) { + // If all the modules have been loaded, call the callback. + if (paths.length === 0) { + return callback.apply(null, modules); + } + // Otherwise load the next one. + require([paths.shift()], function(module) { + modules.push(module); + requireModules(paths, callback, modules); + }); + }; + + window.requireSerial = function(paths, callback) { + requireModules(paths, callback, []); + }; + +}).call(this, window, require || RequireJS.require); diff --git a/common/static/karma_common_requirejs.conf.js b/common/static/karma_common_requirejs.conf.js index 60255e52c8..140ff586be 100644 --- a/common/static/karma_common_requirejs.conf.js +++ b/common/static/karma_common_requirejs.conf.js @@ -46,7 +46,8 @@ var libraryFiles = [ {pattern: 'js/test/i18n.js', included: false}, {pattern: 'coffee/src/jquery.immediateDescendents.js', included: false}, {pattern: 'js/vendor/requirejs/text.js', included: false}, - {pattern: 'js/vendor/sinon-1.17.0.js', included: false} + {pattern: 'js/vendor/sinon-1.17.0.js', included: false}, + {pattern: 'common/js/utils/require-serial.js', included: true} ]; // Paths to source JavaScript files diff --git a/lms/static/js/spec/main.js b/lms/static/js/spec/main.js index 709ec84340..4c90c9f4a5 100644 --- a/lms/static/js/spec/main.js +++ b/lms/static/js/spec/main.js @@ -770,7 +770,9 @@ testFiles[i] = '/base/' + testFiles[i]; } - require(testFiles, function () { + // Jasmine has a global stack for creating a tree of specs. We need to load + // spec files one by one, otherwise some end up getting nested under others. + window.requireSerial(testFiles, function () { // start test run, once Require.js is done window.__karma__.start(); }); diff --git a/lms/static/karma_lms.conf.js b/lms/static/karma_lms.conf.js index b310c7e337..e769f5b9d6 100644 --- a/lms/static/karma_lms.conf.js +++ b/lms/static/karma_lms.conf.js @@ -73,7 +73,8 @@ var libraryFiles = [ {pattern: 'xmodule_js/common_static/js/vendor/slick.core.js', included: true}, {pattern: 'xmodule_js/common_static/js/vendor/slick.grid.js', included: true}, {pattern: 'xmodule_js/common_static/js/libs/jasmine-waituntil.js', included: true}, - {pattern: 'xmodule_js/common_static/js/libs/jasmine-extensions.js', included: true} + {pattern: 'xmodule_js/common_static/js/libs/jasmine-extensions.js', included: true}, + {pattern: 'common/js/utils/require-serial.js', included: true} ]; // Paths to source JavaScript files From 8f79d8013f70e8dec4b713e9acb80e4c9b92ce6b Mon Sep 17 00:00:00 2001 From: Usman Khalid <2200617@gmail.com> Date: Tue, 26 Apr 2016 19:53:54 +0500 Subject: [PATCH 2/3] Turn off global ajaxError handler in afterEach in teams tab factory specs. --- .../teams/js/spec/teams_tab_factory_spec.js | 5 ++-- .../teams/js/spec/views/teams_tab_spec.js | 6 ++++- .../teams/static/teams/js/views/teams_tab.js | 23 ++++++++++++------- 3 files changed, 23 insertions(+), 11 deletions(-) diff --git a/lms/djangoapps/teams/static/teams/js/spec/teams_tab_factory_spec.js b/lms/djangoapps/teams/static/teams/js/spec/teams_tab_factory_spec.js index d41cfce9d5..de6b4e104b 100644 --- a/lms/djangoapps/teams/static/teams/js/spec/teams_tab_factory_spec.js +++ b/lms/djangoapps/teams/static/teams/js/spec/teams_tab_factory_spec.js @@ -1,6 +1,6 @@ -define(['jquery', 'backbone', 'teams/js/teams_tab_factory', +define(['jquery', 'backbone', 'teams/js/teams_tab_factory', 'teams/js/views/teams_tab', 'common/js/spec_helpers/page_helpers', 'teams/js/spec_helpers/team_spec_helpers'], - function($, Backbone, TeamsTabFactory, PageHelpers, TeamSpecHelpers) { + function($, Backbone, TeamsTabFactory, TeamsTabView, PageHelpers, TeamSpecHelpers) { 'use strict'; describe("Teams Tab Factory", function() { @@ -15,6 +15,7 @@ define(['jquery', 'backbone', 'teams/js/teams_tab_factory', afterEach(function() { Backbone.history.stop(); + $(document).off('ajaxError', TeamsTabView.prototype.errorHandler); }); it('can render the "Teams" tab', function() { diff --git a/lms/djangoapps/teams/static/teams/js/spec/views/teams_tab_spec.js b/lms/djangoapps/teams/static/teams/js/spec/views/teams_tab_spec.js index e9f58659dd..2f236433e1 100644 --- a/lms/djangoapps/teams/static/teams/js/spec/views/teams_tab_spec.js +++ b/lms/djangoapps/teams/static/teams/js/spec/views/teams_tab_spec.js @@ -56,7 +56,11 @@ define([ spyOn(Logger, 'log'); }); - afterEach(Backbone.history.stop); + afterEach(function() { + Backbone.history.stop(); + $(document).off('ajaxError', TeamsTabView.prototype.errorHandler); + } + ); describe('Navigation', function () { it('does not render breadcrumbs for the top level tabs', function() { diff --git a/lms/djangoapps/teams/static/teams/js/views/teams_tab.js b/lms/djangoapps/teams/static/teams/js/views/teams_tab.js index 7aadb4fed2..69e5241213 100644 --- a/lms/djangoapps/teams/static/teams/js/views/teams_tab.js +++ b/lms/djangoapps/teams/static/teams/js/views/teams_tab.js @@ -159,14 +159,7 @@ start: function() { Backbone.history.start(); - $(document).ajaxError(function (event, xhr) { - if (xhr.status === 401) { - TeamUtils.showMessage(gettext("Your request could not be completed. Reload the page and try again.")); - } - else if (xhr.status === 500) { - TeamUtils.showMessage(gettext("Your request could not be completed due to a server problem. Reload the page and try again. If the issue persists, click the Help tab to report the problem.")); - } - }); + $(document).ajaxError(this.errorHandler); // Navigate to the default page if there is no history: // 1. If the user belongs to at least one team, jump to the "My Teams" page @@ -180,6 +173,20 @@ } }, + errorHandler: function(event, xhr) { + if (xhr.status === 401) { + TeamUtils.showMessage(gettext( + "Your request could not be completed. Reload the page and try again." + )); + } + else if (xhr.status === 500) { + TeamUtils.showMessage(gettext( + "Your request could not be completed due to a server problem. Reload the page" + + " and try again. If the issue persists, click the Help tab to report the problem." + )); + } + }, + render: function() { this.mainView.setElement(this.$el).render(); TeamUtils.hideMessage(); From 6c26715a142edf9e8d1811264fa75dc3a1750625 Mon Sep 17 00:00:00 2001 From: Usman Khalid <2200617@gmail.com> Date: Tue, 26 Apr 2016 20:30:35 +0500 Subject: [PATCH 3/3] Make hashes unique so that they do not conflict with hashes from module_edit_spec.coffee. --- cms/static/js/spec/views/xblock_spec.js | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/cms/static/js/spec/views/xblock_spec.js b/cms/static/js/spec/views/xblock_spec.js index bcbfabfffa..9777008f05 100644 --- a/cms/static/js/spec/views/xblock_spec.js +++ b/cms/static/js/spec/views/xblock_spec.js @@ -62,8 +62,8 @@ define(["jquery", "URI", "common/js/spec_helpers/ajax_helpers", "common/js/compo mockCssUrl = "mock.css", headHtml; postXBlockRequest(requests, [ - ["hash1", { mimetype: "text/css", kind: "text", data: mockCssText }], - ["hash2", { mimetype: "text/css", kind: "url", data: mockCssUrl }] + ["xblock_spec_hash1", { mimetype: "text/css", kind: "text", data: mockCssText }], + ["xblock_spec_hash2", { mimetype: "text/css", kind: "url", data: mockCssUrl }] ]); headHtml = $('head').html(); expect(headHtml).toContain(mockCssText); @@ -73,7 +73,9 @@ define(["jquery", "URI", "common/js/spec_helpers/ajax_helpers", "common/js/compo it('can render an xblock with required JavaScript', function() { var requests = AjaxHelpers.requests(this); postXBlockRequest(requests, [ - ["hash3", { mimetype: "application/javascript", kind: "text", data: "window.test = 100;" }] + ["xblock_spec_hash3", { + mimetype: "application/javascript", kind: "text", data: "window.test = 100;" + }] ]); expect(window.test).toBe(100); }); @@ -82,7 +84,7 @@ define(["jquery", "URI", "common/js/spec_helpers/ajax_helpers", "common/js/compo var requests = AjaxHelpers.requests(this), mockHeadTag = "Test Title"; postXBlockRequest(requests, [ - ["hash4", { mimetype: "text/html", placement: "head", data: mockHeadTag }] + ["xblock_spec_hash4", { mimetype: "text/html", placement: "head", data: mockHeadTag }] ]); expect($('head').html()).toContain(mockHeadTag); }); @@ -93,7 +95,9 @@ define(["jquery", "URI", "common/js/spec_helpers/ajax_helpers", "common/js/compo promise; spyOn(ViewUtils, 'loadJavaScript').and.returnValue($.Deferred().reject().promise()); promise = postXBlockRequest(requests, [ - ["hash5", { mimetype: "application/javascript", kind: "url", data: missingJavaScriptUrl }] + ["xblock_spec_hash5", { + mimetype: "application/javascript", kind: "url", data: missingJavaScriptUrl + }] ]); expect(promise.isRejected()).toBe(true); });