From 34d1ea9e63093bfeba1fa1239d16158d364a99af Mon Sep 17 00:00:00 2001 From: Will Daly Date: Thu, 30 May 2013 15:09:39 -0400 Subject: [PATCH 01/42] Consolidated coverage report generation into one rake command. Added diff coverage reports using diff-coverage tool. --- doc/testing.md | 5 +++-- jenkins/test.sh | 3 ++- rakefiles/tests.rake | 31 +++++++++---------------------- 3 files changed, 14 insertions(+), 25 deletions(-) diff --git a/doc/testing.md b/doc/testing.md index e5d035d90e..bc58bc387d 100644 --- a/doc/testing.md +++ b/doc/testing.md @@ -197,9 +197,10 @@ To view test coverage: 2. Generate reports: - rake coverage:html + rake coverage -3. HTML reports are located in the `reports` folder. +3. Reports are located in the `reports` folder. The command +generates HTML and XML (Cobertura format) reports. ## Testing using queue servers diff --git a/jenkins/test.sh b/jenkins/test.sh index 35be3a0121..51566f6fb5 100755 --- a/jenkins/test.sh +++ b/jenkins/test.sh @@ -84,7 +84,8 @@ rake phantomjs_jasmine_cms || TESTS_FAILED=1 rake phantomjs_jasmine_common/lib/xmodule || TESTS_FAILED=1 rake phantomjs_jasmine_common/static/coffee || TESTS_FAILED=1 -rake coverage:xml coverage:html +# Generate coverage reports +rake coverage [ $TESTS_FAILED == '0' ] rake autodeploy_properties diff --git a/rakefiles/tests.rake b/rakefiles/tests.rake index 448a482f04..ac7037508c 100644 --- a/rakefiles/tests.rake +++ b/rakefiles/tests.rake @@ -119,30 +119,17 @@ task :test do end end -namespace :coverage do - desc "Build the html coverage reports" - task :html => :report_dirs do - TEST_TASK_DIRS.each do |dir| - report_dir = report_dir_path(dir) +desc "Build the html, xml, and diff coverage reports" +task :coverage => :report_dirs do + TEST_TASK_DIRS.each do |dir| + report_dir = report_dir_path(dir) - if !File.file?("#{report_dir}/.coverage") - next - end - - sh("coverage html --rcfile=#{dir}/.coveragerc") + if !File.file?("#{report_dir}/.coverage") + next end - end - desc "Build the xml coverage reports" - task :xml => :report_dirs do - TEST_TASK_DIRS.each do |dir| - report_dir = report_dir_path(dir) - - if !File.file?("#{report_dir}/.coverage") - next - end - # Why doesn't the rcfile control the xml output file properly?? - sh("coverage xml -o #{report_dir}/coverage.xml --rcfile=#{dir}/.coveragerc") - end + sh("coverage html --rcfile=#{dir}/.coveragerc") + sh("coverage xml -o #{report_dir}/coverage.xml --rcfile=#{dir}/.coveragerc") + sh("diff-cover #{report_dir}/coverage.xml --html-report #{report_dir}/diff_cover.html --git-branch master...HEAD") end end From 76ae9800060d329f7c08ee941c53aff489ad2c8b Mon Sep 17 00:00:00 2001 From: Will Daly Date: Mon, 3 Jun 2013 12:19:13 -0400 Subject: [PATCH 02/42] Clean the report directories before each test run, to ensure we don't get coverage results from a previous test run. --- rakefiles/tests.rake | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/rakefiles/tests.rake b/rakefiles/tests.rake index ac7037508c..ee862d631d 100644 --- a/rakefiles/tests.rake +++ b/rakefiles/tests.rake @@ -45,6 +45,11 @@ end directory REPORT_DIR task :clean_test_files do + + # Delete all non-folder files in the reports directory + sh("find #{REPORT_DIR} -type f -print0 | xargs -0 rm") + + # Reset the test fixtures sh("git clean -fqdx test_root") end @@ -81,12 +86,11 @@ TEST_TASK_DIRS = [] end Dir["common/lib/*"].select{|lib| File.directory?(lib)}.each do |lib| - task_name = "test_#{lib}" report_dir = report_dir_path(lib) desc "Run tests for common lib #{lib}" - task task_name => report_dir do + task "test_#{lib}" => ["clean_test_files", report_dir] do ENV['NOSE_XUNIT_FILE'] = File.join(report_dir, "nosetests.xml") cmd = "nosetests #{lib}" sh(run_under_coverage(cmd, lib)) do |ok, res| @@ -95,10 +99,13 @@ Dir["common/lib/*"].select{|lib| File.directory?(lib)}.each do |lib| end TEST_TASK_DIRS << lib - desc "Run tests for common lib #{lib} (without coverage)" - task "fasttest_#{lib}" do - sh("nosetests #{lib}") - end + # There used to be a fasttest_#{lib} command that ran without coverage. + # However, this is an inconsistent usage of "fast": + # When running tests for lms and cms, "fast" means skipping + # staticfiles collection, but still running under coverage. + # We keep the fasttest_#{lib} command for backwards compatibility, + # but make it an alias to the normal test command. + task "fasttest_#{lib}" => "test_#{lib}" end task :report_dirs From afa6f485fdaf672c0f915f1a203c26320f4fdb85 Mon Sep 17 00:00:00 2001 From: Will Daly Date: Mon, 3 Jun 2013 13:26:01 -0400 Subject: [PATCH 03/42] Added diff-cover as a requirement (installed from github) --- rakefiles/tests.rake | 3 ++- requirements/edx/github.txt | 1 + 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/rakefiles/tests.rake b/rakefiles/tests.rake index ee862d631d..7e6d12c40f 100644 --- a/rakefiles/tests.rake +++ b/rakefiles/tests.rake @@ -46,7 +46,8 @@ directory REPORT_DIR task :clean_test_files do - # Delete all non-folder files in the reports directory + # Delete all files in the reports directory, while preserving + # the directory structure. sh("find #{REPORT_DIR} -type f -print0 | xargs -0 rm") # Reset the test fixtures diff --git a/requirements/edx/github.txt b/requirements/edx/github.txt index b1aef0a108..bcf1b415d7 100644 --- a/requirements/edx/github.txt +++ b/requirements/edx/github.txt @@ -10,3 +10,4 @@ # Our libraries: -e git+https://github.com/edx/XBlock.git@2144a25d#egg=XBlock -e git+https://github.com/edx/codejail.git@5fb5fa0#egg=codejail +-e git+https://github.com/edx/diff-cover.git@v0.1#egg=diff_cover From 9c506020d9f8536c756f143f0a7082d36d46b8bb Mon Sep 17 00:00:00 2001 From: Will Daly Date: Tue, 4 Jun 2013 17:51:12 -0400 Subject: [PATCH 04/42] Fixed bug in tests.rake in which rm would be called with no args. --- rakefiles/tests.rake | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/rakefiles/tests.rake b/rakefiles/tests.rake index 7e6d12c40f..b8919bdf90 100644 --- a/rakefiles/tests.rake +++ b/rakefiles/tests.rake @@ -48,7 +48,7 @@ task :clean_test_files do # Delete all files in the reports directory, while preserving # the directory structure. - sh("find #{REPORT_DIR} -type f -print0 | xargs -0 rm") + sh("find #{REPORT_DIR} -type f -print0 | xargs --no-run-if-empty -0 rm") # Reset the test fixtures sh("git clean -fqdx test_root") @@ -138,6 +138,6 @@ task :coverage => :report_dirs do sh("coverage html --rcfile=#{dir}/.coveragerc") sh("coverage xml -o #{report_dir}/coverage.xml --rcfile=#{dir}/.coveragerc") - sh("diff-cover #{report_dir}/coverage.xml --html-report #{report_dir}/diff_cover.html --git-branch master...HEAD") + sh("diff-cover #{report_dir}/coverage.xml --html-report #{report_dir}/diff_cover.html") end end From 5662ecfde99424cecafa06605d9fc4f9b940a21f Mon Sep 17 00:00:00 2001 From: Will Daly Date: Tue, 4 Jun 2013 18:13:42 -0400 Subject: [PATCH 05/42] Updated to current tag of diff-cover --- requirements/edx/github.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements/edx/github.txt b/requirements/edx/github.txt index bcf1b415d7..fc9070bba3 100644 --- a/requirements/edx/github.txt +++ b/requirements/edx/github.txt @@ -10,4 +10,4 @@ # Our libraries: -e git+https://github.com/edx/XBlock.git@2144a25d#egg=XBlock -e git+https://github.com/edx/codejail.git@5fb5fa0#egg=codejail --e git+https://github.com/edx/diff-cover.git@v0.1#egg=diff_cover +-e git+https://github.com/edx/diff-cover.git@v0.1.0#egg=diff_cover From 99c185f06bf1d7ef83b5b7c05c56a9c251870856 Mon Sep 17 00:00:00 2001 From: Will Daly Date: Tue, 4 Jun 2013 18:39:49 -0400 Subject: [PATCH 06/42] Added diff coverage printout to rake coverage command --- rakefiles/tests.rake | 2 ++ 1 file changed, 2 insertions(+) diff --git a/rakefiles/tests.rake b/rakefiles/tests.rake index b8919bdf90..528290cee5 100644 --- a/rakefiles/tests.rake +++ b/rakefiles/tests.rake @@ -139,5 +139,7 @@ task :coverage => :report_dirs do sh("coverage html --rcfile=#{dir}/.coveragerc") sh("coverage xml -o #{report_dir}/coverage.xml --rcfile=#{dir}/.coveragerc") sh("diff-cover #{report_dir}/coverage.xml --html-report #{report_dir}/diff_cover.html") + sh("diff-cover #{report_dir}/coverage.xml") + puts "\n\n" end end From e1d3fb73012b5af3c5ec81f4577b4d9d1fe1927c Mon Sep 17 00:00:00 2001 From: Will Daly Date: Tue, 4 Jun 2013 18:46:45 -0400 Subject: [PATCH 07/42] Added warning message when no coverage information found. Added comments to tests.rake --- rakefiles/tests.rake | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/rakefiles/tests.rake b/rakefiles/tests.rake index 528290cee5..904ea4a54e 100644 --- a/rakefiles/tests.rake +++ b/rakefiles/tests.rake @@ -129,17 +129,33 @@ end desc "Build the html, xml, and diff coverage reports" task :coverage => :report_dirs do + + found_coverage_info = false + TEST_TASK_DIRS.each do |dir| report_dir = report_dir_path(dir) if !File.file?("#{report_dir}/.coverage") next + else + found_coverage_info = true end + # Generate the coverage.py HTML report sh("coverage html --rcfile=#{dir}/.coveragerc") + + # Generate the coverage.py XML report sh("coverage xml -o #{report_dir}/coverage.xml --rcfile=#{dir}/.coveragerc") + + # Generate the diff coverage HTML report, based on the XML report sh("diff-cover #{report_dir}/coverage.xml --html-report #{report_dir}/diff_cover.html") + + # Print the diff coverage report to the console sh("diff-cover #{report_dir}/coverage.xml") - puts "\n\n" + puts "\n" + end + + if not found_coverage_info + puts "No coverage info found. Run `rake test` before running `rake coverage`." end end From 4391783248ee1338e086175723910e486745ca5e Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Wed, 5 Jun 2013 12:40:09 -0400 Subject: [PATCH 08/42] add exporting of 'about' content as well as adding unit test checks --- cms/djangoapps/contentstore/tests/test_contentstore.py | 3 +++ common/lib/xmodule/xmodule/modulestore/xml_exporter.py | 3 +++ 2 files changed, 6 insertions(+) diff --git a/cms/djangoapps/contentstore/tests/test_contentstore.py b/cms/djangoapps/contentstore/tests/test_contentstore.py index aebfb91126..771871e9bc 100644 --- a/cms/djangoapps/contentstore/tests/test_contentstore.py +++ b/cms/djangoapps/contentstore/tests/test_contentstore.py @@ -486,6 +486,9 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase): # check for custom_tags self.verify_content_existence(module_store, root_dir, location, 'custom_tags', 'custom_tag_template') + # check for about content + self.verify_content_existence(module_store, root_dir, location, 'about', 'about', '.html') + # check for graiding_policy.json filesystem = OSFS(root_dir / 'test_export/policies/6.002_Spring_2012') self.assertTrue(filesystem.exists('grading_policy.json')) diff --git a/common/lib/xmodule/xmodule/modulestore/xml_exporter.py b/common/lib/xmodule/xmodule/modulestore/xml_exporter.py index 50bdf3a252..9fceb51c51 100644 --- a/common/lib/xmodule/xmodule/modulestore/xml_exporter.py +++ b/common/lib/xmodule/xmodule/modulestore/xml_exporter.py @@ -28,6 +28,9 @@ def export_to_xml(modulestore, contentstore, course_location, root_dir, course_d # export the course updates export_extra_content(export_fs, modulestore, course_location, 'course_info', 'info', '.html') + # export the 'about' data (e.g. overview, etc.) + export_extra_content(export_fs, modulestore, course_location, 'about', 'about', '.html') + # export the grading policy policies_dir = export_fs.makeopendir('policies') course_run_policy_dir = policies_dir.makeopendir(course.location.name) From f2c40a71fe3b1d775241efa7109dd897fb834521 Mon Sep 17 00:00:00 2001 From: Nate Hardison Date: Fri, 31 May 2013 15:17:45 -0700 Subject: [PATCH 09/42] Enable theming in base LMS template Provide the appropriate switches to adjust based on whether or not a theme (in particular, the Stanford theme) is enabled in the settings. For now, these changes are very specific to Stanford. This is because the template architecture needs some reworking to generalize nicely. --- lms/templates/main.html | 38 ++++++++++++++++++++++++++++++++++---- 1 file changed, 34 insertions(+), 4 deletions(-) diff --git a/lms/templates/main.html b/lms/templates/main.html index 313025d09a..d6cbe21095 100644 --- a/lms/templates/main.html +++ b/lms/templates/main.html @@ -1,9 +1,28 @@ <%namespace name='static' file='static_content.html'/> <%! from django.utils import html %> + +## Define a couple of helper functions to make life easier when +## embedding theme conditionals into templates. All inheriting +## templates have access to these functions, and we can import these +## into non-inheriting templates via the %namespace tag. +<%def name="theme_enabled()"> + <% return settings.MITX_FEATURES["USE_CUSTOM_THEME"] %> + + +<%def name="stanford_theme_enabled()"> + <% return theme_enabled() and getattr(settings, "THEME_NAME") == "stanford" %> + + - <%block name="title">edX + <%block name="title"> + % if stanford_theme_enabled(): + Home | class.stanford.edu + % else: + edX + % endif + - + <%static:css group='application'/> From f055c6f9893439bdae6148eb313124ef32dc6ae5 Mon Sep 17 00:00:00 2001 From: Nate Hardison Date: Fri, 31 May 2013 15:37:00 -0700 Subject: [PATCH 11/42] Inject theming hooks into navigation bar Allow themes to inherit from the default navigation bar and override pieces of it, including the main logo, the links that display to the right of the logo, and the links inside the dropdown menu (with the exception of the `Log Out` link. In addition, this adds an empty block at the very top so that themes can place a branding bar at the top of the page. (Stanford identity guidelines require this: see https://identity.stanford.edu.) --- lms/templates/navigation.html | 51 ++++++++++++++++++++--------------- 1 file changed, 30 insertions(+), 21 deletions(-) diff --git a/lms/templates/navigation.html b/lms/templates/navigation.html index 13c461173b..104366c8c4 100644 --- a/lms/templates/navigation.html +++ b/lms/templates/navigation.html @@ -1,6 +1,6 @@ ## mako <%namespace name='static' file='static_content.html'/> -<%namespace file='main.html' import="login_query"/> +<%namespace file='main.html' import="login_query, stanford_theme_enabled"/> <%! from django.core.urlresolvers import reverse @@ -10,6 +10,9 @@ import branding from status.status import get_site_status_msg %> +## Provide a hook for themes to inject branding on top. +<%block name="navigation_top" /> + <%block cached="False"> <% try: @@ -38,9 +41,12 @@ site_status_msg = get_site_status_msg(course_id)