Skip to content

Commit a05f167

Browse files
pekkaklarckpekkaklarck
committed
Prefer exact match over embedded match with setup/teardown and Run Keyword
This only affects a situation where - a name of an executed keyword contains a variable and - the name matches a different keyword depending on are variables replaced or not. After this change an exact match after variables have been resolved has a precedence regardless what the original name would match. Fixes #5444.
1 parent 9526d10 commit a05f167

File tree

7 files changed

+107
-44
lines changed

7 files changed

+107
-44
lines changed

‎atest/robot/running/setup_and_teardown_using_embedded_arguments.robot

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,22 @@ Resource atest_resource.robot
44

55
*** Test Cases ***
66
Suite setup and teardown
7-
Should Be Equal ${SUITE.setup.status} PASS
8-
Should Be Equal ${SUITE.teardown.status} PASS
7+
Should Be Equal ${SUITE.setup.name} Embedded \${LIST}
8+
Should Be Equal ${SUITE.teardown.name} Embedded \${LIST}
99

1010
Test setup and teardown
11-
Check Test Case ${TESTNAME}
11+
${tc} = Check Test Case ${TESTNAME}
12+
Should Be Equal ${tc.setup.name} Embedded \${LIST}
13+
Should Be Equal ${tc.teardown.name} Embedded \${LIST}
1214

1315
Keyword setup and teardown
14-
Check Test Case ${TESTNAME}
16+
${tc} = Check Test Case ${TESTNAME}
17+
Should Be Equal ${tc[0].setup.name} Embedded \${LIST}
18+
Should Be Equal ${tc[0].teardown.name} Embedded \${LIST}
19+
20+
Exact match after replacing variables has higher precedence
21+
${tc} = Check Test Case ${TESTNAME}
22+
Should Be Equal ${tc.setup.name} Embedded not, exact match instead
23+
Should Be Equal ${tc.teardown.name} Embedded not, exact match instead
24+
Should Be Equal ${tc[0].setup.name} Embedded not, exact match instead
25+
Should Be Equal ${tc[0].teardown.name} Embedded not, exact match instead

‎atest/robot/standard_libraries/builtin/run_keyword.robot

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -61,10 +61,17 @@ With keyword accepting embedded arguments as variables containing objects
6161

6262
With library keyword accepting embedded arguments as variables containing objects
6363
${tc} = Check Test Case ${TEST NAME}
64-
Check Run Keyword With Embedded Args ${tc[0]} Embedded "\${OBJECT}" in library Robot
64+
Check Run Keyword With Embedded Args ${tc[0]} Embedded "\${OBJECT}" in library Robot
6565
Check Run Keyword With Embedded Args ${tc[1]} Embedded object "\${OBJECT}" in library Robot
6666

67-
Run Keyword In For Loop
67+
Exact match after replacing variables has higher precedence than embedded arguments
68+
${tc} = Check Test Case ${TEST NAME}
69+
Check Run Keyword ${tc[1]} Embedded "not"
70+
Check Log Message ${tc[1][0][0][0]} Nothing embedded in this user keyword!
71+
Check Run Keyword ${tc[2]} embedded_args.Embedded "not" in library
72+
Check Log Message ${tc[2][0][0]} Nothing embedded in this library keyword!
73+
74+
Run Keyword In FOR Loop
6875
${tc} = Check Test Case ${TEST NAME}
6976
Check Run Keyword ${tc[0, 0, 0]} BuiltIn.Log hello from for loop
7077
Check Run Keyword In UK ${tc[0, 2, 0]} BuiltIn.Log hei maailma

‎atest/testdata/running/setup_and_teardown_using_embedded_arguments.robot

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,8 @@ Suite Setup Embedded ${LIST}
33
Suite Teardown Embedded ${LIST}
44

55
*** Variables ***
6-
@{LIST} one ${2}
6+
@{LIST} one ${2}
7+
${NOT} not, exact match instead
78

89
*** Test Cases ***
910
Test setup and teardown
@@ -14,6 +15,11 @@ Test setup and teardown
1415
Keyword setup and teardown
1516
Keyword setup and teardown
1617

18+
Exact match after replacing variables has higher precedence
19+
[Setup] Embedded ${NOT}
20+
Exact match after replacing variables has higher precedence
21+
[Teardown] Embedded ${NOT}
22+
1723
*** Keywords ***
1824
Keyword setup and teardown
1925
[Setup] Embedded ${LIST}
@@ -22,3 +28,11 @@ Keyword setup and teardown
2228

