Merge pull request #12831 from cpennington/xunit-only-in-ci

Clean up paver arguments

lms/djangoapps/course_structure_api/v0/tests.py

 """
 Run these tests @ Devstack:
-    paver test_system -s lms --fasttest --verbose --test_id=lms/djangoapps/course_structure_api
+    paver test_system -s lms --fasttest --verbose --test-id=lms/djangoapps/course_structure_api
 """
 # pylint: disable=missing-docstring,invalid-name,maybe-no-member,attribute-defined-outside-init
 from datetime import datetime

pavelib/acceptance_test.py

 """
 Acceptance test tasks
 """
-from paver.easy import task, cmdopts, needs
+from paver.easy import cmdopts, needs
 from pavelib.utils.test.suites import AcceptanceTestSuite
+from pavelib.utils.passthrough_opts import PassthroughTask
 from optparse import make_option
 
 try:
@@ -13,22 +14,22 @@ except ImportError:
 __test__ = False  # do not collect
 
 
-@task
 @needs(
     'pavelib.prereqs.install_prereqs',
     'pavelib.utils.test.utils.clean_reports_dir',
 )
 @cmdopts([
     ("system=", "s", "System to act on"),
-    ("default_store=", "m", "Default modulestore to use for course creation"),
+    ("default-store=", "m", "Default modulestore to use for course creation"),
     ("fasttest", "a", "Run without collectstatic"),
-    ("extra_args=", "e", "adds as extra args to the test command"),
     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("--pdb", action="store_true", help="Launches an interactive debugger upon error"),
+    ("default_store=", None, "deprecated in favor of default-store"),
+    ('extra_args=', 'e', 'deprecated, pass extra options directly in the paver commandline'),
 ])
-def test_acceptance(options):
+@PassthroughTask
+def test_acceptance(options, passthrough_options):
     """
     Run the acceptance tests for either lms or cms
     """
@@ -39,6 +40,7 @@ def test_acceptance(options):
         'verbosity': getattr(options, 'verbosity', 3),
         'extra_args': getattr(options, 'extra_args', ''),
         'pdb': getattr(options, 'pdb', False),
+        'passthrough_options': passthrough_options,
     }
 
     if opts['system'] not in ['cms', 'lms']:

pavelib/bok_choy.py

@@ -6,6 +6,7 @@ from paver.easy import task, needs, cmdopts, sh
 from pavelib.utils.test.suites.bokchoy_suite import BokChoyTestSuite, Pa11yCrawler
 from pavelib.utils.envs import Env
 from pavelib.utils.test.utils import check_firefox_version
+from pavelib.utils.passthrough_opts import PassthroughTask
 from optparse import make_option
 import os
 
@@ -17,32 +18,53 @@ except ImportError:
 __test__ = False  # do not collect
 
 BOKCHOY_OPTS = [
-    ('test_spec=', 't', 'Specific test to run'),
+    ('test-spec=', 't', 'Specific test to run'),
     ('fasttest', 'a', 'Skip some setup'),
-    ('skip_clean', 'C', 'Skip cleaning repository before running tests'),
+    ('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'),
-    ('extra_args=', 'e', 'adds as extra args to the test command'),
-    ('default_store=', 's', 'Default modulestore'),
-    ('test_dir=', 'd', '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'),
+    ('default-store=', 's', 'Default modulestore'),
+    ('test-dir=', 'd', '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("--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("--pdb", action="store_true", help="Drop into debugger on failures or errors"),
-    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("--skip-firefox-version-validation", action='store_false', dest="validate_firefox_version"),
+    make_option("--save-screenshots", action='store_true', dest="save_screenshots"),
+    ('default_store=', None, 'deprecated in favor of default-store'),
+    ('extra_args=', 'e', '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'),
+    ('skip_clean', None, 'deprecated in favor of skip-clean'),
+    ('test_dir=', None, 'deprecated in favor of test-dir'),
+    ('test_spec=', None, 'Specific test to run'),
+    ('verify_xss', None, 'deprecated in favor of verify-xss'),
+    make_option(
+        "--skip_firefox_version_validation",
+        action='store_false',
+        dest="validate_firefox_version",
+        help="deprecated in favor of --skip-firefox-version-validation"
+    ),
+    make_option(
+        "--save_screenshots",
+        action='store_true',
+        dest="save_screenshots",
+        help="deprecated in favor of save-screenshots"
+    ),
 ]
 
 
-def parse_bokchoy_opts(options):
+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),
@@ -57,13 +79,14 @@ def parse_bokchoy_opts(options):
         '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
     }
 
 
-@task
 @needs('pavelib.prereqs.install_prereqs')
 @cmdopts(BOKCHOY_OPTS)
-def test_bokchoy(options):
+@PassthroughTask
+def test_bokchoy(options, passthrough_options):
     """
     Run acceptance tests that use the bok-choy framework.
     Skips some static asset steps if `fasttest` is True.
@@ -86,14 +109,14 @@ def test_bokchoy(options):
     if validate_firefox:
         check_firefox_version()
 
-    opts = parse_bokchoy_opts(options)
+    opts = parse_bokchoy_opts(options, passthrough_options)
     run_bokchoy(**opts)
 
 
-@task
 @needs('pavelib.prereqs.install_prereqs')
 @cmdopts(BOKCHOY_OPTS)
-def test_a11y(options):
+@PassthroughTask
+def test_a11y(options, passthrough_options):
     """
     Run accessibility tests that use the bok-choy framework.
     Skips some static asset steps if `fasttest` is True.
