From c279eb1c036f7ce8ed42ad2e927297e24b5a1745 Mon Sep 17 00:00:00 2001
From: Calen Pennington <cale@edx.org>
Date: Wed, 22 Jun 2016 14:37:16 -0400
Subject: [PATCH 1/7] Add a new Task subclass that will pass through unknown
 options as the unknown_options argument

---
 pavelib/utils/passthrough_opts.py | 153 ++++++++++++++++++++++++++++++
 1 file changed, 153 insertions(+)
 create mode 100644 pavelib/utils/passthrough_opts.py

diff --git a/pavelib/utils/passthrough_opts.py b/pavelib/utils/passthrough_opts.py
new file mode 100644
index 0000000000..88273ca0cc
--- /dev/null
+++ b/pavelib/utils/passthrough_opts.py
@@ -0,0 +1,153 @@
+"""
+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.unknown_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.unknown_options

From 206cedf55ef2580544aaf16421854f684376650b Mon Sep 17 00:00:00 2001
From: Calen Pennington <cale@edx.org>
Date: Wed, 22 Jun 2016 15:06:50 -0400
Subject: [PATCH 2/7] Deprecate extra_args in favor of just passing through all
 unknown options

---
 pavelib/acceptance_test.py                    | 10 +++--
 pavelib/bok_choy.py                           | 33 ++++++++-------
 pavelib/tests.py                              | 29 +++++++------
 pavelib/utils/passthrough_opts.py             |  4 +-
 pavelib/utils/test/suites/acceptance_suite.py |  3 +-
 pavelib/utils/test/suites/bokchoy_suite.py    |  1 +
 pavelib/utils/test/suites/nose_suite.py       |  6 ++-
 pavelib/utils/test/suites/suite.py            |  1 +
 scripts/circle-ci-tests.sh                    | 12 +++---
 scripts/generic-ci-tests.sh                   | 41 +++++++++----------
 10 files changed, 79 insertions(+), 61 deletions(-)

diff --git a/pavelib/acceptance_test.py b/pavelib/acceptance_test.py
index 254af313bd..6d91d9d18f 100644
--- a/pavelib/acceptance_test.py
+++ b/pavelib/acceptance_test.py
@@ -1,8 +1,9 @@
 """
 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,7 +14,6 @@ except ImportError:
 __test__ = False  # do not collect
 
 
-@task
 @needs(
     'pavelib.prereqs.install_prereqs',
     'pavelib.utils.test.utils.clean_reports_dir',
@@ -22,13 +22,14 @@ __test__ = False  # do not collect
     ("system=", "s", "System to act on"),
     ("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"),
+    ('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']:
diff --git a/pavelib/bok_choy.py b/pavelib/bok_choy.py
index 446b7be797..23d752173c 100644
--- a/pavelib/bok_choy.py
+++ b/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
 
@@ -22,7 +23,6 @@ BOKCHOY_OPTS = [
     ('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'),
@@ -34,15 +34,19 @@ BOKCHOY_OPTS = [
     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"),
+    ('extra_args=', 'e', 'deprecated, pass extra options directly in the paver commandline'),
 ]
 
 
-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 +61,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 +91,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 +114,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 +145,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 +155,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'])
diff --git a/pavelib/tests.py b/pavelib/tests.py
index e1bde07e39..a34a43a3dd 100644
--- a/pavelib/tests.py
+++ b/pavelib/tests.py
@@ -7,6 +7,7 @@ import sys
 from paver.easy import sh, task, cmdopts, needs, call_task
 from pavelib.utils.test import suites
 from pavelib.utils.envs import Env
+from pavelib.utils.passthrough_opts import PassthroughTask
 from optparse import make_option
 
 try:
@@ -17,7 +18,6 @@ except ImportError:
 __test__ = False  # do not collect
 
 
-@task
 @needs(
     'pavelib.prereqs.install_prereqs',
     'pavelib.utils.test.utils.clean_reports_dir',
@@ -28,7 +28,6 @@ __test__ = False  # do not collect
     ("failed", "f", "Run only failed tests"),
     ("fail_fast", "x", "Fail suite on first failed test"),
     ("fasttest", "a", "Run without collectstatic"),
-    ('extra_args=', 'e', 'adds as extra args to the test command'),
     ('cov_args=', 'c', 'adds as args to coverage for the test run'),
     ('skip_clean', 'C', 'skip cleaning repository before running tests'),
     ('processes=', 'p', 'number of processes to use running tests'),
@@ -44,8 +43,10 @@ __test__ = False  # do not collect
         dest='disable_migrations',
         help="Create tables directly from apps' models. Can also be used by exporting DISABLE_MIGRATIONS=1."
     ),
+    ('extra_args=', 'e', 'deprecated, pass extra options directly in the paver commandline'),
 ], share_with=['pavelib.utils.test.utils.clean_reports_dir'])
-def test_system(options):
+@PassthroughTask
+def test_system(options, passthrough_options):
     """
     Run tests on our djangoapps for lms and cms
     """
@@ -64,6 +65,7 @@ def test_system(options):
         'disable_migrations': getattr(options, 'disable_migrations', False),
         'processes': getattr(options, 'processes', None),
         'randomize': getattr(options, 'randomize', None),
+        'passthrough_options': passthrough_options
     }
 
     if test_id:
@@ -84,7 +86,6 @@ def test_system(options):
     test_suite.run()
 
 
-@task
 @needs(
     'pavelib.prereqs.install_prereqs',
     'pavelib.utils.test.utils.clean_reports_dir',
@@ -94,15 +95,16 @@ def test_system(options):
     ("test_id=", "t", "Test id"),
     ("failed", "f", "Run only failed tests"),
     ("fail_fast", "x", "Run only failed tests"),
-    ('extra_args=', 'e', 'adds as extra args to the test command'),
     ('cov_args=', 'c', 'adds as args to coverage for the test run'),
     ('skip_clean', 'C', 'skip cleaning repository before running 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", default=1),
     make_option("--pdb", action="store_true", help="Drop into debugger on failures or errors"),
+    ('extra_args=', 'e', 'deprecated, pass extra options directly in the paver commandline'),
 ], share_with=['pavelib.utils.test.utils.clean_reports_dir'])
-def test_lib(options):
+@PassthroughTask
+def test_lib(options, passthrough_options):
     """
     Run tests for common/lib/ and pavelib/ (paver-tests)
     """
@@ -117,6 +119,7 @@ def test_lib(options):
         'cov_args': getattr(options, 'cov_args', ''),
         'skip_clean': getattr(options, 'skip_clean', False),
         'pdb': getattr(options, 'pdb', False),
+        'passthrough_options': passthrough_options
     }
 
     if test_id:
@@ -133,7 +136,6 @@ def test_lib(options):
     test_suite.run()
 
 
-@task
 @needs(
     'pavelib.prereqs.install_prereqs',
     'pavelib.utils.test.utils.clean_reports_dir',
@@ -141,7 +143,6 @@ def test_lib(options):
 @cmdopts([
     ("failed", "f", "Run only failed tests"),
     ("fail_fast", "x", "Run only failed tests"),
-    ('extra_args=', 'e', 'adds as extra args to the test command'),
     ('cov_args=', 'c', 'adds as args to coverage for the test run'),
     make_option("--verbose", action="store_const", const=2, dest="verbosity"),
     make_option("-q", "--quiet", action="store_const", const=0, dest="verbosity"),
@@ -153,8 +154,10 @@ def test_lib(options):
         dest='disable_migrations',
         help="Create tables directly from apps' models. Can also be used by exporting DISABLE_MIGRATIONS=1."
     ),
+    ('extra_args=', 'e', 'deprecated, pass extra options directly in the paver commandline'),
 ])
