Get rid of the extra parse_bokchoy_opts step for passing arguments to a BokChoySuite

pavelib/bok_choy.py

@@ -4,7 +4,7 @@ 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, parse_bokchoy_opts
+from pavelib.utils.test.bokchoy_options import BOKCHOY_OPTS
 from pavelib.utils.envs import Env
 from pavelib.utils.test.utils import check_firefox_version
 from pavelib.utils.passthrough_opts import PassthroughTask
@@ -47,8 +47,7 @@ def test_bokchoy(options, passthrough_options):
     if validate_firefox:
         check_firefox_version()
 
-    opts = parse_bokchoy_opts(options, passthrough_options)
-    run_bokchoy(**opts)
+    run_bokchoy(passthrough_options=passthrough_options, **options)
 
 
 @needs('pavelib.prereqs.install_prereqs')
@@ -73,11 +72,10 @@ def test_a11y(options, passthrough_options):
     """
     # Modify the options object directly, so that any subsequently called tasks
     # that share with this task get the modified options
-    options['report_dir'] = Env.BOK_CHOY_A11Y_REPORT_DIR
-    options['coveragerc'] = Env.BOK_CHOY_A11Y_COVERAGERC
-    options['extra_args'] = options.get('extra_args', '') + ' -a "a11y" '
-    opts = parse_bokchoy_opts(options, passthrough_options)
-    run_bokchoy(**opts)
+    options['test_a11y']['report_dir'] = Env.BOK_CHOY_A11Y_REPORT_DIR
+    options['test_a11y']['coveragerc'] = Env.BOK_CHOY_A11Y_COVERAGERC
+    options['test_a11y']['extra_args'] = options.get('extra_args', '') + ' -a "a11y" '
+    run_bokchoy(passthrough_options=passthrough_options, **options['test_a11y'])
 
 
 @needs('pavelib.prereqs.install_prereqs')
@@ -90,10 +88,9 @@ def perf_report_bokchoy(options, passthrough_options):
     """
     # Modify the options object directly, so that any subsequently called tasks
     # that share with this task get the modified options
-    options['test_dir'] = 'performance'
-    opts = parse_bokchoy_opts(options, passthrough_options)
+    options['perf_report_bokchoy']['test_dir'] = 'performance'
 
-    run_bokchoy(**opts)
+    run_bokchoy(passthrough_options=passthrough_options, **options['perf_report_bokchoy'])
 
 
 @needs('pavelib.prereqs.install_prereqs')