@@ -109,27 +132,26 @@ def test_a11y(options):
     It can also be left blank to run all tests in the suite that are tagged
     with `@attr("a11y")`.
     """
-    opts = parse_bokchoy_opts(options)
+    opts = parse_bokchoy_opts(options, passthrough_options)
     opts['report_dir'] = Env.BOK_CHOY_A11Y_REPORT_DIR
     opts['coveragerc'] = Env.BOK_CHOY_A11Y_COVERAGERC
     opts['extra_args'] = opts['extra_args'] + ' -a "a11y" '
     run_bokchoy(**opts)
 
 
-@task
 @needs('pavelib.prereqs.install_prereqs')
 @cmdopts(BOKCHOY_OPTS)
-def perf_report_bokchoy(options):
+@PassthroughTask
+def perf_report_bokchoy(options, passthrough_options):
     """
     Generates a har file for with page performance info.
     """
-    opts = parse_bokchoy_opts(options)
+    opts = parse_bokchoy_opts(options, passthrough_options)
     opts['test_dir'] = 'performance'
 
     run_bokchoy(**opts)
 
 
-@task
 @needs('pavelib.prereqs.install_prereqs')
 @cmdopts(BOKCHOY_OPTS + [
     ('with-html', 'w', 'Include html reports'),
@@ -141,7 +163,8 @@ def perf_report_bokchoy(options):
         help='Course key for test course',
     ),
 ])
-def pa11ycrawler(options):
+@PassthroughTask
+def pa11ycrawler(options, passthrough_options):
     """
     Runs pa11ycrawler against the demo-test-course to generates accessibility
     reports. (See https://github.com/edx/demo-test-course)
@@ -150,7 +173,7 @@ def pa11ycrawler(options):
     flag to get an environment running. The setup for this is the same as
     for bok-choy tests, only test course is imported as well.
     """
-    opts = parse_bokchoy_opts(options)
+    opts = parse_bokchoy_opts(options, passthrough_options)
     opts['report_dir'] = Env.PA11YCRAWLER_REPORT_DIR
     opts['coveragerc'] = Env.PA11YCRAWLER_COVERAGERC
     opts['should_fetch_course'] = getattr(options, 'should_fetch_course', not opts['fasttest'])

pavelib/js_test.py

@@ -19,7 +19,8 @@ __test__ = False  # do not collect
     ("mode=", "m", "dev or run"),
     ("coverage", "c", "Run test under coverage"),
     ("port=", "p", "Port to run test server on (dev mode only)"),
-    ('skip_clean', 'C', 'skip cleaning repository before running tests'),
+    ('skip-clean', 'C', 'skip cleaning repository before running tests'),
+    ('skip_clean', None, 'deprecated in favor of skip-clean'),
 ], share_with=["pavelib.utils.tests.utils.clean_reports_dir"])
 def test_js(options):
     """

pavelib/paver_tests/test_js_test.py

@@ -137,7 +137,7 @@ class TestPaverJavaScriptTestTasks(PaverTestCase):
                     suite=suite
                 )
             if port:
-                expected_test_tool_command += u" --port {port}".format(port=port)
+                expected_test_tool_command += u" --port={port}".format(port=port)
             expected_messages.append(expected_test_tool_command)
 
         self.assertEquals(self.task_messages, expected_messages)

pavelib/paver_tests/test_paver_bok_choy_cmds.py

@@ -24,25 +24,23 @@ class TestPaverBokChoyCmd(unittest.TestCase):
         and store.
         """
 
-        expected_statement = (
-            "DEFAULT_STORE={default_store} "
-            "SCREENSHOT_DIR='{repo_dir}/test_root/log{shard_str}' "
-            "BOK_CHOY_HAR_DIR='{repo_dir}/test_root/log{shard_str}/hars' "
-            "BOKCHOY_A11Y_CUSTOM_RULES_FILE='{repo_dir}/{a11y_custom_file}' "
-            "SELENIUM_DRIVER_LOG_DIR='{repo_dir}/test_root/log{shard_str}' "
-            "VERIFY_XSS='{verify_xss}' "
-            "nosetests {repo_dir}/common/test/acceptance/{exp_text} "
-            "--with-xunit "
-            "--xunit-file={repo_dir}/reports/bok_choy{shard_str}/xunit.xml "
-            "--verbosity=2 "
-        ).format(
-            default_store=store,
-            repo_dir=REPO_DIR,
-            shard_str='/shard_' + self.shard if self.shard else '',
-            exp_text=name,
-            a11y_custom_file='node_modules/edx-custom-a11y-rules/lib/custom_a11y_rules.js',
-            verify_xss=verify_xss
-        )
+        shard_str = '/shard_' + self.shard if self.shard else ''
+
+        expected_statement = [
+            "DEFAULT_STORE={}".format(store),
+            "SCREENSHOT_DIR='{}/test_root/log{}'".format(REPO_DIR, shard_str),
+            "BOK_CHOY_HAR_DIR='{}/test_root/log{}/hars'".format(REPO_DIR, shard_str),
+            "BOKCHOY_A11Y_CUSTOM_RULES_FILE='{}/{}'".format(
+                REPO_DIR,
+                'node_modules/edx-custom-a11y-rules/lib/custom_a11y_rules.js'
+            ),
+            "SELENIUM_DRIVER_LOG_DIR='{}/test_root/log{}'".format(REPO_DIR, shard_str),
+            "VERIFY_XSS='{}'".format(verify_xss),
+            "nosetests",
+            "{}/common/test/acceptance/{}".format(REPO_DIR, name),
+            "--xunit-file={}/reports/bok_choy{}/xunit.xml".format(REPO_DIR, shard_str),
+            "--verbosity=2",
+        ]
         return expected_statement
 
     def setUp(self):
@@ -93,7 +91,7 @@ class TestPaverBokChoyCmd(unittest.TestCase):
 
     def test_serversonly(self):
         suite = BokChoyTestSuite('', serversonly=True)
-        self.assertEqual(suite.cmd, "")
+        self.assertEqual(suite.cmd, None)
 
     def test_verify_xss(self):
         suite = BokChoyTestSuite('', verify_xss=True)
@@ -119,14 +117,15 @@ class TestPaverBokChoyCmd(unittest.TestCase):
         """
         Using 1 process means paver should ask for the traditional xunit plugin for plugin results
         """
-        expected_verbosity_string = (
-            "--with-xunit --xunit-file={repo_dir}/reports/bok_choy{shard_str}/xunit.xml --verbosity=2".format(
+        expected_verbosity_command = [
+            "--xunit-file={repo_dir}/reports/bok_choy{shard_str}/xunit.xml".format(
                 repo_dir=REPO_DIR,
                 shard_str='/shard_' + self.shard if self.shard else ''
-            )
-        )
+            ),
+            "--verbosity=2",
+        ]
         suite = BokChoyTestSuite('', num_processes=1)
-        self.assertEqual(BokChoyTestSuite.verbosity_processes_string(suite), expected_verbosity_string)
+        self.assertEqual(suite.verbosity_processes_command, expected_verbosity_command)
 
     def test_verbosity_settings_2_processes(self):
         """
