A few tweaks for issue16662 based on feedback from Robert Collins.

2 files changed, 46 insertions, 38 deletions

diff --git a/Lib/unittest/loader.py b/Lib/unittest/loader.py
index 590e227..a8c6492 100644
--- a/Lib/unittest/loader.py
+++ b/Lib/unittest/loader.py
@@ -79,12 +79,15 @@ class TestLoader(object):
         # use_load_tests argument.  For backward compatibility, we still
         # accept the argument (which can also be the first position) but we
         # ignore it and issue a deprecation warning if it's present.
-        if len(args) == 1 or 'use_load_tests' in kws:
+        if len(args) > 0 or 'use_load_tests' in kws:
             warnings.warn('use_load_tests is deprecated and ignored',
                           DeprecationWarning)
             kws.pop('use_load_tests', None)
         if len(args) > 1:
-            raise TypeError('loadTestsFromModule() takes 1 positional argument but {} were given'.format(len(args)))
+            # Complain about the number of arguments, but don't forget the
+            # required `module` argument.
+            complaint = len(args) + 1
+            raise TypeError('loadTestsFromModule() takes 1 positional argument but {} were given'.format(complaint))
         if len(kws) != 0:
             # Since the keyword arguments are unsorted (see PEP 468), just
             # pick the alphabetically sorted first argument to complain about,
diff --git a/Lib/unittest/test/test_loader.py b/Lib/unittest/test/test_loader.py
index d363394..7c23414 100644
--- a/Lib/unittest/test/test_loader.py
+++ b/Lib/unittest/test/test_loader.py
@@ -196,23 +196,23 @@ class Test_TestLoader(unittest.TestCase):
 
     @warningregistry
     def test_loadTestsFromModule__use_load_tests_deprecated_positional(self):
+        m = types.ModuleType('m')
+        class MyTestCase(unittest.TestCase):
+            def test(self):
+                pass
+        m.testcase_1 = MyTestCase
+
+        load_tests_args = []
+        def load_tests(loader, tests, pattern):
+            self.assertIsInstance(tests, unittest.TestSuite)
+            load_tests_args.extend((loader, tests, pattern))
+            return tests
+        m.load_tests = load_tests
+        # The method still works.
+        loader = unittest.TestLoader()
+        # use_load_tests=True as a positional argument.
         with warnings.catch_warnings(record=True) as w:
             warnings.simplefilter('always')
-            m = types.ModuleType('m')
-            class MyTestCase(unittest.TestCase):
-                def test(self):
-                    pass
-            m.testcase_1 = MyTestCase
-
-            load_tests_args = []
-            def load_tests(loader, tests, pattern):
-                self.assertIsInstance(tests, unittest.TestSuite)
-                load_tests_args.extend((loader, tests, pattern))
-                return tests
-            m.load_tests = load_tests
-            # The method still works.
-            loader = unittest.TestLoader()
-            # use_load_tests=True as a positional argument.
             suite = loader.loadTestsFromModule(m, False)
             self.assertIsInstance(suite, unittest.TestSuite)
             # load_tests was still called because use_load_tests is deprecated
@@ -225,22 +225,22 @@ class Test_TestLoader(unittest.TestCase):
 
     @warningregistry
     def test_loadTestsFromModule__use_load_tests_deprecated_keyword(self):
+        m = types.ModuleType('m')
+        class MyTestCase(unittest.TestCase):
+            def test(self):
+                pass
+        m.testcase_1 = MyTestCase
+
+        load_tests_args = []
+        def load_tests(loader, tests, pattern):
+            self.assertIsInstance(tests, unittest.TestSuite)
+            load_tests_args.extend((loader, tests, pattern))
+            return tests
+        m.load_tests = load_tests
+        # The method still works.
+        loader = unittest.TestLoader()
         with warnings.catch_warnings(record=True) as w:
             warnings.simplefilter('always')
-            m = types.ModuleType('m')
-            class MyTestCase(unittest.TestCase):
-                def test(self):
-                    pass
-            m.testcase_1 = MyTestCase
-
-            load_tests_args = []
-            def load_tests(loader, tests, pattern):
-                self.assertIsInstance(tests, unittest.TestSuite)
-                load_tests_args.extend((loader, tests, pattern))
-                return tests
-            m.load_tests = load_tests
-            # The method still works.
-            loader = unittest.TestLoader()
             suite = loader.loadTestsFromModule(m, use_load_tests=False)
             self.assertIsInstance(suite, unittest.TestSuite)
             # load_tests was still called because use_load_tests is deprecated
@@ -251,6 +251,7 @@ class Test_TestLoader(unittest.TestCase):
             self.assertEqual(str(w[-1].message),
                                  'use_load_tests is deprecated and ignored')
 
+    @warningregistry
     def test_loadTestsFromModule__too_many_positional_args(self):
         m = types.ModuleType('m')
         class MyTestCase(unittest.TestCase):
@@ -265,14 +266,18 @@ class Test_TestLoader(unittest.TestCase):
             return tests
         m.load_tests = load_tests
         loader = unittest.TestLoader()
-        with self.assertRaises(TypeError) as cm:
+        with self.assertRaises(TypeError) as cm, \
+             warnings.catch_warning(record=True) as w:
             loader.loadTestsFromModule(m, False, 'testme.*')
-        self.assertEqual(type(cm.exception), TypeError)
-        # The error message names the first bad argument alphabetically,
-        # however use_load_tests (which sorts first) is ignored.
-        self.assertEqual(
-            str(cm.exception),
-            'loadTestsFromModule() takes 1 positional argument but 2 were given')
+            # We still got the deprecation warning.
+            self.assertIs(w[-1].category, DeprecationWarning)
+            self.assertEqual(str(w[-1].message),
+                                 'use_load_tests is deprecated and ignored')
+            # We also got a TypeError for too many positional arguments.
+            self.assertEqual(type(cm.exception), TypeError)
+            self.assertEqual(
+                str(cm.exception),
+                'loadTestsFromModule() takes 1 positional argument but 3 were given')
 
     @warningregistry
     def test_loadTestsFromModule__use_load_tests_other_bad_keyword(self):