-def test_python(options):
+@PassthroughTask
+def test_python(options, passthrough_options):
     """
     Run all python tests
     """
@@ -166,27 +169,28 @@ def test_python(options):
         'cov_args': getattr(options, 'cov_args', ''),
         'pdb': getattr(options, 'pdb', False),
         'disable_migrations': getattr(options, 'disable_migrations', False),
+        'passthrough_options': passthrough_options,
     }
 
     python_suite = suites.PythonTestSuite('Python Tests', **opts)
     python_suite.run()
 
 
-@task
 @needs(
     'pavelib.prereqs.install_prereqs',
     'pavelib.utils.test.utils.clean_reports_dir',
 )
 @cmdopts([
     ("suites", "s", "List of unit test suites to run. (js, lib, cms, lms)"),
-    ('extra_args=', 'e', 'adds as extra args to the test command'),
     ('cov_args=', 'c', 'adds as args to coverage for the test run'),
     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", default=1),
     make_option("--pdb", action="store_true", help="Drop into debugger on failures or errors"),
+    ('extra_args=', 'e', 'deprecated, pass extra options directly in the paver commandline'),
 ])
-def test(options):
+@PassthroughTask
+def test(options, passthrough_options):
     """
     Run all tests
     """
@@ -195,6 +199,7 @@ def test(options):
         'extra_args': getattr(options, 'extra_args', ''),
         'cov_args': getattr(options, 'cov_args', ''),
         'pdb': getattr(options, 'pdb', False),
+        'passthrough_options': passthrough_options,
     }
     # Subsuites to be added to the main suite
     python_suite = suites.PythonTestSuite('Python Tests', **opts)
diff --git a/pavelib/utils/passthrough_opts.py b/pavelib/utils/passthrough_opts.py
index 88273ca0cc..515c22ca5c 100644
--- a/pavelib/utils/passthrough_opts.py
+++ b/pavelib/utils/passthrough_opts.py
@@ -29,7 +29,7 @@ class PassthroughOptionParser(OptionParser):
     "-s" short options.
     """
     def __init__(self, *args, **kwargs):
-        self.unknown_options = []
+        self.passthrough_options = []
 
         # N.B. OptionParser is an old-style class, which is why
         # this isn't using super()
@@ -150,4 +150,4 @@ class PassthroughTask(paver.tasks.Task):
         try:
             return super(PassthroughTask, self).__call__(*args, **kwargs)
         finally:
-            del paver.tasks.environment.unknown_options
+            del paver.tasks.environment.passthrough_options
diff --git a/pavelib/utils/test/suites/acceptance_suite.py b/pavelib/utils/test/suites/acceptance_suite.py
index a9081acb9a..2fbfdab3af 100644
--- a/pavelib/utils/test/suites/acceptance_suite.py
+++ b/pavelib/utils/test/suites/acceptance_suite.py
@@ -38,13 +38,14 @@ class AcceptanceTest(TestSuite):
 
         cmd = (
             "DEFAULT_STORE={default_store} ./manage.py {system} --settings acceptance harvest --traceback "
-            "--debug-mode --verbosity {verbosity} {pdb}{report_args} {extra_args}".format(
+            "--debug-mode --verbosity {verbosity} {pdb}{report_args} {extra_args} {passthrough}".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,
+                passthrough=self.passthrough_options
             )
         )
 
diff --git a/pavelib/utils/test/suites/bokchoy_suite.py b/pavelib/utils/test/suites/bokchoy_suite.py
index 327b6b9c3c..4dfa4af09c 100644
--- a/pavelib/utils/test/suites/bokchoy_suite.py
+++ b/pavelib/utils/test/suites/bokchoy_suite.py
@@ -255,6 +255,7 @@ class BokChoyTestSuite(TestSuite):
         if self.save_screenshots:
             cmd.append("--with-save-baseline")
         cmd.append(self.extra_args)
+        cmd.extend(self.passthrough_options)
 
         cmd = (" ").join(cmd)
         return cmd
diff --git a/pavelib/utils/test/suites/nose_suite.py b/pavelib/utils/test/suites/nose_suite.py
index dbdaa28fed..a199191096 100644
--- a/pavelib/utils/test/suites/nose_suite.py
+++ b/pavelib/utils/test/suites/nose_suite.py
@@ -166,6 +166,8 @@ class SystemTestSuite(NoseTestSuite):
         if self.randomize:
             cmd.append('--with-randomly')
 
+        cmd.extend(self.passthrough_options)
+
         return self._under_coverage_cmd(" ".join(cmd))
 
     @property
@@ -215,13 +217,15 @@ class LibTestSuite(NoseTestSuite):
         cmd = (
             "nosetests --id-file={test_ids} {test_id} {test_opts} "
             "--with-xunit --xunit-file={xunit_report} {extra} "
-            "--verbosity={verbosity}".format(
+            "--verbosity={verbosity} "
+            "{passthrough}".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,
+                passthrough=self.passthrough_options
             )
         )
 
diff --git a/pavelib/utils/test/suites/suite.py b/pavelib/utils/test/suites/suite.py
index 5675400e5f..0ab2503731 100644
--- a/pavelib/utils/test/suites/suite.py
+++ b/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):
         """
diff --git a/scripts/circle-ci-tests.sh b/scripts/circle-ci-tests.sh
index db992912e6..e2243f4798 100755
--- a/scripts/circle-ci-tests.sh
+++ b/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" || 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" || 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" || 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"
             ;;
 
         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"
             ;;
 
         3)  # run the commonlib unit tests
-            paver test_lib --extra_args="--with-flaky" --cov_args="-p"
+            paver test_lib --with-flaky --cov_args="-p"
             ;;
 
         *)
diff --git a/scripts/generic-ci-tests.sh b/scripts/generic-ci-tests.sh
index cbdee47be5..06642d3b27 100755
--- a/scripts/generic-ci-tests.sh
+++ b/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"
         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
         ;;
 
     "commonlib-unit")
-        paver test_lib --extra_args="--with-flaky" --cov_args="-p" -v
+        paver test_lib --with-flaky --cov_args="-p" -v
         ;;
 
     "js-unit")
@@ -143,7 +142,7 @@ 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_lib --skip_clean --with-flaky --cov_args="-p" || { 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
         ;;
 
     "cms-acceptance")
-        paver test_acceptance -s cms --extra_args="-v 3"
+        paver test_acceptance -s cms -vvv
         ;;
 
     "bok-choy")
@@ -182,39 +181,39 @@ case "$TEST_SUITE" in
                 ;;
 
             "1")
-                paver test_bokchoy -n $NUMBER_OF_BOKCHOY_THREADS --extra_args="-a shard_1 --with-flaky"
+                paver test_bokchoy -n $NUMBER_OF_BOKCHOY_THREADS --attr='shard_1' --with-flaky
                 ;;
 
             "2")
-                paver test_bokchoy -n $NUMBER_OF_BOKCHOY_THREADS --extra_args="-a 'shard_2' --with-flaky"
+                paver test_bokchoy -n $NUMBER_OF_BOKCHOY_THREADS --attr='shard_2' --with-flaky
                 ;;
 
             "3")
-                paver test_bokchoy -n $NUMBER_OF_BOKCHOY_THREADS --extra_args="-a 'shard_3' --with-flaky"
+                paver test_bokchoy -n $NUMBER_OF_BOKCHOY_THREADS --attr='shard_3' --with-flaky
                 ;;
 
             "4")
-                paver test_bokchoy -n $NUMBER_OF_BOKCHOY_THREADS --extra_args="-a 'shard_4' --with-flaky"
+                paver test_bokchoy -n $NUMBER_OF_BOKCHOY_THREADS --attr='shard_4' --with-flaky
                 ;;
 
             "5")
