Clarify how options are passed to paver tasks, so that pa11y subtasks get needed information

pavelib/bok_choy.py

@@ -4,7 +4,12 @@ http://bok-choy.readthedocs.org/en/latest/
 """
 from paver.easy import task, needs, cmdopts, sh
 from pavelib.utils.test.suites.bokchoy_suite import BokChoyTestSuite, Pa11yCrawler
-from pavelib.utils.test.bokchoy_options import BOKCHOY_OPTS
+from pavelib.utils.test.bokchoy_options import (
+    BOKCHOY_OPTS,
+    PA11Y_HTML,
+    PA11Y_COURSE_KEY,
+    PA11Y_FETCH_COURSE,
+)
 from pavelib.utils.envs import Env
 from pavelib.utils.test.utils import check_firefox_version
 from pavelib.utils.passthrough_opts import PassthroughTask
@@ -93,17 +98,11 @@ def perf_report_bokchoy(options, passthrough_options):
     run_bokchoy(options.perf_report_bokchoy, passthrough_options)
 
 
-@needs('pavelib.prereqs.install_prereqs')
-@cmdopts(BOKCHOY_OPTS + [
-    ('with-html', 'w', 'Include html reports'),
-    make_option('--course-key', help='Course key for test course'),
-    make_option(
-        "--fetch-course",
-        action="store_true",
-        dest="should_fetch_course",
-        help='Course key for test course',
-    ),
-])
+@needs('pavelib.prereqs.install_prereqs', 'get_test_course')
+@cmdopts(
+    BOKCHOY_OPTS + [PA11Y_HTML, PA11Y_COURSE_KEY, PA11Y_FETCH_COURSE],
+    share_with=['get_test_course', 'prepare_bokchoy_run', 'load_courses']
+)
 @PassthroughTask
 @timed
 def pa11ycrawler(options, passthrough_options):

pavelib/paver_tests/test_paver_bok_choy_cmds.py

@@ -5,10 +5,12 @@ Run just this test with: paver test_lib -t pavelib/paver_tests/test_paver_bok_ch
 import os
 import unittest
 
+import ddt
 from mock import patch, call
 from test.test_support import EnvironmentVarGuard
-from paver.easy import BuildFailure
+from paver.easy import BuildFailure, call_task, environment
 from pavelib.utils.test.suites import BokChoyTestSuite, Pa11yCrawler
+from pavelib.utils.test.suites.bokchoy_suite import DEMO_COURSE_TAR_GZ, DEMO_COURSE_IMPORT_DIR
 
 REPO_DIR = os.getcwd()
 
@@ -173,6 +175,7 @@ class TestPaverBokChoyCmd(unittest.TestCase):
             suite.verbosity_processes_command
 
 
+@ddt.ddt
 class TestPaverPa11yCrawlerCmd(unittest.TestCase):
 
     """
@@ -190,6 +193,9 @@ class TestPaverPa11yCrawlerCmd(unittest.TestCase):
         # Cleanup mocks
         self.addCleanup(mock_sh.stop)
 