@@ -134,32 +133,34 @@ class TestPaverBokChoyCmd(unittest.TestCase):
         be used.
         """
         process_count = 2
-        expected_verbosity_string = (
-            "--with-xunitmp --xunitmp-file={repo_dir}/reports/bok_choy{shard_str}/xunit.xml"
-            " --processes={procs} --no-color --process-timeout=1200".format(
+        expected_verbosity_command = [
+            "--xunitmp-file={repo_dir}/reports/bok_choy{shard_str}/xunit.xml".format(
                 repo_dir=REPO_DIR,
                 shard_str='/shard_' + self.shard if self.shard else '',
-                procs=process_count
-            )
-        )
+            ),
+            "--processes={}".format(process_count),
+            "--no-color",
+            "--process-timeout=1200",
+        ]
         suite = BokChoyTestSuite('', num_processes=process_count)
-        self.assertEqual(BokChoyTestSuite.verbosity_processes_string(suite), expected_verbosity_string)
+        self.assertEqual(suite.verbosity_processes_command, expected_verbosity_command)
 
     def test_verbosity_settings_3_processes(self):
         """
         With the above test, validate that num_processes can be set to various values
         """
         process_count = 3
-        expected_verbosity_string = (
-            "--with-xunitmp --xunitmp-file={repo_dir}/reports/bok_choy{shard_str}/xunit.xml"
-            " --processes={procs} --no-color --process-timeout=1200".format(
+        expected_verbosity_command = [
+            "--xunitmp-file={repo_dir}/reports/bok_choy{shard_str}/xunit.xml".format(
                 repo_dir=REPO_DIR,
                 shard_str='/shard_' + self.shard if self.shard else '',
-                procs=process_count
-            )
-        )
+            ),
+            "--processes={}".format(process_count),
+            "--no-color",
+            "--process-timeout=1200",
+        ]
         suite = BokChoyTestSuite('', num_processes=process_count)
-        self.assertEqual(BokChoyTestSuite.verbosity_processes_string(suite), expected_verbosity_string)
+        self.assertEqual(suite.verbosity_processes_command, expected_verbosity_command)
 
     def test_invalid_verbosity_and_processes(self):
         """
@@ -168,7 +169,8 @@ class TestPaverBokChoyCmd(unittest.TestCase):
         """
         suite = BokChoyTestSuite('', num_processes=2, verbosity=3)
         with self.assertRaises(BuildFailure):
-            BokChoyTestSuite.verbosity_processes_string(suite)
+            # pylint: disable=pointless-statement
+            suite.verbosity_processes_command
 
 
 class TestPaverPa11yCrawlerCmd(unittest.TestCase):
@@ -192,17 +194,16 @@ class TestPaverPa11yCrawlerCmd(unittest.TestCase):
         """
         Returns the expected command to run pa11ycrawler.
         """
-        expected_statement = (
-            'pa11ycrawler run {start_urls} '
-            '--pa11ycrawler-allowed-domains=localhost '
-            '--pa11ycrawler-reports-dir={report_dir} '
-            '--pa11ycrawler-deny-url-matcher=logout '
-            '--pa11y-reporter="1.0-json" '
-            '--depth-limit=6 '
-        ).format(
-            start_urls=' '.join(start_urls),
-            report_dir=report_dir,
-        )
+        expected_statement = [
+            'pa11ycrawler',
+            'run',
+        ] + start_urls + [
+            '--pa11ycrawler-allowed-domains=localhost',
+            '--pa11ycrawler-reports-dir={}'.format(report_dir),
+            '--pa11ycrawler-deny-url-matcher=logout',
+            '--pa11y-reporter="1.0-json"',
+            '--depth-limit=6',
+        ]
         return expected_statement
 
     def test_default(self):

pavelib/servers.py

@@ -164,14 +164,21 @@ def celery(options):
 @needs('pavelib.prereqs.install_prereqs')
 @cmdopts([
     ("settings=", "s", "Django settings for both LMS and Studio"),
-    ("asset_settings=", "a", "Django settings for updating assets for both LMS and Studio (defaults to settings)"),
-    ("worker_settings=", "w", "Celery worker Django settings"),
+    ("asset-settings=", "a", "Django settings for updating assets for both LMS and Studio (defaults to settings)"),
+    ("worker-settings=", "w", "Celery worker Django settings"),
     ("fast", "f", "Skip updating assets"),
     ("optimized", "o", "Run with optimized assets"),
-    ("settings_lms=", "l", "Set LMS only, overriding the value from --settings (if provided)"),
-    ("asset_settings_lms=", None, "Set LMS only, overriding the value from --asset_settings (if provided)"),
-    ("settings_cms=", "c", "Set Studio only, overriding the value from --settings (if provided)"),
-    ("asset_settings_cms=", None, "Set Studio only, overriding the value from --asset_settings (if provided)"),
+    ("settings-lms=", "l", "Set LMS only, overriding the value from --settings (if provided)"),
+    ("asset-settings-lms=", None, "Set LMS only, overriding the value from --asset-settings (if provided)"),
+    ("settings-cms=", "c", "Set Studio only, overriding the value from --settings (if provided)"),
+    ("asset-settings-cms=", None, "Set Studio only, overriding the value from --asset-settings (if provided)"),
+
+    ("asset_settings=", None, "deprecated in favor of asset-settings"),
+    ("asset_settings_cms=", None, "deprecated in favor of asset-settings-cms"),
+    ("asset_settings_lms=", None, "deprecated in favor of asset-settings-lms"),
+    ("settings_cms=", None, "deprecated in favor of settings-cms"),
+    ("settings_lms=", None, "deprecated in favor of settings-lms"),
+    ("worker_settings=", None, "deprecated in favor of worker-settings"),
 ])
 def run_all_servers(options):
     """