-                paver test_bokchoy -n $NUMBER_OF_BOKCHOY_THREADS --extra_args="-a 'shard_5' --with-flaky"
+                paver test_bokchoy -n $NUMBER_OF_BOKCHOY_THREADS --attr='shard_5' --with-flaky
                 ;;
 
             "6")
-                paver test_bokchoy -n $NUMBER_OF_BOKCHOY_THREADS --extra_args="-a 'shard_6' --with-flaky"
+                paver test_bokchoy -n $NUMBER_OF_BOKCHOY_THREADS --attr='shard_6' --with-flaky
                 ;;
 
             "7")
-                paver test_bokchoy -n $NUMBER_OF_BOKCHOY_THREADS --extra_args="-a 'shard_7' --with-flaky"
+                paver test_bokchoy -n $NUMBER_OF_BOKCHOY_THREADS --attr='shard_7' --with-flaky
                 ;;
 
             "8")
-                paver test_bokchoy -n $NUMBER_OF_BOKCHOY_THREADS --extra_args="-a 'shard_8' --with-flaky"
+                paver test_bokchoy -n $NUMBER_OF_BOKCHOY_THREADS --attr='shard_8' --with-flaky
                 ;;
 
             "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 -n $NUMBER_OF_BOKCHOY_THREADS --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' --with-flaky
                 ;;
 
             # Default case because if we later define another bok-choy shard on Jenkins

From ef89c75b192aad4752850d30d8bbfad01098990a Mon Sep 17 00:00:00 2001
From: Calen Pennington <cale@edx.org>
Date: Wed, 22 Jun 2016 16:34:24 -0400
Subject: [PATCH 3/7] Deprecate paver arguments with '_' in them in favor of
 versions with '-'

---
 .../course_structure_api/v0/tests.py          |  2 +-
 pavelib/acceptance_test.py                    |  3 +-
 pavelib/bok_choy.py                           | 37 +++++++++++++-----
 pavelib/js_test.py                            |  3 +-
 pavelib/servers.py                            | 19 ++++++---
 pavelib/tests.py                              | 39 ++++++++++++-------
 pavelib/utils/test/suites/nose_suite.py       |  2 +-
 pavelib/utils/test/utils.py                   |  5 ++-
 scripts/accessibility-tests.sh                |  2 +-
 scripts/circle-ci-tests.sh                    | 12 +++---
 scripts/generic-ci-tests.sh                   | 10 ++---
 11 files changed, 88 insertions(+), 46 deletions(-)

diff --git a/lms/djangoapps/course_structure_api/v0/tests.py b/lms/djangoapps/course_structure_api/v0/tests.py
index 954c9beb45..a83baaee27 100644
--- a/lms/djangoapps/course_structure_api/v0/tests.py
+++ b/lms/djangoapps/course_structure_api/v0/tests.py
@@ -1,6 +1,6 @@
 """
 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
diff --git a/pavelib/acceptance_test.py b/pavelib/acceptance_test.py
index 6d91d9d18f..e5fc548053 100644
--- a/pavelib/acceptance_test.py
+++ b/pavelib/acceptance_test.py
@@ -20,12 +20,13 @@ __test__ = False  # do not collect
 )
 @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"),
     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'),
 ])
 @PassthroughTask
diff --git a/pavelib/bok_choy.py b/pavelib/bok_choy.py
index 23d752173c..7d663f9e7a 100644
--- a/pavelib/bok_choy.py
+++ b/pavelib/bok_choy.py
@@ -18,23 +18,42 @@ 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'),
-    ('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"
+    ),
 ]
 
 
diff --git a/pavelib/js_test.py b/pavelib/js_test.py
index fb1a8933e6..058ffebe83 100644
--- a/pavelib/js_test.py
+++ b/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):
     """
diff --git a/pavelib/servers.py b/pavelib/servers.py
index 19d9302409..db4076ccf6 100644
--- a/pavelib/servers.py
+++ b/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):
     """
diff --git a/pavelib/tests.py b/pavelib/tests.py
index a34a43a3dd..f3a8861003 100644
--- a/pavelib/tests.py
+++ b/pavelib/tests.py
@@ -24,12 +24,12 @@ __test__ = False  # do not collect
 )
 @cmdopts([
     ("system=", "s", "System to act on"),
-    ("test_id=", "t", "Test id"),
+    ("test-id=", "t", "Test id"),
     ("failed", "f", "Run only failed tests"),
-    ("fail_fast", "x", "Fail suite on first failed test"),
+    ("fail-fast", "x", "Fail suite on first failed test"),
     ("fasttest", "a", "Run without collectstatic"),
-    ('cov_args=', 'c', 'adds as args to coverage for the test run'),
-    ('skip_clean', 'C', 'skip cleaning repository before running tests'),
+    ('cov-args=', 'c', 'adds as args to coverage for the test run'),
+    ('skip-clean', 'C', 'skip cleaning repository before running tests'),
     ('processes=', 'p', 'number of processes to use running tests'),
     make_option('-r', '--randomize', action='store_true', dest='randomize', help='run the tests in a random order'),
     make_option('--no-randomize', action='store_false', dest='randomize', help="don't run the tests in a random order"),
@@ -43,7 +43,11 @@ __test__ = False  # do not collect
         dest='disable_migrations',
         help="Create tables directly from apps' models. Can also be used by exporting DISABLE_MIGRATIONS=1."
     ),
+    ("fail_fast", None, "deprecated in favor of fail-fast"),
+    ("test_id=", None, "deprecated in favor of test-id"),
+    ('cov_args=', None, 'deprecated in favor of cov-args'),
     ('extra_args=', 'e', 'deprecated, pass extra options directly in the paver commandline'),
+    ('skip_clean', None, 'deprecated in favor of skip-clean'),
 ], share_with=['pavelib.utils.test.utils.clean_reports_dir'])
 @PassthroughTask
 def test_system(options, passthrough_options):
@@ -92,16 +96,20 @@ def test_system(options, passthrough_options):
 )
 @cmdopts([
     ("lib=", "l", "lib to test"),
-    ("test_id=", "t", "Test id"),
+    ("test-id=", "t", "Test id"),
     ("failed", "f", "Run only failed tests"),
-    ("fail_fast", "x", "Run only failed tests"),
-    ('cov_args=', 'c', 'adds as args to coverage for the test run'),
-    ('skip_clean', 'C', 'skip cleaning repository before running tests'),
+    ("fail-fast", "x", "Run only failed tests"),
+    ('cov-args=', 'c', 'adds as args to coverage for the test run'),
+    ('skip-clean', 'C', 'skip cleaning repository before running 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", default=1),
     make_option("--pdb", action="store_true", help="Drop into debugger on failures or errors"),
+    ('cov_args=', None, 'deprecated in favor of cov-args'),
     ('extra_args=', 'e', 'deprecated, pass extra options directly in the paver commandline'),
+    ("fail_fast", None, "deprecated in favor of fail-fast"),
+    ('skip_clean', None, 'deprecated in favor of skip-clean'),
+    ("test_id=", None, "deprecated in favor of test-id"),
 ], share_with=['pavelib.utils.test.utils.clean_reports_dir'])
 @PassthroughTask
 def test_lib(options, passthrough_options):
@@ -142,8 +150,8 @@ def test_lib(options, passthrough_options):
 )
 @cmdopts([
     ("failed", "f", "Run only failed tests"),
-    ("fail_fast", "x", "Run only failed tests"),
-    ('cov_args=', 'c', 'adds as args to coverage for the test run'),
+    ("fail-fast", "x", "Run only failed tests"),
+    ('cov-args=', 'c', 'adds as args to coverage for the test run'),
     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", default=1),
@@ -154,7 +162,9 @@ def test_lib(options, passthrough_options):
         dest='disable_migrations',
         help="Create tables directly from apps' models. Can also be used by exporting DISABLE_MIGRATIONS=1."
     ),
+    ('cov_args=', None, 'deprecated in favor of cov-args'),
     ('extra_args=', 'e', 'deprecated, pass extra options directly in the paver commandline'),
+    ("fail_fast", None, "deprecated in favor of fail-fast"),
 ])
 @PassthroughTask
 def test_python(options, passthrough_options):
@@ -182,11 +192,12 @@ def test_python(options, passthrough_options):
 )
 @cmdopts([
     ("suites", "s", "List of unit test suites to run. (js, lib, cms, lms)"),
-    ('cov_args=', 'c', 'adds as args to coverage for the test run'),
+    ('cov-args=', 'c', 'adds as args to coverage for the test run'),
     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", default=1),
     make_option("--pdb", action="store_true", help="Drop into debugger on failures or errors"),
+    ('cov_args=', None, 'deprecated in favor of cov-args'),
     ('extra_args=', 'e', 'deprecated, pass extra options directly in the paver commandline'),
 ])
 @PassthroughTask
@@ -213,7 +224,8 @@ def test(options, passthrough_options):
 @task
 @needs('pavelib.prereqs.install_prereqs')
 @cmdopts([
-    ("compare_branch=", "b", "Branch to compare against, defaults to origin/master"),
+    ("compare-branch=", "b", "Branch to compare against, defaults to origin/master"),
+    ("compare_branch=", None, "deprecated in favor of compare-branch"),
 ])
 def coverage(options):
     """