+        # reset the options for all tasks
+        environment.options.clear()
+
     def _expected_command(self, report_dir, start_urls):
         """
         Returns the expected command to run pa11ycrawler.
@@ -213,15 +219,31 @@ class TestPaverPa11yCrawlerCmd(unittest.TestCase):
             self._expected_command(suite.pa11y_report_dir, suite.start_urls)
         )
 
-    def test_get_test_course(self):
-        suite = Pa11yCrawler('')
-        suite.get_test_course()
-        self._mock_sh.assert_has_calls([
-            call(
-                'wget {targz} -O {dir}demo_course.tar.gz'.format(targz=suite.tar_gz_file, dir=suite.imports_dir)),
-            call(
-                'tar zxf {dir}demo_course.tar.gz -C {dir}'.format(dir=suite.imports_dir)),
-        ])
+    @ddt.data(
+        (True, True, None),
+        (True, False, None),
+        (False, True, DEMO_COURSE_IMPORT_DIR),
+        (False, False, None),
+    )
+    @ddt.unpack
+    def test_get_test_course(self, import_dir_set, should_fetch_course_set, downloaded_to):
+        options = {}
+        if import_dir_set:
+            options['imports_dir'] = 'some_import_dir'
+        if should_fetch_course_set:
+            options['should_fetch_course'] = True
+
+        call_task('pavelib.utils.test.suites.bokchoy_suite.get_test_course', options=options)
+
+        if downloaded_to is None:
+            self._mock_sh.assert_has_calls([])
+        else:
+            self._mock_sh.assert_has_calls([
+                call(
+                    'wget {targz} -O {dir}demo_course.tar.gz'.format(targz=DEMO_COURSE_TAR_GZ, dir=downloaded_to)),
+                call(
+                    'tar zxf {dir}demo_course.tar.gz -C {dir}'.format(dir=downloaded_to)),
+            ])
 
     def test_generate_html_reports(self):
         suite = Pa11yCrawler('')

pavelib/utils/test/bokchoy_options.py

@@ -8,19 +8,34 @@ import os
 from pavelib.utils.envs import Env
 
 
+BOKCHOY_IMPORTS_DIR = ('imports-dir=', 'i', 'Directory containing (un-archived) courses to be imported')
+BOKCHOY_IMPORTS_DIR_DEPR = ('imports_dir=', None, 'deprecated in favor of imports-dir')
+BOKCHOY_DEFAULT_STORE = make_option(
+    "-s", "--default-store",
+    default=os.environ.get('DEFAULT_STORE', 'split'),
+    help='Default modulestore'
+)
+BOKCHOY_DEFAULT_STORE_DEPR = make_option(
+    "--default_store",
+    default=os.environ.get('DEFAULT_STORE', 'split'),
+    help='deprecated in favor of default-store'
+)
+BOKCHOY_FASTTEST = make_option('-a', '--fasttest', action='store_true', help='Skip some setup')
+BOKCHOY_COVERAGERC = make_option('--coveragerc', help='coveragerc file to use during this test')
+
 BOKCHOY_OPTS = [
     ('test-spec=', 't', 'Specific test to run'),
-    make_option('-a', '--fasttest', action='store_true', help='Skip some setup'),
+    BOKCHOY_FASTTEST,
     ('skip-clean', 'C', 'Skip cleaning repository before running tests'),
     make_option('-r', '--serversonly', action='store_true', help='Prepare suite and leave servers running'),
     make_option('-o', '--testsonly', action='store_true', help='Assume servers are running and execute tests only'),
-    make_option("-s", "--default-store", default=os.environ.get('DEFAULT_STORE', 'split'), help='Default modulestore'),
+    BOKCHOY_DEFAULT_STORE,
     make_option(
         '-d', '--test-dir',
         default='tests',
         help='Directory for finding tests (relative to common/test/acceptance)'
     ),
-    ('imports-dir=', 'i', 'Directory containing (un-archived) courses to be imported'),
+    BOKCHOY_IMPORTS_DIR,
     make_option('-n', '--num-processes', type='int', help='Number of test threads (for multiprocessing)'),
     make_option(
         '-x', '--verify-xss',
@@ -35,17 +50,13 @@ BOKCHOY_OPTS = [
     make_option("--save-screenshots", action='store_true', dest="save_screenshots"),
     make_option("--report-dir", default=Env.BOK_CHOY_REPORT_DIR, help="Directory to store reports in"),
 
-    make_option(
-        "--default_store",
-        default=os.environ.get('DEFAULT_STORE', 'split'),
-        help='deprecated in favor of default-store'
-    ),
+    BOKCHOY_DEFAULT_STORE_DEPR,
     make_option(
         '-e', '--extra_args',
         default='',
         help='deprecated, pass extra options directly in the paver commandline'
     ),
-    ('imports_dir=', None, 'deprecated in favor of imports-dir'),
+    BOKCHOY_IMPORTS_DIR_DEPR,
     make_option('--num_processes', type='int', help='deprecated in favor of num-processes'),
     ('skip_clean', None, 'deprecated in favor of skip-clean'),
     make_option('--test_dir', default='tests', help='deprecated in favor of test-dir'),
@@ -69,3 +80,12 @@ BOKCHOY_OPTS = [
         help="deprecated in favor of save-screenshots"
     ),
 ]
+
+PA11Y_HTML = ('with-html', 'w', 'Include html reports')
+PA11Y_COURSE_KEY = make_option('--course-key', help='Course key for test course')
+PA11Y_FETCH_COURSE = make_option(
+    "--fetch-course",
+    action="store_true",
+    dest="should_fetch_course",
+    help='Course key for test course',
+)

pavelib/utils/test/bokchoy_utils.py

@@ -6,10 +6,13 @@ import os
 import time
 import httplib
 import subprocess
-from paver.easy import sh, task, cmdopts
+from paver import tasks
+from paver.easy import sh, task, cmdopts, needs
 from pavelib.utils.envs import Env
 from pavelib.utils.process import run_background_process
-from pavelib.utils.test.bokchoy_options import BOKCHOY_OPTS
+from pavelib.utils.test.bokchoy_options import (
+    BOKCHOY_COVERAGERC, BOKCHOY_DEFAULT_STORE, BOKCHOY_DEFAULT_STORE_DEPR
+)
 from pavelib.utils.timer import timed
 
 try:
@@ -21,7 +24,7 @@ __test__ = False  # do not collect
 
 
 @task
-@cmdopts(BOKCHOY_OPTS, share_with=['test_bokchoy', 'test_a11y', 'pa11ycrawler'])
+@cmdopts([BOKCHOY_COVERAGERC, BOKCHOY_DEFAULT_STORE, BOKCHOY_DEFAULT_STORE_DEPR])
 @timed
 def start_servers(options):
     """

