Skip to content

Commit

Permalink
Fix nested arg conversion when value is object, not string.
Browse files Browse the repository at this point in the history 
Now arguments with nested types are always converted. An alternative
would have been checking do nested values have correct types. That
could have been better, but it would have also been lot more work.

Fixes #4705.
  • Loading branch information
@pekkaklarck
pekkaklarck committed Mar 28, 2023
1 parent bf589e6 commit 72cde3b
Show file tree
Hide file tree
Showing 4 changed files with 50 additions and 10 deletions.
4 changes: 4 additions & 0 deletions atest/testdata/keywords/type_conversion/annotations.robot
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ ${FRACTION 1/2} ${{fractions.Fraction(1,2)}}
${DECIMAL 1/2} ${{decimal.Decimal('0.5')}}
${DEQUE} ${{collections.deque([1, 2, 3])}}
${MAPPING} ${{type('M', (collections.abc.Mapping,), {'__getitem__': lambda s, k: {'a': 1}[k], '__iter__': lambda s: iter({'a': 1}), '__len__': lambda s: 1})()}}
${SEQUENCE} ${{type('S', (collections.abc.Sequence,), {'__getitem__': lambda s, i: ['x'][i], '__len__': lambda s: 1})()}}
${PATH} ${{pathlib.Path('x/y')}}
${PUREPATH} ${{pathlib.PurePath('x/y')}}

Expand Down Expand Up @@ -354,6 +355,7 @@ List
List ${{[1, 2]}} [1, 2]
List ${{(1, 2)}} [1, 2]
List ${DEQUE} [1, 2, 3]
List ${SEQUENCE} ['x']

Invalid list
[Template] Conversion Should Fail
Expand All @@ -370,9 +372,11 @@ Sequence (abc)
Sequence [] []
Sequence ['foo', 'bar'] ${LIST}
Sequence ${DEQUE} collections.deque([1, 2, 3])
Sequence ${SEQUENCE} ${SEQUENCE}
Mutable sequence [1, 2, 3.14, -42] [1, 2, 3.14, -42]
Mutable sequence ['\\x00', '\\x52'] ['\\x00', 'R']
Mutable sequence ${DEQUE} collections.deque([1, 2, 3])
Mutable sequence ${SEQUENCE} ['x']

Invalid sequence (abc)
[Template] Conversion Should Fail
Expand Down
28 changes: 23 additions & 5 deletions atest/testdata/keywords/type_conversion/annotations_with_typing.robot
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,14 @@ List with types
List with types [] []
List with types [1, 2, 3, -42] [1, 2, 3, -42]
List with types [1, '2', 3.0] [1, 2, 3]
List with types ${{[1, '2', 3.0]}} [1, 2, 3]

List with incompatible types
[Template] Conversion Should Fail
List with types ['foo', 'bar'] type=List[int] error=Item '0' got value 'foo' that cannot be converted to integer.
List with types [0, 1, 2, 3, 4, 5, 6.1] type=List[int] error=Item '6' got value '6.1' (float) that cannot be converted to integer: Conversion would lose precision.
List with types ${{[0.0, 1.1]}} type=List[int] error=Item '1' got value '1.1' (float) that cannot be converted to integer: Conversion would lose precision.
... arg_type=list

Invalid list
[Template] Conversion Should Fail
Expand All @@ -34,17 +37,22 @@ Tuple
Tuple with types
Tuple with types ('true', 1) (True, 1)
Tuple with types ('ei', '2') (False, 2) # 'ei' -> False is due to language config
Tuple with types ${{('no', '3')}} (False, 3)

Tuple with homogenous types
Homogenous tuple () ()
Homogenous tuple (1,) (1,)
Homogenous tuple (1, 2) (1, 2)
Homogenous tuple (1, 2, 3, 4, 5, 6, 7) (1, 2, 3, 4, 5, 6, 7)
Homogenous tuple ('1',) (1,)
Homogenous tuple (1, '2') (1, 2)
Homogenous tuple (1, '2', 3.0, 4, 5) (1, 2, 3, 4, 5)
Homogenous tuple ${{(1, '2', 3.0)}} (1, 2, 3)

Tuple with incompatible types
[Template] Conversion Should Fail
Tuple with types ('bad', 'values') type=Tuple[bool, int] error=Item '1' got value 'values' that cannot be converted to integer.
Homogenous tuple ('bad', 'values') type=Tuple[int, ...] error=Item '0' got value 'bad' that cannot be converted to integer.
Tuple with types ${{('bad', 'values')}} type=Tuple[bool, int] error=Item '1' got value 'values' that cannot be converted to integer.
... arg_type=tuple

Tuple with wrong number of values
[Template] Conversion Should Fail
Expand All @@ -67,6 +75,7 @@ Sequence with types
Sequence with types [1, 2.3, '4', '5.6'] [1, 2.3, 4, 5.6]
Mutable sequence with types
... [1, 2, 3.0, '4'] [1, 2, 3, 4]
Sequence with types ${{[1, 2.3, '4', '5.6']}} [1, 2.3, 4, 5.6]

Sequence with incompatible types
[Template] Conversion Should Fail
Expand All @@ -88,6 +97,7 @@ Dict with types
Dict with types {} {}
Dict with types {1: 1.1, 2: 2.2} {1: 1.1, 2: 2.2}
Dict with types {'1': '2', 3.0: 4} {1: 2, 3: 4}
Dict with types ${{{'1': '2', 3.0: 4}}} {1: 2, 3: 4}

