From 6738a37ec1531446590f49c5c1fd5ee2d50579d7 Mon Sep 17 00:00:00 2001 From: Ned Batchelder Date: Tue, 27 Aug 2013 11:08:22 -0400 Subject: [PATCH 01/13] Keep comments in capa XML from causing failures Comments (and processing instructions!) are handled oddly in lxml. This change will keep them from causing failures. They will be omitted from the HTML generated, which is fine, since they aren't needed there. --- common/lib/capa/capa/capa_problem.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/common/lib/capa/capa/capa_problem.py b/common/lib/capa/capa/capa_problem.py index c2bdeadc2143..08a223f609b8 100644 --- a/common/lib/capa/capa/capa_problem.py +++ b/common/lib/capa/capa/capa_problem.py @@ -555,6 +555,13 @@ def _extract_html(self, problemtree): # private Used by get_html. ''' + if not isinstance(problemtree.tag, basestring): + # Comment and ProcessingInstruction nodes are not Elements, + # and we're ok leaving those behind. + # BTW: etree gives us no good way to distinguish these things + # other than to examine .tag to see if it's a string. :( + return + if (problemtree.tag == 'script' and problemtree.get('type') and 'javascript' in problemtree.get('type')): # leave javascript intact. From e88e04d3a6704ccdabe48f7b0fb6fe0b09ae8078 Mon Sep 17 00:00:00 2001 From: Ned Batchelder Date: Tue, 27 Aug 2013 11:44:41 -0400 Subject: [PATCH 02/13] A test that our XML-comments fix works. --- .../lib/capa/capa/tests/test_html_render.py | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/common/lib/capa/capa/tests/test_html_render.py b/common/lib/capa/capa/tests/test_html_render.py index 9bc326d7b924..8e343ee1cf8a 100644 --- a/common/lib/capa/capa/tests/test_html_render.py +++ b/common/lib/capa/capa/tests/test_html_render.py @@ -226,6 +226,26 @@ def test_substitute_python_vars(self): span_element = rendered_html.find('span') self.assertEqual(span_element.get('attr'), "TEST") + def test_xml_comments_and_other_odd_things(self): + # Comments and processing instructions should be skipped. + xml_str = textwrap.dedent("""\ + + + ]> + + + + + """) + + # Create the problem + problem = new_loncapa_problem(xml_str) + + # Render the HTML + the_html = problem.get_html() + self.assertRegexpMatches(the_html, r"
\s+
") + def _create_test_file(self, path, content_str): test_fp = self.system.filestore.open(path, "w") test_fp.write(content_str) From 6cb2e0b2064c023e884077bfdd8daab0d0801013 Mon Sep 17 00:00:00 2001 From: Julian Arni Date: Wed, 28 Aug 2013 11:13:20 -0400 Subject: [PATCH 03/13] Check extension rather than mimetype --- cms/templates/import.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cms/templates/import.html b/cms/templates/import.html index a5c6b9f41261..27337bf235d3 100644 --- a/cms/templates/import.html +++ b/cms/templates/import.html @@ -72,7 +72,7 @@

${_("Course to import:")}

