From 7173afc93eee051c7b4296b718a36cb2712f7f4a Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Mon, 12 Jun 2017 12:12:24 +0200 Subject: [PATCH 1/4] bpo-30635: Fix refleak in test_c_locale_coercion When checking for reference leaks, test_c_locale_coercion is run multiple times and so _LocaleCoercionTargetsTestCase.setUpClass() is called multiple times. setUpClass() appends new value at each call. Use _LocaleCoercionTargetsTestCase.available_targets as a marker to check if the class is already initialized or not. --- Lib/test/test_c_locale_coercion.py | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/Lib/test/test_c_locale_coercion.py b/Lib/test/test_c_locale_coercion.py index c14d820a2d7931..363d1d7e992ec5 100644 --- a/Lib/test/test_c_locale_coercion.py +++ b/Lib/test/test_c_locale_coercion.py @@ -146,20 +146,24 @@ def test_library_c_locale_warning(self): class _LocaleCoercionTargetsTestCase(_ChildProcessEncodingTestCase): # Base class for test cases that rely on coercion targets being defined - available_targets = [] + available_targets = None targets_required = True @classmethod def setUpClass(cls): + if cls.available_targets is not None: + # initialization already done + return + cls.available_targets = [] + first_target_locale = None - available_targets = cls.available_targets # Find the target locales available in the current system for target_locale in _C_UTF8_LOCALES: if _set_locale_in_subprocess(target_locale): - available_targets.append(target_locale) + cls.available_targets.append(target_locale) if first_target_locale is None: first_target_locale = target_locale - if cls.targets_required and not available_targets: + if cls.targets_required and not cls.available_targets: raise unittest.SkipTest("No C-with-UTF-8 locale available") # Expect coercion to use the first available locale warning_msg = CLI_COERCION_WARNING_FMT.format(first_target_locale) From 40a8f4bb977a9a1d0ba041096a531483bfb699f2 Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Mon, 12 Jun 2017 12:14:25 +0200 Subject: [PATCH 2/4] test_c_locale_coercion: remove unused targets_required --- Lib/test/test_c_locale_coercion.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/Lib/test/test_c_locale_coercion.py b/Lib/test/test_c_locale_coercion.py index 363d1d7e992ec5..8a91f7c6649496 100644 --- a/Lib/test/test_c_locale_coercion.py +++ b/Lib/test/test_c_locale_coercion.py @@ -147,7 +147,6 @@ class _LocaleCoercionTargetsTestCase(_ChildProcessEncodingTestCase): # Base class for test cases that rely on coercion targets being defined available_targets = None - targets_required = True @classmethod def setUpClass(cls): @@ -163,7 +162,7 @@ def setUpClass(cls): cls.available_targets.append(target_locale) if first_target_locale is None: first_target_locale = target_locale - if cls.targets_required and not cls.available_targets: + if not cls.available_targets: raise unittest.SkipTest("No C-with-UTF-8 locale available") # Expect coercion to use the first available locale warning_msg = CLI_COERCION_WARNING_FMT.format(first_target_locale) From 7ab9253be5e6ff58c57c002b45b519388def72eb Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Mon, 12 Jun 2017 12:16:49 +0200 Subject: [PATCH 3/4] test_c_locale_coercion: inline EXPECTED_COERCION_WARNING --- Lib/test/test_c_locale_coercion.py | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/Lib/test/test_c_locale_coercion.py b/Lib/test/test_c_locale_coercion.py index 8a91f7c6649496..2e628803f6a006 100644 --- a/Lib/test/test_c_locale_coercion.py +++ b/Lib/test/test_c_locale_coercion.py @@ -155,18 +155,12 @@ def setUpClass(cls): return cls.available_targets = [] - first_target_locale = None # Find the target locales available in the current system for target_locale in _C_UTF8_LOCALES: if _set_locale_in_subprocess(target_locale): cls.available_targets.append(target_locale) - if first_target_locale is None: - first_target_locale = target_locale if not cls.available_targets: raise unittest.SkipTest("No C-with-UTF-8 locale available") - # Expect coercion to use the first available locale - warning_msg = CLI_COERCION_WARNING_FMT.format(first_target_locale) - cls.EXPECTED_COERCION_WARNING = warning_msg class LocaleConfigurationTests(_LocaleCoercionTargetsTestCase): @@ -218,7 +212,9 @@ def _check_c_locale_coercion(self, expected_fsencoding, coerce_c_locale): expected_warning = [] if coerce_c_locale != "0": - expected_warning.append(self.EXPECTED_COERCION_WARNING) + # Expect coercion to use the first available locale + warning_msg = CLI_COERCION_WARNING_FMT.format(self.available_targets[0]) + expected_warning.append(warning_msg) base_var_dict = { "LANG": "", From ec744867ab602227e09e0970755d6191fa17dae7 Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Mon, 12 Jun 2017 12:20:31 +0200 Subject: [PATCH 4/4] test_c_locale_coercion: only create AVAILABLE_TARGETS once Convert setUpClass() to setUpModule() to only compute AVAILABLE_TARGETS once to spawn less processes. Speed up test_c_locale_coercion: 3.2 sec => 2.9 sec. --- Lib/test/test_c_locale_coercion.py | 39 +++++++++++++++++------------- 1 file changed, 22 insertions(+), 17 deletions(-) diff --git a/Lib/test/test_c_locale_coercion.py b/Lib/test/test_c_locale_coercion.py index 2e628803f6a006..f537f1d22aa47d 100644 --- a/Lib/test/test_c_locale_coercion.py +++ b/Lib/test/test_c_locale_coercion.py @@ -143,24 +143,29 @@ def test_library_c_locale_warning(self): "or PYTHONCOERCECLOCALE=0 to disable this locale coercion behavior)." ) -class _LocaleCoercionTargetsTestCase(_ChildProcessEncodingTestCase): - # Base class for test cases that rely on coercion targets being defined - available_targets = None +AVAILABLE_TARGETS = None - @classmethod - def setUpClass(cls): - if cls.available_targets is not None: - # initialization already done - return - cls.available_targets = [] +def setUpModule(): + global AVAILABLE_TARGETS + + if AVAILABLE_TARGETS is not None: + # initialization already done + return + AVAILABLE_TARGETS = [] + + # Find the target locales available in the current system + for target_locale in _C_UTF8_LOCALES: + if _set_locale_in_subprocess(target_locale): + AVAILABLE_TARGETS.append(target_locale) + if not AVAILABLE_TARGETS: + raise unittest.SkipTest("No C-with-UTF-8 locale available") - # Find the target locales available in the current system - for target_locale in _C_UTF8_LOCALES: - if _set_locale_in_subprocess(target_locale): - cls.available_targets.append(target_locale) - if not cls.available_targets: - raise unittest.SkipTest("No C-with-UTF-8 locale available") + + +class _LocaleCoercionTargetsTestCase(_ChildProcessEncodingTestCase): + # Base class for test cases that rely on coercion targets being defined + pass class LocaleConfigurationTests(_LocaleCoercionTargetsTestCase): @@ -180,7 +185,7 @@ def test_external_target_locale_configuration(self): "LC_ALL": "", } for env_var in ("LANG", "LC_CTYPE"): - for locale_to_set in self.available_targets: + for locale_to_set in AVAILABLE_TARGETS: with self.subTest(env_var=env_var, configured_locale=locale_to_set): var_dict = base_var_dict.copy() @@ -213,7 +218,7 @@ def _check_c_locale_coercion(self, expected_fsencoding, coerce_c_locale): expected_warning = [] if coerce_c_locale != "0": # Expect coercion to use the first available locale - warning_msg = CLI_COERCION_WARNING_FMT.format(self.available_targets[0]) + warning_msg = CLI_COERCION_WARNING_FMT.format(AVAILABLE_TARGETS[0]) expected_warning.append(warning_msg) base_var_dict = {