@@ -249,7 +261,8 @@ def coverage(options):
 @task
 @needs('pavelib.prereqs.install_prereqs')
 @cmdopts([
-    ("compare_branch=", "b", "Branch to compare against, defaults to origin/master"),
+    ("compare-branch=", "b", "Branch to compare against, defaults to origin/master"),
+    ("compare_branch=", None, "deprecated in favor of compare-branch"),
 ])
 def diff_coverage(options):
     """
diff --git a/pavelib/utils/test/suites/nose_suite.py b/pavelib/utils/test/suites/nose_suite.py
index a199191096..89c114d621 100644
--- a/pavelib/utils/test/suites/nose_suite.py
+++ b/pavelib/utils/test/suites/nose_suite.py
@@ -95,7 +95,7 @@ class NoseTestSuite(TestSuite):
             opts += "--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']
diff --git a/pavelib/utils/test/utils.py b/pavelib/utils/test/utils.py
index 8e3e3e62b1..769fe17386 100644
--- a/pavelib/utils/test/utils.py
+++ b/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
diff --git a/scripts/accessibility-tests.sh b/scripts/accessibility-tests.sh
index d9ec4d250d..3730215bb2 100755
--- a/scripts/accessibility-tests.sh
+++ b/scripts/accessibility-tests.sh
@@ -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
diff --git a/scripts/circle-ci-tests.sh b/scripts/circle-ci-tests.sh
index e2243f4798..11c05239a3 100755
--- a/scripts/circle-ci-tests.sh
+++ b/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 --with-flaky --cov_args="-p" || EXIT=1
+    paver test_lib --with-flaky --cov-args="-p" || EXIT=1
     echo "Running python tests for Studio"
-    paver test_system -s cms --with-flaky --cov_args="-p" || EXIT=1
+    paver test_system -s cms --with-flaky --cov-args="-p" || EXIT=1
     echo "Running python tests for lms"
-    paver test_system -s lms --with-flaky --cov_args="-p" || EXIT=1
+    paver test_system -s lms --with-flaky --cov-args="-p" || EXIT=1
 
     exit $EXIT
 else
@@ -74,15 +74,15 @@ else
             ;;
 
         1)  # run all of the lms unit tests
-            paver test_system -s lms --with-flaky --cov_args="-p"
+            paver test_system -s lms --with-flaky --cov-args="-p"
             ;;
 
         2)  # run all of the cms unit tests
-            paver test_system -s cms --with-flaky --cov_args="-p"
+            paver test_system -s cms --with-flaky --cov-args="-p"
             ;;
 
         3)  # run the commonlib unit tests
-            paver test_lib --with-flaky --cov_args="-p"
+            paver test_lib --with-flaky --cov-args="-p"
             ;;
 
         *)
diff --git a/scripts/generic-ci-tests.sh b/scripts/generic-ci-tests.sh
index 06642d3b27..363a851e0f 100755
--- a/scripts/generic-ci-tests.sh
+++ b/scripts/generic-ci-tests.sh
@@ -99,7 +99,7 @@ case "$TEST_SUITE" in
         ;;
 
     "lms-unit")
-        PAVER_ARGS="--with-flaky --processes=-1 --cov_args='-p' -v"
+        PAVER_ARGS="--with-flaky --processes=-1 --cov-args='-p' -v"
         case "$SHARD" in
             "all")
                 paver test_system -s lms $PAVER_ARGS
@@ -128,11 +128,11 @@ case "$TEST_SUITE" in
         ;;
 
     "cms-unit")
-        paver test_system -s cms --with-flaky --cov_args="-p" -v
+        paver test_system -s cms --with-flaky --cov-args="-p" -v
         ;;
 
     "commonlib-unit")
-        paver test_lib --with-flaky --cov_args="-p" -v
+        paver test_lib --with-flaky --cov-args="-p" -v
         ;;
 
     "js-unit")
@@ -141,8 +141,8 @@ case "$TEST_SUITE" in
         ;;
 
     "commonlib-js-unit")
-        paver test_js --coverage --skip_clean || { EXIT=1; }
-        paver test_lib --skip_clean --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" || { 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

From 6b61bc44d98cc868642f58cc16ac88d85a5a6c34 Mon Sep 17 00:00:00 2001
From: Calen Pennington <cale@edx.org>
Date: Thu, 23 Jun 2016 11:12:09 -0400
Subject: [PATCH 4/7] Covert all paver command manipulation to happen on lists
 of args/options, rather than strings

---
 pavelib/paver_tests/test_js_test.py           |   2 +-
 .../paver_tests/test_paver_bok_choy_cmds.py   | 107 +++++++++---------
 pavelib/utils/test/suites/acceptance_suite.py |  29 +++--
 pavelib/utils/test/suites/bokchoy_suite.py    |  57 +++++-----
 pavelib/utils/test/suites/js_suite.py         |  25 ++--
 pavelib/utils/test/suites/nose_suite.py       |  58 +++++-----
 pavelib/utils/test/suites/suite.py            |   2 +-
 7 files changed, 138 insertions(+), 142 deletions(-)

diff --git a/pavelib/paver_tests/test_js_test.py b/pavelib/paver_tests/test_js_test.py
index 8689776908..085fdcab8d 100644
--- a/pavelib/paver_tests/test_js_test.py
+++ b/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)
diff --git a/pavelib/paver_tests/test_paver_bok_choy_cmds.py b/pavelib/paver_tests/test_paver_bok_choy_cmds.py
index 9f37700463..865b585950 100644
--- a/pavelib/paver_tests/test_paver_bok_choy_cmds.py
+++ b/pavelib/paver_tests/test_paver_bok_choy_cmds.py
@@ -24,25 +24,24 @@ 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),
+            "--with-xunit",
+            "--xunit-file={}/reports/bok_choy{}/xunit.xml".format(REPO_DIR, shard_str),
+            "--verbosity=2",
+        ]
         return expected_statement
 
     def setUp(self):
@@ -93,7 +92,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 +118,16 @@ 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 = [
+            "--with-xunit",
+            "--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 +135,36 @@ 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 = [
+            "--with-xunitmp",
+            "--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 = [
+            "--with-xunitmp",
+            "--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 +173,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 +198,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):
diff --git a/pavelib/utils/test/suites/acceptance_suite.py b/pavelib/utils/test/suites/acceptance_suite.py
index 2fbfdab3af..f9ea7f438a 100644
--- a/pavelib/utils/test/suites/acceptance_suite.py
+++ b/pavelib/utils/test/suites/acceptance_suite.py
@@ -34,22 +34,21 @@ class AcceptanceTest(TestSuite):
     def cmd(self):
 
         report_file = self.report_dir / "{}.xml".format(self.system)
-        report_args = "--with-xunit --xunit-file {}".format(report_file)
+        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} {passthrough}".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,
-                passthrough=self.passthrough_options
-            )
-        )
-
-        return cmd
+        return [
+            "DEFAULT_STORE={}".format(self.default_store),
+            "./manage.py",
+            self.system,
+            "--settings=acceptance",
+            "harvest",
+            "--traceback",
+            "--debug-mode",
+            "--verbosity={}".format(self.verbosity),
+            "--pdb" if self.pdb else "",
+        ] + report_args + [
+            self.extra_args
+        ] + self.passthrough_options
 
     def _update_assets(self):
         """
