From f0475c9066d23d63d3b626d7ee485a112b5fb446 Mon Sep 17 00:00:00 2001 From: Ben Hsing Date: Tue, 18 Jun 2024 04:03:23 +0000 Subject: [PATCH 1/5] gh-120665: make unittest loaders avoid loading test cases that are abstract base classes --- Lib/test/test_unittest/test_loader.py | 49 +++++++++++++++++++++++++++ Lib/unittest/loader.py | 11 ++++-- 2 files changed, 58 insertions(+), 2 deletions(-) diff --git a/Lib/test/test_unittest/test_loader.py b/Lib/test/test_unittest/test_loader.py index 83dd25ca54623f8..a172beca12b8557 100644 --- a/Lib/test/test_unittest/test_loader.py +++ b/Lib/test/test_unittest/test_loader.py @@ -1,3 +1,4 @@ +import abc import functools import sys import types @@ -98,6 +99,22 @@ def test_loadTestsFromTestCase__from_FunctionTestCase(self): self.assertIsInstance(suite, loader.suiteClass) self.assertEqual(list(suite), []) + # "Do not load any tests from a TestCase-derived class that is an abstract + # base class." + def test_loadTestsFromTestCase__from_abc_TestCase(self): + class FooBase(unittest.TestCase, metaclass=abc.ABCMeta): + @abc.abstractmethod + def test(self): ... + class Foo(FooBase): + def test(self): pass + + empty_suite = unittest.TestSuite() + + loader = unittest.TestLoader() + suite = loader.loadTestsFromTestCase(Foo) + self.assertEqual(loader.loadTestsFromTestCase(FooBase), empty_suite) + self.assertEqual(list(suite), [Foo('test')]) + ################################################################ ### /Tests for TestLoader.loadTestsFromTestCase @@ -252,6 +269,24 @@ def load_tests(loader, tests, pattern): self.assertRaisesRegex(TypeError, "some failure", test.m) + # Check that loadTestsFromModule skips abstract base classes derived from + # TestCase, which can't be instantiated. + def test_loadTestsFromModule__skip_abc_TestCase(self): + m = types.ModuleType('m') + class MyTestCaseBase(unittest.TestCase, metaclass=abc.ABCMeta): + @abc.abstractmethod + def test(self): + ... + class MyTestCase(MyTestCaseBase): + def test(self): + pass + m.testcase_1 = MyTestCaseBase + m.testcase_2 = MyTestCase + loader = unittest.TestLoader() + suite = loader.loadTestsFromModule(m) + expected = [loader.suiteClass([MyTestCase('test')])] + self.assertEqual(list(suite), expected) + ################################################################ ### /Tests for TestLoader.loadTestsFromModule() @@ -1052,6 +1087,20 @@ def test_loadTestsFromNames__module_not_loaded(self): if module_name in sys.modules: del sys.modules[module_name] + # "The specifier should not refer to a test method in a TestCase-derived + # subclass that is an abstract base class" + def test_loadTestsFromNames__testmethod_in_abc_TestCase(self): + m = types.ModuleType('m') + class Foo(unittest.TestCase, metaclass=abc.ABCMeta): + @abc.abstractmethod + def test_1(self): ... + def test_2(self): pass + m.Foo = Foo + + loader = unittest.TestLoader() + with self.assertRaises(TypeError): + loader.loadTestsFromNames(['Foo.test_1', 'Foo.test_2'], m) + ################################################################ ### /Tests for TestLoader.loadTestsFromNames() diff --git a/Lib/unittest/loader.py b/Lib/unittest/loader.py index 22797b83a68bc87..797f2950f8cbba7 100644 --- a/Lib/unittest/loader.py +++ b/Lib/unittest/loader.py @@ -1,5 +1,6 @@ """Loading unittests.""" +import inspect import os import re import sys @@ -84,8 +85,10 @@ def loadTestsFromTestCase(self, testCaseClass): raise TypeError("Test cases should not be derived from " "TestSuite. Maybe you meant to derive from " "TestCase?") - if testCaseClass in (case.TestCase, case.FunctionTestCase): - # We don't load any tests from base types that should not be loaded. + if (testCaseClass in (case.TestCase, case.FunctionTestCase) or + inspect.isabstract(testCaseClass)): + # We don't load any tests from base types that should not be loaded, + # and abstract base classes that can't be instantiated testCaseNames = [] else: testCaseNames = self.getTestCaseNames(testCaseClass) @@ -103,6 +106,7 @@ def loadTestsFromModule(self, module, *, pattern=None): isinstance(obj, type) and issubclass(obj, case.TestCase) and obj not in (case.TestCase, case.FunctionTestCase) + and not inspect.isabstract(obj) ): tests.append(self.loadTestsFromTestCase(obj)) @@ -181,6 +185,9 @@ def loadTestsFromName(self, name, module=None): elif (isinstance(obj, types.FunctionType) and isinstance(parent, type) and issubclass(parent, case.TestCase)): + if inspect.isabstract(parent): + raise TypeError( + "Cannot instantiate abstract base class %s" % parent.__name__) name = parts[-1] inst = parent(name) # static methods follow a different path From 4a43e04880bd7e20f771ae95b25642c3c8c54b44 Mon Sep 17 00:00:00 2001 From: "blurb-it[bot]" <43283697+blurb-it[bot]@users.noreply.github.com> Date: Tue, 18 Jun 2024 04:08:42 +0000 Subject: [PATCH 2/5] =?UTF-8?q?=F0=9F=93=9C=F0=9F=A4=96=20Added=20by=20blu?= =?UTF-8?q?rb=5Fit.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../next/Library/2024-06-18-04-08-37.gh-issue-120665.x7T1hV.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 Misc/NEWS.d/next/Library/2024-06-18-04-08-37.gh-issue-120665.x7T1hV.rst diff --git a/Misc/NEWS.d/next/Library/2024-06-18-04-08-37.gh-issue-120665.x7T1hV.rst b/Misc/NEWS.d/next/Library/2024-06-18-04-08-37.gh-issue-120665.x7T1hV.rst new file mode 100644 index 000000000000000..5ce5e9d458c1833 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2024-06-18-04-08-37.gh-issue-120665.x7T1hV.rst @@ -0,0 +1 @@ +Fixed an issue where :module:`unittest` loaders would load and instantiate :class:`TestCase`-derived subclasses that are also abstract base classes, which can't be instantiated. From 97845abb9813a122daa901d10b381eca42a28d7b Mon Sep 17 00:00:00 2001 From: blhsing Date: Tue, 18 Jun 2024 13:15:56 +0800 Subject: [PATCH 3/5] Update 2024-06-18-04-08-37.gh-issue-120665.x7T1hV.rst --- .../next/Library/2024-06-18-04-08-37.gh-issue-120665.x7T1hV.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Misc/NEWS.d/next/Library/2024-06-18-04-08-37.gh-issue-120665.x7T1hV.rst b/Misc/NEWS.d/next/Library/2024-06-18-04-08-37.gh-issue-120665.x7T1hV.rst index 5ce5e9d458c1833..27e93988ed11efb 100644 --- a/Misc/NEWS.d/next/Library/2024-06-18-04-08-37.gh-issue-120665.x7T1hV.rst +++ b/Misc/NEWS.d/next/Library/2024-06-18-04-08-37.gh-issue-120665.x7T1hV.rst @@ -1 +1 @@ -Fixed an issue where :module:`unittest` loaders would load and instantiate :class:`TestCase`-derived subclasses that are also abstract base classes, which can't be instantiated. +Fixed an issue where ``unittest`` loaders would load and instantiate :class:`unittest.TestCase`-derived subclasses that are also abstract base classes, which can't be instantiated. From 55a4994f38b8761afe8ae807940d65deadc426ff Mon Sep 17 00:00:00 2001 From: Ben Hsing Date: Tue, 18 Jun 2024 07:17:17 +0000 Subject: [PATCH 4/5] gh-120665: split the test for loadTestsFromNames into two sub-tests --- Lib/test/test_unittest/test_loader.py | 12 ++++++++++-- Lib/unittest/loader.py | 2 +- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/Lib/test/test_unittest/test_loader.py b/Lib/test/test_unittest/test_loader.py index a172beca12b8557..26a38763a2c6c7f 100644 --- a/Lib/test/test_unittest/test_loader.py +++ b/Lib/test/test_unittest/test_loader.py @@ -1098,8 +1098,16 @@ def test_2(self): pass m.Foo = Foo loader = unittest.TestLoader() - with self.assertRaises(TypeError): - loader.loadTestsFromNames(['Foo.test_1', 'Foo.test_2'], m) + for name in 'Foo.test_1', 'Foo.test_2': + with self.subTest(name=name): + try: + loader.loadTestsFromNames([name], m) + except TypeError as e: + self.assertEqual(str(e), + "Cannot instantiate abstract test case Foo") + else: + self.fail( + "TestLoader.loadTestsFromNames failed to raise TypeError") ################################################################ ### /Tests for TestLoader.loadTestsFromNames() diff --git a/Lib/unittest/loader.py b/Lib/unittest/loader.py index 797f2950f8cbba7..2bb9af39b737c87 100644 --- a/Lib/unittest/loader.py +++ b/Lib/unittest/loader.py @@ -187,7 +187,7 @@ def loadTestsFromName(self, name, module=None): issubclass(parent, case.TestCase)): if inspect.isabstract(parent): raise TypeError( - "Cannot instantiate abstract base class %s" % parent.__name__) + "Cannot instantiate abstract test case %s" % parent.__name__) name = parts[-1] inst = parent(name) # static methods follow a different path From d74e37c07441534671a457dc435ec9fa84ba754d Mon Sep 17 00:00:00 2001 From: Ben Hsing Date: Tue, 18 Jun 2024 07:32:50 +0000 Subject: [PATCH 5/5] gh-120665: use self.assertRaisesRegex instead of a try block --- Lib/test/test_unittest/test_loader.py | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/Lib/test/test_unittest/test_loader.py b/Lib/test/test_unittest/test_loader.py index 26a38763a2c6c7f..560e7bebae1462e 100644 --- a/Lib/test/test_unittest/test_loader.py +++ b/Lib/test/test_unittest/test_loader.py @@ -1099,15 +1099,9 @@ def test_2(self): pass loader = unittest.TestLoader() for name in 'Foo.test_1', 'Foo.test_2': - with self.subTest(name=name): - try: - loader.loadTestsFromNames([name], m) - except TypeError as e: - self.assertEqual(str(e), - "Cannot instantiate abstract test case Foo") - else: - self.fail( - "TestLoader.loadTestsFromNames failed to raise TypeError") + with self.subTest(name=name), self.assertRaisesRegex(TypeError, + "Cannot instantiate abstract test case Foo"): + loader.loadTestsFromNames([name], m) ################################################################ ### /Tests for TestLoader.loadTestsFromNames()