Skip to content

Commit

Permalink
Tests: Change ListParamType to check value type instead of setting an…
Browse files Browse the repository at this point in the history
… attribute when parsing an argument twice
  • Loading branch information
gutzbenj committed Feb 11, 2023
1 parent daf87d9 commit 980bf6e
Show file tree
Hide file tree
Showing 3 changed files with 18 additions and 5 deletions.
4 changes: 1 addition & 3 deletions click_params/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,6 @@ def __init__(self, param_type: click.ParamType, separator: str = ',', name: str
self._separator = separator
self._name = name or self.name
self._param_type = param_type
self._convert_called = False
self._error_message = 'These items are not %s: {errors}' % self._name
self._ignore_empty = ignore_empty

Expand All @@ -115,7 +114,7 @@ def convert(self, value, param, ctx):
# For an unknown reason, when a user is prompted for a value using "prompt" argument from click.option
# the convert method seems to be called more than once, so this is necessary to avoid an error
# when calling self._strip_separator below since the value passed will be already converted to a list
if self._convert_called:
if isinstance(value, list):
return value

if self._ignore_empty and value == "":
Expand All @@ -125,7 +124,6 @@ def convert(self, value, param, ctx):
if errors:
self.fail(self._error_message.format(errors=errors), param, ctx)

self._convert_called = True
return converted_list

def __repr__(self):
Expand Down
4 changes: 2 additions & 2 deletions tests/test_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -137,8 +137,9 @@ def test_should_raise_error_when_instantiating_with_non_string_parameter(self, s
'separator', [{}, {'separator': ' '}, {'separator': ';'}] # default separator should be used i.e ","
)
def test_should_not_raise_error_when_instantiating_with_a_string(self, separator):
expected_separator = separator.get("separator", ",") # default separator
base_list = ListParamType(click.INT, **separator)
assert not base_list._convert_called
assert base_list._separator == expected_separator

# we test method _strip_separator

Expand Down Expand Up @@ -189,7 +190,6 @@ def test_should_return_correct_items_when_giving_correct_expression(self, expres
# this is to test the scenario when a user is prompted for a value, it seems that
# the convert method is called more than once
# after the first call, all subsequent calls will have the converted value as first argument
assert base_list._convert_called
assert values == base_list.convert(values, None, None)

@pytest.mark.parametrize('param_type', [click.INT, click.FLOAT, click.STRING, FRACTION, COMPLEX])
Expand Down
15 changes: 15 additions & 0 deletions tests/test_miscellaneous.py
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,21 @@ def test_miscellaneous_list_param_types_ignore_empty_string(param_type):
assert misc_list_type.convert("", None, None) == []


def test_cli_with_multiple_similar_string_list_param_types(runner):
@click.command()
@click.option('-v', 'values', type=StringListParamType(","))
def cli(values):
click.echo(values)

result = runner.invoke(cli, ['-v', "abc,def"])

assert result.output == "['abc', 'def']\n"

result = runner.invoke(cli, ['-v', "abc,def"])
# TODO: the following will currently fail!
assert result.output == "['abc', 'def']\n"


class TestJsonParamType:
"""Tests JsonParamType specific cases"""

Expand Down

0 comments on commit 980bf6e

Please sign in to comment.