2329
Embedded ${args}
2430
Should Be Equal ${args} ${LIST}
31+
32+
Embedded not, exact match instead
33+
No Operation
34+
35+
Exact match after replacing variables has higher precedence
36+
[Setup] Embedded ${NOT}
37+
No Operation
38+
[Teardown] Embedded ${NOT}

‎atest/testdata/standard_libraries/builtin/embedded_args.py

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,3 +11,8 @@ def embedded_object(obj):
1111
print(obj)
1212
if obj.name != "Robot":
1313
raise AssertionError(f"'{obj.name}' != 'Robot'")
14+
15+
16+
@keyword('Embedded "not" in library')
17+
def embedded_not():
18+
print("Nothing embedded in this library keyword!")

‎atest/testdata/standard_libraries/builtin/run_keyword.robot

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,12 @@ With library keyword accepting embedded arguments as variables containing object
7373
Run Keyword Embedded "${OBJECT}" in library
7474
Run Keyword Embedded object "${OBJECT}" in library
7575

76-
Run Keyword In For Loop
76+
Exact match after replacing variables has higher precedence than embedded arguments
77+
VAR ${not} not
78+
Run Keyword Embedded "${not}"
79+
Run Keyword Embedded "${{'NOT'}}" in library
80+
81+
Run Keyword In FOR Loop
7782
[Documentation] FAIL Expected failure in For Loop
7883
FOR ${kw} ${arg1} ${arg2} IN
7984
... Log hello from for loop INFO
@@ -131,3 +136,6 @@ Embedded "${arg}"
131136
Embedded object "${obj}"
132137
Log ${obj}
133138
Should Be Equal ${obj.name} Robot
139+
140+
Embedded "not"
141+
Log Nothing embedded in this user keyword!

‎src/robot/libraries/BuiltIn.py

