gh-103558: Add coverage tests for argparse (#103570)

gaogaotiantian · hauntsaninja · web-flow · commit 9efaff5fd31a · 2023-06-05T00:14:00.000-07:00
Co-authored-by: Shantanu &lt;12621235+hauntsaninja@users.noreply.github.com&gt;
Co-authored-by: hauntsaninja &lt;hauntsaninja@gmail.com&gt;
diff --git a/Lib/argparse.py b/Lib/argparse.py
@@ -1528,6 +1528,8 @@ def _add_container_actions(self, container):
         title_group_map = {}
         for group in self._action_groups:
             if group.title in title_group_map:
+                # This branch could happen if a derived class added
+                # groups with duplicated titles in __init__
                 msg = _('cannot merge actions - two groups are named %r')
                 raise ValueError(msg % (group.title))
             title_group_map[group.title] = group
@@ -1811,13 +1813,11 @@ def identity(string):
 
         # add parent arguments and defaults
         for parent in parents:
+            if not isinstance(parent, ArgumentParser):
+                raise TypeError('parents must be a list of ArgumentParser')
             self._add_container_actions(parent)
-            try:
-                defaults = parent._defaults
-            except AttributeError:
-                pass
-            else:
-                self._defaults.update(defaults)
+            defaults = parent._defaults
+            self._defaults.update(defaults)
 
     # =======================
     # Pretty __repr__ methods
diff --git a/Lib/test/test_argparse.py b/Lib/test/test_argparse.py
@@ -15,7 +15,7 @@
 import argparse
 import warnings
 
-from test.support import os_helper
+from test.support import os_helper, captured_stderr
 from unittest import mock
 
 
@@ -1382,6 +1382,19 @@ class TestPositionalsActionAppend(ParserTestCase):
         ('a b c', NS(spam=['a', ['b', 'c']])),
     ]
 
+
+class TestPositionalsActionExtend(ParserTestCase):
+    """Test the 'extend' action"""
+
+    argument_signatures = [
+        Sig('spam', action='extend'),
+        Sig('spam', action='extend', nargs=2),
+    ]
+    failures = ['', '--foo', 'a', 'a b', 'a b c d']
+    successes = [
+        ('a b c', NS(spam=['a', 'b', 'c'])),
+    ]
+
 # ========================================
 # Combined optionals and positionals tests
 # ========================================
@@ -1419,6 +1432,32 @@ class TestOptionalsAlmostNumericAndPositionals(ParserTestCase):
     ]
 
 
+class TestOptionalsAndPositionalsAppend(ParserTestCase):
+    argument_signatures = [
+        Sig('foo', nargs='*', action='append'),
+        Sig('--bar'),
+    ]
+    failures = ['-foo']
+    successes = [
+        ('a b', NS(foo=[['a', 'b']], bar=None)),
+        ('--bar a b', NS(foo=[['b']], bar='a')),
+        ('a b --bar c', NS(foo=[['a', 'b']], bar='c')),
+    ]
+
+
+class TestOptionalsAndPositionalsExtend(ParserTestCase):
+    argument_signatures = [
+        Sig('foo', nargs='*', action='extend'),
+        Sig('--bar'),
+    ]
+    failures = ['-foo']
+    successes = [
+        ('a b', NS(foo=['a', 'b'], bar=None)),
+        ('--bar a b', NS(foo=['b'], bar='a')),
+        ('a b --bar c', NS(foo=['a', 'b'], bar='c')),
+    ]
+
+
 class TestEmptyAndSpaceContainingArguments(ParserTestCase):
 
     argument_signatures = [
@@ -1899,6 +1938,10 @@ def test_open_args(self):
                 type('foo')
                 m.assert_called_with('foo', *args)
 
+    def test_invalid_file_type(self):
+        with self.assertRaises(ValueError):
+            argparse.FileType('b')('-test')
+
 
 class TestFileTypeMissingInitialization(TestCase):
     """
@@ -2092,6 +2135,27 @@ class TestActionExtend(ParserTestCase):
         ('--foo f1 --foo f2 f3 f4', NS(foo=['f1', 'f2', 'f3', 'f4'])),
     ]
 
+
+class TestInvalidAction(TestCase):
+    """Test invalid user defined Action"""
+
+    class ActionWithoutCall(argparse.Action):
+        pass
+
+    def test_invalid_type(self):
+        parser = argparse.ArgumentParser()
+
+        parser.add_argument('--foo', action=self.ActionWithoutCall)
+        self.assertRaises(NotImplementedError, parser.parse_args, ['--foo', 'bar'])
+
+    def test_modified_invalid_action(self):
+        parser = ErrorRaisingArgumentParser()
+        action = parser.add_argument('--foo')
+        # Someone got crazy and did this
+        action.type = 1
+        self.assertRaises(ArgumentParserError, parser.parse_args, ['--foo', 'bar'])
+
+
 # ================
 # Subparsers tests
 # ================
@@ -2727,6 +2791,9 @@ def test_groups_parents(self):
               -x X
         '''.format(progname, ' ' if progname else '' )))
 
+    def test_wrong_type_parents(self):
+        self.assertRaises(TypeError, ErrorRaisingArgumentParser, parents=[1])
+
 # ==============================
 # Mutually exclusive group tests
 # ==============================
@@ -4743,6 +4810,9 @@ def test_invalid_option_strings(self):
         self.assertValueError('--')
         self.assertValueError('---')
 
+    def test_invalid_prefix(self):
+        self.assertValueError('--foo', '+foo')
+
     def test_invalid_type(self):
         self.assertValueError('--foo', type='int')
         self.assertValueError('--foo', type=(int, float))
@@ -4807,6 +4877,9 @@ def test_parsers_action_missing_params(self):
         self.assertTypeError('command', action='parsers',
                              parser_class=argparse.ArgumentParser)
 
+    def test_version_missing_params(self):
+        self.assertTypeError('command', action='version')
+
     def test_required_positional(self):
         self.assertTypeError('foo', required=True)
 
@@ -5400,6 +5473,17 @@ def test_exclusive_incompatible(self):
         self.assertRaises(TypeError, parser.parse_intermixed_args, [])
         self.assertEqual(group.required, True)
 
+    def test_invalid_args(self):
+        parser = ErrorRaisingArgumentParser(prog='PROG')
+        self.assertRaises(ArgumentParserError, parser.parse_intermixed_args, ['a'])
+
+        parser = ErrorRaisingArgumentParser(prog='PROG')
+        parser.add_argument('--foo', nargs="*")
+        parser.add_argument('foo')
+        with captured_stderr() as stderr:
+            parser.parse_intermixed_args(['hello', '--foo'])
+            self.assertIn("UserWarning", stderr.getvalue())
+
 class TestIntermixedMessageContentError(TestCase):
     # case where Intermixed gives different error message
     # error is raised by 1st parsing step
diff --git a/Misc/NEWS.d/next/Library/2023-04-15-23-26-16.gh-issue-103558.w9OzK4.rst b/Misc/NEWS.d/next/Library/2023-04-15-23-26-16.gh-issue-103558.w9OzK4.rst
@@ -0,0 +1 @@
+Fixed ``parent`` argument validation mechanism of :mod:`argparse`. Improved test coverage.

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1 @@`
	`1`	+Fixed ``parent`` argument validation mechanism of :mod:`argparse`. Improved test coverage.