Dict with incompatible types
[Template] Conversion Should Fail
Expand All @@ -110,7 +120,11 @@ Mapping
Mapping with types
Mapping with types {} {}
Mapping with types {1: 2, '3': 4.0} {1: 2, 3: 4}
Mutable mapping with types {1: 2, '3': 4.0} {1: 2, 3: 4}
Mapping with types ${{{1: 2, '3': 4.0}}} {1: 2, 3: 4}
Mutable mapping with types
... {1: 2, '3': 4.0} {1: 2, 3: 4}
Mutable mapping with types
... ${{{1: 2, '3': 4.0}}} {1: 2, 3: 4}

Mapping with incompatible types
[Template] Conversion Should Fail
Expand All @@ -124,12 +138,14 @@ Invalid mapping
Mapping with types ooops type=Mapping[int, float] error=Invalid expression.

TypedDict
TypedDict {'x': 1, 'y': 2} {'x': 1, 'y': 2}
TypedDict {'x': 1, 'y': 2.0} {'x': 1, 'y': 2}
TypedDict {'x': -10_000, 'y': '2'} {'x': -10000, 'y': 2}
TypedDict ${{{'x': 1, 'y': '2'}}} {'x': 1, 'y': 2}
TypedDict with optional {'x': 1, 'y': 2, 'z': 3} {'x': 1, 'y': 2, 'z': 3}

Optional TypedDict keys can be omitted
TypedDict with optional {'x': 0, 'y': 0} {'x': 0, 'y': 0}
TypedDict with optional {'x': 0, 'y': '0'} {'x': 0, 'y': 0}
TypedDict with optional ${{{'x': 0, 'y': '0'}}} {'x': 0, 'y': 0}

Required TypedDict keys cannot be omitted
[Documentation] This test would fail if using Python 3.8 without typing_extensions!
Expand Down Expand Up @@ -160,7 +176,9 @@ Set
Set with types
Set with types set() set()
Set with types {1, 2.0, '3'} {1, 2, 3}
Set with types ${{{1, 2.0, '3'}}} {1, 2, 3}
Mutable set with types {1, 2, 3.14, -42} {1, 2, 3.14, -42}
Mutable set with types ${{{1, 2, 3.14, -42}}} {1, 2, 3.14, -42}

Set with incompatible types
[Template] Conversion Should Fail
Expand Down
5 changes: 5 additions & 0 deletions atest/testdata/keywords/type_conversion/custom_converters.robot
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,11 @@ With generics
... ('28.9.2022', '9/28/2022')
... {'one': '28.9.2022'}
... {'one', 'two', 'three'}
With generics
... ${{['one', 'two', 'three']}}
... ${{('28.9.2022', '9/28/2022')}}
... ${{{'one': '28.9.2022'}}}
... ${{{'one', 'two', 'three'}}}

With TypedDict
TypedDict {'fi': '29.9.2022', 'us': '9/29/2022'}
Expand Down
23 changes: 18 additions & 5 deletions src/robot/running/arguments/typeconverters.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -100,11 +100,11 @@ def convert(self, name, value, explicit_type=True, strict=True, kind='Argument')
return self._handle_error(name, value, kind, error, strict)

def no_conversion_needed(self, value):
used_type = getattr(self.used_type, '__origin__', self.used_type)
try:
return isinstance(value, self.used_type)
return isinstance(value, used_type)
except TypeError:
# If the used type doesn't like `isinstance` (e.g. TypedDict),
# compare the value to the generic type instead.
# Used type wasn't a class. Compare to generic type instead.
if self.type and self.type is not self.used_type:
return isinstance(value, self.type)
raise
Expand Down Expand Up @@ -460,7 +460,7 @@ def handles(cls, type_):
return super().handles(type_) and type_ is not Tuple

def no_conversion_needed(self, value):
if isinstance(value, str):
if isinstance(value, str) or self.converter:
return False
return super().no_conversion_needed(value)

Expand Down Expand Up @@ -500,6 +500,9 @@ def __init__(self, used_type, custom_converters=None, languages=None):
self.converters = tuple(self.converter_for(t, custom_converters, languages)
for t in types)

def no_conversion_needed(self, value):
return super().no_conversion_needed(value) and not self.converters

def _non_string_convert(self, value, explicit_type=True):
return self._convert_items(tuple(value), explicit_type)

Expand Down Expand Up @@ -587,11 +590,18 @@ def __init__(self, used_type, custom_converters=None, languages=None):
self.converters = tuple(self.converter_for(t, custom_converters, languages)
for t in types)

def no_conversion_needed(self, value):
return super().no_conversion_needed(value) and not self.converters

def _non_string_convert(self, value, explicit_type=True):
if issubclass(self.used_type, dict) and not isinstance(value, dict):
if self._used_type_is_dict() and not isinstance(value, dict):
value = dict(value)
return self._convert_items(value, explicit_type)

def _used_type_is_dict(self):
used_type = getattr(self.used_type, '__origin__', self.used_type)
return issubclass(used_type, dict)

def _convert(self, value, explicit_type=True):
return self._convert_items(self._literal_eval(value, dict), explicit_type)

Expand Down Expand Up @@ -625,6 +635,9 @@ def __init__(self, used_type, custom_converters=None, languages=None):
self.type_name = type_repr(used_type)
self.converter = self.converter_for(types[0], custom_converters, languages)

def no_conversion_needed(self, value):
return super().no_conversion_needed(value) and not self.converter

def _non_string_convert(self, value, explicit_type=True):
return self._convert_items(set(value), explicit_type)

Expand Down

0 comments on commit 72cde3b

Please sign in to comment.