pavelib/utils/passthrough_opts.py

+"""
+Provides:
+    PassthroughOptionParser:
+        A subclass of :class:`optparse.OptionParser` that captures unknown options
+        into its ``passthrough_options`` attribute.
+    PassthroughTask:
+        A subclass of :class:`paver.tasks.Task` that supplies unknown options
+        as the `passthrough_options` argument to the decorated function
+"""
+
+from optparse import OptionParser, BadOptionError
+import paver.tasks
+from mock import patch
+
+
+try:
+    from gettext import gettext
+except ImportError:
+    def gettext(message):
+        """Dummy gettext"""
+        return message
+_ = gettext
+
+
+class PassthroughOptionParser(OptionParser):
+    """
+    An :class:`optparse.OptionParser` which captures any unknown options into
+    the ``passthrough_options`` attribute. Handles both "--long-options" and
+    "-s" short options.
+    """
+    def __init__(self, *args, **kwargs):
+        self.passthrough_options = []
+
+        # N.B. OptionParser is an old-style class, which is why
+        # this isn't using super()
+        OptionParser.__init__(self, *args, **kwargs)
+
+    def _process_long_opt(self, rargs, values):
+        # This is a copy of the OptionParser._process_long_opt method,
+        # modified to capture arguments that aren't understood
+
+        arg = rargs.pop(0)
+
+        # Value explicitly attached to arg?  Pretend it's the next
+        # argument.
+
+        if "=" in arg:
+            (opt, next_arg) = arg.split("=", 1)
+            rargs.insert(0, next_arg)
+            had_explicit_value = True
+        else:
+            opt = arg
+            had_explicit_value = False
+
+        try:
+            opt = self._match_long_opt(opt)
+        except BadOptionError:
+            self.passthrough_options.append(arg)
+            if had_explicit_value:
+                rargs.pop(0)
+            return
+
+        option = self._long_opt[opt]
+        if option.takes_value():
+            nargs = option.nargs
+
+            if len(rargs) < nargs:
+                if nargs == 1:
+                    self.error(_("%s option requires an argument") % opt)
+                else:
+                    self.error(_("%s option requires %d arguments")
+                               % (opt, nargs))
+            elif nargs == 1:
+                value = rargs.pop(0)
+            else:
+                value = tuple(rargs[0:nargs])
+                del rargs[0:nargs]
+
+        elif had_explicit_value:
+            self.error(_("%s option does not take a value") % opt)
+
+        else:
+            value = None
+
+        option.process(opt, value, values, self)
+
+    def _process_short_opts(self, rargs, values):
+        arg = rargs.pop(0)
+        stop = False
+        i = 1
+
+        passthrough_opts = []
+
+        for char in arg[1:]:
+            opt = "-" + char
+            option = self._short_opt.get(opt)
+            i += 1                      # we have consumed a character
+
+            if not option:
+                passthrough_opts.append(char)
+                continue
+
+            if option.takes_value():
+                # Any characters left in arg?  Pretend they're the
+                # next arg, and stop consuming characters of arg.
+
+                if i < len(arg):
+                    rargs.insert(0, arg[i:])
+                    stop = True
+
+                nargs = option.nargs
+                if len(rargs) < nargs:
+                    if nargs == 1:
+                        self.error(_("%s option requires an argument") % opt)
+                    else:
+                        self.error(_("%s option requires %d arguments")
+                                   % (opt, nargs))
+
+                elif nargs == 1:
+                    value = rargs.pop(0)
+                else:
+                    value = tuple(rargs[0:nargs])
+                    del rargs[0:nargs]
+
+            else:                       # option doesn't take a value
+                value = None
+
+            option.process(opt, value, values, self)
+
+            if stop:
+                break
+
+        if passthrough_opts:
+            self.passthrough_options.append('-{}'.format("".join(passthrough_opts)))
+
+
+class PassthroughTask(paver.tasks.Task):
+    """
+    A :class:`paver.tasks.Task` subclass that supplies any options that it doesn't
+    understand to the task function as the ``passthrough_options`` argument.
+    """
+
+    @property
+    def parser(self):
+        with patch.object(paver.tasks.optparse, 'OptionParser', PassthroughOptionParser):
+            return super(PassthroughTask, self).parser
+
+    def __call__(self, *args, **kwargs):
+        paver.tasks.environment.passthrough_options = self._parser.passthrough_options  # pylint: disable=no-member
+        try:
+            return super(PassthroughTask, self).__call__(*args, **kwargs)
+        finally:
+            del paver.tasks.environment.passthrough_options