pavelib/utils/test/suites/bokchoy_suite.py

@@ -7,13 +7,18 @@ from urllib import urlencode
 from common.test.acceptance.fixtures.course import CourseFixture, FixtureError
 
 from path import Path as path
-from paver.easy import sh, BuildFailure, cmdopts, task, needs, call_task
+from paver.easy import sh, BuildFailure, cmdopts, task, needs, might_call, call_task
 from pavelib.utils.test.suites.suite import TestSuite
 from pavelib.utils.envs import Env
 from pavelib.utils.test.bokchoy_utils import (
     clear_mongo, start_servers, check_services, wait_for_test_servers
 )
-from pavelib.utils.test.bokchoy_options import BOKCHOY_OPTS
+from pavelib.utils.test.bokchoy_options import (
+    BOKCHOY_IMPORTS_DIR, BOKCHOY_IMPORTS_DIR_DEPR,
+    BOKCHOY_DEFAULT_STORE, BOKCHOY_DEFAULT_STORE_DEPR,
+    BOKCHOY_FASTTEST,
+    PA11Y_FETCH_COURSE
+)
 from pavelib.utils.test import utils as test_utils
 from pavelib.utils.timer import timed
 
@@ -29,9 +34,12 @@ __test__ = False  # do not collect
 DEFAULT_NUM_PROCESSES = 1
 DEFAULT_VERBOSITY = 2
 
+DEMO_COURSE_TAR_GZ = "https://github.com/edx/demo-test-course/archive/master.tar.gz"
+DEMO_COURSE_IMPORT_DIR = path('test_root/courses/')
+
 
 @task
-@cmdopts(BOKCHOY_OPTS, share_with=['test_bokchoy', 'test_a11y', 'pa11ycrawler'])
+@cmdopts([BOKCHOY_DEFAULT_STORE, BOKCHOY_DEFAULT_STORE_DEPR])
 @timed
 def load_bok_choy_data(options):
     """
@@ -48,7 +56,10 @@ def load_bok_choy_data(options):
 
 
 @task
-@cmdopts(BOKCHOY_OPTS, share_with=['test_bokchoy', 'test_a11y', 'pa11ycrawler'])
+@cmdopts([
+    BOKCHOY_IMPORTS_DIR, BOKCHOY_IMPORTS_DIR_DEPR, BOKCHOY_DEFAULT_STORE,
+    BOKCHOY_DEFAULT_STORE_DEPR
+])
 @timed
 def load_courses(options):
     """
@@ -70,6 +81,51 @@ def load_courses(options):
                 import_dir=options.imports_dir
             )
         )
