Adds enum support to the typeconverter. #2908
Conversation
| Run Keyword And Expect Error | ||
| ... ValueError: Argument 'argument' cannot be converted to Foo, got 'foobar'. | ||
| ... Enum foobar | ||
|
|
||
| *** Keywords *** | ||
| Conversion Should Fail | ||
| [Arguments] ${kw} ${arg} |
There was a problem hiding this comment.
You ought to be able to add new argument ${type}=${kw.lower()} here and use this keyword also with the new Invalid Enum test like
Conversion Should Fail Enum foobar type=Foo
or with [Template] if you want to test multiple values like existing tests do.
| @@ -14,6 +14,7 @@ | |||
| # limitations under the License. | |||
|
|
|||
| from ast import literal_eval | |||
| from enum import EnumMeta | |||
There was a problem hiding this comment.
Enums being new in Python 3.4, this is probably the first change not compatible with Python 3.3. Good that we decided to drop its support along with Python 2.6 in #2276.
| @@ -39,12 +40,17 @@ def convert(self, positional, named): | |||
| def _convert(self, name, value): | |||
| if name not in self._argspec.types or not is_string(value): | |||
| return value | |||
| if value.upper() == 'NONE': | |||
| return None | |||
There was a problem hiding this comment.
Is moving these two lines related to enums? For me it feels a bit wrong to convert NONE string to None always if an argument has annotation. For example, if we have def example(arg: str), we should pass all strings (and all other arguments) without changes. This conversions is ought to be safe if the annotation is a type we are going to convert and know wouldn't work with strings in the first place.
| try: | ||
| return type_[value] | ||
| except KeyError: | ||
| self._raise_convert_failed(name, type_.__name__, value) |
There was a problem hiding this comment.
Although enums need to be recognized differently than other types, it would be better to have a separate _conver_enum helper for conversion. Code here could hen be change to something like:
if isinstance(type_, EnumMeta):
converter = self._convert_enum
else:
converter = self._converters.get(type_)
|
I incorporated your feedback, is this enum converter function what you had in mind? |
|
Didn't realize |
|
Didn't notice earlier that you haven't added new tests to |
|
I'll add the missing tests myself. I got some extra time for coding and want to merge this PR to avoid conflicts. |
This PR adds Enum support to the typeconverter and relevant tests.
Because the
enumI added for the test starts with an upper case character I wasn't able to use the existing template for the expected failure.