Support kwargs same way as Robot Framework documentation states

[Hi-Fi]

In RF documentation (http://robotframework.org/robotframework/latest/RobotFrameworkUserGuide.html#free-keyword-arguments-kwargs) it's mentioned that kwargs could be used in Java libraries by adding Map<String,Object> -kind of argument to method. In Javalib-core there's internal handling, that converts those kwargs to String[] causing it to be harder to parse those to usable form.

In javalib-core documentation there was no clear examples about the same kind of cases than in RF documentation, which would be nice to have, too.

[aronaks]

I would agree. Please add more examples for that fucntionality.

[Hi-Fi]

Implemented in #24, documentation needs to be updated after that's merged.

[shan-96]

#24 is merged to master but I am still able to reproduce issue #19

I think the argument collector should

1. number of args is one
2. check if arg is of type map
3. set this arg as kwarg parsing the object into Map<String,Object>

[shan-96]

Because if feel Free keyword arguments (**kwargs) are always going to be NULL in KeywordOverload.class for remote server because Java itself has no kwargs syntax, but keywords can have java.util.Map as the last argument to specify that they accept kwargs.

Which is conflicting in KeywordInvoker.class

[Hi-Fi]

@shan-96 could you please create test case showing the error? Kwargs are supported, but parsed before callung to keyword for named arguments. And there're tests also for cases that were encountered in own usage.

[Hi-Fi]

KeywordOverload shouldn't be used, but replace it with real default values. See e.g. MarketSquare/robotframework-seleniumlibrary-java@63b3542

[shan-96]

@shan-96 could you please create test case showing the error? Kwargs are supported, but parsed before callung to keyword for named arguments. And there're tests also for cases that were encountered in own usage.

@Hi-Fi I can create a Junit for ArgumentCollector.class and KeywordInvoker.class and push it to develop branch which can elaborate the example

[shan-96]

@Hi-Fi my bad, while debugging I was using .class files from release version 2.0.3

I have anyway written a test and pushed to develop. I'll open a PR for the same. Can we release 2.0.4 with fix #19 asap?

Thanks

[Hi-Fi]

So this is working, and change was only to include passing test?

This issue is still open because of documentation updates that are in wiki are missing.

[shan-96]

So this is working, and change was only to include passing test?

This issue is still open because of documentation updates that are in wiki are missing.

Yes, but I wont be able to test it end to end (fire a robot test case from tool like RIDE and validate java methods)

[Hi-Fi]

There are that kind of E2E tests already, and those can be added more. They're running with Maven.

But as said, there are plenty of tests for this functionality created, do have to check carefully if those new ones provide anything new.

[shan-96]

My E2E cases are still failing in production even when I deploy snapshot version of robotframework-javalibCore. I am still figuring out what is different

[shan-96]

@Hi-Fi I have corrected the junit caught an error please check

[Hi-Fi]

Created keywords were not returning anything, but were used manually to check things (from RF documentation): https://github.com/robotframework/JavalibCore/blob/develop/src/test/robotframework/acceptance/annotationlibrary.robot

[shan-96]

I feel the problem is with KeywordInvoker.java

Can you explain what does the method KeywordInvoker.getParameterNamesFromMethod() do?

[Hi-Fi]

For kwargs parser (KeywordCollector) needs the names of the arguments. Preferably these can be get from annotation (which also marks vararg with * and kwargs with **). This is change from old library, as it was not so strict with correct format. If annotation is not present, names are collected using Reflection and vararg and kwargs guessed by positions and types.

[shan-96]

Understood. But with this new way of parsing keyword and other args should or should not cases recorded in old library fail?

When annotation is not present and names are collected with reflection, I am testing a few cases

1. pass a dict from robot keyword to a java method as argument
2. pass a fixed number of string arguments from robot keyword to java argument

Both of them fail

Here is my workflow.

JAVA KEYWORD MAPPING FUNCTION

public void exampleKeyword(Map<String, String> map):
    for (String key: map.keySet())
        System.out.println(key + " " + map.get(key));

ROBOT LIBRARY FUNCTION

*** Settings ***
Library           Collections
Library           Remote    http://${env_var}/RPC2/Remote_lib_name   javalib

*** Test Case ***
testRobotKeyword   ${map}   Create Dictionary    Key   Value   Key2    Value2                           
exampleKeyword     ${map}

java keyword execution throws NPE as map is empty

Similar case with fixed number of String args is also failing

[Hi-Fi]

Major version was increased, as there's changes that cause existing things to fail without changes. You can see those e.g. at the Seleniumlibrary-java I linked earlier.

KeywordCollector doesn't determine the vararg/kwarg presence from arguments anymore, but names parsed with that getParameterNamesFromMethod.

With your example you're not passing kwargs, you're passing map as an argument which is different case. That might be something that's not working (only map that's wanted to be used as map and not kwargs) without annotation. Also Jrobotremoteserver has to be newest one, as there was also breaking changes.

[shan-96]

Understood. But you agree that these cases should pass right? Jrobotremoteserver is latest (4.0.0)

[shan-96]

I think there should be a different logical workflow to determine args and collect them when var args and kw args are both null / empty

[Hi-Fi]

I think that case shouldn't pass, because passing map as an argument is different thing than passing kwargs, and now it seems to be failing with that. I think your example would work if you just give names arguments in the keyword call instead of Map, like:
exampleKeyword Key=Value Key2=Value2

It also might work by using annotation library and stating that the argument is "normal" argument. I think now it gets confused because of name thinks argument is kwargs, and there's map instead coming. And that part (I think) is handled already before JavalibCore or jroborremoteserver, as those have kwargs already at first call (https://github.com/robotframework/JavalibCore/blob/develop/src/main/java/org/robotframework/javalib/library/RobotFrameworkDynamicAPI.java#L53). If using remote, you can also check the determined names by making REST call to remote server:

curl --location --request POST 'http://${env_var}/RPC2/Remote_lib_name' \
--header 'Content-Type: application/xml' \
--data-raw '<?xml version="1.0"?>
<methodCall>
   <methodName>get_keyword_arguments</methodName>
      <params>
      	<param><value><string> exampleKeyword </string></value></param>
      </params>
</methodCall>'

[shan-96]

Gviing names arguments in the keyword call instead of Map, like:
exampleKeyword Key=Value Key2=Value2 is not feasible as map size can be huge

[Hi-Fi]

Then you're not really using kwargs, but keyword with argument with type of map. And that (I think) can be done by annotation and giving that argument name without "**". With Classpathlibrary it's not working, and not sure if it's even possible to distinguish between kwargs and map arguments (especialy when there's only one argument).

[shan-96]

Yes, argument name is without ** which is why in KeywordOverload.java (jrobotremoteserver)
args is not null with one non empty map but kwargs are null... but the argument collector parses it differently.

[Hi-Fi]

If it works OK without jrobotremoteserver (so directly using library), please create issue about it to jrobotremoteserver. If it doesn't work directly, please create new issue to this repo. I think this is one special case that should be thought separately and not within this issue.

[shan-96]

I will create a new issue, but let me first understand your point. You mean when robot passes an argument with just one map, we must treat it as a kwarg right?

[Hi-Fi]

If you give map as an argument, you pass object that has type of map, and it should be treated up to keyword as single object.

If you pass named arguments, those are put to map by RF, and those are handles as kwargs. And if all given named arguments match to keyword arguments, keyword receives only empty map as JavalibCore has moved those named arguments already out from RF generated map to actual arguments.

So you can never just give plain map and think that as kwargs. It can only be value of named arg, but nothing more.

[shan-96]

It doesnt work without jrobotremoteserver, I will create the issue here