+    else:
+        print colorize('blue', "--imports-dir not set, skipping import")
+
+
+@task
+@cmdopts([BOKCHOY_IMPORTS_DIR, BOKCHOY_IMPORTS_DIR_DEPR, PA11Y_FETCH_COURSE])
+@timed
+def get_test_course(options):
+    """
+    Fetches the test course.
+    """
+
+    if options.get('imports_dir'):
+        print colorize("green", "--imports-dir specified, skipping fetch of test course")
+        return
+
+    if not options.get('should_fetch_course', False):
+        print colorize("green", "--skip-fetch specified, skipping fetch of test course")
+        return
+
+    # Set the imports_dir for use by other tasks
+    options.imports_dir = DEMO_COURSE_IMPORT_DIR
+
+    options.imports_dir.makedirs_p()
+    zipped_course = options.imports_dir + 'demo_course.tar.gz'
+
+    msg = colorize('green', "Fetching the test course from github...")
+    print msg
+
+    sh(
+        'wget {tar_gz_file} -O {zipped_course}'.format(
+            tar_gz_file=DEMO_COURSE_TAR_GZ,
+            zipped_course=zipped_course,
+        )
+    )
+
+    msg = colorize('green', "Uncompressing the test course...")
+    print msg
+
+    sh(
+        'tar zxf {zipped_course} -C {courses_dir}'.format(
+            zipped_course=zipped_course,
+            courses_dir=options.imports_dir,
+        )
+    )
 
 
 @task
@@ -83,7 +139,8 @@ def reset_test_database():
 
 @task
 @needs(['reset_test_database', 'clear_mongo', 'load_bok_choy_data', 'load_courses'])
-@cmdopts(BOKCHOY_OPTS, share_with=['test_bokchoy', 'test_a11y', 'pa11ycrawler'])
+@might_call('start_servers')
+@cmdopts([BOKCHOY_FASTTEST], share_with=['start_servers'])
 @timed
 def prepare_bokchoy_run(options):
     """
@@ -289,21 +346,7 @@ class Pa11yCrawler(BokChoyTestSuite):
     def __init__(self, *args, **kwargs):
         super(Pa11yCrawler, self).__init__(*args, **kwargs)
         self.course_key = kwargs.get('course_key')
-        if self.imports_dir:
-            # If imports_dir has been specified, assume the files are
-            # already there -- no need to fetch them from github. This
-            # allows someome to crawl a different course. They are responsible
-            # for putting it, un-archived, in the directory.
-            self.should_fetch_course = False
-        else:
-            # Otherwise, obey `--skip-fetch` command and use the default
-            # test course.  Note that the fetch will also be skipped when
-            # using `--fast`.
-            self.should_fetch_course = kwargs.get('should_fetch_course')
-            self.imports_dir = path('test_root/courses/')
-
         self.pa11y_report_dir = os.path.join(self.report_dir, 'pa11ycrawler_reports')
-        self.tar_gz_file = "https://github.com/edx/demo-test-course/archive/master.tar.gz"
 
         self.start_urls = []
         auto_auth_params = {
@@ -325,38 +368,6 @@ class Pa11yCrawler(BokChoyTestSuite):
         lms_params = urlencode(auto_auth_params)
         self.start_urls.append("\"http://localhost:8003/auto_auth?{}\"".format(lms_params))
 
-    def __enter__(self):
-        if self.should_fetch_course:
-            self.get_test_course()
-        super(Pa11yCrawler, self).__enter__()
-
-    def get_test_course(self):
-        """
-        Fetches the test course.
-        """
-        self.imports_dir.makedirs_p()
-        zipped_course = self.imports_dir + 'demo_course.tar.gz'
-
-        msg = colorize('green', "Fetching the test course from github...")
-        print msg
-
-        sh(
-            'wget {tar_gz_file} -O {zipped_course}'.format(
-                tar_gz_file=self.tar_gz_file,
-                zipped_course=zipped_course,
-            )
-        )
-
-        msg = colorize('green', "Uncompressing the test course...")
-        print msg
-
-        sh(
-            'tar zxf {zipped_course} -C {courses_dir}'.format(
-                zipped_course=zipped_course,
-                courses_dir=self.imports_dir,
-            )
-        )
-
     def generate_html_reports(self):
         """
         Runs pa11ycrawler json-to-html