Skip to content

Support RF 3.2 get_keyword_arguments new format #31

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Mar 28, 2020
Merged

Support RF 3.2 get_keyword_arguments new format #31

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
23 changes: 22 additions & 1 deletion src/robotlibcore.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@


from robot.api.deco import keyword # noqa F401
from robot import __version__ as robot_version

PY2 = sys.version_info < (3,)

Expand Down Expand Up @@ -97,7 +98,27 @@ def run_keyword(self, name, args, kwargs=None):
return self.keywords[name](*args, **(kwargs or {}))

def get_keyword_arguments(self, name):
kw = self.keywords[name] if name != '__init__' else self.__init__
if robot_version >= '3.2':
return self.__new_arg_spec(name)
return self.__old_arg_spec(name)

def __new_arg_spec(self, keyword_name):
kw = self.__get_keyword(keyword_name)
if kw is None:
return [tuple(), ]
Copy link
Member

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

Copy link
Member

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, None is used for other __special__ values, expect __init__.

args, defaults, varargs, kwargs = self.__get_arg_spec(kw)
args = [(arg, ) for arg in args]
args += [(name, value) for name, value in defaults]
if varargs:
args.append(('*{}'.format(varargs), ))
if kwargs:
args.append(('**{}'.format(kwargs), ))
Copy link
Member

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

  1. I thought all are tuples, will change.

  2. 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.

return args

def __old_arg_spec(self, keyword_name):
kw = self.__get_keyword(keyword_name)
if kw is None:
return []
args, defaults, varargs, kwargs = self.__get_arg_spec(kw)
args += ['{}={}'.format(name, value) for name, value in defaults]
if varargs:
Expand Down
22 changes: 19 additions & 3 deletions utest/test_robotlibcore.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
import sys

import pytest
from robot import __version__ as robot__version

from robotlibcore import HybridCore

from HybridLibrary import HybridLibrary
from DynamicLibrary import DynamicLibrary

Expand Down Expand Up @@ -36,6 +36,8 @@ def test_dir():
'_DynamicCore__get_keyword_tags_supported',
'_DynamicCore__get_typing_hints',
'_DynamicCore__join_defaults_with_types',
'_DynamicCore__new_arg_spec',
'_DynamicCore__old_arg_spec',
'_HybridCore__get_members',
'_HybridCore__get_members_from_instance',
'_custom_name',
Expand Down Expand Up @@ -72,6 +74,8 @@ def test_dir():
'_DynamicCore__get_keyword_path',
'_DynamicCore__get_keyword_tags_supported',
'_DynamicCore__join_defaults_with_types',
'_DynamicCore__new_arg_spec',
'_DynamicCore__old_arg_spec',
'get_keyword_arguments',
'get_keyword_documentation',
'get_keyword_source',
Expand All @@ -94,14 +98,26 @@ def test_getattr():
assert str(exc_info.value) == \
"'%s' object has no attribute 'non_existing'" % type(lib).__name__


def test_get_keyword_arguments():
@pytest.mark.skipif(robot__version >= '3.2', reason='For RF 3.1')
def test_get_keyword_arguments_rf31():
args = DynamicLibrary().get_keyword_arguments
assert args('mandatory') == ['arg1', 'arg2']
assert args('defaults') == ['arg1', 'arg2=default', 'arg3=3']
assert args('varargs_and_kwargs') == ['*args', '**kws']
assert args('all_arguments') == ['mandatory', 'default=value', '*varargs', '**kwargs']
assert args('__init__') == ['arg=None']
assert args('__foobar__') == []


@pytest.mark.skipif(robot__version < '3.2', reason='For RF 3.2 or greater')
def test_get_keyword_arguments_rf32():
args = DynamicLibrary().get_keyword_arguments
assert args('mandatory') == [('arg1', ), ('arg2', )]
assert args('defaults') == [('arg1', ), ('arg2', 'default'), ('arg3', 3)]
assert args('varargs_and_kwargs') == [('*args', ), ('**kws', )]
assert args('all_arguments') == [('mandatory', ), ('default', 'value'), ('*varargs', ), ('**kwargs', )]
assert args('__init__') == [('arg', None)]
assert args('__foobar__') == [()]


def test_get_keyword_documentation():
Expand Down