Lines changed: 32 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@
3131
DotDict, escape, format_assign_message, get_error_message, get_time, html_escape,
3232
is_falsy, is_list_like, is_truthy, Matcher, normalize, normalize_whitespace,
3333
parse_re_flags, parse_time, plural_or_not as s, prepr, safe_str, secs_to_timestr,
34-
seq2str, split_from_equals, timestr_to_secs
34+
seq2str, split_from_equals, timestr_to_secs, unescape
3535
)
3636
from robot.utils.asserts import assert_equal, assert_not_equal
3737
from robot.variables import (
@@ -2149,11 +2149,10 @@ def run_keyword(self, name, *args):
21492149
can be a variable and thus set dynamically, e.g. from a return value of
21502150
another keyword or from the command line.
21512151
"""
2152+
ctx = self._context
2153+
name, args = self._replace_variables_in_name(name, args, ctx)
21522154
if not isinstance(name, str):
21532155
raise RuntimeError("Keyword name must be a string.")
2154-
ctx = self._context
2155-
if not (ctx.dry_run or self._accepts_embedded_arguments(name, ctx)):
2156-
name, args = self._replace_variables_in_name([name, *args])
21572156
if ctx.steps:
21582157
data, result, _ = ctx.steps[-1]
21592158
lineno = data.lineno
@@ -2169,26 +2168,41 @@ def run_keyword(self, name, *args):
21692168
with ctx.paused_timeouts:
21702169
return kw.run(result, ctx)
21712170

2172-
def _accepts_embedded_arguments(self, name, ctx):
2173-
# KeywordRunner.run has similar logic that's used with setups/teardowns.
2174-
if "{" in name:
2175-
runner = ctx.get_runner(name, recommend_on_failure=False)
2176-
return hasattr(runner, "embedded_args")
2177-
return False
2178-
2179-
def _replace_variables_in_name(self, name_and_args):
2180-
resolved = self._variables.replace_list(
2181-
name_and_args,
2171+
def _replace_variables_in_name(self, name, args, ctx):
2172+
match = search_variable(name)
2173+
if not match or ctx.dry_run:
2174+
return unescape(name), args
2175+
if match.is_list_variable():
2176+
return self._replace_variables_in_name_with_list_variable(name, args, ctx)
2177+
# If the matched runner accepts embedded arguments, use the original name
2178+
# instead of the one where variables are already replaced and converted to
2179+
# strings. This allows using non-string values as embedded arguments also
2180+
# in this context. An exact match after variables have been replaced has
2181+
# a precedence over a possible embedded match with the original name, though.
2182+
# TODO: This functionality exists also in 'KeywordRunner.run'. Reuse that to
2183+
# avoid duplication. We probably could pass an argument like 'dynamic_name=True'
2184+
# to 'Keyword.run', but then it would be better if 'Run Keyword' would support
2185+
# 'NONE' as a special value to not run anything similarly as setup/teardown.
2186+
replaced = ctx.variables.replace_scalar(name, ignore_errors=ctx.in_teardown)
2187+
runner = ctx.get_runner(replaced, recommend_on_failure=False)
2188+
if hasattr(runner, "embedded_args"):
2189+
return name, args
2190+
return replaced, args
2191+
2192+
def _replace_variables_in_name_with_list_variable(self, name, args, ctx):
2193+
# TODO: This seems to be the only place where `replace_until` is used.
2194+
# That functionality should be removed from `replace_list` and implemented
2195+
# here. Alternatively we could disallow passing name as a list variable.
2196+
resolved = ctx.variables.replace_list(
2197+
[name, *args],
21822198
replace_until=1,
2183-
ignore_errors=self._context.in_teardown,
2199+
ignore_errors=ctx.in_teardown,
21842200
)
21852201
if not resolved:
21862202
raise DataError(
2187-
f"Keyword name missing: Given arguments {name_and_args} resolved "
2203+
f"Keyword name missing: Given arguments {[name, *args]} resolved "
21882204
f"to an empty list."
21892205
)
2190-
if not isinstance(resolved[0], str):
2191-
raise RuntimeError("Keyword name must be a string.")
21922206
return resolved[0], resolved[1:]
21932207

21942208
@run_keyword_variant(resolve=0, dry_run=True)

‎src/robot/running/bodyrunner.py

Lines changed: 22 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -81,31 +81,35 @@ def __init__(self, context, run=True):
8181

8282
def run(self, data, result, setup_or_teardown=False):
8383
context = self._context
84-
runner = self._get_runner(data.name, setup_or_teardown, context)
84+
if setup_or_teardown:
85+
runner = self._get_setup_teardown_runner(data, context)
86+
else:
87+
runner = context.get_runner(data.name, recommend_on_failure=self._run)
8588
if not runner:
8689
return None
8790
if context.dry_run:
8891
return runner.dry_run(data, result, context)
8992
return runner.run(data, result, context, self._run)
9093

91-
def _get_runner(self, name, setup_or_teardown, context):
92-
if setup_or_teardown:
93-
# Don't replace variables in name if it contains embedded arguments
94-
# to support non-string values. BuiltIn.run_keyword has similar
95-
# logic, but, for example, handling 'NONE' differs.
96-
if "{" in name:
97-
runner = context.get_runner(name, recommend_on_failure=False)
98-
if hasattr(runner, "embedded_args"):
99-
return runner
100-
try:
101-
name = context.variables.replace_string(name)
102-
except DataError as err:
103-
if context.dry_run:
104-
return None
105-
raise ExecutionFailed(err.message)
106-
if name.upper() in ("", "NONE"):
94+
def _get_setup_teardown_runner(self, data, context):
95+
try:
96+
name = context.variables.replace_string(data.name)
97+
except DataError as err:
98+
if context.dry_run:
10799
return None
108-
return context.get_runner(name, recommend_on_failure=self._run)
100+
raise ExecutionFailed(err.message)
101+
if name.upper() in ("NONE", ""):
102+
return None
103+
# If the matched runner accepts embedded arguments, use the original name
104+
# instead of the one where variables are already replaced and converted to
105+
# strings. This allows using non-string values as embedded arguments also
106+
# in this context. An exact match after variables have been replaced has
107+
# a precedence over a possible embedded match with the original name, though.
108+
# BuiltIn.run_keyword has the same logic.
109+
runner = context.get_runner(name, recommend_on_failure=self._run)
110+
if hasattr(runner, "embedded_args") and name != data.name:
111+
runner = context.get_runner(data.name)
112+
return runner
109113

110114

111115
def ForRunner(context, flavor="IN", run=True, templated=False):

0 commit comments

Comments
 (0)