diff --git a/pavelib/utils/test/suites/bokchoy_suite.py b/pavelib/utils/test/suites/bokchoy_suite.py
index 4dfa4af09c..805d507f29 100644
--- a/pavelib/utils/test/suites/bokchoy_suite.py
+++ b/pavelib/utils/test/suites/bokchoy_suite.py
@@ -118,33 +118,36 @@ 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 = [
+                "--with-xunitmp",
+                "--xunitmp-file={}".format(self.xunit_report),
                 "--processes={}".format(self.num_processes),
-                "--no-color --process-timeout=1200"
+                "--no-color",
+                "--process-timeout=1200",
             ]
 
         else:
-            substring = [
+            command = [
                 "--with-xunit",
                 "--xunit-file={}".format(self.xunit_report),
                 "--verbosity={}".format(self.verbosity),
             ]
 
-        return " ".join(substring)
+        return command
 
     def prepare_bokchoy_run(self):
         """
@@ -224,7 +227,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 +238,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,16 +251,15 @@ class BokChoyTestSuite(TestSuite):
             "VERIFY_XSS='{}'".format(self.verify_xss),
             "nosetests",
             test_spec,
-            "{}".format(self.verbosity_processes_string())
-        ]
+        ] + self.verbosity_processes_command
         if self.pdb:
             cmd.append("--pdb")
         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
 
 
@@ -353,19 +355,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
diff --git a/pavelib/utils/test/suites/js_suite.py b/pavelib/utils/test/suites/js_suite.py
index 6d810c4ca6..d5d9e9c732 100644
--- a/pavelib/utils/test/suites/js_suite.py
+++ b/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
diff --git a/pavelib/utils/test/suites/nose_suite.py b/pavelib/utils/test/suites/nose_suite.py
index 89c114d621..a3e901a4d4 100644
--- a/pavelib/utils/test/suites/nose_suite.py
+++ b/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)
+            if not cmd[0].endswith('.py'):
+                cmd[0] = "`which {}`".format(cmd[0])
 
-            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,
-                )
-            )
+            cmd = [
+                "python",
+                "-m",
+                "coverage",
+                "run",
+                self.cov_args,
+                "--rcfile={}".format(Env.PYTHON_COVERAGERC),
+            ] + cmd
 
         return cmd
 
@@ -85,14 +83,14 @@ 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
@@ -102,13 +100,13 @@ class NoseTestSuite(TestSuite):
         )
 
         if self.fail_fast or env_fail_fast_set:
-            opts += " --stop"
+            opts.append("--stop")
 
         if self.pdb:
             opts += " --pdb"
 
         if self.use_ids:
-            opts += " --with-id"
+            opts.append("--with-id")
 
         return opts
 
@@ -152,7 +150,7 @@ 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',
@@ -168,7 +166,7 @@ class SystemTestSuite(NoseTestSuite):
 
         cmd.extend(self.passthrough_options)
 
-        return self._under_coverage_cmd(" ".join(cmd))
+        return self._under_coverage_cmd(cmd)
 
     @property
     def _default_test_id(self):
@@ -214,19 +212,15 @@ 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} "
-            "{passthrough}".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,
-                passthrough=self.passthrough_options
-            )
-        )
+        cmd = [
+            "nosetests",
+            "--id-file={}".format(self.test_ids),
+            self.test_id,
+        ] + self.test_options_flags + [
+            "--with-xunit",
+            "--xunit-file={}".format(self.xunit_report),
+            self.extra_args,
+            "--verbosity={}".format(self.verbosity),
+        ] + self.passthrough_options
 
         return self._under_coverage_cmd(cmd)
diff --git a/pavelib/utils/test/suites/suite.py b/pavelib/utils/test/suites/suite.py
index 0ab2503731..f11bb62498 100644
--- a/pavelib/utils/test/suites/suite.py
+++ b/pavelib/utils/test/suites/suite.py
@@ -76,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)

From 2690c044c6822639ab3a8328bc04a3a4cf2bdd3a Mon Sep 17 00:00:00 2001
From: Calen Pennington <cale@edx.org>
Date: Thu, 23 Jun 2016 16:18:41 -0400
Subject: [PATCH 5/7] Only generate xunit files during CI builds, so that pudb
 works during development

---
 .../paver_tests/test_paver_bok_choy_cmds.py   |  4 ---
 pavelib/utils/test/suites/acceptance_suite.py |  3 +-
 pavelib/utils/test/suites/bokchoy_suite.py    |  2 --
 pavelib/utils/test/suites/nose_suite.py       |  2 --
 scripts/accessibility-tests.sh                |  2 +-
 scripts/circle-ci-tests.sh                    | 12 +++----
 scripts/generic-ci-tests.sh                   | 34 ++++++++++---------
 setup.cfg                                     |  1 -
 8 files changed, 26 insertions(+), 34 deletions(-)

diff --git a/pavelib/paver_tests/test_paver_bok_choy_cmds.py b/pavelib/paver_tests/test_paver_bok_choy_cmds.py
index 865b585950..f2093932d2 100644
--- a/pavelib/paver_tests/test_paver_bok_choy_cmds.py
+++ b/pavelib/paver_tests/test_paver_bok_choy_cmds.py
@@ -38,7 +38,6 @@ class TestPaverBokChoyCmd(unittest.TestCase):
             "VERIFY_XSS='{}'".format(verify_xss),
             "nosetests",
             "{}/common/test/acceptance/{}".format(REPO_DIR, name),
-            "--with-xunit",
             "--xunit-file={}/reports/bok_choy{}/xunit.xml".format(REPO_DIR, shard_str),
             "--verbosity=2",
         ]
@@ -119,7 +118,6 @@ class TestPaverBokChoyCmd(unittest.TestCase):
         Using 1 process means paver should ask for the traditional xunit plugin for plugin results
         """
         expected_verbosity_command = [
-            "--with-xunit",
             "--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 ''
@@ -136,7 +134,6 @@ class TestPaverBokChoyCmd(unittest.TestCase):
         """
         process_count = 2
         expected_verbosity_command = [
-            "--with-xunitmp",
             "--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 '',
@@ -154,7 +151,6 @@ class TestPaverBokChoyCmd(unittest.TestCase):
         """
         process_count = 3
         expected_verbosity_command = [
-            "--with-xunitmp",
             "--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 '',
diff --git a/pavelib/utils/test/suites/acceptance_suite.py b/pavelib/utils/test/suites/acceptance_suite.py
index f9ea7f438a..d87d7b6898 100644
--- a/pavelib/utils/test/suites/acceptance_suite.py
+++ b/pavelib/utils/test/suites/acceptance_suite.py
@@ -34,8 +34,7 @@ class AcceptanceTest(TestSuite):
     def cmd(self):
 
         report_file = self.report_dir / "{}.xml".format(self.system)
-        report_args = ["--with-xunit", "--xunit-file {}".format(report_file)]
-
+        report_args = ["--xunit-file {}".format(report_file)]
         return [
             "DEFAULT_STORE={}".format(self.default_store),
             "./manage.py",
diff --git a/pavelib/utils/test/suites/bokchoy_suite.py b/pavelib/utils/test/suites/bokchoy_suite.py
index 805d507f29..bda6098ace 100644
--- a/pavelib/utils/test/suites/bokchoy_suite.py
+++ b/pavelib/utils/test/suites/bokchoy_suite.py
@@ -133,7 +133,6 @@ class BokChoyTestSuite(TestSuite):
         if self.num_processes != 1:
             # Construct "multiprocess" nosetest command
             command = [
-                "--with-xunitmp",
                 "--xunitmp-file={}".format(self.xunit_report),
                 "--processes={}".format(self.num_processes),
                 "--no-color",
@@ -142,7 +141,6 @@ class BokChoyTestSuite(TestSuite):
 
         else:
             command = [
-                "--with-xunit",
                 "--xunit-file={}".format(self.xunit_report),
                 "--verbosity={}".format(self.verbosity),
             ]
diff --git a/pavelib/utils/test/suites/nose_suite.py b/pavelib/utils/test/suites/nose_suite.py
index a3e901a4d4..cef5fcfa11 100644
--- a/pavelib/utils/test/suites/nose_suite.py
+++ b/pavelib/utils/test/suites/nose_suite.py
@@ -153,7 +153,6 @@ class SystemTestSuite(NoseTestSuite):
         ] + self.test_options_flags + [
             '--settings=test',
             self.extra_args,
-            '--with-xunitmp',
             '--xunitmp-file={}'.format(self.report_dir / "nosetests.xml"),
             '--with-database-isolation',
         ]
@@ -217,7 +216,6 @@ class LibTestSuite(NoseTestSuite):
             "--id-file={}".format(self.test_ids),
             self.test_id,
         ] + self.test_options_flags + [
-            "--with-xunit",
             "--xunit-file={}".format(self.xunit_report),
             self.extra_args,
             "--verbosity={}".format(self.verbosity),
diff --git a/scripts/accessibility-tests.sh b/scripts/accessibility-tests.sh
index 3730215bb2..fbf262aa67 100755
--- a/scripts/accessibility-tests.sh
+++ b/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
diff --git a/scripts/circle-ci-tests.sh b/scripts/circle-ci-tests.sh
index 11c05239a3..b9e1683cde 100755
--- a/scripts/circle-ci-tests.sh
+++ b/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 --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 --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 --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 --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 --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 --with-flaky --cov-args="-p"
+            paver test_lib --with-flaky --cov-args="-p" --with-xunitmp
             ;;
 
         *)
diff --git a/scripts/generic-ci-tests.sh b/scripts/generic-ci-tests.sh
index 363a851e0f..dba3705b38 100755
--- a/scripts/generic-ci-tests.sh
+++ b/scripts/generic-ci-tests.sh
@@ -99,7 +99,7 @@ case "$TEST_SUITE" in
         ;;
 
     "lms-unit")
-        PAVER_ARGS="--with-flaky --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 $PAVER_ARGS
@@ -128,11 +128,11 @@ case "$TEST_SUITE" in
         ;;
 
     "cms-unit")
-        paver test_system -s cms --with-flaky --cov-args="-p" -v
+        paver test_system -s cms --with-flaky --cov-args="-p" -v --with-xunitmp
         ;;
 
     "commonlib-unit")