pavelib/utils/test/suites/acceptance_suite.py

@@ -34,21 +34,19 @@ class AcceptanceTest(TestSuite):
     def cmd(self):
 
         report_file = self.report_dir / "{}.xml".format(self.system)
-        report_args = "--with-xunit --xunit-file {}".format(report_file)
-
-        cmd = (
-            "DEFAULT_STORE={default_store} ./manage.py {system} --settings acceptance harvest --traceback "
-            "--debug-mode --verbosity {verbosity} {pdb}{report_args} {extra_args}".format(
-                default_store=self.default_store,
-                system=self.system,
-                verbosity=self.verbosity,
-                pdb="--pdb " if self.pdb else "",
-                report_args=report_args,
-                extra_args=self.extra_args,
-            )
-        )
-
-        return cmd
+        report_args = ["--xunit-file {}".format(report_file)]
+        return [
+            "DEFAULT_STORE={}".format(self.default_store),
+            "./manage.py",
+            self.system,
+            "--settings=acceptance",
+            "harvest",
+            "--traceback",
+            "--debug-mode",
+            "--verbosity={}".format(self.verbosity),
+        ] + report_args + [
+            self.extra_args
+        ] + self.passthrough_options
 
     def _update_assets(self):
         """

pavelib/utils/test/suites/bokchoy_suite.py

@@ -118,33 +118,34 @@ class BokChoyTestSuite(TestSuite):
             sh("./manage.py lms --settings bok_choy flush --traceback --noinput")
             bokchoy_utils.clear_mongo()
 
-    def verbosity_processes_string(self):
+    @property
+    def verbosity_processes_command(self):
         """
         Multiprocessing, xunit, color, and verbosity do not work well together. We need to construct
         the proper combination for use with nosetests.
         """
-        substring = []
+        command = []
 
         if self.verbosity != DEFAULT_VERBOSITY and self.num_processes != DEFAULT_NUM_PROCESSES:
             msg = 'Cannot pass in both num_processors and verbosity. Quitting'
             raise BuildFailure(msg)
 
         if self.num_processes != 1:
-            # Construct "multiprocess" nosetest substring
-            substring = [
-                "--with-xunitmp --xunitmp-file={}".format(self.xunit_report),
+            # Construct "multiprocess" nosetest command
+            command = [
+                "--xunitmp-file={}".format(self.xunit_report),
                 "--processes={}".format(self.num_processes),
-                "--no-color --process-timeout=1200"
+                "--no-color",
+                "--process-timeout=1200",
             ]
 
         else:
-            substring = [
-                "--with-xunit",
+            command = [
                 "--xunit-file={}".format(self.xunit_report),
                 "--verbosity={}".format(self.verbosity),
             ]
 
-        return " ".join(substring)
+        return command
 
     def prepare_bokchoy_run(self):
         """
@@ -224,7 +225,7 @@ class BokChoyTestSuite(TestSuite):
     def cmd(self):
         """
         This method composes the nosetests command to send to the terminal. If nosetests aren't being run,
-         the command returns an empty string.
+         the command returns None.
         """
         # Default to running all tests if no specific test is specified
         if not self.test_spec:
@@ -235,7 +236,7 @@ class BokChoyTestSuite(TestSuite):
         # Skip any additional commands (such as nosetests) if running in
         # servers only mode
         if self.serversonly:
-            return ""
+            return None
 
         # Construct the nosetests command, specifying where to save
         # screenshots and XUnit XML reports
@@ -248,15 +249,13 @@ class BokChoyTestSuite(TestSuite):
             "VERIFY_XSS='{}'".format(self.verify_xss),
             "nosetests",
             test_spec,
-            "{}".format(self.verbosity_processes_string())
-        ]
-        if self.pdb:
-            cmd.append("--pdb")
+        ] + self.verbosity_processes_command
         if self.save_screenshots:
             cmd.append("--with-save-baseline")
-        cmd.append(self.extra_args)
+        if self.extra_args:
+            cmd.append(self.extra_args)
+        cmd.extend(self.passthrough_options)
 
-        cmd = (" ").join(cmd)
         return cmd
 
 
@@ -352,19 +351,14 @@ class Pa11yCrawler(BokChoyTestSuite):
         """
         Runs pa11ycrawler as staff user against the test course.
         """
