Deprecate extra_args in favor of just passing through all unknown options

pavelib/acceptance_test.py

 """
 Acceptance test tasks
 """
-from paver.easy import task, cmdopts, needs
+from paver.easy import cmdopts, needs
 from pavelib.utils.test.suites import AcceptanceTestSuite
+from pavelib.utils.passthrough_opts import PassthroughTask
 from optparse import make_option
 
 try:
@@ -13,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']:

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'])

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)

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

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
             )
         )
 

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

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
             )
         )
 

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):
         """

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"
             ;;
 
         *)

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