@@ -120,12 +117,15 @@ def pa11ycrawler(options, passthrough_options):
     """
     # Modify the options object directly, so that any subsequently called tasks
     # that share with this task get the modified options
-    options['report_dir'] = Env.PA11YCRAWLER_REPORT_DIR
-    options['coveragerc'] = Env.PA11YCRAWLER_COVERAGERC
-    options['should_fetch_course'] = getattr(options, 'should_fetch_course', not options.get('fasttest'))
-    options['course_key'] = getattr(options, 'course-key', "course-v1:edX+Test101+course")
-    opts = parse_bokchoy_opts(options, passthrough_options)
-    test_suite = Pa11yCrawler('a11y_crawler', **opts)
+    options['pa11ycrawler']['report_dir'] = Env.PA11YCRAWLER_REPORT_DIR
+    options['pa11ycrawler']['coveragerc'] = Env.PA11YCRAWLER_COVERAGERC
+    options['pa11ycrawler']['should_fetch_course'] = getattr(
+        options,
+        'should_fetch_course',
+        not options.get('fasttest')
+    )
+    options['pa11ycrawler']['course_key'] = getattr(options, 'course-key', "course-v1:edX+Test101+course")
+    test_suite = Pa11yCrawler('a11y_crawler', passthrough_options=passthrough_options, **options['pa11ycrawler'])
     test_suite.run()
 
     if getattr(options, 'with_html', False):

pavelib/utils/test/bokchoy_options.py

+"""
+Definitions of all options used by the various bok_choy tasks.
+"""
+
 from optparse import make_option
 import os
 
+from pavelib.utils.envs import Env
+
 
 BOKCHOY_OPTS = [
     ('test-spec=', 't', 'Specific test to run'),
-    ('fasttest', 'a', 'Skip some setup'),
+    make_option('-a', '--fasttest', action='store_true', help='Skip some setup'),
     ('skip-clean', 'C', 'Skip cleaning repository before running tests'),
-    ('serversonly', 'r', 'Prepare suite and leave servers running'),
-    ('testsonly', 'o', 'Assume servers are running and execute tests only'),
+    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'),
-    ('test-dir=', 'd', 'Directory for finding tests (relative to common/test/acceptance)'),
+    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'),
-    ('num-processes=', 'n', 'Number of test threads (for multiprocessing)'),
-    ('verify-xss', 'x', 'Run XSS vulnerability tests'),
+    make_option('-n', '--num-processes', type='int', help='Number of test threads (for multiprocessing)'),
+    make_option(
+        '-x', '--verify-xss',
+        action='store_true',
+        default=os.environ.get('VERIFY_XSS', False),
+        help='Run XSS vulnerability tests'
+    ),
     make_option("--verbose", action="store_const", const=2, dest="verbosity"),
     make_option("-q", "--quiet", action="store_const", const=0, dest="verbosity"),
     make_option("-v", "--verbosity", action="count", dest="verbosity"),
     make_option("--skip-firefox-version-validation", action='store_false', dest="validate_firefox_version"),
     make_option("--save-screenshots", action='store_true', dest="save_screenshots"),
-    make_option("--default_store", default=os.environ.get('DEFAULT_STORE', 'split'), help='deprecated in favor of default-store'),
-    ('extra_args=', 'e', 'deprecated, pass extra options directly in the paver commandline'),
+    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'
+    ),
+    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'),
-    ('num_processes=', None, 'deprecated in favor of num-processes'),
+    make_option('--num_processes', type='int', help='deprecated in favor of num-processes'),
     ('skip_clean', None, 'deprecated in favor of skip-clean'),
-    ('test_dir=', None, 'deprecated in favor of test-dir'),
+    make_option('--test_dir', default='tests', help='deprecated in favor of test-dir'),
     ('test_spec=', None, 'Specific test to run'),
-    ('verify_xss', None, 'deprecated in favor of verify-xss'),
+    make_option(
+        '--verify_xss',
+        action='store_true',
+        default=os.environ.get('VERIFY_XSS', False),
+        help='deprecated in favor of verify-xss'
+    ),
     make_option(
         "--skip_firefox_version_validation",
         action='store_false',
@@ -39,31 +69,3 @@ BOKCHOY_OPTS = [
         help="deprecated in favor of save-screenshots"
     ),
 ]
-
-
-def parse_bokchoy_opts(options, passthrough_options=None):
-    """
-    Parses bok choy options.
-
-    Returns: dict of options.
-    """
-    if passthrough_options is None:
-        passthrough_options = []
-
-    return {
-        'test_spec': getattr(options, 'test_spec', None),
-        'fasttest': getattr(options, 'fasttest', False),
-        'num_processes': int(getattr(options, 'num_processes', 1)),
-        'verify_xss': getattr(options, 'verify_xss', os.environ.get('VERIFY_XSS', False)),
-        'serversonly': getattr(options, 'serversonly', False),
-        'testsonly': getattr(options, 'testsonly', False),
-        'default_store': getattr(options, 'default_store', os.environ.get('DEFAULT_STORE', 'split')),
-        'verbosity': getattr(options, 'verbosity', 2),
-        'extra_args': getattr(options, 'extra_args', ''),
-        'pdb': getattr(options, 'pdb', False),
-        'test_dir': getattr(options, 'test_dir', 'tests'),
-        'imports_dir': getattr(options, 'imports_dir', None),
-        'save_screenshots': getattr(options, 'save_screenshots', False),
-        'passthrough_options': passthrough_options,
-        'report_dir': getattr(options, 'report_dir', Env.BOK_CHOY_REPORT_DIR),
-    }