-        paver test_lib --with-flaky --cov-args="-p" -v
+        paver test_lib --with-flaky --cov-args="-p" -v --with-xunitmp
         ;;
 
     "js-unit")
@@ -142,7 +142,7 @@ case "$TEST_SUITE" in
 
     "commonlib-js-unit")
         paver test_js --coverage --skip-clean || { EXIT=1; }
-        paver test_lib --skip-clean --with-flaky --cov-args="-p" || { 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
@@ -159,11 +159,11 @@ case "$TEST_SUITE" in
         ;;
 
     "lms-acceptance")
-        paver test_acceptance -s lms -vvv
+        paver test_acceptance -s lms -vvv --with-xunit
         ;;
 
     "cms-acceptance")
-        paver test_acceptance -s cms -vvv
+        paver test_acceptance -s cms -vvv --with-xunit
         ;;
 
     "bok-choy")
@@ -174,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 --attr='shard_1' --with-flaky
+                paver test_bokchoy --attr='shard_1' $PAVER_ARGS
                 ;;
 
             "2")
-                paver test_bokchoy -n $NUMBER_OF_BOKCHOY_THREADS --attr='shard_2' --with-flaky
+                paver test_bokchoy --attr='shard_2' $PAVER_ARGS
                 ;;
 
             "3")
-                paver test_bokchoy -n $NUMBER_OF_BOKCHOY_THREADS --attr='shard_3' --with-flaky
+                paver test_bokchoy --attr='shard_3' $PAVER_ARGS
                 ;;
 
             "4")
-                paver test_bokchoy -n $NUMBER_OF_BOKCHOY_THREADS --attr='shard_4' --with-flaky
+                paver test_bokchoy --attr='shard_4' $PAVER_ARGS
                 ;;
 
             "5")
-                paver test_bokchoy -n $NUMBER_OF_BOKCHOY_THREADS --attr='shard_5' --with-flaky
+                paver test_bokchoy --attr='shard_5' $PAVER_ARGS
                 ;;
 
             "6")
-                paver test_bokchoy -n $NUMBER_OF_BOKCHOY_THREADS --attr='shard_6' --with-flaky
+                paver test_bokchoy --attr='shard_6' $PAVER_ARGS
                 ;;
 
             "7")
-                paver test_bokchoy -n $NUMBER_OF_BOKCHOY_THREADS --attr='shard_7' --with-flaky
+                paver test_bokchoy --attr='shard_7' $PAVER_ARGS
                 ;;
 
             "8")
-                paver test_bokchoy -n $NUMBER_OF_BOKCHOY_THREADS --attr='shard_8' --with-flaky
+                paver test_bokchoy --attr='shard_8' $PAVER_ARGS
                 ;;
 
             "9")
-                paver test_bokchoy -n $NUMBER_OF_BOKCHOY_THREADS --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' --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
diff --git a/setup.cfg b/setup.cfg
index 97aed7995b..3bf713d337 100644
--- a/setup.cfg
+++ b/setup.cfg
@@ -1,6 +1,5 @@
 [nosetests]
 logging-clear-handlers=1
-with-xunitmp=1
 with-ignore-docstrings=1
 exclude-dir=lms/envs
             cms/envs

From 81d347d93f162d4656c57abbf290389659a294ce Mon Sep 17 00:00:00 2001
From: Calen Pennington <cale@edx.org>
Date: Mon, 27 Jun 2016 14:20:32 -0400
Subject: [PATCH 6/7] Push defaults up to option definition to limit
 option-parsing repetitiveness

---
 pavelib/tests.py | 146 ++++++++++++++++++++++++++---------------------
 1 file changed, 81 insertions(+), 65 deletions(-)

