From 36714be874fa9ba7c0229d97607906669615e409 Mon Sep 17 00:00:00 2001 From: Mariusz Felisiak Date: Wed, 4 Aug 2021 10:49:30 +0200 Subject: [PATCH] Refs #31621 -- Fixed handling --parallel option in test management command and runtests.py. Regression in ae89daf46f83a7b39d599d289624c3377bfa4ab1. Thanks Tim Graham for the report. --- django/core/management/commands/test.py | 4 +++ django/test/runner.py | 22 +++++++----- docs/ref/django-admin.txt | 5 ++- tests/runtests.py | 42 +++++++++++------------ tests/test_runner/test_discover_runner.py | 28 +++++++-------- tests/test_runner/tests.py | 18 ++++++++++ 6 files changed, 72 insertions(+), 47 deletions(-) diff --git a/django/core/management/commands/test.py b/django/core/management/commands/test.py index 35e7b73871..7a76033424 100644 --- a/django/core/management/commands/test.py +++ b/django/core/management/commands/test.py @@ -3,6 +3,7 @@ import sys from django.conf import settings from django.core.management.base import BaseCommand from django.core.management.utils import get_command_line_option +from django.test.runner import get_max_test_processes from django.test.utils import NullTimeKeeper, TimeKeeper, get_runner @@ -50,6 +51,9 @@ class Command(BaseCommand): TestRunner = get_runner(settings, options['testrunner']) time_keeper = TimeKeeper() if options.get('timing', False) else NullTimeKeeper() + parallel = options.get('parallel') + if parallel == 'auto': + options['parallel'] = get_max_test_processes() test_runner = TestRunner(**options) with time_keeper.timed('Total run'): failures = test_runner.run_tests(test_labels) diff --git a/django/test/runner.py b/django/test/runner.py index 878e62b1a8..eb5026b212 100644 --- a/django/test/runner.py +++ b/django/test/runner.py @@ -336,14 +336,24 @@ class RemoteTestRunner: return result -def parallel_type(value): - """Parse value passed to the --parallel option.""" +def get_max_test_processes(): + """ + The maximum number of test processes when using the --parallel option. + """ # The current implementation of the parallel test runner requires # multiprocessing to start subprocesses with fork(). if multiprocessing.get_start_method() != 'fork': return 1 - if value == 'auto': + try: + return int(os.environ['DJANGO_TEST_PROCESSES']) + except KeyError: return multiprocessing.cpu_count() + + +def parallel_type(value): + """Parse value passed to the --parallel option.""" + if value == 'auto': + return value try: return int(value) except ValueError: @@ -616,12 +626,8 @@ class DiscoverRunner: '-d', '--debug-sql', action='store_true', help='Prints logged SQL queries on failure.', ) - try: - default_parallel = int(os.environ['DJANGO_TEST_PROCESSES']) - except KeyError: - default_parallel = 0 parser.add_argument( - '--parallel', nargs='?', const='auto', default=default_parallel, + '--parallel', nargs='?', const='auto', default=0, type=parallel_type, metavar='N', help=( 'Run tests using up to N parallel processes. Use the value ' diff --git a/docs/ref/django-admin.txt b/docs/ref/django-admin.txt index 0a346b5af7..c9618aa5fb 100644 --- a/docs/ref/django-admin.txt +++ b/docs/ref/django-admin.txt @@ -1470,9 +1470,8 @@ multiple cores, this allows running tests significantly faster. Using ``--parallel`` without a value, or with the value ``auto``, runs one test process per core according to :func:`multiprocessing.cpu_count()`. You can override this by passing the desired number of processes, e.g. -``--parallel 4``. You can also enable ``--parallel`` without passing the flag -by setting the :envvar:`DJANGO_TEST_PROCESSES` environment variable to the -desired number of processes. +``--parallel 4``, or by setting the :envvar:`DJANGO_TEST_PROCESSES` environment +variable. Django distributes test cases — :class:`unittest.TestCase` subclasses — to subprocesses. If there are fewer test cases than configured processes, Django diff --git a/tests/runtests.py b/tests/runtests.py index dfbc70818c..f922bcd44d 100755 --- a/tests/runtests.py +++ b/tests/runtests.py @@ -23,7 +23,7 @@ else: from django.conf import settings from django.db import connection, connections from django.test import TestCase, TransactionTestCase - from django.test.runner import parallel_type + from django.test.runner import get_max_test_processes, parallel_type from django.test.selenium import SeleniumTestCaseBase from django.test.utils import NullTimeKeeper, TimeKeeper, get_runner from django.utils.deprecation import ( @@ -325,17 +325,6 @@ def teardown_run_tests(state): del os.environ['RUNNING_DJANGOS_TEST_SUITE'] -def actual_test_processes(parallel): - if parallel == 0: - # This doesn't work before django.setup() on some databases. - if all(conn.features.can_clone_databases for conn in connections.all()): - return parallel_type('auto') - else: - return 1 - else: - return parallel - - class ActionSelenium(argparse.Action): """ Validate the comma-separated list of requested browsers. @@ -354,18 +343,29 @@ def django_tests(verbosity, interactive, failfast, keepdb, reverse, test_labels, debug_sql, parallel, tags, exclude_tags, test_name_patterns, start_at, start_after, pdb, buffer, timing, shuffle): - actual_parallel = actual_test_processes(parallel) + if parallel in {0, 'auto'}: + max_parallel = get_max_test_processes() + else: + max_parallel = parallel if verbosity >= 1: msg = "Testing against Django installed in '%s'" % os.path.dirname(django.__file__) - if actual_parallel > 1: - msg += " with up to %d processes" % actual_parallel + if max_parallel > 1: + msg += " with up to %d processes" % max_parallel print(msg) test_labels, state = setup_run_tests(verbosity, start_at, start_after, test_labels) # Run the test suite, including the extra validation tests. if not hasattr(settings, 'TEST_RUNNER'): settings.TEST_RUNNER = 'django.test.runner.DiscoverRunner' + + if parallel in {0, 'auto'}: + # This doesn't work before django.setup() on some databases. + if all(conn.features.can_clone_databases for conn in connections.all()): + parallel = max_parallel + else: + parallel = 1 + TestRunner = get_runner(settings) test_runner = TestRunner( verbosity=verbosity, @@ -374,7 +374,7 @@ def django_tests(verbosity, interactive, failfast, keepdb, reverse, keepdb=keepdb, reverse=reverse, debug_sql=debug_sql, - parallel=actual_parallel, + parallel=parallel, tags=tags, exclude_tags=exclude_tags, test_name_patterns=test_name_patterns, @@ -563,13 +563,11 @@ if __name__ == "__main__": '--debug-sql', action='store_true', help='Turn on the SQL query logger within tests.', ) - try: - default_parallel = int(os.environ['DJANGO_TEST_PROCESSES']) - except KeyError: - # actual_test_processes() converts this to "auto" later on. - default_parallel = 0 + # 0 is converted to "auto" or 1 later on, depending on a method used by + # multiprocessing to start subprocesses and on the backend support for + # cloning databases. parser.add_argument( - '--parallel', nargs='?', const='auto', default=default_parallel, + '--parallel', nargs='?', const='auto', default=0, type=parallel_type, metavar='N', help=( 'Run tests using up to N parallel processes. Use the value "auto" ' diff --git a/tests/test_runner/test_discover_runner.py b/tests/test_runner/test_discover_runner.py index b73c1581e3..625de94067 100644 --- a/tests/test_runner/test_discover_runner.py +++ b/tests/test_runner/test_discover_runner.py @@ -9,7 +9,7 @@ from unittest import TestSuite, TextTestRunner, defaultTestLoader, mock from django.db import connections from django.test import SimpleTestCase -from django.test.runner import DiscoverRunner +from django.test.runner import DiscoverRunner, get_max_test_processes from django.test.utils import ( NullTimeKeeper, TimeKeeper, captured_stderr, captured_stdout, ) @@ -54,11 +54,11 @@ class DiscoverRunnerParallelArgumentTests(SimpleTestCase): def test_parallel_flag(self, *mocked_objects): result = self.get_parser().parse_args(['--parallel']) - self.assertEqual(result.parallel, 12) + self.assertEqual(result.parallel, 'auto') def test_parallel_auto(self, *mocked_objects): result = self.get_parser().parse_args(['--parallel', 'auto']) - self.assertEqual(result.parallel, 12) + self.assertEqual(result.parallel, 'auto') def test_parallel_count(self, *mocked_objects): result = self.get_parser().parse_args(['--parallel', '17']) @@ -70,20 +70,20 @@ class DiscoverRunnerParallelArgumentTests(SimpleTestCase): msg = "argument --parallel: 'unaccepted' is not an integer or the string 'auto'" self.assertIn(msg, stderr.getvalue()) + def test_get_max_test_processes(self, *mocked_objects): + self.assertEqual(get_max_test_processes(), 12) + @mock.patch.dict(os.environ, {'DJANGO_TEST_PROCESSES': '7'}) - def test_parallel_env_var(self, *mocked_objects): - result = self.get_parser().parse_args([]) - self.assertEqual(result.parallel, 7) + def test_get_max_test_processes_env_var(self, *mocked_objects): + self.assertEqual(get_max_test_processes(), 7) - @mock.patch.dict(os.environ, {'DJANGO_TEST_PROCESSES': 'typo'}) - def test_parallel_env_var_non_int(self, *mocked_objects): - with self.assertRaises(ValueError): - self.get_parser().parse_args([]) - - def test_parallel_spawn(self, mocked_get_start_method, mocked_cpu_count): + def test_get_max_test_processes_spawn( + self, mocked_get_start_method, mocked_cpu_count, + ): mocked_get_start_method.return_value = 'spawn' - result = self.get_parser().parse_args(['--parallel']) - self.assertEqual(result.parallel, 1) + self.assertEqual(get_max_test_processes(), 1) + with mock.patch.dict(os.environ, {'DJANGO_TEST_PROCESSES': '7'}): + self.assertEqual(get_max_test_processes(), 1) class DiscoverRunnerTests(SimpleTestCase): diff --git a/tests/test_runner/tests.py b/tests/test_runner/tests.py index 485a35be9c..62b489abc7 100644 --- a/tests/test_runner/tests.py +++ b/tests/test_runner/tests.py @@ -434,6 +434,12 @@ class ManageCommandParallelTests(SimpleTestCase): ) self.assertEqual(stderr.getvalue(), '') + @mock.patch.dict(os.environ, {'DJANGO_TEST_PROCESSES': '7'}) + def test_no_parallel_django_test_processes_env(self, *mocked_objects): + with captured_stderr() as stderr: + call_command('test', testrunner='test_runner.tests.MockTestRunner') + self.assertEqual(stderr.getvalue(), '') + @mock.patch.dict(os.environ, {'DJANGO_TEST_PROCESSES': 'invalid'}) def test_django_test_processes_env_non_int(self, *mocked_objects): with self.assertRaises(ValueError): @@ -443,6 +449,18 @@ class ManageCommandParallelTests(SimpleTestCase): testrunner='test_runner.tests.MockTestRunner', ) + @mock.patch.dict(os.environ, {'DJANGO_TEST_PROCESSES': '7'}) + def test_django_test_processes_parallel_default(self, *mocked_objects): + for parallel in ['--parallel', '--parallel=auto']: + with self.subTest(parallel=parallel): + with captured_stderr() as stderr: + call_command( + 'test', + parallel, + testrunner='test_runner.tests.MockTestRunner', + ) + self.assertIn('parallel=7', stderr.getvalue()) + class CustomTestRunnerOptionsSettingsTests(AdminScriptTestCase): """