-        cmd_str = (
-            'pa11ycrawler run {start_urls} '
-            '--pa11ycrawler-allowed-domains={allowed_domains} '
-            '--pa11ycrawler-reports-dir={report_dir} '
-            '--pa11ycrawler-deny-url-matcher={dont_go_here} '
-            '--pa11y-reporter="{reporter}" '
-            '--depth-limit={depth} '
-        ).format(
-            start_urls=' '.join(self.start_urls),
-            allowed_domains='localhost',
-            report_dir=self.pa11y_report_dir,
-            reporter="1.0-json",
-            dont_go_here="logout",
-            depth="6",
-        )
-        return cmd_str
+        cmd = [
+            'pa11ycrawler',
+            'run',
+        ] + self.start_urls + [
+            '--pa11ycrawler-allowed-domains=localhost',
+            '--pa11ycrawler-reports-dir={}'.format(self.pa11y_report_dir),
+            '--pa11ycrawler-deny-url-matcher=logout',
+            '--pa11y-reporter="1.0-json"',
+            '--depth-limit=6',
+        ]
+        return cmd

pavelib/utils/test/suites/js_suite.py

@@ -76,21 +76,22 @@ class JsTestSubSuite(TestSuite):
         """
         Run the tests using karma runner.
         """
-        cmd = (
-            "karma start {test_conf_file} --single-run={single_run} --capture-timeout=60000 "
-            "--junitreportpath={xunit_report}".format(
-                single_run='false' if self.mode == 'dev' else 'true',
-                test_conf_file=self.test_conf_file,
-                xunit_report=self.xunit_report,
-            )
-        )
+        cmd = [
+            "karma",
+            "start",
+            self.test_conf_file,
+            "--single-run={}".format('false' if self.mode == 'dev' else 'true'),
+            "--capture-timeout=60000",
+            "--junitreportpath={}".format(self.xunit_report),
+        ]
 
         if self.port:
-            cmd += " --port {port}".format(port=self.port)
+            cmd.append("--port={}".format(self.port))
 
         if self.run_under_coverage:
-            cmd += " --coverage --coveragereportpath={report_path}".format(
-                report_path=self.coverage_report
-            )
+            cmd.extend([
+                "--coverage",
+                "--coveragereportpath={}".format(self.coverage_report),
+            ])
 
         return cmd

pavelib/utils/test/suites/nose_suite.py

@@ -59,23 +59,21 @@ class NoseTestSuite(TestSuite):
         unaltered otherwise.
         """
         if self.run_under_coverage:
-            cmd0, cmd_rest = cmd.split(" ", 1)
             # We use "python -m coverage" so that the proper python
             # will run the importable coverage rather than the
             # coverage that OS path finds.
 
-            if not cmd0.endswith('.py'):
-                cmd0 = "`which {}`".format(cmd0)
-
-            cmd = (
-                "python -m coverage run {cov_args} --rcfile={rcfile} "
-                "{cmd0} {cmd_rest}".format(
-                    cov_args=self.cov_args,
-                    rcfile=Env.PYTHON_COVERAGERC,
-                    cmd0=cmd0,
-                    cmd_rest=cmd_rest,
-                )
-            )
+            if not cmd[0].endswith('.py'):
+                cmd[0] = "`which {}`".format(cmd[0])
+
+            cmd = [
+                "python",
+                "-m",
+                "coverage",
+                "run",
+                self.cov_args,
+                "--rcfile={}".format(Env.PYTHON_COVERAGERC),
+            ] + cmd
 
         return cmd
 
@@ -85,30 +83,27 @@ class NoseTestSuite(TestSuite):
         Takes the test options and returns the appropriate flags
         for the command.
         """
-        opts = " "
+        opts = []
 
         # Handle "--failed" as a special case: we want to re-run only
         # the tests that failed within our Django apps
         # This sets the --failed flag for the nosetests command, so this
         # functionality is the same as described in the nose documentation
         if self.failed_only:
-            opts += "--failed"
+            opts.append("--failed")
 
         # This makes it so we use nose's fail-fast feature in two cases.
-        # Case 1: --fail_fast is passed as an arg in the paver command
+        # Case 1: --fail-fast is passed as an arg in the paver command
         # Case 2: The environment variable TESTS_FAIL_FAST is set as True
         env_fail_fast_set = (
             'TESTS_FAIL_FAST' in os.environ and os.environ['TEST_FAIL_FAST']
         )
 
         if self.fail_fast or env_fail_fast_set:
-            opts += " --stop"
-
-        if self.pdb:
-            opts += " --pdb"
+            opts.append("--stop")
 
         if self.use_ids:
-            opts += " --with-id"
+            opts.append("--with-id")
 
         return opts
 
@@ -152,10 +147,9 @@ class SystemTestSuite(NoseTestSuite):
             './manage.py', self.root, 'test',
             '--verbosity={}'.format(self.verbosity),
             self.test_id,
-            self.test_options_flags,
+        ] + self.test_options_flags + [
             '--settings=test',
             self.extra_args,
-            '--with-xunitmp',
             '--xunitmp-file={}'.format(self.report_dir / "nosetests.xml"),
             '--with-database-isolation',
         ]
@@ -166,7 +160,9 @@ class SystemTestSuite(NoseTestSuite):
         if self.randomize:
             cmd.append('--with-randomly')
 
-        return self._under_coverage_cmd(" ".join(cmd))
+        cmd.extend(self.passthrough_options)
+
+        return self._under_coverage_cmd(cmd)
 
     @property
     def _default_test_id(self):
@@ -212,17 +208,14 @@ class LibTestSuite(NoseTestSuite):
 
     @property
     def cmd(self):