diff --git a/pavelib/tests.py b/pavelib/tests.py
index f3a8861003..58b6e9154d 100644
--- a/pavelib/tests.py
+++ b/pavelib/tests.py
@@ -28,10 +28,13 @@ __test__ = False  # do not collect
     ("failed", "f", "Run only failed tests"),
     ("fail-fast", "x", "Fail suite on first failed test"),
     ("fasttest", "a", "Run without collectstatic"),
-    ('cov-args=', 'c', 'adds as args to coverage for the test run'),
+    make_option(
+        '-c', '--cov-args', default='',
+        help='adds as args to coverage for the test run'
+    ),
     ('skip-clean', 'C', 'skip cleaning repository before running tests'),
     ('processes=', 'p', 'number of processes to use running tests'),
-    make_option('-r', '--randomize', action='store_true', dest='randomize', help='run the tests in a random order'),
+    make_option('-r', '--randomize', action='store_true', help='run the tests in a random order'),
     make_option('--no-randomize', action='store_false', dest='randomize', help="don't run the tests in a random order"),
     make_option("--verbose", action="store_const", const=2, dest="verbosity"),
     make_option("-q", "--quiet", action="store_const", const=0, dest="verbosity"),
@@ -46,7 +49,10 @@ __test__ = False  # do not collect
     ("fail_fast", None, "deprecated in favor of fail-fast"),
     ("test_id=", None, "deprecated in favor of test-id"),
     ('cov_args=', None, 'deprecated in favor of cov-args'),
-    ('extra_args=', 'e', 'deprecated, pass extra options directly in the paver commandline'),
+    make_option(
+        "-e", "--extra_args", default="",
+        help="deprecated, pass extra options directly in the paver commandline"
+    ),
     ('skip_clean', None, 'deprecated in favor of skip-clean'),
 ], share_with=['pavelib.utils.test.utils.clean_reports_dir'])
 @PassthroughTask
@@ -57,36 +63,34 @@ def test_system(options, passthrough_options):
     system = getattr(options, 'system', None)
     test_id = getattr(options, 'test_id', None)
 
-    opts = {
-        'failed_only': getattr(options, 'failed', None),
-        'fail_fast': getattr(options, 'fail_fast', None),
-        'fasttest': getattr(options, 'fasttest', None),
-        'verbosity': getattr(options, 'verbosity', 1),
-        'extra_args': getattr(options, 'extra_args', ''),
-        'cov_args': getattr(options, 'cov_args', ''),
-        'skip_clean': getattr(options, 'skip_clean', False),
-        'pdb': getattr(options, 'pdb', False),
-        'disable_migrations': getattr(options, 'disable_migrations', False),
-        'processes': getattr(options, 'processes', None),
-        'randomize': getattr(options, 'randomize', None),
-        'passthrough_options': passthrough_options
-    }
-
     if test_id:
         if not system:
             system = test_id.split('/')[0]
         if system in ['common', 'openedx']:
             system = 'lms'
-        opts['test_id'] = test_id
+        options.test_system['test_id'] = test_id
 
     if test_id or system:
-        system_tests = [suites.SystemTestSuite(system, **opts)]
+        system_tests = [suites.SystemTestSuite(
+            system,
+            passthrough_options=passthrough_options,
+            **options.test_system
+        )]
     else:
         system_tests = []
         for syst in ('cms', 'lms'):
-            system_tests.append(suites.SystemTestSuite(syst, **opts))
+            system_tests.append(suites.SystemTestSuite(
+                syst,
+                passthrough_options=passthrough_options,
+                **options.test_system
+            ))
 
-    test_suite = suites.PythonTestSuite('python tests', subsuites=system_tests, **opts)
+    test_suite = suites.PythonTestSuite(
+        'python tests',
+        subsuites=system_tests,
+        passthrough_options=passthrough_options,
+        **options.test_system
+    )
     test_suite.run()
 
 
@@ -99,14 +103,20 @@ def test_system(options, passthrough_options):
     ("test-id=", "t", "Test id"),
     ("failed", "f", "Run only failed tests"),
     ("fail-fast", "x", "Run only failed tests"),
-    ('cov-args=', 'c', 'adds as args to coverage for the test run'),
+    make_option(
+        '-c', '--cov-args', default='',
+        help='adds as args to coverage for the test run'
+    ),
     ('skip-clean', 'C', 'skip cleaning repository before running 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", default=1),
     make_option("--pdb", action="store_true", help="Drop into debugger on failures or errors"),
     ('cov_args=', None, 'deprecated in favor of cov-args'),
-    ('extra_args=', 'e', 'deprecated, pass extra options directly in the paver commandline'),
+    make_option(
+        '-e', '--extra_args', default='',
+        help='deprecated, pass extra options directly in the paver commandline'
+    ),
     ("fail_fast", None, "deprecated in favor of fail-fast"),
     ('skip_clean', None, 'deprecated in favor of skip-clean'),
     ("test_id=", None, "deprecated in favor of test-id"),
@@ -119,28 +129,32 @@ def test_lib(options, passthrough_options):
     lib = getattr(options, 'lib', None)
     test_id = getattr(options, 'test_id', lib)
 
-    opts = {
-        'failed_only': getattr(options, 'failed', None),
-        'fail_fast': getattr(options, 'fail_fast', None),
-        'verbosity': getattr(options, 'verbosity', 1),
-        'extra_args': getattr(options, 'extra_args', ''),
-        'cov_args': getattr(options, 'cov_args', ''),
-        'skip_clean': getattr(options, 'skip_clean', False),
-        'pdb': getattr(options, 'pdb', False),
-        'passthrough_options': passthrough_options
-    }
-
     if test_id:
         if '/' in test_id:
             lib = '/'.join(test_id.split('/')[0:3])
         else:
             lib = 'common/lib/' + test_id.split('.')[0]
-        opts['test_id'] = test_id
-        lib_tests = [suites.LibTestSuite(lib, **opts)]
+        options.test_lib['test_id'] = test_id
+        lib_tests = [suites.LibTestSuite(
+            lib,
+            passthrough_options=passthrough_options,
+            **options.test_lib
+        )]
     else:
-        lib_tests = [suites.LibTestSuite(d, **opts) for d in Env.LIB_TEST_DIRS]
+        lib_tests = [
+            suites.LibTestSuite(
+                d,
+                passthrough_options=passthrough_options,
+                **options.test_lib
+            ) for d in Env.LIB_TEST_DIRS
+        ]
 
-    test_suite = suites.PythonTestSuite('python tests', subsuites=lib_tests, **opts)
+    test_suite = suites.PythonTestSuite(
+        'python tests',
+        subsuites=lib_tests,
+        passthrough_options=passthrough_options,
+        **options.test_lib
+    )
     test_suite.run()
 
 
@@ -151,7 +165,10 @@ def test_lib(options, passthrough_options):
 @cmdopts([
     ("failed", "f", "Run only failed tests"),
     ("fail-fast", "x", "Run only failed tests"),
-    ('cov-args=', 'c', 'adds as args to coverage for the test run'),
+    make_option(
+        '-c', '--cov-args', default='',
+        help='adds as args to coverage for the test run'
+    ),
     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", default=1),
@@ -163,7 +180,10 @@ def test_lib(options, passthrough_options):
         help="Create tables directly from apps' models. Can also be used by exporting DISABLE_MIGRATIONS=1."
     ),
     ('cov_args=', None, 'deprecated in favor of cov-args'),
-    ('extra_args=', 'e', 'deprecated, pass extra options directly in the paver commandline'),
+    make_option(
+        '-e', '--extra_args', default='',
+        help='deprecated, pass extra options directly in the paver commandline'
+    ),
     ("fail_fast", None, "deprecated in favor of fail-fast"),
 ])
 @PassthroughTask
@@ -171,18 +191,11 @@ def test_python(options, passthrough_options):
     """
     Run all python tests
     """
