Support user keywords with both embedded and normal arguments

[JockeJarre]

it could be useful to support embedded arguments mixed with normal arguments

Number of ${animals} should be
    [Arguments]    ${count}
    # ...

that could be called like

Number of horses should be    2
Number of dogs should be    3

Or if you want to use it for library aliases:

${alias}.Call keyword on different connections using the alias
    [Arguments]    &{args}

Called like this:

aliasone.Call Keyword on different connections using the alias    &{Parameters}
aliastwo.Call Keyword on different connections using the alias    &{Parameters}

[pekkaklarck]

If I remember correctly, the reasons why we decided not to support normal arguments along with embedded args when this functionality was added (#370) were the following:

• This feature was implemented mainly for ATDD/BDD usage and there you in general don't want to have normal arguments.
• Related to the above, there were no concrete use cases where mixing arguments would have provided benefits.
• Simpler implementation.
• Simpler usage. The whole embedded args was new and we didn't want to make it more complicated.
• Backwards compatibility if someone had created keyword with variables in their name. Such usage was prohibited later (Disallow using keyword with embedded arguments as normal keywords #1962) and isn't thus anymore relevant.

I don't think implementing this would be overly complicated, so I guess the only really valid reason these days is the lack of concrete use cases. Please add such examples to comments!

[JockeJarre]

As mentioned above, the use case I am thinking of, is when working with remote connections. Maybe I am miss using the use-case??? Will explain further now.

We use Remote Libraries a lot, by using Remote Library connections we connect to the system to test many times, at the same time, as different users. The simulated users process the workflow as if it would be using the Windows Fat client.

when you call the same remote library keywords within the same test, but for different connections, you need to prepend the remote library alias in front of the keyword

connection1.Run something in the connection    Parameter1    Parameter2    Parameter3
connection2.Run something in the connection    Parameter1    Parameter2    Parameter3

otherwise RF will (of course) complain that it doesn't know which "Run something in the connection" keyword to use. All fine.

If you want to create a resource user keyword which uses these connection keywords you can do like this: (as long as you do not have any additional parameters)

${Connection}.Do Complex things on a connection
    Run Keyword    ${Connection}.Run something in the connection     Parameter1    Parameter2    Parameter3

or you need to embed all additional parameters in the keyword name...

If you need additional parameters, currently the only chance you have is to create it like this:

Do Complex things on a connection
    [Arguments]    ${Connection}    ${Param1}    ${Param2}    ${Param3}
    Run Keyword    ${Connection}.Run something in the connection    ${Param1}    ${Param2}    ${Param3}

When you call it, it would look like this:

Do Complex things on a connection    connection1    Parameter1    Parameter3    Parameter4

So my wishful thinking would be:

${Connection}.Do Complex things on a connection
    [Arguments]    ${Param1}    ${Param2}    ${Param3}
    Run Keyword    ${Connection}.Run something in the connection     ${Param1}    ${Param2}    ${Param3}

The connection name coming from the Keyword name and the rest through standard parameters:

connection1.Do Complex things on a connection      Parameter1      Parameter3      Parameter4

[Serhiy1]

I think this feature will help make more readable test cases. Typically when dealing with optional arguments

For example, in the suites I currently maintain, I have a lot of keywords that take 1 or 2 required arguments and then several or more optional arguments. Making the required arguments embedded and the other arguments optional would increase readability and convey intent closer.

# custom keyword with no embedded arguments 
${event}=     Record Evidence    ${user}    ${user_asset}    title=vulnerability report

# custom keyword with mixed embedded and optional arguments.
${event}=    ${user} Records Evidence on ${user_asset}    title=vulnrability report

[JockeJarre]

@pekkaklarck, can you give me a hint what needs to be changed to get this done? I am happy to support as usual.

[yggdr]

If you want another use case I can give you an example of what I'm currently working on.

It's an existing Robot code base, I'm trying to refactor it and make it a bit easier to use. As part of that I would like to introduce embedded arguments, so I can have one Keyword like Login to router ${routername} as ${user}, instead of 10 Keywords Login to router A1 as user admin, Login to router B2 as user root, etc, or one Keyword Login to router router=A1 user=root with two mandatory arguments. The latter isn't a nice English sentence, which is part of the appeal of Robot.

But sometimes we want to give those Keywords extra arguments that overwrite some default values we otherwise assume. By having the main arguments embedded and the optional arguments as classic ones this could be nicely done, and I was disappointed when my testing revealed that they cannot be mixed yet.

[pekkaklarck]

Thanks for the use case examples! I especially like the idea of having a keyword that can be normally used with easy-to-understand embedded arguments but that accepts optional arguments that can be used in special cases.

I think adding this functionality is fine and I add this issue to RF 5.1 scope. @JockeJarre already promised to look at creating a PR.

[pekkaklarck]

@JockeJarre asked where to get started so I did a quick prototype. The following changes seem to work pretty well but there certainly can be things that aren't taken into account (e.g. dry-run). The code isn't particularly elegant either, but that's mainly due to the original code using inheritance badly. Code can be cleaned up, but a bigger and more important task in the beginning would be adding tests and fixing existing. Embedded args are mainly tested in atest/robot/keywords/embedded_arguments.robot.

diff --git a/src/robot/running/userkeyword.py b/src/robot/running/userkeyword.py
index 725aac009..e5598f859 100644
--- a/src/robot/running/userkeyword.py
+++ b/src/robot/running/userkeyword.py
@@ -56,8 +56,6 @@ class UserLibrary:
         embedded = EmbeddedArguments(kw.name)
         if not embedded:
             return UserKeywordHandler(kw, self.name)
-        if kw.args:
-            raise DataError('Keyword cannot have both normal and embedded arguments.')
         return EmbeddedArgumentsHandler(kw, self.name, embedded)
 
     def _log_creating_failed(self, handler, error):
diff --git a/src/robot/running/userkeywordrunner.py b/src/robot/running/userkeywordrunner.py
index 7516e2f47..d9d7f87ad 100644
--- a/src/robot/running/userkeywordrunner.py
+++ b/src/robot/running/userkeywordrunner.py
@@ -231,15 +231,15 @@ class EmbeddedArgumentsRunner(UserKeywordRunner):
         self.embedded_args = list(zip(handler.embedded_args, match.groups()))
 
     def _resolve_arguments(self, args, variables=None):
-        # Validates that no arguments given.
-        self.arguments.resolve(args, variables)
-        if not variables:
-            return []
-        return [(n, variables.replace_scalar(v)) for n, v in self.embedded_args]
+        if variables:
+            self.embedded_args = [(n, variables.replace_scalar(v))
+                                  for n, v in self.embedded_args]
+        return super()._resolve_arguments(args, variables)
 
-    def _set_arguments(self, embedded_args, context):
+    def _set_arguments(self, args, context):
+        super()._set_arguments(args, context)
         variables = context.variables
-        for name, value in embedded_args:
+        for name, value in self.embedded_args:
             variables['${%s}' % name] = value
         context.output.trace(lambda: self._trace_log_args_message(variables))

[JockeJarre]

Thanks pekkaklarck, I added your changes and also amended tests for the new feature, but I got some results I did not understand.

I also found how to do the same for python modules, is that what you expected?

Currently it is stored here: https://github.com/JockeJarre/robotframework/tree/topic/mixed_arguments

Shall I still pack it into a PR, for you to check, or how shall we proceed?

[pekkaklarck]

Could you clarify what you didn't understand? We can discuss further on Slack as well.

Did you mean you got mixed arguments working with Python based keywords? How does that work in practice?

[pekkaklarck]

Are you @JockeJarre working with this issue? If not, I'll move it to RF 5.2 scope.

[JockeJarre]

yes, please move it.

[pekkaklarck]

This has been brought up few times lately and, for example, @Snooz82 had some interesting use case for this functionality. I hope we get this done in RF 6.1 and raised priority accordingly.

[pekkaklarck]

This feature still lacks documentation.

[oboehmer]

Thanks for implementing this, @yanne ! Is it correct that this feature currently only works for user keywords and not for library keywords?

[pekkaklarck]

Yes, this only works with user keywords. Need to update the title accordingly. Support for library keywords can be added later if it's considered useful.

[oianson]

This is a nice feature. Showing interest for support for library keywords too :)

[pekkaklarck]

If you'd like to see this support with library keywords, please submit an issue referencing this issue. With library keywords we need to decide how to pass different arguments to the actual Python function. With user keywords that was easy because we get embedded arguments from the name and others from [Arguments], but with Python functions we need to pass all arguments used in the data as "normal" arguments. Probably it could work like this:

@keyword('Number of ${animals} should be') 
def example(animals, count):
    ...

We could easily detect the argument count mismatch (one embedded to the name, two accepted by the function) and require user to pass the missing ones as normal arguments. That would make the same usages as with user keywords possible:

Number of horses should be    2
Number of dogs should be    3

There are, however, design decisions to be made at least about the named argument usage:

Number of horses should be    count=2                    # This probably should be allowed
Number of dogs should be    animals=horses    count=2    # This probably should cause an error