-        cmd = (
-            "nosetests --id-file={test_ids} {test_id} {test_opts} "
-            "--with-xunit --xunit-file={xunit_report} {extra} "
-            "--verbosity={verbosity}".format(
-                test_ids=self.test_ids,
-                test_id=self.test_id,
-                test_opts=self.test_options_flags,
-                xunit_report=self.xunit_report,
-                verbosity=self.verbosity,
-                extra=self.extra_args,
-            )
-        )
+        cmd = [
+            "nosetests",
+            "--id-file={}".format(self.test_ids),
+            self.test_id,
+        ] + self.test_options_flags + [
+            "--xunit-file={}".format(self.xunit_report),
+            self.extra_args,
+            "--verbosity={}".format(self.verbosity),
+        ] + self.passthrough_options
 
         return self._under_coverage_cmd(cmd)

pavelib/utils/test/suites/suite.py

@@ -28,6 +28,7 @@ class TestSuite(object):
         self.verbosity = int(kwargs.get('verbosity', 1))
         self.skip_clean = kwargs.get('skip_clean', False)
         self.pdb = kwargs.get('pdb', False)
+        self.passthrough_options = kwargs.get('passthrough_options', [])
 
     def __enter__(self):
         """
@@ -75,7 +76,7 @@ class TestSuite(object):
         It returns False if errors or failures occur. Otherwise, it
         returns True.
         """
-        cmd = self.cmd
+        cmd = " ".join(self.cmd)
 
         if tasks.environment.dry_run:
             tasks.environment.info(cmd)

pavelib/utils/test/utils.py

@@ -39,14 +39,15 @@ def clean_dir(directory):
 
 @task
 @cmdopts([
-    ('skip_clean', 'C', 'skip cleaning repository before running tests'),
+    ('skip-clean', 'C', 'skip cleaning repository before running tests'),
+    ('skip_clean', None, 'deprecated in favor of skip-clean'),
 ])
 def clean_reports_dir(options):
     """
     Clean coverage files, to ensure that we don't use stale data to generate reports.
     """
     if getattr(options, 'skip_clean', False):
-        print '--skip_clean is set, skipping...'
+        print '--skip-clean is set, skipping...'
         return
 
     # We delete the files but preserve the directory structure

scripts/accessibility-tests.sh

@@ -16,7 +16,7 @@ echo "Setting up for accessibility tests..."
 source scripts/jenkins-common.sh
 
 echo "Running explicit accessibility tests..."
-SELENIUM_BROWSER=phantomjs paver test_a11y
+SELENIUM_BROWSER=phantomjs paver test_a11y --with-xunitmp
 
 echo "Generating coverage report..."
 paver a11y_coverage
@@ -24,7 +24,7 @@ paver a11y_coverage
 if [ "$RUN_PA11YCRAWLER" = "1" ]
 then
     echo "Running pa11ycrawler against test course..."
-    paver pa11ycrawler --fasttest --skip_clean --fetch-course --with-html
+    paver pa11ycrawler --fasttest --skip-clean --fetch-course --with-html
 
     echo "Generating coverage report..."
     paver pa11ycrawler_coverage

scripts/circle-ci-tests.sh

@@ -37,11 +37,11 @@ if [ "$CIRCLE_NODE_TOTAL" == "1" ] ; then
     echo "via the CircleCI UI and adjust scripts/circle-ci-tests.sh to match."
 
     echo "Running tests for common/lib/ and pavelib/"
-    paver test_lib --extra_args="--with-flaky" --cov_args="-p" || EXIT=1
+    paver test_lib --with-flaky --cov-args="-p" --with-xunitmp || EXIT=1
     echo "Running python tests for Studio"
-    paver test_system -s cms --extra_args="--with-flaky" --cov_args="-p" || EXIT=1
+    paver test_system -s cms --with-flaky --cov-args="-p" --with-xunitmp || EXIT=1
     echo "Running python tests for lms"
-    paver test_system -s lms --extra_args="--with-flaky" --cov_args="-p" || EXIT=1
+    paver test_system -s lms --with-flaky --cov-args="-p" --with-xunitmp || EXIT=1
 
     exit $EXIT
 else
@@ -74,15 +74,15 @@ else
             ;;
 
         1)  # run all of the lms unit tests
-            paver test_system -s lms --extra_args="--with-flaky" --cov_args="-p"
+            paver test_system -s lms --with-flaky --cov-args="-p" --with-xunitmp
             ;;
 
         2)  # run all of the cms unit tests
-            paver test_system -s cms --extra_args="--with-flaky" --cov_args="-p"
+            paver test_system -s cms --with-flaky --cov-args="-p" --with-xunitmp
             ;;
 
         3)  # run the commonlib unit tests
-            paver test_lib --extra_args="--with-flaky" --cov_args="-p"
+            paver test_lib --with-flaky --cov-args="-p" --with-xunitmp
             ;;
 
         *)

scripts/generic-ci-tests.sh

@@ -99,23 +99,22 @@ case "$TEST_SUITE" in
         ;;
 
     "lms-unit")
-        EXTRA_ARGS="--with-flaky"
-        PAVER_ARGS="--processes=-1 --cov_args='-p' -v"
+        PAVER_ARGS="--with-flaky --processes=-1 --cov-args='-p' -v --with-xunitmp"
         case "$SHARD" in
             "all")
-                paver test_system -s lms --extra_args="$EXTRA_ARGS" $PAVER_ARGS
+                paver test_system -s lms $PAVER_ARGS
                 ;;
             "1")
-                paver test_system -s lms --extra_args="--attr='shard_1' $EXTRA_ARGS" $PAVER_ARGS
+                paver test_system -s lms --attr='shard_1' $PAVER_ARGS
                 ;;
             "2")
-                paver test_system -s lms --extra_args="--attr='shard_2' $EXTRA_ARGS" $PAVER_ARGS
+                paver test_system -s lms --attr='shard_2' $PAVER_ARGS
                 ;;
             "3")