-    opts = {
-        'failed_only': getattr(options, 'failed', None),
-        'fail_fast': getattr(options, 'fail_fast', None),
-        'verbosity': getattr(options, 'verbosity', 1),
-        'extra_args': getattr(options, 'extra_args', ''),
-        'cov_args': getattr(options, 'cov_args', ''),
-        'pdb': getattr(options, 'pdb', False),
-        'disable_migrations': getattr(options, 'disable_migrations', False),
-        'passthrough_options': passthrough_options,
-    }
-
-    python_suite = suites.PythonTestSuite('Python Tests', **opts)
+    python_suite = suites.PythonTestSuite(
+        'Python Tests',
+        passthrough_options=passthrough_options,
+        **options.test_python
+    )
     python_suite.run()
 
 
@@ -192,28 +205,31 @@ def test_python(options, passthrough_options):
 )
 @cmdopts([
     ("suites", "s", "List of unit test suites to run. (js, lib, cms, lms)"),
-    ('cov-args=', 'c', 'adds as args to coverage for the test run'),
+    make_option(
+        '-c', '--cov-args', default='',
+        help='adds as args to coverage for the test run'
+    ),
     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", default=1),
     make_option("--pdb", action="store_true", help="Drop into debugger on failures or errors"),
     ('cov_args=', None, 'deprecated in favor of cov-args'),
-    ('extra_args=', 'e', 'deprecated, pass extra options directly in the paver commandline'),
+    make_option(
+        '-e', '--extra_args', default='',
+        help='deprecated, pass extra options directly in the paver commandline'
+    ),
 ])
 @PassthroughTask
 def test(options, passthrough_options):
     """
     Run all tests
     """
-    opts = {
-        'verbosity': getattr(options, 'verbosity', 1),
-        'extra_args': getattr(options, 'extra_args', ''),
-        'cov_args': getattr(options, 'cov_args', ''),
-        'pdb': getattr(options, 'pdb', False),
-        'passthrough_options': passthrough_options,
-    }
     # Subsuites to be added to the main suite
-    python_suite = suites.PythonTestSuite('Python Tests', **opts)
+    python_suite = suites.PythonTestSuite(
+        'Python Tests',
+        passthrough_options=passthrough_options,
+        **options.test
+    )
     js_suite = suites.JsTestSuite('JS Tests', mode='run', with_coverage=True)
 
     # Main suite to be run
@@ -255,7 +271,7 @@ def coverage(options):
     sh("coverage xml --rcfile={}".format(rcfile))
     # Generate the coverage.py HTML report
     sh("coverage html --rcfile={}".format(rcfile))
-    call_task('diff_coverage', options=dict(options))
+    call_task('diff_coverage', options=options.coverage)
 
 
 @task
@@ -268,7 +284,7 @@ def diff_coverage(options):
     """
     Build the diff coverage reports
     """
-    compare_branch = getattr(options, 'compare_branch', 'origin/master')
+    compare_branch = options.diff_coverage.get('compare_branch', 'origin/master')
 
     # Find all coverage XML files (both Python and JavaScript)
     xml_reports = []

From 0e90567f37e961d11131dce96b4448fc0bfb2cdc Mon Sep 17 00:00:00 2001
From: Calen Pennington <cale@edx.org>
Date: Tue, 28 Jun 2016 09:53:25 -0400
Subject: [PATCH 7/7] Remove the --pdb option, because it was just a
 passthrough to nose

---
 pavelib/acceptance_test.py                    | 1 -
 pavelib/bok_choy.py                           | 1 -
 pavelib/tests.py                              | 4 ----
 pavelib/utils/test/suites/acceptance_suite.py | 1 -
 pavelib/utils/test/suites/bokchoy_suite.py    | 2 --
 pavelib/utils/test/suites/nose_suite.py       | 3 ---
 6 files changed, 12 deletions(-)

diff --git a/pavelib/acceptance_test.py b/pavelib/acceptance_test.py
index e5fc548053..271c1e6c89 100644
--- a/pavelib/acceptance_test.py
+++ b/pavelib/acceptance_test.py
@@ -25,7 +25,6 @@ __test__ = False  # do not collect
     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'),
 ])
diff --git a/pavelib/bok_choy.py b/pavelib/bok_choy.py
index 7d663f9e7a..a8d5048e8f 100644
--- a/pavelib/bok_choy.py
+++ b/pavelib/bok_choy.py
@@ -31,7 +31,6 @@ BOKCHOY_OPTS = [
     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"),
     ('default_store=', None, 'deprecated in favor of default-store'),
diff --git a/pavelib/tests.py b/pavelib/tests.py
index 58b6e9154d..c56f247dce 100644
--- a/pavelib/tests.py
+++ b/pavelib/tests.py
@@ -39,7 +39,6 @@ __test__ = False  # do not collect
     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", default=1),
-    make_option("--pdb", action="store_true", help="Drop into debugger on failures or errors"),
     make_option(
         '--disable-migrations',
         action='store_true',
@@ -111,7 +110,6 @@ def test_system(options, passthrough_options):
     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", default=1),
-    make_option("--pdb", action="store_true", help="Drop into debugger on failures or errors"),
     ('cov_args=', None, 'deprecated in favor of cov-args'),
     make_option(
         '-e', '--extra_args', default='',
@@ -172,7 +170,6 @@ def test_lib(options, passthrough_options):
     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", default=1),
-    make_option("--pdb", action="store_true", help="Drop into debugger on failures or errors"),
     make_option(
         '--disable-migrations',
         action='store_true',
@@ -212,7 +209,6 @@ def test_python(options, passthrough_options):
     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", default=1),
-    make_option("--pdb", action="store_true", help="Drop into debugger on failures or errors"),
     ('cov_args=', None, 'deprecated in favor of cov-args'),
     make_option(
         '-e', '--extra_args', default='',
diff --git a/pavelib/utils/test/suites/acceptance_suite.py b/pavelib/utils/test/suites/acceptance_suite.py
index d87d7b6898..e21f6088bd 100644
--- a/pavelib/utils/test/suites/acceptance_suite.py
+++ b/pavelib/utils/test/suites/acceptance_suite.py
@@ -44,7 +44,6 @@ class AcceptanceTest(TestSuite):
             "--traceback",
             "--debug-mode",
             "--verbosity={}".format(self.verbosity),
-            "--pdb" if self.pdb else "",
         ] + report_args + [
             self.extra_args
         ] + self.passthrough_options
diff --git a/pavelib/utils/test/suites/bokchoy_suite.py b/pavelib/utils/test/suites/bokchoy_suite.py
index bda6098ace..4929b589ea 100644
--- a/pavelib/utils/test/suites/bokchoy_suite.py
+++ b/pavelib/utils/test/suites/bokchoy_suite.py
@@ -250,8 +250,6 @@ class BokChoyTestSuite(TestSuite):
             "nosetests",
             test_spec,
         ] + self.verbosity_processes_command
-        if self.pdb:
-            cmd.append("--pdb")
         if self.save_screenshots:
             cmd.append("--with-save-baseline")
         if self.extra_args:
diff --git a/pavelib/utils/test/suites/nose_suite.py b/pavelib/utils/test/suites/nose_suite.py
index cef5fcfa11..3a1cc4e892 100644
--- a/pavelib/utils/test/suites/nose_suite.py
+++ b/pavelib/utils/test/suites/nose_suite.py
@@ -102,9 +102,6 @@ class NoseTestSuite(TestSuite):
         if self.fail_fast or env_fail_fast_set:
             opts.append("--stop")
 
-        if self.pdb:
-            opts += " --pdb"
-
         if self.use_ids:
             opts.append("--with-id")