Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

gh-103558: Add coverage tests for argparse #103570

Merged
merged 16 commits into from
Jun 5, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions Lib/argparse.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -1811,13 +1813,11 @@ def identity(string):

# add parent arguments and defaults
for parent in parents:
if not isinstance(parent, ArgumentParser):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are there any other options on what can be actually passed here? Why it was like that? It is introduced in a pretty old commit without any mentions of "why": 698a18a

Maybe some structural subtyping? optparse?

However, it is documented that parents must be a list of ArgumentParser objects:

parents - A list of ArgumentParser objects whose arguments should also be included

So, I guess it is ok.

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
Expand Down
86 changes: 85 additions & 1 deletion Lib/test/test_argparse.py
Original file line number Diff line number Diff line change
Expand Up @@ -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


Expand Down Expand Up @@ -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
# ========================================
Expand Down Expand Up @@ -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 = [
Expand Down Expand Up @@ -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):
"""
Expand Down Expand Up @@ -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
# ================
Expand Down Expand Up @@ -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
# ==============================
Expand Down Expand Up @@ -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))
Expand Down Expand Up @@ -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)

Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fixed ``parent`` argument validation mechanism of :mod:`argparse`. Improved test coverage.