-                paver test_system -s lms --extra_args="--attr='shard_3' $EXTRA_ARGS" $PAVER_ARGS
+                paver test_system -s lms --attr='shard_3' $PAVER_ARGS
                 ;;
             "4")
-                paver test_system -s lms --extra_args="--attr='shard_1=False,shard_2=False,shard_3=False' $EXTRA_ARGS" $PAVER_ARGS
+                paver test_system -s lms --attr='shard_1=False,shard_2=False,shard_3=False' $PAVER_ARGS
                 ;;
             *)
                 # If no shard is specified, rather than running all tests, create an empty xunit file. This is a
@@ -129,11 +128,11 @@ case "$TEST_SUITE" in
         ;;
 
     "cms-unit")
-        paver test_system -s cms --extra_args="--with-flaky" --cov_args="-p" -v
+        paver test_system -s cms --with-flaky --cov-args="-p" -v --with-xunitmp
         ;;
 
     "commonlib-unit")
-        paver test_lib --extra_args="--with-flaky" --cov_args="-p" -v
+        paver test_lib --with-flaky --cov-args="-p" -v --with-xunitmp
         ;;
 
     "js-unit")
@@ -142,8 +141,8 @@ case "$TEST_SUITE" in
         ;;
 
     "commonlib-js-unit")
-        paver test_js --coverage --skip_clean || { EXIT=1; }
-        paver test_lib --skip_clean --extra_args="--with-flaky" --cov_args="-p" || { EXIT=1; }
+        paver test_js --coverage --skip-clean || { EXIT=1; }
+        paver test_lib --skip-clean --with-flaky --cov-args="-p" --with-xunitmp || { EXIT=1; }
 
         # This is to ensure that the build status of the shard is properly set.
         # Because we are running two paver commands in a row, we need to capture
@@ -160,11 +159,11 @@ case "$TEST_SUITE" in
         ;;
 
     "lms-acceptance")
-        paver test_acceptance -s lms --extra_args="-v 3"
+        paver test_acceptance -s lms -vvv --with-xunit
         ;;
 
     "cms-acceptance")
-        paver test_acceptance -s cms --extra_args="-v 3"
+        paver test_acceptance -s cms -vvv --with-xunit
         ;;
 
     "bok-choy")
@@ -175,46 +174,48 @@ case "$TEST_SUITE" in
         cp -R $HOME/firefox/ firefox/
         export SELENIUM_FIREFOX_PATH=firefox/firefox
 
+        PAVER_ARGS="-n $NUMBER_OF_BOKCHOY_THREADS --with-flaky --with-xunitmp"
+
         case "$SHARD" in
 
             "all")
-                paver test_bokchoy
+                paver test_bokchoy $PAVER_ARGS
                 ;;
 
             "1")
-                paver test_bokchoy -n $NUMBER_OF_BOKCHOY_THREADS --extra_args="-a shard_1 --with-flaky"
+                paver test_bokchoy --attr='shard_1' $PAVER_ARGS
                 ;;
 
             "2")
-                paver test_bokchoy -n $NUMBER_OF_BOKCHOY_THREADS --extra_args="-a 'shard_2' --with-flaky"
+                paver test_bokchoy --attr='shard_2' $PAVER_ARGS
                 ;;
 
             "3")
-                paver test_bokchoy -n $NUMBER_OF_BOKCHOY_THREADS --extra_args="-a 'shard_3' --with-flaky"
+                paver test_bokchoy --attr='shard_3' $PAVER_ARGS
                 ;;
 
             "4")
-                paver test_bokchoy -n $NUMBER_OF_BOKCHOY_THREADS --extra_args="-a 'shard_4' --with-flaky"
+                paver test_bokchoy --attr='shard_4' $PAVER_ARGS
                 ;;
 
             "5")
-                paver test_bokchoy -n $NUMBER_OF_BOKCHOY_THREADS --extra_args="-a 'shard_5' --with-flaky"
+                paver test_bokchoy --attr='shard_5' $PAVER_ARGS
                 ;;
 
             "6")
-                paver test_bokchoy -n $NUMBER_OF_BOKCHOY_THREADS --extra_args="-a 'shard_6' --with-flaky"
+                paver test_bokchoy --attr='shard_6' $PAVER_ARGS
                 ;;
 
             "7")
-                paver test_bokchoy -n $NUMBER_OF_BOKCHOY_THREADS --extra_args="-a 'shard_7' --with-flaky"
+                paver test_bokchoy --attr='shard_7' $PAVER_ARGS
                 ;;
 
             "8")
-                paver test_bokchoy -n $NUMBER_OF_BOKCHOY_THREADS --extra_args="-a 'shard_8' --with-flaky"
+                paver test_bokchoy --attr='shard_8' $PAVER_ARGS
                 ;;
 
             "9")
-                paver test_bokchoy -n $NUMBER_OF_BOKCHOY_THREADS --extra_args="-a shard_1=False,shard_2=False,shard_3=False,shard_4=False,shard_5=False,shard_6=False,shard_7=False,shard_8=False,a11y=False --with-flaky"
+                paver test_bokchoy --attr='shard_1=False,shard_2=False,shard_3=False,shard_4=False,shard_5=False,shard_6=False,shard_7=False,shard_8=False,a11y=False' $PAVER_ARGS
                 ;;
 
             # Default case because if we later define another bok-choy shard on Jenkins

setup.cfg

 [nosetests]
 logging-clear-handlers=1
-with-xunitmp=1
 with-ignore-docstrings=1
 exclude-dir=lms/envs
             cms/envs