add: function(e, data) { submitBtn.unbind('click'); var file = data.files[0]; - if (file.type == "application/x-gzip") { + if (file.name.match(/tar\.gz$/)) { submitBtn.click(function(e){ e.preventDefault(); submitBtn.hide(); From 1f2f0bf348fc31108e554b50bead3bcfff61eb28 Mon Sep 17 00:00:00 2001 From: Will Daly Date: Wed, 28 Aug 2013 21:02:42 -0400 Subject: [PATCH 04/13] Skip acceptance test DB migrations if the DB already exists and running fasttest_acceptance --- cms/envs/acceptance.py | 4 +- cms/envs/acceptance_static.py | 77 ----------------------------------- lms/envs/acceptance.py | 4 +- lms/envs/acceptance_static.py | 77 ----------------------------------- rakelib/tests.rake | 32 ++++++++++++--- 5 files changed, 30 insertions(+), 164 deletions(-) delete mode 100644 cms/envs/acceptance_static.py delete mode 100644 lms/envs/acceptance_static.py diff --git a/cms/envs/acceptance.py b/cms/envs/acceptance.py index 3b89e2e98848..32bae3c34caf 100644 --- a/cms/envs/acceptance.py +++ b/cms/envs/acceptance.py @@ -68,8 +68,8 @@ def seed(): DATABASES = { 'default': { 'ENGINE': 'django.db.backends.sqlite3', - 'NAME': TEST_ROOT / "db" / "test_mitx_%s.db" % seed(), - 'TEST_NAME': TEST_ROOT / "db" / "test_mitx_%s.db" % seed(), + 'NAME': TEST_ROOT / "db" / "test_edx.db", + 'TEST_NAME': TEST_ROOT / "db" / "test_edx.db" } } diff --git a/cms/envs/acceptance_static.py b/cms/envs/acceptance_static.py deleted file mode 100644 index f7d69794fb82..000000000000 --- a/cms/envs/acceptance_static.py +++ /dev/null @@ -1,77 +0,0 @@ -""" -This config file extends the test environment configuration -so that we can run the lettuce acceptance tests. -This is used in the django-admin call as acceptance.py -contains random seeding, causing django-admin to create a random collection -""" - -# We intentionally define lots of variables that aren't used, and -# want to import all variables from base settings files -# pylint: disable=W0401, W0614 - -from .test import * - -# You need to start the server in debug mode, -# otherwise the browser will not render the pages correctly -DEBUG = True - -# Disable warnings for acceptance tests, to make the logs readable -import logging -logging.disable(logging.ERROR) -import os -import random - -MODULESTORE_OPTIONS = { - 'default_class': 'xmodule.raw_module.RawDescriptor', - 'host': 'localhost', - 'db': 'acceptance_xmodule', - 'collection': 'acceptance_modulestore', - 'fs_root': TEST_ROOT / "data", - 'render_template': 'mitxmako.shortcuts.render_to_string', -} - -MODULESTORE = { - 'default': { - 'ENGINE': 'xmodule.modulestore.draft.DraftModuleStore', - 'OPTIONS': MODULESTORE_OPTIONS - }, - 'direct': { - 'ENGINE': 'xmodule.modulestore.mongo.MongoModuleStore', - 'OPTIONS': MODULESTORE_OPTIONS - }, - 'draft': { - 'ENGINE': 'xmodule.modulestore.draft.DraftModuleStore', - 'OPTIONS': MODULESTORE_OPTIONS - } -} - -CONTENTSTORE = { - 'ENGINE': 'xmodule.contentstore.mongo.MongoContentStore', - 'OPTIONS': { - 'host': 'localhost', - 'db': 'acceptance_xcontent', - }, - # allow for additional options that can be keyed on a name, e.g. 'trashcan' - 'ADDITIONAL_OPTIONS': { - 'trashcan': { - 'bucket': 'trash_fs' - } - } -} - -# Set this up so that rake lms[acceptance] and running the -# harvest command both use the same (test) database -# which they can flush without messing up your dev db -DATABASES = { - 'default': { - 'ENGINE': 'django.db.backends.sqlite3', - 'NAME': TEST_ROOT / "db" / "test_mitx.db", - 'TEST_NAME': TEST_ROOT / "db" / "test_mitx.db", - } -} - -# Include the lettuce app for acceptance testing, including the 'harvest' django-admin command -INSTALLED_APPS += ('lettuce.django',) -LETTUCE_APPS = ('contentstore',) -LETTUCE_SERVER_PORT = random.randint(1024, 65535) -LETTUCE_BROWSER = 'chrome' diff --git a/lms/envs/acceptance.py b/lms/envs/acceptance.py index e866a250d91b..5d0e350bb6da 100644 --- a/lms/envs/acceptance.py +++ b/lms/envs/acceptance.py @@ -65,8 +65,8 @@ def seed(): DATABASES = { 'default': { 'ENGINE': 'django.db.backends.sqlite3', - 'NAME': TEST_ROOT / "db" / "test_mitx_%s.db" % seed(), - 'TEST_NAME': TEST_ROOT / "db" / "test_mitx_%s.db" % seed(), + 'NAME': TEST_ROOT / "db" / "test_edx.db", + 'TEST_NAME': TEST_ROOT / "db" / "test_edx.db", } } diff --git a/lms/envs/acceptance_static.py b/lms/envs/acceptance_static.py deleted file mode 100644 index 27efb6160d48..000000000000 --- a/lms/envs/acceptance_static.py +++ /dev/null @@ -1,77 +0,0 @@ -""" -This config file extends the test environment configuration -so that we can run the lettuce acceptance tests. -""" - -# We intentionally define lots of variables that aren't used, and -# want to import all variables from base settings files -# pylint: disable=W0401, W0614 - -from .test import * - -# You need to start the server in debug mode, -# otherwise the browser will not render the pages correctly -DEBUG = True - -# Disable warnings for acceptance tests, to make the logs readable -import logging -logging.disable(logging.ERROR) -import random - -# Use the mongo store for acceptance tests -modulestore_options = { - 'default_class': 'xmodule.raw_module.RawDescriptor', - 'host': 'localhost', - 'db': 'acceptance_xmodule', - 'collection': 'acceptance_modulestore', - 'fs_root': TEST_ROOT / "data", - 'render_template': 'mitxmako.shortcuts.render_to_string', -} - -MODULESTORE = { - 'default': { - 'ENGINE': 'xmodule.modulestore.mongo.MongoModuleStore', - 'OPTIONS': modulestore_options - }, - 'direct': { - 'ENGINE': 'xmodule.modulestore.mongo.MongoModuleStore', - 'OPTIONS': modulestore_options - } -} - -CONTENTSTORE = { - 'ENGINE': 'xmodule.contentstore.mongo.MongoContentStore', - 'OPTIONS': { - 'host': 'localhost', - 'db': 'acceptance_xcontent', - } -} - -# Set this up so that rake lms[acceptance] and running the -# harvest command both use the same (test) database -# which they can flush without messing up your dev db -DATABASES = { - 'default': { - 'ENGINE': 'django.db.backends.sqlite3', - 'NAME': TEST_ROOT / "db" / "test_mitx.db", - 'TEST_NAME': TEST_ROOT / "db" / "test_mitx.db", - } -} - -# Set up XQueue information so that the lms will send -# requests to a mock XQueue server running locally -XQUEUE_PORT = random.randint(1024, 65535) -XQUEUE_INTERFACE = { - "url": "http://127.0.0.1:%d" % XQUEUE_PORT, - "django_auth": { - "username": "lms", - "password": "***REMOVED***" - }, - "basic_auth": ('anant', 'agarwal'), -} - -# Include the lettuce app for acceptance testing, including the 'harvest' django-admin command -INSTALLED_APPS += ('lettuce.django',) -LETTUCE_APPS = ('courseware',) -LETTUCE_SERVER_PORT = random.randint(1024, 65535) -LETTUCE_BROWSER = 'chrome' diff --git a/rakelib/tests.rake b/rakelib/tests.rake index 1c976b1c05d1..f7c0fbdc4f43 100644 --- a/rakelib/tests.rake +++ b/rakelib/tests.rake @@ -4,6 +4,8 @@ CLOBBER.include(REPORT_DIR, 'test_root/*_repo', 'test_root/staticfiles') # Create the directory to hold coverage reports, if it doesn't already exist. directory REPORT_DIR +ACCEPTANCE_DB = 'test_root/db/test_edx.db' + def test_id_dir(path) return File.join(".testids", path.to_s) end @@ -30,7 +32,7 @@ def run_tests(system, report_dir, test_id=nil, stop_on_failure=true) test_sh(run_under_coverage(cmd, system)) end -def run_acceptance_tests(system, report_dir, harvest_args) +def create_acceptance_db(system) # HACK: Since now the CMS depends on the existence of some database tables # that used to be in LMS (Role/Permissions for Forums) we need to make # sure the acceptance tests create/migrate the database tables @@ -43,6 +45,22 @@ def run_acceptance_tests(system, report_dir, harvest_args) end sh(django_admin(system, 'acceptance', 'syncdb', '--noinput')) sh(django_admin(system, 'acceptance', 'migrate', '--noinput')) +end + +def setup_acceptance_db(system, fasttest=false) + # If running under fasttest mode and the database already + # exists, skip the migrations. + if File.exists?(ACCEPTANCE_DB) + if not fasttest + File.delete(ACCEPTANCE_DB) + create_acceptance_db(system) + end + else + create_acceptance_db(system) + end +end + +def run_acceptance_tests(system, report_dir, harvest_args) test_sh(django_admin(system, 'acceptance', 'harvest', '--debug-mode', '--tag -skip', harvest_args)) end @@ -58,7 +76,7 @@ end task :clean_test_files do desc "Clean fixture files used by tests" - sh("git clean -fqdx test_root") + sh("git clean -fqdx test_root/logs test_root/data test_root/staticfiles test_root/uploads") end task :clean_reports_dir => REPORT_DIR do @@ -91,13 +109,15 @@ TEST_TASK_DIRS = [] # Run acceptance tests desc "Run acceptance tests" - #gather_assets uses its own env because acceptance contains seeds to make the information unique - #acceptance_static is acceptance without the random seeding - task "test_acceptance_#{system}", [:harvest_args] => [:clean_test_files, "#{system}:gather_assets:acceptance_static", "fasttest_acceptance_#{system}"] + task "test_acceptance_#{system}", [:harvest_args] => [:clean_test_files, "#{system}:gather_assets:acceptance"] do |t, args| + setup_acceptance_db(system) + Rake::Task["fasttest_acceptance_#{system}"].invoke(*args) + end - desc "Run acceptance tests without collectstatic" + desc "Run acceptance tests without collectstatic or database migrations" task "fasttest_acceptance_#{system}", [:harvest_args] => [report_dir, :clean_reports_dir, :predjango] do |t, args| args.with_defaults(:harvest_args => '') + setup_acceptance_db(system, fasttest=true) run_acceptance_tests(system, report_dir, args.harvest_args) end From 1ab59a2c3d10896d51cc5e24bed2ef57b4aa28c0 Mon Sep 17 00:00:00 2001 From: Adam Palay Date: Tue, 3 Sep 2013 08:07:07 -0400 Subject: [PATCH 05/13] add pyparsing explicitly to sandbox requirements --- requirements/edx-sandbox/base.txt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/requirements/edx-sandbox/base.txt b/requirements/edx-sandbox/base.txt index d5f05083c8e8..f88cc899cf25 100644 --- a/requirements/edx-sandbox/base.txt +++ b/requirements/edx-sandbox/base.txt @@ -1,3 +1,4 @@ numpy==1.6.2 networkx==1.7 -sympy==0.7.1 \ No newline at end of file +sympy==0.7.1 +pyparsing==1.5.6 \ No newline at end of file From a7159d8afd1312dd1b561cc46de5757327c86d1c Mon Sep 17 00:00:00 2001 From: Adam Palay Date: Tue, 3 Sep 2013 10:49:04 -0400 Subject: [PATCH 06/13] add pyparsing to edx requirements --- requirements/edx/base.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/requirements/edx/base.txt b/requirements/edx/base.txt index 3edce34f5478..8d609e05f192 100644 --- a/requirements/edx/base.txt +++ b/requirements/edx/base.txt @@ -41,6 +41,7 @@ pycrypto>=2.6 pygments==1.5 pygraphviz==1.1 pymongo==2.4.1 +pyparsing==1.5.6 python-memcached==1.48 python-openid==2.2.5 pytz==2012h From e94515df8091ef9aa06997f3bf9bbed132ee0cbe Mon Sep 17 00:00:00 2001 From: Will Daly Date: Tue, 3 Sep 2013 11:36:23 -0400 Subject: [PATCH 07/13] Fix for acceptance test static file issue --- common/djangoapps/terrain/browser.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/common/djangoapps/terrain/browser.py b/common/djangoapps/terrain/browser.py index 75c0764b1bb4..498197b59045 100644 --- a/common/djangoapps/terrain/browser.py +++ b/common/djangoapps/terrain/browser.py @@ -29,11 +29,13 @@ # to use staticfiles. try: import staticfiles + import staticfiles.handlers except ImportError: pass else: import sys sys.modules['django.contrib.staticfiles'] = staticfiles + sys.modules['django.contrib.staticfiles.handlers'] = staticfiles.handlers LOGGER = getLogger(__name__) LOGGER.info("Loading the lettuce acceptance testing terrain file...") From a09e6104036d253c65a573c05c4ee6ca2ee4ffd3 Mon Sep 17 00:00:00 2001 From: Anton Stupak Date: Fri, 30 Aug 2013 09:51:48 +0300 Subject: [PATCH 08/13] Fix multiple video bug --- .../xmodule/xmodule/css/video/display.scss | 27 ++++ .../xmodule/xmodule/js/fixtures/video.html | 2 + .../xmodule/js/fixtures/video_all.html | 4 +- .../xmodule/js/fixtures/video_html5.html | 4 +- .../js/fixtures/video_no_captions.html | 2 + .../lib/xmodule/xmodule/js/spec/helper.coffee | 16 ++- .../xmodule/js/spec/video/general_spec.js | 60 +++------ .../xmodule/js/src/video/01_initialize.js | 116 ++++++++++++++---- .../js/src/video/08_video_speed_control.js | 18 ++- .../xmodule/xmodule/js/src/video/10_main.js | 11 +- common/lib/xmodule/xmodule/video_module.py | 12 +- .../courseware/features/video.feature | 33 ++++- lms/djangoapps/courseware/features/video.py | 45 +++++++ .../courseware/features/youtube_setup.py | 45 +++++++ .../mock_youtube_server/__init__.py | 0 .../mock_youtube_server.py | 81 ++++++++++++ .../test_mock_youtube_server.py | 53 ++++++++ .../courseware/tests/test_video_mongo.py | 8 +- .../courseware/tests/test_video_xml.py | 4 +- lms/envs/acceptance.py | 5 + lms/envs/acceptance_static.py | 4 + lms/templates/video.html | 3 + 22 files changed, 463 insertions(+), 90 deletions(-) create mode 100644 lms/djangoapps/courseware/features/youtube_setup.py create mode 100644 lms/djangoapps/courseware/mock_youtube_server/__init__.py create mode 100644 lms/djangoapps/courseware/mock_youtube_server/mock_youtube_server.py create mode 100644 lms/djangoapps/courseware/mock_youtube_server/test_mock_youtube_server.py diff --git a/common/lib/xmodule/xmodule/css/video/display.scss b/common/lib/xmodule/xmodule/css/video/display.scss index 533ab2aec0dc..dc801be0f99a 100644 --- a/common/lib/xmodule/xmodule/css/video/display.scss +++ b/common/lib/xmodule/xmodule/css/video/display.scss @@ -40,6 +40,12 @@ div.video { padding-bottom: 56.25%; position: relative; + div { + &.hidden { + display: none; + } + } + object, iframe { border: none; height: 100%; @@ -48,6 +54,15 @@ div.video { top: 0; width: 100%; } + + h3 { + text-align: center; + color: white; + + &.hidden { + display: none; + } + } } section.video-controls { @@ -516,6 +531,12 @@ div.video { height: 0px; } + article.video-wrapper section.video-player { + h3 { + color: black; + } + } + ol.subtitles { width: 0; height: 0; @@ -563,6 +584,12 @@ div.video { position: static; } + article.video-wrapper section.video-player { + h3 { + color: white; + } + } + div.tc-wrapper { @include clearfix; display: table; diff --git a/common/lib/xmodule/xmodule/js/fixtures/video.html b/common/lib/xmodule/xmodule/js/fixtures/video.html index 341e18ae9d66..6e4df9ec9c77 100644 --- a/common/lib/xmodule/xmodule/js/fixtures/video.html +++ b/common/lib/xmodule/xmodule/js/fixtures/video.html @@ -10,6 +10,8 @@ data-end="" data-caption-asset-path="/static/subs/" data-autoplay="False" + data-yt-test-timeout="1500" + data-yt-test-url="https://gdata.youtube.com/feeds/api/videos/" >
diff --git a/common/lib/xmodule/xmodule/js/fixtures/video_all.html b/common/lib/xmodule/xmodule/js/fixtures/video_all.html index 25a3c2c0ab4f..85fd004976b9 100644 --- a/common/lib/xmodule/xmodule/js/fixtures/video_all.html +++ b/common/lib/xmodule/xmodule/js/fixtures/video_all.html @@ -13,6 +13,8 @@ data-webm-source="test_files/test.webm" data-ogg-source="test_files/test.ogv" data-autoplay="False" + data-yt-test-timeout="1500" + data-yt-test-url="https://gdata.youtube.com/feeds/api/videos/" >
@@ -55,4 +57,4 @@

Speed

- \ No newline at end of file + diff --git a/common/lib/xmodule/xmodule/js/fixtures/video_html5.html b/common/lib/xmodule/xmodule/js/fixtures/video_html5.html index 677ab9b24738..f2c749ef2746 100644 --- a/common/lib/xmodule/xmodule/js/fixtures/video_html5.html +++ b/common/lib/xmodule/xmodule/js/fixtures/video_html5.html @@ -13,6 +13,8 @@ data-webm-source="test_files/test.webm" data-ogg-source="test_files/test.ogv" data-autoplay="False" + data-yt-test-timeout="1500" + data-yt-test-url="https://gdata.youtube.com/feeds/api/videos/" >
@@ -27,4 +29,4 @@
- \ No newline at end of file + diff --git a/common/lib/xmodule/xmodule/js/fixtures/video_no_captions.html b/common/lib/xmodule/xmodule/js/fixtures/video_no_captions.html index c611acfffdd9..69207230fa5d 100644 --- a/common/lib/xmodule/xmodule/js/fixtures/video_no_captions.html +++ b/common/lib/xmodule/xmodule/js/fixtures/video_no_captions.html @@ -10,6 +10,8 @@ data-end="" data-caption-asset-path="/static/subs/" data-autoplay="False" + data-yt-test-timeout="1500" + data-yt-test-url="https://gdata.youtube.com/feeds/api/videos/" >
diff --git a/common/lib/xmodule/xmodule/js/spec/helper.coffee b/common/lib/xmodule/xmodule/js/spec/helper.coffee index f3cecf71cbe6..7b5d3156e909 100644 --- a/common/lib/xmodule/xmodule/js/spec/helper.coffee +++ b/common/lib/xmodule/xmodule/js/spec/helper.coffee @@ -90,12 +90,24 @@ jasmine.stubbedHtml5Speeds = ['0.75', '1.0', '1.25', '1.50'] jasmine.stubRequests = -> spyOn($, 'ajax').andCallFake (settings) -> if match = settings.url.match /youtube\.com\/.+\/videos\/(.+)\?v=2&alt=jsonc/ - if settings.success + status = match[1].split('_') + if status and status[0] is 'status' + { + always: (callback) -> + callback.call(window, {}, status[1]) + error: (callback) -> + callback.call(window, {}, status[1]) + done: (callback) -> + callback.call(window, {}, status[1]) + } + else if settings.success # match[1] - it's video ID settings.success data: jasmine.stubbedMetadata[match[1]] else { always: (callback) -> - callback.call(window, {}, 'success'); + callback.call(window, {}, 'success') + done: (callback) -> + callback.call(window, {}, 'success') } else if match = settings.url.match /static(\/.*)?\/subs\/(.+)\.srt\.sjson/ settings.success jasmine.stubbedCaption diff --git a/common/lib/xmodule/xmodule/js/spec/video/general_spec.js b/common/lib/xmodule/xmodule/js/spec/video/general_spec.js index 9194106fff12..54f952bffbcd 100644 --- a/common/lib/xmodule/xmodule/js/spec/video/general_spec.js +++ b/common/lib/xmodule/xmodule/js/spec/video/general_spec.js @@ -55,46 +55,6 @@ expect(this.state.speed).toEqual('0.75'); }); }); - - describe('Check Youtube link existence', function () { - var statusList = { - error: 'html5', - timeout: 'html5', - abort: 'html5', - parsererror: 'html5', - success: 'youtube', - notmodified: 'youtube' - }; - - function stubDeffered(data, status) { - return { - always: function(callback) { - callback.call(window, data, status); - } - } - } - - function checkPlayer(videoType, data, status) { - this.state = new window.Video('#example'); - spyOn(this.state , 'getVideoMetadata') - .andReturn(stubDeffered(data, status)); - this.state.initialize('#example'); - - expect(this.state.videoType).toEqual(videoType); - } - - it('if video id is incorrect', function () { - checkPlayer('html5', { error: {} }, 'success'); - }); - - $.each(statusList, function(status, mode){ - it('Status:' + status + ', mode:' + mode, function () { - checkPlayer(mode, {}, status); - }); - }); - - }); - }); describe('HTML5', function () { @@ -154,10 +114,22 @@ it('parse Html5 sources', function () { var html5Sources = { - mp4: 'test_files/test.mp4', - webm: 'test_files/test.webm', - ogg: 'test_files/test.ogv' - }; + mp4: null, + webm: null, + ogg: null + }, v = document.createElement('video'); + + if (!!(v.canPlayType && v.canPlayType('video/webm; codecs="vp8, vorbis"').replace(/no/, ''))) { + html5Sources['webm'] = 'xmodule/include/fixtures/test.webm'; + } + + if (!!(v.canPlayType && v.canPlayType('video/mp4; codecs="avc1.42E01E, mp4a.40.2"').replace(/no/, ''))) { + html5Sources['mp4'] = 'xmodule/include/fixtures/test.mp4'; + } + + if (!!(v.canPlayType && v.canPlayType('video/ogg; codecs="theora"').replace(/no/, ''))) { + html5Sources['ogg'] = 'xmodule/include/fixtures/test.ogv'; + } expect(state.html5Sources).toEqual(html5Sources); }); 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..b41bdd6f1cad 100644 --- a/common/lib/xmodule/xmodule/js/src/video/01_initialize.js +++ b/common/lib/xmodule/xmodule/js/src/video/01_initialize.js @@ -143,8 +143,6 @@ function (VideoPlayer) { if (state.parseYoutubeStreams(state.config.youtubeStreams)) { state.videoType = 'youtube'; - state.fetchMetadata(); - state.parseSpeed(); return true; } return false; @@ -153,9 +151,7 @@ function (VideoPlayer) { // function _prepareHTML5Video(state) // The function prepare HTML5 video, parse HTML5 // video sources etc. - function _prepareHTML5Video(state) { - state.videoType = 'html5'; - + function _prepareHTML5Video(state, html5Mode) { state.parseVideoSources( { mp4: state.config.mp4Source, @@ -164,20 +160,39 @@ function (VideoPlayer) { } ); + if (html5Mode) { + state.speeds = ['0.75', '1.0', '1.25', '1.50']; + state.videos = { + '0.75': state.config.sub, + '1.0': state.config.sub, + '1.25': state.config.sub, + '1.5': state.config.sub + }; + } + + // We must have at least one non-YouTube video source available. + // Otherwise, return a negative. + if ( + state.html5Sources.webm === null && + state.html5Sources.mp4 === null && + state.html5Sources.ogg === null + ) { + state.el.find('.video-player div').addClass('hidden'); + state.el.find('.video-player h3').removeClass('hidden'); + + return false; + } + + state.videoType = 'html5'; + if (!state.config.sub || !state.config.sub.length) { state.config.sub = ''; state.config.show_captions = false; } - state.speeds = ['0.75', '1.0', '1.25', '1.50']; - state.videos = { - '0.75': state.config.sub, - '1.0': state.config.sub, - '1.25': state.config.sub, - '1.5': state.config.sub - }; - state.setSpeed($.cookie('video_speed')); + + return true; } function _setConfigurations(state) { @@ -201,7 +216,7 @@ function (VideoPlayer) { // The function set initial configuration and preparation. function initialize(element) { - var _this = this; + var _this = this, tempYtTestTimeout; // This is used in places where we instead would have to check if an element has a CSS class 'fullscreen'. this.isFullScreen = false; @@ -227,28 +242,61 @@ function (VideoPlayer) { webmSource: this.el.data('webm-source'), oggSource: this.el.data('ogg-source'), + ytTestUrl: this.el.data('yt-test-url'), + fadeOutTimeout: 1400, availableQualities: ['hd720', 'hd1080', 'highres'] }; + // Check if the YT test timeout has been set. If not, or it is in + // improper format, then set to default value. + tempYtTestTimeout = parseInt(this.el.data('yt-test-timeout'), 10); + if (!isFinite(tempYtTestTimeout)) { + tempYtTestTimeout = 1500; + } + this.config.ytTestTimeout = tempYtTestTimeout; + if (!(_parseYouTubeIDs(this))) { // If we do not have YouTube ID's, try parsing HTML5 video sources. - _prepareHTML5Video(this); + if (!_prepareHTML5Video(this, true)) { + // Non-YouTube sources were not found either. + return; + } + _setConfigurations(this); _renderElements(this); } else { - this.getVideoMetadata() + if (!this.youtubeXhr) { + this.youtubeXhr = this.getVideoMetadata(); + } + + this.youtubeXhr .always(function(json, status) { var err = $.isPlainObject(json.error) || - (status !== "success" && status !== "notmodified"); - - if (err){ + (status !== 'success' && status !== 'notmodified'); + if (err) { // When the youtube link doesn't work for any reason // (for example, the great firewall in china) any // alternate sources should automatically play. - _prepareHTML5Video(_this); - _this.el.find('a.quality_control').hide(); + if (!_prepareHTML5Video(_this)) { + // Non-YouTube sources were not found either. + + _this.el.find('.video-player div').removeClass('hidden'); + _this.el.find('.video-player h3').addClass('hidden'); + + // If in reality the timeout was to short, try to + // continue loading the YouTube video anyways. + _this.fetchMetadata(); + _this.parseSpeed(); + } else { + // In-browser HTML5 player does not support quality + // control. + _this.el.find('a.quality_control').hide(); + } + } else { + _this.fetchMetadata(); + _this.parseSpeed(); } _setConfigurations(_this); @@ -294,7 +342,13 @@ function (VideoPlayer) { // Take the HTML5 sources (URLs of videos), and make them available explictly for each type // of video format (mp4, webm, ogg). function parseVideoSources(sources) { - var _this = this; + var _this = this, + v = document.createElement('video'), + sourceCodecs = { + mp4: 'video/mp4; codecs="avc1.42E01E, mp4a.40.2"', + webm: 'video/webm; codecs="vp8, vorbis"', + ogg: 'video/ogg; codecs="theora"' + }; this.html5Sources = { mp4: null, @@ -304,7 +358,14 @@ function (VideoPlayer) { $.each(sources, function (name, source) { if (source && source.length) { - _this.html5Sources[name] = source; + if ( + Boolean( + v.canPlayType && + v.canPlayType(sourceCodecs[name]).replace(/no/, '') + ) + ) { + _this.html5Sources[name] = source; + } } }); } @@ -321,7 +382,9 @@ function (VideoPlayer) { $.each(this.videos, function (speed, url) { _this.getVideoMetadata(url, function(data) { - _this.metadata[data.data.id] = data.data; + if (data.data) { + _this.metadata[data.data.id] = data.data; + } }); }); } @@ -358,12 +421,11 @@ function (VideoPlayer) { if (typeof url !== 'string') { url = this.videos['1.0'] || ''; } - successHandler = ($.isFunction(callback)) ? callback : null; xhr = $.ajax({ - url: 'https://gdata.youtube.com/feeds/api/videos/' + url + '?v=2&alt=jsonc', - timeout: 500, + url: this.config.ytTestUrl + url + '?v=2&alt=jsonc', dataType: 'jsonp', + timeout: this.config.ytTestTimeout, success: successHandler }); diff --git a/common/lib/xmodule/xmodule/js/src/video/08_video_speed_control.js b/common/lib/xmodule/xmodule/js/src/video/08_video_speed_control.js index c315e4afbced..91d2ba6fba75 100644 --- a/common/lib/xmodule/xmodule/js/src/video/08_video_speed_control.js +++ b/common/lib/xmodule/xmodule/js/src/video/08_video_speed_control.js @@ -10,21 +10,31 @@ function () { return function (state) { state.videoSpeedControl = {}; + if (state.videoType === 'html5') { + _initialize(state); + } else if (state.videoType === 'youtube' && state.youtubeXhr) { + state.youtubeXhr.done(function () { + _initialize(state); + }); + } + if (state.videoType === 'html5' && !(_checkPlaybackRates())) { _hideSpeedControl(state); return; } - - _makeFunctionsPublic(state); - _renderElements(state); - _bindHandlers(state); }; // *************************************************************** // Private functions start here. // *************************************************************** + function _initialize(state) { + _makeFunctionsPublic(state); + _renderElements(state); + _bindHandlers(state); + } + // function _makeFunctionsPublic(state) // // Functions which will be accessible via 'state' object. When called, diff --git a/common/lib/xmodule/xmodule/js/src/video/10_main.js b/common/lib/xmodule/xmodule/js/src/video/10_main.js index 70fdbc580d2b..457433592a8c 100644 --- a/common/lib/xmodule/xmodule/js/src/video/10_main.js +++ b/common/lib/xmodule/xmodule/js/src/video/10_main.js @@ -20,7 +20,8 @@ function ( VideoSpeedControl, VideoCaption ) { - var previousState; + var previousState, + youtubeXhr = null; // Because this constructor can be called multiple times on a single page (when // the user switches verticals, the page doesn't reload, but the content changes), we must @@ -53,7 +54,11 @@ function ( state = {}; previousState = state; + state.youtubeXhr = youtubeXhr; Initialize(state, element); + if (!youtubeXhr) { + youtubeXhr = state.youtubeXhr; + } VideoControl(state); VideoQualityControl(state); @@ -67,6 +72,10 @@ function ( // Video with Jasmine. return state; }; + + window.Video.clearYoutubeXhr = function () { + youtubeXhr = null; + }; }); }(RequireJS.requirejs, RequireJS.require, RequireJS.define)); diff --git a/common/lib/xmodule/xmodule/video_module.py b/common/lib/xmodule/xmodule/video_module.py index be77cd268426..8ea87b2d41b8 100644 --- a/common/lib/xmodule/xmodule/video_module.py +++ b/common/lib/xmodule/xmodule/video_module.py @@ -167,6 +167,12 @@ def get_html(self): sources = {get_ext(src): src for src in self.html5_sources} sources['main'] = self.source + # for testing Youtube timeout in acceptance tests + if getattr(settings, 'VIDEO_PORT', None): + yt_test_url = "http://127.0.0.1:" + str(settings.VIDEO_PORT) + '/test_youtube/' + else: + yt_test_url = 'https://gdata.youtube.com/feeds/api/videos/' + return self.system.render_template('video.html', { 'youtube_streams': _create_youtube_string(self), 'id': self.location.html_id(), @@ -181,7 +187,11 @@ def get_html(self): 'show_captions': json.dumps(self.show_captions), 'start': self.start_time, 'end': self.end_time, - 'autoplay': settings.MITX_FEATURES.get('AUTOPLAY_VIDEOS', True) + 'autoplay': settings.MITX_FEATURES.get('AUTOPLAY_VIDEOS', True), + # TODO: Later on the value 1500 should be taken from some global + # configuration setting field. + 'yt_test_timeout': 1500, + 'yt_test_url': yt_test_url }) diff --git a/lms/djangoapps/courseware/features/video.feature b/lms/djangoapps/courseware/features/video.feature index 6c8299f2c531..b741c8bee11c 100644 --- a/lms/djangoapps/courseware/features/video.feature +++ b/lms/djangoapps/courseware/features/video.feature @@ -1,18 +1,39 @@ Feature: Video component As a student, I want to view course videos in LMS. - Scenario: Video component is fully rendered in the LMS in HTML5 mode Given the course has a Video component in HTML5 mode Then when I view the video it has rendered in HTML5 mode And all sources are correct - Scenario: Video component is fully rendered in the LMS in Youtube mode - Given the course has a Video component in Youtube mode - Then when I view the video it has rendered in Youtube mode - - # Firefox doesn't have HTML5 + # Firefox doesn't have HTML5 (only mp4 - fix here) @skip_firefox Scenario: Autoplay is enabled in LMS for a Video component Given the course has a Video component in HTML5 mode Then when I view the video it has autoplay enabled + +# Youtube testing +Scenario: Video component is fully rendered in the LMS in Youtube mode with HTML5 sources +Given youtube server is up and response time is 0.4 seconds +And the course has a Video component in Youtube_HTML5 mode +Then when I view the video it has rendered in Youtube mode + +Scenario: Video component is not rendered in the LMS in Youtube mode with HTML5 sources +Given youtube server is up and response time is 2 seconds +And the course has a Video component in Youtube_HTML5 mode +Then when I view the video it has rendered in HTML5 mode + +Scenario: Video component is rendered in the LMS in Youtube mode without HTML5 sources +Given youtube server is up and response time is 2 seconds +And the course has a Video component in Youtube mode +Then when I view the video it has rendered in Youtube mode + +Scenario: Video component is rendered in the LMS in Youtube mode with HTML5 sources that doesn't supported by browser +Given youtube server is up and response time is 2 seconds +And the course has a Video component in Youtube_HTML5_Unsupported_Video mode +Then when I view the video it has rendered in Youtube mode + +Scenario: Video component is rendered in the LMS in HTML5 mode with HTML5 sources that doesn't supported by browser +Given the course has a Video component in HTML5_Unsupported_Video mode +Then error message is shown +And error message has correct text diff --git a/lms/djangoapps/courseware/features/video.py b/lms/djangoapps/courseware/features/video.py index f5977920197d..e0a1461aea5e 100644 --- a/lms/djangoapps/courseware/features/video.py +++ b/lms/djangoapps/courseware/features/video.py @@ -3,6 +3,7 @@ from lettuce import world, step from lettuce.django import django_url from common import i_am_registered_for_the_course, section_location +from django.utils.translation import ugettext as _ ############### ACTIONS #################### @@ -11,6 +12,9 @@ 'https://s3.amazonaws.com/edx-course-videos/edx-intro/edX-FA12-cware-1_100.webm', 'https://s3.amazonaws.com/edx-course-videos/edx-intro/edX-FA12-cware-1_100.ogv' ] +HTML5_SOURCES_INCORRECT = [ + 'https://s3.amazonaws.com/edx-course-videos/edx-intro/edX-FA12-cware-1_100.mp99' +] @step('when I view the (.*) it has autoplay enabled$') def does_autoplay_video(_step, video_type): @@ -51,10 +55,37 @@ def add_video_to_course(course, player_mode): 'html5_sources': HTML5_SOURCES } }) + if player_mode == 'youtube_html5': + kwargs.update({ + 'metadata': { + 'html5_sources': HTML5_SOURCES + } + }) + if player_mode == 'youtube_html5_unsupported_video': + kwargs.update({ + 'metadata': { + 'html5_sources': HTML5_SOURCES_INCORRECT + } + }) + if player_mode == 'html5_unsupported_video': + kwargs.update({ + 'metadata': { + 'youtube_id_1_0': '', + 'youtube_id_0_75': '', + 'youtube_id_1_25': '', + 'youtube_id_1_5': '', + 'html5_sources': HTML5_SOURCES_INCORRECT + } + }) world.ItemFactory.create(**kwargs) +@step('youtube server is up and response time is (.*) seconds$') +def set_youtube_response_timeout(_step, time): + world.youtube_server.time_to_response = time + + @step('when I view the video it has rendered in (.*) mode$') def video_is_rendered(_step, mode): modes = { @@ -64,9 +95,23 @@ def video_is_rendered(_step, mode): html_tag = modes[mode.lower()] assert world.css_find('.video {0}'.format(html_tag)).first + @step('all sources are correct$') def all_sources_are_correct(_step): sources = world.css_find('.video video source') assert set(source['src'] for source in sources) == set(HTML5_SOURCES) +@step('error message is shown$') +def error_message_is_shown(_step): + selector = '.video .video-player h3' + assert world.css_visible(selector) + + +@step('error message has correct text$') +def error_message_has_correct_text(_step): + selector = '.video .video-player h3' + text = _('ERROR: No playable video sources found!') + assert world.css_has_text(selector, text) + + diff --git a/lms/djangoapps/courseware/features/youtube_setup.py b/lms/djangoapps/courseware/features/youtube_setup.py new file mode 100644 index 000000000000..8233d1f4586e --- /dev/null +++ b/lms/djangoapps/courseware/features/youtube_setup.py @@ -0,0 +1,45 @@ +#pylint: disable=C0111 +#pylint: disable=W0621 + +from courseware.mock_youtube_server.mock_youtube_server import MockYoutubeServer +from lettuce import before, after, world +from django.conf import settings +import threading + +from logging import getLogger +logger = getLogger(__name__) + + +@before.all +def setup_mock_youtube_server(): + # import ipdb; ipdb.set_trace() + server_host = '127.0.0.1' + + server_port = settings.VIDEO_PORT + + address = (server_host, server_port) + + # Create the mock server instance + server = MockYoutubeServer(address) + logger.debug("Youtube server started at {} port".format(str(server_port))) + + server.time_to_response = 1 # seconds + + # Start the server running in a separate daemon thread + # Because the thread is a daemon, it will terminate + # when the main thread terminates. + server_thread = threading.Thread(target=server.serve_forever) + server_thread.daemon = True + server_thread.start() + + # Store the server instance in lettuce's world + # so that other steps can access it + # (and we can shut it down later) + world.youtube_server = server + + +@after.all +def teardown_mock_youtube_server(total): + + # Stop the LTI server and free up the port + world.youtube_server.shutdown() diff --git a/lms/djangoapps/courseware/mock_youtube_server/__init__.py b/lms/djangoapps/courseware/mock_youtube_server/__init__.py new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/lms/djangoapps/courseware/mock_youtube_server/mock_youtube_server.py b/lms/djangoapps/courseware/mock_youtube_server/mock_youtube_server.py new file mode 100644 index 000000000000..46b269dda683 --- /dev/null +++ b/lms/djangoapps/courseware/mock_youtube_server/mock_youtube_server.py @@ -0,0 +1,81 @@ +from BaseHTTPServer import HTTPServer, BaseHTTPRequestHandler +import urlparse +from requests.packages.oauthlib.oauth1.rfc5849 import signature +import mock +import threading +import json +from logging import getLogger +logger = getLogger(__name__) +import time + +class MockYoutubeRequestHandler(BaseHTTPRequestHandler): + ''' + A handler for Youtube GET requests. + ''' + + protocol = "HTTP/1.0" + + def do_HEAD(self): + self._send_head() + + def do_GET(self): + ''' + Handle a GET request from the client and sends response back. + ''' + self._send_head() + + logger.debug("Youtube provider received GET request to path {}".format( + self.path) + ) # Log the request + + status_message = "I'm youtube." + response_timeout = float(self.server.time_to_response) + + # threading timer produces TypeError: 'NoneType' object is not callable here + # so we use time.sleep, as we already in separate thread. + time.sleep(response_timeout) + self._send_response(status_message) + + def _send_head(self): + ''' + Send the response code and MIME headers + ''' + self.send_response(200) + self.send_header('Content-type', 'text/html') + self.end_headers() + + def _send_response(self, message): + ''' + Send message back to the client + ''' + callback = urlparse.parse_qs(self.path)['callback'][0] + response = callback + '({})'.format(json.dumps({'message': message})) + # Log the response + logger.debug("Youtube: sent response {}".format(message)) + + self.wfile.write(response) + + +class MockYoutubeServer(HTTPServer): + ''' + A mock Youtube provider server that responds + to GET requests to localhost. + ''' + + def __init__(self, address): + ''' + Initialize the mock XQueue server instance. + + *address* is the (host, host's port to listen to) tuple. + ''' + handler = MockYoutubeRequestHandler + HTTPServer.__init__(self, address, handler) + + def shutdown(self): + ''' + Stop the server and free up the port + ''' + # First call superclass shutdown() + HTTPServer.shutdown(self) + # We also need to manually close the socket + self.socket.close() diff --git a/lms/djangoapps/courseware/mock_youtube_server/test_mock_youtube_server.py b/lms/djangoapps/courseware/mock_youtube_server/test_mock_youtube_server.py new file mode 100644 index 000000000000..4ccd7cdc58db --- /dev/null +++ b/lms/djangoapps/courseware/mock_youtube_server/test_mock_youtube_server.py @@ -0,0 +1,53 @@ +""" +Test for Mock_Youtube_Server +""" +import unittest +import threading +import urllib +from mock_youtube_server import MockYoutubeServer + +from nose.plugins.skip import SkipTest + + +class MockYoutubeServerTest(unittest.TestCase): + ''' + A mock version of the Youtube provider server that listens on a local + port and responds with jsonp. + + Used for lettuce BDD tests in lms/courseware/features/video.feature + ''' + + def setUp(self): + + # This is a test of the test setup, + # so it does not need to run as part of the unit test suite + # You can re-enable it by commenting out the line below + raise SkipTest + + # Create the server + server_port = 8034 + server_host = '127.0.0.1' + address = (server_host, server_port) + self.server = MockYoutubeServer(address, ) + self.server.time_to_response = 0.5 + # Start the server in a separate daemon thread + server_thread = threading.Thread(target=self.server.serve_forever) + server_thread.daemon = True + server_thread.start() + + def tearDown(self): + + # Stop the server, freeing up the port + self.server.shutdown() + + def test_request(self): + """ + Tests that Youtube server processes request with right program + path, and responses with incorrect signature. + """ + # GET request + response_handle = urllib.urlopen( + 'http://127.0.0.1:8034/feeds/api/videos/OEoXaMPEzfM?v=2&alt=jsonc&callback=callback_func', + ) + response = response_handle.read() + self.assertEqual("""callback_func({"message": "I\'m youtube."})""", response) diff --git a/lms/djangoapps/courseware/tests/test_video_mongo.py b/lms/djangoapps/courseware/tests/test_video_mongo.py index 3436938cc09b..b393b33da857 100644 --- a/lms/djangoapps/courseware/tests/test_video_mongo.py +++ b/lms/djangoapps/courseware/tests/test_video_mongo.py @@ -64,7 +64,9 @@ def test_video_constructor(self): 'sub': u'a_sub_file.srt.sjson', 'track': '', 'youtube_streams': _create_youtube_string(self.item_module), - 'autoplay': settings.MITX_FEATURES.get('AUTOPLAY_VIDEOS', True) + 'autoplay': settings.MITX_FEATURES.get('AUTOPLAY_VIDEOS', True), + 'yt_test_timeout': 1500, + 'yt_test_url': 'https://gdata.youtube.com/feeds/api/videos/' } self.maxDiff = None @@ -114,7 +116,9 @@ def test_video_constructor(self): 'sub': 'a_sub_file.srt.sjson', 'track': '', 'youtube_streams': '1.00:OEoXaMPEzfM', - 'autoplay': settings.MITX_FEATURES.get('AUTOPLAY_VIDEOS', True) + 'autoplay': settings.MITX_FEATURES.get('AUTOPLAY_VIDEOS', True), + 'yt_test_timeout': 1500, + 'yt_test_url': 'https://gdata.youtube.com/feeds/api/videos/' } self.assertEqual(context, expected_context) diff --git a/lms/djangoapps/courseware/tests/test_video_xml.py b/lms/djangoapps/courseware/tests/test_video_xml.py index 33df1432c029..d79017346854 100644 --- a/lms/djangoapps/courseware/tests/test_video_xml.py +++ b/lms/djangoapps/courseware/tests/test_video_xml.py @@ -92,7 +92,9 @@ def test_video_get_html(self): 'sources': sources, 'youtube_streams': _create_youtube_string(module), 'track': '', - 'autoplay': settings.MITX_FEATURES.get('AUTOPLAY_VIDEOS', True) + 'autoplay': settings.MITX_FEATURES.get('AUTOPLAY_VIDEOS', True), + 'yt_test_timeout': 1500, + 'yt_test_url': 'https://gdata.youtube.com/feeds/api/videos/' } self.assertEqual(module.get_html(), expected_context) diff --git a/lms/envs/acceptance.py b/lms/envs/acceptance.py index e866a250d91b..7924780f3abc 100644 --- a/lms/envs/acceptance.py +++ b/lms/envs/acceptance.py @@ -82,6 +82,11 @@ def seed(): "basic_auth": ('anant', 'agarwal'), } + +# Set up Video information so that the lms will send +# requests to a mock Youtube server running locally +VIDEO_PORT = XQUEUE_PORT + 2 + # Forums are disabled in test.py to speed up unit tests, but we do not have # per-test control for acceptance tests MITX_FEATURES['ENABLE_DISCUSSION_SERVICE'] = True diff --git a/lms/envs/acceptance_static.py b/lms/envs/acceptance_static.py index 27efb6160d48..c09c9e29e816 100644 --- a/lms/envs/acceptance_static.py +++ b/lms/envs/acceptance_static.py @@ -70,6 +70,10 @@ "basic_auth": ('anant', 'agarwal'), } +# Set up Video information so that the lms will send +# requests to a mock Youtube server running locally +VIDEO_PORT = XQUEUE_PORT + 2 + # Include the lettuce app for acceptance testing, including the 'harvest' django-admin command INSTALLED_APPS += ('lettuce.django',) LETTUCE_APPS = ('courseware',) diff --git a/lms/templates/video.html b/lms/templates/video.html index 43f36915a0f7..3f06f0051181 100644 --- a/lms/templates/video.html +++ b/lms/templates/video.html @@ -23,6 +23,8 @@

${display_name}

data-end="${end}" data-caption-asset-path="${caption_asset_path}" data-autoplay="${autoplay}" + data-yt-test-timeout="${yt_test_timeout}" + data-yt-test-url="${yt_test_url}" >
@@ -30,6 +32,7 @@

${display_name}

+
From 300f7f13a6908fc958c8c9a4948c1a8167595096 Mon Sep 17 00:00:00 2001 From: Adam Palay Date: Tue, 3 Sep 2013 13:35:04 -0400 Subject: [PATCH 09/13] fix pep8 and pylint errors Conflicts: cms/djangoapps/contentstore/views/course.py common/lib/xmodule/xmodule/tests/test_combined_open_ended.py lms/envs/common.py --- .../management/commands/check_course.py | 1 - cms/djangoapps/contentstore/views/course.py | 85 ++++++++++--------- common/lib/capa/capa/inputtypes.py | 57 +++++++------ .../xmodule/tests/test_combined_open_ended.py | 18 ++-- lms/envs/common.py | 14 +-- 5 files changed, 94 insertions(+), 81 deletions(-) diff --git a/cms/djangoapps/contentstore/management/commands/check_course.py b/cms/djangoapps/contentstore/management/commands/check_course.py index 13ac6af50c21..541f5dee75cd 100644 --- a/cms/djangoapps/contentstore/management/commands/check_course.py +++ b/cms/djangoapps/contentstore/management/commands/check_course.py @@ -60,4 +60,3 @@ def _get_discussion_items(module): for item in queried_discussion_items: if item.location.url() not in discussion_items: print 'Found dangling discussion module = {0}'.format(item.location.url()) - diff --git a/cms/djangoapps/contentstore/views/course.py b/cms/djangoapps/contentstore/views/course.py index 939286a7656c..772dfd277896 100644 --- a/cms/djangoapps/contentstore/views/course.py +++ b/cms/djangoapps/contentstore/views/course.py @@ -124,29 +124,33 @@ def create_new_course(request): pass if existing_course is not None: return JsonResponse({ - 'ErrMsg': _('There is already a course defined with the same ' - 'organization, course number, and course run. Please ' - 'change either organization or course number to be ' - 'unique.'), - 'OrgErrMsg': _('Please change either the organization or ' - 'course number so that it is unique.'), - 'CourseErrMsg': _('Please change either the organization or ' - 'course number so that it is unique.'), + 'ErrMsg': _('There is already a course defined with the same ' + 'organization, course number, and course run. Please ' + 'change either organization or course number to be ' + 'unique.'), + 'OrgErrMsg': _('Please change either the organization or ' + 'course number so that it is unique.'), + 'CourseErrMsg': _('Please change either the organization or ' + 'course number so that it is unique.'), }) - course_search_location = ['i4x', dest_location.org, dest_location.course, - 'course', None + course_search_location = [ + 'i4x', + dest_location.org, + dest_location.course, + 'course', + None ] courses = modulestore().get_items(course_search_location) if len(courses) > 0: return JsonResponse({ - 'ErrMsg': _('There is already a course defined with the same ' - 'organization and course number. Please ' - 'change at least one field to be unique.'), - 'OrgErrMsg': _('Please change either the organization or ' - 'course number so that it is unique.'), - 'CourseErrMsg': _('Please change either the organization or ' - 'course number so that it is unique.'), + 'ErrMsg': _('There is already a course defined with the same ' + 'organization and course number. Please ' + 'change at least one field to be unique.'), + 'OrgErrMsg': _('Please change either the organization or ' + 'course number so that it is unique.'), + 'CourseErrMsg': _('Please change either the organization or ' + 'course number so that it is unique.'), }) # instantiate the CourseDescriptor and then persist it @@ -156,15 +160,15 @@ def create_new_course(request): else: metadata = {'display_name': display_name} modulestore('direct').create_and_save_xmodule( - dest_location, - metadata=metadata + dest_location, + metadata=metadata ) new_course = modulestore('direct').get_item(dest_location) # clone a default 'about' overview module as well dest_about_location = dest_location.replace( - category='about', - name='overview' + category='about', + name='overview' ) overview_template = AboutDescriptor.get_template('overview.yaml') modulestore('direct').create_and_save_xmodule( @@ -203,13 +207,16 @@ def course_info(request, org, course, name, provided_id=None): # get current updates location = Location(['i4x', org, course, 'course_info', "updates"]) - return render_to_response('course_info.html', { - 'context_course': course_module, - 'url_base': "/" + org + "/" + course + "/", - 'course_updates': json.dumps(get_course_updates(location)), - 'handouts_location': Location(['i4x', org, course, 'course_info', 'handouts']).url(), - 'base_asset_url': StaticContent.get_base_url_path_for_course_assets(location) + '/'}) - + return render_to_response( + 'course_info.html', + { + 'context_course': course_module, + 'url_base': "/" + org + "/" + course + "/", + 'course_updates': json.dumps(get_course_updates(location)), + 'handouts_location': Location(['i4x', org, course, 'course_info', 'handouts']).url(), + 'base_asset_url': StaticContent.get_base_url_path_for_course_assets(location) + '/' + } + ) @expect_json @require_http_methods(("GET", "POST", "PUT", "DELETE")) @@ -245,7 +252,7 @@ def course_info_updates(request, org, course, provided_id=None): content_type="text/plain" ) # can be either and sometimes django is rewriting one to the other: - elif request.method in ('POST', 'PUT'): + elif request.method in ('POST', 'PUT'): try: return JsonResponse(update_course_updates(location, request.POST, provided_id)) except: @@ -380,7 +387,7 @@ def course_grader_updates(request, org, course, name, grader_index=None): if request.method == 'GET': # Cannot just do a get w/o knowing the course name :-( return JsonResponse(CourseGradingModel.fetch_grader( - Location(location), grader_index + Location(location), grader_index )) elif request.method == "DELETE": # ??? Should this return anything? Perhaps success fail? @@ -388,8 +395,8 @@ def course_grader_updates(request, org, course, name, grader_index=None): return JsonResponse() else: # post or put, doesn't matter. return JsonResponse(CourseGradingModel.update_grader_from_json( - Location(location), - request.POST + Location(location), + request.POST )) @@ -411,8 +418,8 @@ def course_advanced_updates(request, org, course, name): return JsonResponse(CourseMetadata.fetch(location)) elif request.method == 'DELETE': return JsonResponse(CourseMetadata.delete_key( - location, - json.loads(request.body) + location, + json.loads(request.body) )) else: # NOTE: request.POST is messed up because expect_json @@ -479,9 +486,9 @@ def course_advanced_updates(request, org, course, name): filter_tabs = False try: return JsonResponse(CourseMetadata.update_from_json( - location, - request_body, - filter_tabs=filter_tabs + location, + request_body, + filter_tabs=filter_tabs )) except (TypeError, ValueError) as err: return HttpResponseBadRequest( @@ -585,8 +592,8 @@ def textbook_index(request, org, course, name): # MongoKeyValueStore before we update the mongo datastore. course_module.save() store.update_metadata( - course_module.location, - own_metadata(course_module) + course_module.location, + own_metadata(course_module) ) return JsonResponse(course_module.pdf_textbooks) else: diff --git a/common/lib/capa/capa/inputtypes.py b/common/lib/capa/capa/inputtypes.py index d27893d44d62..58a3ed8b1265 100644 --- a/common/lib/capa/capa/inputtypes.py +++ b/common/lib/capa/capa/inputtypes.py @@ -208,10 +208,10 @@ def process_requirements(self): # end up in a partially-initialized state. loaded = {} to_render = set() - for a in self.get_attributes(): - loaded[a.name] = a.parse_from_xml(self.xml) - if a.render: - to_render.add(a.name) + for attribute in self.get_attributes(): + loaded[attribute.name] = attribute.parse_from_xml(self.xml) + if attribute.render: + to_render.add(attribute.name) self.loaded_attributes = loaded self.to_render = to_render @@ -493,17 +493,17 @@ def get_attributes(cls): """ Register the attributes. """ - return [Attribute('params', None), # extra iframe params - Attribute('html_file', None), - Attribute('gradefn', "gradefn"), - Attribute('get_statefn', None), # Function to call in iframe - # to get current state. - Attribute('set_statefn', None), # Function to call iframe to - # set state - Attribute('width', "400"), # iframe width - Attribute('height', "300")] # iframe height - - + return [ + Attribute('params', None), # extra iframe params + Attribute('html_file', None), + Attribute('gradefn', "gradefn"), + Attribute('get_statefn', None), # Function to call in iframe + # to get current state. + Attribute('set_statefn', None), # Function to call iframe to + # set state + Attribute('width', "400"), # iframe width + Attribute('height', "300") # iframe height + ] def _extra_context(self): context = { @@ -514,7 +514,6 @@ def _extra_context(self): return context - registry.register(JSInput) #----------------------------------------------------------------------------- @@ -1048,8 +1047,8 @@ def preview_chemcalc(self, data): try: result['preview'] = chemcalc.render_to_html(formula) - except pyparsing.ParseException as p: - result['error'] = u"Couldn't parse formula: {0}".format(p.msg) + except pyparsing.ParseException as err: + result['error'] = u"Couldn't parse formula: {0}".format(err.msg) except Exception: # this is unexpected, so log log.warning( @@ -1189,15 +1188,19 @@ def parse(tag, tag_type): 'can_reuse': smth}. """ tag_attrs = dict() - tag_attrs['draggable'] = {'id': Attribute._sentinel, - 'label': "", 'icon': "", - 'can_reuse': ""} - - tag_attrs['target'] = {'id': Attribute._sentinel, - 'x': Attribute._sentinel, - 'y': Attribute._sentinel, - 'w': Attribute._sentinel, - 'h': Attribute._sentinel} + tag_attrs['draggable'] = { + 'id': Attribute._sentinel, + 'label': "", 'icon': "", + 'can_reuse': "" + } + + tag_attrs['target'] = { + 'id': Attribute._sentinel, + 'x': Attribute._sentinel, + 'y': Attribute._sentinel, + 'w': Attribute._sentinel, + 'h': Attribute._sentinel + } dic = dict() diff --git a/common/lib/xmodule/xmodule/tests/test_combined_open_ended.py b/common/lib/xmodule/xmodule/tests/test_combined_open_ended.py index 8a32f7e8223c..4eadca110c95 100644 --- a/common/lib/xmodule/xmodule/tests/test_combined_open_ended.py +++ b/common/lib/xmodule/xmodule/tests/test_combined_open_ended.py @@ -23,9 +23,11 @@ from xmodule.modulestore import Location from xmodule.tests import get_test_system, test_util_open_ended from xmodule.progress import Progress -from xmodule.tests.test_util_open_ended import (MockQueryDict, DummyModulestore, TEST_STATE_SA_IN, +from xmodule.tests.test_util_open_ended import ( + MockQueryDict, DummyModulestore, TEST_STATE_SA_IN, MOCK_INSTANCE_STATE, TEST_STATE_SA, TEST_STATE_AI, TEST_STATE_AI2, TEST_STATE_AI2_INVALID, - TEST_STATE_SINGLE, TEST_STATE_PE_SINGLE) + TEST_STATE_SINGLE, TEST_STATE_PE_SINGLE +) import capa.xqueue_interface as xqueue_interface @@ -69,7 +71,7 @@ class OpenEndedChildTest(unittest.TestCase): 'peer_grader_count': 1, 'min_to_calibrate': 3, 'max_to_calibrate': 6, - } + } } definition = Mock() descriptor = Mock() @@ -192,7 +194,7 @@ class OpenEndedModuleTest(unittest.TestCase): 'peer_grader_count': 1, 'min_to_calibrate': 3, 'max_to_calibrate': 6, - } + } } oeparam = etree.XML(''' @@ -553,7 +555,7 @@ def test_alternate_orderings(self): descriptor, static_data=self.static_data, metadata=self.metadata, - instance_state={'task_states' : TEST_STATE_SA}) + instance_state={'task_states': TEST_STATE_SA}) combinedoe = CombinedOpenEndedV1Module(self.test_system, self.location, @@ -561,7 +563,7 @@ def test_alternate_orderings(self): descriptor, static_data=self.static_data, metadata=self.metadata, - instance_state={'task_states' : TEST_STATE_SA_IN}) + instance_state={'task_states': TEST_STATE_SA_IN}) def test_get_score_realistic(self): @@ -605,7 +607,7 @@ def generate_oe_module(self, task_state, task_number, task_xml): descriptor = Mock(data=definition) instance_state = {'task_states': task_state, 'graded': True} if task_number is not None: - instance_state.update({'current_task_number' : task_number}) + instance_state.update({'current_task_number': task_number}) combinedoe = CombinedOpenEndedV1Module(self.test_system, self.location, definition, @@ -711,7 +713,7 @@ def test_open_ended_flow_reset(self): #Simulate a student saving an answer html = module.handle_ajax("get_html", {}) - module.handle_ajax("save_answer", {"student_answer": self.answer, "can_upload_files" : False, "student_file" : None}) + module.handle_ajax("save_answer", {"student_answer": self.answer, "can_upload_files": False, "student_file": None}) html = module.handle_ajax("get_html", {}) #Mock a student submitting an assessment diff --git a/lms/envs/common.py b/lms/envs/common.py index 9d886f8dc190..4e70f3ade1ae 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -290,12 +290,14 @@ ### COURSE_DEFAULT = '6.002x_Fall_2012' -COURSE_SETTINGS = {'6.002x_Fall_2012': {'number': '6.002x', - 'title': 'Circuits and Electronics', - 'xmlpath': '6002x/', - 'location': 'i4x://edx/6002xs12/course/6.002x_Fall_2012', - } - } +COURSE_SETTINGS = { + '6.002x_Fall_2012': { + 'number': '6.002x', + 'title': 'Circuits and Electronics', + 'xmlpath': '6002x/', + 'location': 'i4x://edx/6002xs12/course/6.002x_Fall_2012', + } +} # IP addresses that are allowed to reload the course, etc. # TODO (vshnayder): Will probably need to change as we get real access control in. From b2b23fa9c57d04ae0de8c404cb5f38a2f083031c Mon Sep 17 00:00:00 2001 From: Will Daly Date: Tue, 3 Sep 2013 21:17:15 -0400 Subject: [PATCH 10/13] Pass in harvest args to fasttest_acceptance, not the args array --- rakelib/tests.rake | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rakelib/tests.rake b/rakelib/tests.rake index f7c0fbdc4f43..b0c8f4979d1b 100644 --- a/rakelib/tests.rake +++ b/rakelib/tests.rake @@ -111,7 +111,7 @@ TEST_TASK_DIRS = [] desc "Run acceptance tests" task "test_acceptance_#{system}", [:harvest_args] => [:clean_test_files, "#{system}:gather_assets:acceptance"] do |t, args| setup_acceptance_db(system) - Rake::Task["fasttest_acceptance_#{system}"].invoke(*args) + Rake::Task["fasttest_acceptance_#{system}"].invoke(args.harvest_args) end desc "Run acceptance tests without collectstatic or database migrations" From fb84541bbebf7bae9b8f5da1261c2f4321bce1b2 Mon Sep 17 00:00:00 2001 From: Adam Palay Date: Wed, 4 Sep 2013 08:28:52 -0400 Subject: [PATCH 11/13] add nltk to sandbox reqs --- requirements/edx-sandbox/base.txt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/requirements/edx-sandbox/base.txt b/requirements/edx-sandbox/base.txt index f88cc899cf25..58f99d422260 100644 --- a/requirements/edx-sandbox/base.txt +++ b/requirements/edx-sandbox/base.txt @@ -1,4 +1,5 @@ numpy==1.6.2 networkx==1.7 sympy==0.7.1 -pyparsing==1.5.6 \ No newline at end of file +pyparsing==1.5.6 +nltk==2.0.4 From 961bd4c9cbd80dff617030e64c6484d1daa190bf Mon Sep 17 00:00:00 2001 From: Anton Stupak Date: Thu, 5 Sep 2013 10:49:42 +0300 Subject: [PATCH 12/13] Fix selector. --- common/lib/xmodule/xmodule/js/src/tabs/tabs-aggregator.coffee | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/lib/xmodule/xmodule/js/src/tabs/tabs-aggregator.coffee b/common/lib/xmodule/xmodule/js/src/tabs/tabs-aggregator.coffee index 7c6e85e34b04..521b19dfbc33 100644 --- a/common/lib/xmodule/xmodule/js/src/tabs/tabs-aggregator.coffee +++ b/common/lib/xmodule/xmodule/js/src/tabs/tabs-aggregator.coffee @@ -9,7 +9,7 @@ class @TabsEditingDescriptor ### # hide editor/settings bar - $('.component-edit-header').hide() + @element.closest('.component-editor').find('.component-edit-header').hide() @$tabs = $(".tab", @element) @$content = $(".component-tab", @element) From 5d55714cadc73084616a38a66ca57e4d582e6dad Mon Sep 17 00:00:00 2001 From: Anton Stupak Date: Thu, 5 Sep 2013 17:02:53 +0300 Subject: [PATCH 13/13] Fix test. --- .../xmodule/js/fixtures/tabs-edit.html | 56 ++++++++++--------- .../xmodule/xmodule/js/spec/tabs/edit.coffee | 2 +- 2 files changed, 30 insertions(+), 28 deletions(-) diff --git a/common/lib/xmodule/xmodule/js/fixtures/tabs-edit.html b/common/lib/xmodule/xmodule/js/fixtures/tabs-edit.html index c83a1456226c..7c0f7be2b78a 100644 --- a/common/lib/xmodule/xmodule/js/fixtures/tabs-edit.html +++ b/common/lib/xmodule/xmodule/js/fixtures/tabs-edit.html @@ -1,33 +1,35 @@ -
-
-
-
- -
-
-
- +
+
+
+
+
+ +
+
+
+ +
+
+ Transcripts +
+
+ Subtitles +
-
- Transcripts +
+
-
- Subtitles -
-
-
-
-
-
+
-
+
+
diff --git a/common/lib/xmodule/xmodule/js/spec/tabs/edit.coffee b/common/lib/xmodule/xmodule/js/spec/tabs/edit.coffee index 734e398c743d..d5502b7a29eb 100644 --- a/common/lib/xmodule/xmodule/js/spec/tabs/edit.coffee +++ b/common/lib/xmodule/xmodule/js/spec/tabs/edit.coffee @@ -65,7 +65,7 @@ describe "TabsEditingDescriptor", -> describe "editor/settings header", -> it "is hidden", -> - expect(@descriptor.element.find(".component-edit-header").css('display')).toEqual('none') + expect(@descriptor.element.closest(".component-editor").find(".component-edit-header")).toBeHidden() describe "TabsEditingDescriptor special save cases", -> beforeEach ->