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)