Support RF 3.2 get_keyword_arguments new format #31
Conversation
| def __new_arg_spec(self, keyword_name): | ||
| kw = self.__get_keyword(keyword_name) | ||
| if kw is None: | ||
| return [tuple(), ] |
There was a problem hiding this comment.
This is invalid return value. The syntax also looks a bit strange when [()] could be used instead, but that's not really relevant.
It would probably be better to handle kw being none in get_keyword_arguments before calling new/old methods. I'm not actually sure what would be a good return value in that case. Probably None to let Robot handle the fact that we don't know the spec. Returning [] would mean that we say the kw accepts no args.
There was a problem hiding this comment.
I was not sure what should be returned. Other thing that I was considering, that it would be possible to raise an exception. But if None is better from Robot Framework side, let's go with that.
There was a problem hiding this comment.
Items in the list returned from get_keyword_names must be strings or tuples with 1-2 items so that the first item is a string.
Isn't it so that kw can be None only if RF calls get_keyword_arguments with some new special value like __special__? It's very unlikely to happen in general, but I guess returning None as an indication that we don't know is the best solution. This could be tested with unit tests but I'm no certain is it worth the effort.
There was a problem hiding this comment.
Yes, None is used for other __special__ values, expect __init__.
| if varargs: | ||
| args.append(('*{}'.format(varargs), )) | ||
| if kwargs: | ||
| args.append(('**{}'.format(kwargs), )) |
There was a problem hiding this comment.
There's no need to return other than defaults as tuples. That's ok but also ['mandatory, ('optional', 'default')] is fine. That means only defaults need to be handled differently depending on RF versions and that means some duplication could be removed.
Looking at this code I remember that '{}'.format(value) fails on Python 2 with UnicodeErrorError if value is Unicode and contains non-ASCII stuff. With Python 2 varargs and kwards cannot contain such characters (they aren't even Unicode) but perhaps it would be good to go through the library code and change all '{}'.format(value)s to u'{}'.format(value) or '%s' % value.
There was a problem hiding this comment.
-
I thought all are tuples, will change.
-
I am already forgetting all the Python 2 quirks. Although that is almost same as the from the old format where
format()has served pretty well. But it will be consistent with other code to use%s. Just can't wait to ditch Python 2.
Fixes #27