From 50243c0edd569fb0f7e9751160f1690e3c59cca6 Mon Sep 17 00:00:00 2001 From: sobolevn Date: Sat, 24 Dec 2022 12:51:56 +0300 Subject: [PATCH 1/4] gh-84867: Do not load tests from `TestCase` and `FunctionTestCase` --- Lib/test/test_unittest/test_loader.py | 56 +++++++++++++++++++ Lib/unittest/case.py | 10 ++++ Lib/unittest/loader.py | 25 +++++++-- ...2-12-24-12-50-54.gh-issue-84867.OhaLbU.rst | 2 + 4 files changed, 88 insertions(+), 5 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2022-12-24-12-50-54.gh-issue-84867.OhaLbU.rst diff --git a/Lib/test/test_unittest/test_loader.py b/Lib/test/test_unittest/test_loader.py index bbdfb247ebaada..61f4f9f56577bf 100644 --- a/Lib/test/test_unittest/test_loader.py +++ b/Lib/test/test_unittest/test_loader.py @@ -83,6 +83,22 @@ def runTest(self): self.assertIsInstance(suite, loader.suiteClass) self.assertEqual(list(suite), [Foo('runTest')]) + # "Do not load any tests from `TestCase` class itself." + def test_loadTestsFromTestCase__from_TestCase(self): + loader = unittest.TestLoader() + + suite = loader.loadTestsFromTestCase(unittest.TestCase) + self.assertIsInstance(suite, loader.suiteClass) + self.assertEqual(list(suite), []) + + # "Do not load any tests from `FunctionTestCase` class." + def test_loadTestsFromTestCase__from_FunctionTestCase(self): + loader = unittest.TestLoader() + + suite = loader.loadTestsFromTestCase(unittest.FunctionTestCase) + self.assertIsInstance(suite, loader.suiteClass) + self.assertEqual(list(suite), []) + ################################################################ ### /Tests for TestLoader.loadTestsFromTestCase @@ -104,6 +120,46 @@ def test(self): expected = [loader.suiteClass([MyTestCase('test')])] self.assertEqual(list(suite), expected) + # "This test ensures that internal `TestCase` subclasses are not loaded" + def test_loadTestsFromModule__TestCase_subclass_internals(self): + # See https://github.com/python/cpython/issues/84867 + m = types.ModuleType('m') + # Simulate imported names: + m.TestCase = unittest.TestCase + m.FunctionTestCase = unittest.FunctionTestCase + + loader = unittest.TestLoader() + suite = loader.loadTestsFromModule(m) + self.assertIsInstance(suite, loader.suiteClass) + + expected = [] + self.assertEqual(list(suite), expected) + + # "This test ensures that subclasses of internal type are still loaded." + def test_loadTestsFromModule__TestCase_subclass_custom(self): + class MyBaseTestCase(unittest.TestCase): + # Base class for some 3rd-party library that should not be loaded. + @classmethod + def _shouldBeLoaded(cls): + if not super()._shouldBeLoaded(): + return False + return cls != MyBaseTestCase + + class MyTestCase(MyBaseTestCase): + def test_my(self): + pass + + m = types.ModuleType('m') + m.MyBaseTestCase = MyBaseTestCase + m.MyTestCase = MyTestCase + + loader = unittest.TestLoader() + suite = loader.loadTestsFromModule(m) + self.assertIsInstance(suite, loader.suiteClass) + + expected = [loader.suiteClass([MyTestCase('test_my')])] + self.assertEqual(list(suite), expected) + # "This method searches `module` for classes derived from TestCase" # # What happens if no tests are found (no TestCase instances)? diff --git a/Lib/unittest/case.py b/Lib/unittest/case.py index 5167c5f843f085..25e03e567de7be 100644 --- a/Lib/unittest/case.py +++ b/Lib/unittest/case.py @@ -464,6 +464,16 @@ def enterClassContext(cls, cm): """Same as enterContext, but class-wide.""" return _enter_context(cm, cls.addClassCleanup) + @classmethod + def _shouldBeLoaded(cls): + """Should the subclass of `TestCase` be loaded? + + We skip known unittest subclasses.""" + return ( + issubclass(cls, TestCase) + and cls not in (TestCase, FunctionTestCase) + ) + def setUp(self): "Hook method for setting up the test fixture before exercising it." pass diff --git a/Lib/unittest/loader.py b/Lib/unittest/loader.py index 80d4fbdd8e3606..b68ac6e3c1f85c 100644 --- a/Lib/unittest/loader.py +++ b/Lib/unittest/loader.py @@ -85,9 +85,16 @@ def loadTestsFromTestCase(self, testCaseClass): raise TypeError("Test cases should not be derived from " "TestSuite. Maybe you meant to derive from " "TestCase?") - testCaseNames = self.getTestCaseNames(testCaseClass) - if not testCaseNames and hasattr(testCaseClass, 'runTest'): - testCaseNames = ['runTest'] + if ( + issubclass(testCaseClass, case.TestCase) + and not testCaseClass._shouldBeLoaded() + ): + # We don't load any tests from base types that should not be loaded. + testCaseNames = [] + else: + testCaseNames = self.getTestCaseNames(testCaseClass) + if not testCaseNames and hasattr(testCaseClass, 'runTest'): + testCaseNames = ['runTest'] loaded_suite = self.suiteClass(map(testCaseClass, testCaseNames)) return loaded_suite @@ -96,7 +103,11 @@ def loadTestsFromModule(self, module, *, pattern=None): tests = [] for name in dir(module): obj = getattr(module, name) - if isinstance(obj, type) and issubclass(obj, case.TestCase): + if ( + isinstance(obj, type) + and issubclass(obj, case.TestCase) + and obj._shouldBeLoaded() + ): tests.append(self.loadTestsFromTestCase(obj)) load_tests = getattr(module, 'load_tests', None) @@ -165,7 +176,11 @@ def loadTestsFromName(self, name, module=None): if isinstance(obj, types.ModuleType): return self.loadTestsFromModule(obj) - elif isinstance(obj, type) and issubclass(obj, case.TestCase): + elif ( + isinstance(obj, type) + and issubclass(obj, case.TestCase) + and obj._shouldBeLoaded() + ): return self.loadTestsFromTestCase(obj) elif (isinstance(obj, types.FunctionType) and isinstance(parent, type) and diff --git a/Misc/NEWS.d/next/Library/2022-12-24-12-50-54.gh-issue-84867.OhaLbU.rst b/Misc/NEWS.d/next/Library/2022-12-24-12-50-54.gh-issue-84867.OhaLbU.rst new file mode 100644 index 00000000000000..f581315fc467fe --- /dev/null +++ b/Misc/NEWS.d/next/Library/2022-12-24-12-50-54.gh-issue-84867.OhaLbU.rst @@ -0,0 +1,2 @@ +Do not load test cases from exact :class:`unittest.case.TestCase` and +:class:`unittest.case.FunctionTestCase` classes. From 22bdd120600faa89a559400deab9a51ff36da03e Mon Sep 17 00:00:00 2001 From: sobolevn Date: Mon, 11 Sep 2023 14:38:20 +0300 Subject: [PATCH 2/4] Address review --- Lib/test/test_unittest/test_loader.py | 29 +-------------------------- Lib/unittest/case.py | 10 --------- Lib/unittest/loader.py | 6 +++--- 3 files changed, 4 insertions(+), 41 deletions(-) diff --git a/Lib/test/test_unittest/test_loader.py b/Lib/test/test_unittest/test_loader.py index 61f4f9f56577bf..1cb6ae759c1a17 100644 --- a/Lib/test/test_unittest/test_loader.py +++ b/Lib/test/test_unittest/test_loader.py @@ -131,34 +131,7 @@ def test_loadTestsFromModule__TestCase_subclass_internals(self): loader = unittest.TestLoader() suite = loader.loadTestsFromModule(m) self.assertIsInstance(suite, loader.suiteClass) - - expected = [] - self.assertEqual(list(suite), expected) - - # "This test ensures that subclasses of internal type are still loaded." - def test_loadTestsFromModule__TestCase_subclass_custom(self): - class MyBaseTestCase(unittest.TestCase): - # Base class for some 3rd-party library that should not be loaded. - @classmethod - def _shouldBeLoaded(cls): - if not super()._shouldBeLoaded(): - return False - return cls != MyBaseTestCase - - class MyTestCase(MyBaseTestCase): - def test_my(self): - pass - - m = types.ModuleType('m') - m.MyBaseTestCase = MyBaseTestCase - m.MyTestCase = MyTestCase - - loader = unittest.TestLoader() - suite = loader.loadTestsFromModule(m) - self.assertIsInstance(suite, loader.suiteClass) - - expected = [loader.suiteClass([MyTestCase('test_my')])] - self.assertEqual(list(suite), expected) + self.assertEqual(list(suite), []) # "This method searches `module` for classes derived from TestCase" # diff --git a/Lib/unittest/case.py b/Lib/unittest/case.py index 25e03e567de7be..5167c5f843f085 100644 --- a/Lib/unittest/case.py +++ b/Lib/unittest/case.py @@ -464,16 +464,6 @@ def enterClassContext(cls, cm): """Same as enterContext, but class-wide.""" return _enter_context(cm, cls.addClassCleanup) - @classmethod - def _shouldBeLoaded(cls): - """Should the subclass of `TestCase` be loaded? - - We skip known unittest subclasses.""" - return ( - issubclass(cls, TestCase) - and cls not in (TestCase, FunctionTestCase) - ) - def setUp(self): "Hook method for setting up the test fixture before exercising it." pass diff --git a/Lib/unittest/loader.py b/Lib/unittest/loader.py index b68ac6e3c1f85c..1130ec44935dc8 100644 --- a/Lib/unittest/loader.py +++ b/Lib/unittest/loader.py @@ -87,7 +87,7 @@ def loadTestsFromTestCase(self, testCaseClass): "TestCase?") if ( issubclass(testCaseClass, case.TestCase) - and not testCaseClass._shouldBeLoaded() + and cls in (case.TestCase, case.FunctionTestCase) ): # We don't load any tests from base types that should not be loaded. testCaseNames = [] @@ -106,7 +106,7 @@ def loadTestsFromModule(self, module, *, pattern=None): if ( isinstance(obj, type) and issubclass(obj, case.TestCase) - and obj._shouldBeLoaded() + and cls not in (case.TestCase, case.FunctionTestCase) ): tests.append(self.loadTestsFromTestCase(obj)) @@ -179,7 +179,7 @@ def loadTestsFromName(self, name, module=None): elif ( isinstance(obj, type) and issubclass(obj, case.TestCase) - and obj._shouldBeLoaded() + and cls not in (case.TestCase, case.FunctionTestCase) ): return self.loadTestsFromTestCase(obj) elif (isinstance(obj, types.FunctionType) and From ee614ef047fea8ea188f83ee90834bfa89472a0a Mon Sep 17 00:00:00 2001 From: sobolevn Date: Mon, 11 Sep 2023 14:58:21 +0300 Subject: [PATCH 3/4] Fix typo --- Lib/unittest/loader.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Lib/unittest/loader.py b/Lib/unittest/loader.py index 818b1369722390..d7ae4be6076df1 100644 --- a/Lib/unittest/loader.py +++ b/Lib/unittest/loader.py @@ -86,7 +86,7 @@ def loadTestsFromTestCase(self, testCaseClass): "TestCase?") if ( issubclass(testCaseClass, case.TestCase) - and cls in (case.TestCase, case.FunctionTestCase) + and testCaseClass in (case.TestCase, case.FunctionTestCase) ): # We don't load any tests from base types that should not be loaded. testCaseNames = [] @@ -105,7 +105,7 @@ def loadTestsFromModule(self, module, *, pattern=None): if ( isinstance(obj, type) and issubclass(obj, case.TestCase) - and cls not in (case.TestCase, case.FunctionTestCase) + and obj not in (case.TestCase, case.FunctionTestCase) ): tests.append(self.loadTestsFromTestCase(obj)) @@ -178,7 +178,7 @@ def loadTestsFromName(self, name, module=None): elif ( isinstance(obj, type) and issubclass(obj, case.TestCase) - and cls not in (case.TestCase, case.FunctionTestCase) + and obj not in (case.TestCase, case.FunctionTestCase) ): return self.loadTestsFromTestCase(obj) elif (isinstance(obj, types.FunctionType) and From bc31adefd587f9a19f6cbace7364c790607fd8d7 Mon Sep 17 00:00:00 2001 From: Nikita Sobolev Date: Mon, 11 Sep 2023 21:48:16 +0300 Subject: [PATCH 4/4] Apply suggestions from code review Co-authored-by: Serhiy Storchaka --- Lib/unittest/loader.py | 5 +---- .../Library/2022-12-24-12-50-54.gh-issue-84867.OhaLbU.rst | 4 ++-- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/Lib/unittest/loader.py b/Lib/unittest/loader.py index d7ae4be6076df1..9a3e5cc4bf30e5 100644 --- a/Lib/unittest/loader.py +++ b/Lib/unittest/loader.py @@ -84,10 +84,7 @@ def loadTestsFromTestCase(self, testCaseClass): raise TypeError("Test cases should not be derived from " "TestSuite. Maybe you meant to derive from " "TestCase?") - if ( - issubclass(testCaseClass, case.TestCase) - and testCaseClass in (case.TestCase, case.FunctionTestCase) - ): + if testCaseClass in (case.TestCase, case.FunctionTestCase): # We don't load any tests from base types that should not be loaded. testCaseNames = [] else: diff --git a/Misc/NEWS.d/next/Library/2022-12-24-12-50-54.gh-issue-84867.OhaLbU.rst b/Misc/NEWS.d/next/Library/2022-12-24-12-50-54.gh-issue-84867.OhaLbU.rst index f581315fc467fe..8b45dcee481916 100644 --- a/Misc/NEWS.d/next/Library/2022-12-24-12-50-54.gh-issue-84867.OhaLbU.rst +++ b/Misc/NEWS.d/next/Library/2022-12-24-12-50-54.gh-issue-84867.OhaLbU.rst @@ -1,2 +1,2 @@ -Do not load test cases from exact :class:`unittest.case.TestCase` and -:class:`unittest.case.FunctionTestCase` classes. +:class:`unittest.TestLoader` no longer loads test cases from exact +:class:`unittest.TestCase` and :class:`unittest.FunctionTestCase` classes.