Skip to content
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.

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

Changes to supported evaluation syntax in Python 3 #3207

Closed
MrBIN89 opened this issue Jun 7, 2019 · 11 comments
Closed

Changes to supported evaluation syntax in Python 3 #3207

MrBIN89 opened this issue Jun 7, 2019 · 11 comments

Comments

@MrBIN89
Copy link

MrBIN89 commented Jun 7, 2019

Initial topic - https://groups.google.com/forum/#!topic/robotframework-users/OX5OvCmVwio

For some reasons expression Evaluate [$PAGE_COMPONENTS[k]['name'] for k in $PAGE_COMPONENTS] doesn't work for python 3.7, but works perfectly for python 2.7

Evaluating expression '[RF_VAR_PAGE_COMPONENTS [k ]['name']for k in RF_VAR_PAGE_COMPONENTS ]' failed: NameError: name 'RF_VAR_PAGE_COMPONENTS' is not defined

*** Settings ***
Documentation    Handle nested dicts

Library          Collections

*** Variables ***
&{PAGE_COMPONENTS}         component_one=&{COMPONENT_ONE_PARAMS}
                    ...    component_two=&{COMPONENT_TWO_PARAMS}
&{COMPONENT_ONE_PARAMS}    name=ComponentOne    parameter=ParameterOne
&{COMPONENT_TWO_PARAMS}    name=ComponentTwo    parameter=ParameterTwo

@{EXPECTED_NAMES}          ComponentOne    ComponentTwo

*** Test cases ***
Access To Nested Dict Values In Evaluate Keyword
    ${robot_version} =  Evaluate    robot.version.get_full_version(program=None, naked=False)    modules=robot
   Log    ${robot_version}    level=WARN
   @{component_names} =  Evaluate    [$PAGE_COMPONENTS[k]['name'] for k in $PAGE_COMPONENTS]
   # @{component_names} =  Evaluate    [${PAGE_COMPONENTS}\[k]['name'] for k in $PAGE_COMPONENTS]  -  it works for both versions, but I don't have any idea why the previous line doesn't work
   Log    ${component_names}    level=WARN

   Lists Should Be Equal    ${component_names}    ${EXPECTED_NAMES}

*** Keywords ***
d:\rf_bug_report>robot handle_nested_dicts.robot
==============================================================================
Handle Nested Dicts :: Handle nested dicts
==============================================================================
**[ WARN ] 3.1.2 (Python 2.7.13 on win32)**
[ WARN ] [u'ComponentOne', u'ComponentTwo']
Access To Nested Dict Values In Evaluate Keyword                      | PASS |
------------------------------------------------------------------------------
Handle Nested Dicts :: Handle nested dicts                            | PASS |
1 critical test, 1 passed, 0 failed
1 test total, 1 passed, 0 failed
==============================================================================
Output:  d:\rf_bug_report\output.xml
Log:     d:\rf_bug_report\log.html
Report:  d:\rf_bug_report\report.html

d:\rf_bug_report>robot3 handle_nested_dicts.robot
==============================================================================
Handle Nested Dicts :: Handle nested dicts
==============================================================================
**[ WARN ] 3.1.2 (Python 3.7.3 on win32)**
Access To Nested Dict Values In Evaluate Keyword                      | **FAIL** |
Evaluating expression '[RF_VAR_PAGE_COMPONENTS [k ]['name']for k in  RF_VAR_PAGE_COMPONENTS ]' failed: NameError: name 'RF_VAR_PAGE_COMPONENTS' is not defined
------------------------------------------------------------------------------
Handle Nested Dicts :: Handle nested dicts                            | FAIL |
1 critical test, 0 passed, 1 failed
1 test total, 0 passed, 1 failed
==============================================================================
Output:  d:\rf_bug_report\output.xml
Log:     d:\rf_bug_report\log.html
Report:  d:\rf_bug_report\report.html
@pekkaklarck
Copy link
Member

The problem isn't in the $variable syntax itself. In most cases it works exactly the same in Python 2 and Python 3.

It seems there's been some small change in how Python 3 handles local variables when list comprehension is evaluated with eval(). This can be reproduced also on the Python interpreter:

>>> d = {'a': 1, 'b': 2}
>>> [d[k] for k in d]
[1, 2]
>>> eval('[d[k] for k in d]')
[1, 2]
>>> eval('[x[k] for k in x]', {'x': d})
[1, 2]
>>> eval('[x[k] for k in x]', {}, {'x': d})
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "<string>", line 1, in <module>
  File "<string>", line 1, in <listcomp>
NameError: name 'x' is not defined

The above was run using Python 3 but with Python 2 also the last eval() would have returned [1, 2].

This change looks really strange especially when local variables work just fine when using a list comprehension without eval:

>>> def func1():
...     x = {'a': 1, 'b': 2}
...     return [x[k] for k in x]
... 
>>> def func2():
...     x = {'a': 1, 'b': 2}
...     return eval('[x[k] for k in x]')
... 
>>> func1()
[2, 1]
>>> func2()
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "<stdin>", line 3, in func2
  File "<string>", line 1, in <module>
  File "<string>", line 1, in <listcomp>
NameError: name 'x' is not defined
>>> 

And even more strange is that possible global variables change the behavior:

>>> x = {'a': 'new', 'b': 'values'}
>>> func1()
[2, 1]
>>> func2()
['values', 'new']

This is most likely somehow related to changes handling internal variables in list comprehensions in Python 3. The behavior is so strange that it looks like a bug to me, but it's possible the change is by design. Need to study this a bit more and possibly submit a bug to Python.

@pekkaklarck
Copy link
Member

pekkaklarck commented Jun 7, 2019

Notice that because the behavior comes directly from Python, we cannot really change it with Robot. Good news is that the original example can be changed to this:

@{component_names} =  Evaluate    [v['name'] for v in $PAGE_COMPONENTS.values()]

This approach also works with Python:

>>> d = {'a': 1, 'b': 2}
>>> eval('[x[k] for k in x]', {}, {'x': d})
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "<string>", line 1, in <module>
  File "<string>", line 1, in <listcomp>
NameError: name 'x' is not defined
>>> eval('[v for v in x.values()]', {}, {'x': d})
[2, 1]

I'll nevertheless leave this issue open until we know the root cause for the change in Python.

@pekkaklarck pekkaklarck changed the title $varibale syntax doesn't work for python 3 and RF 3.1 Changes to supported evaluation syntax in Python 3 Jun 7, 2019
@pekkaklarck
Copy link
Member

This seems to be a known issue in Python 3: https://bugs.python.org/issue36300. Unfortunately it seems there are no plans to fix it. Anyway, there's nothing Robot Framework can do about it.

@MisterChild
Copy link
Contributor

@pekkaklarck

Would it be possible to fix this on RFW side by adding the variables to the globals?

    def evaluate(self, expression, modules=None, namespace=None):
        [...]
        if is_string(expression) and '$' in expression:
            expression, variables = self._handle_variables_in_expression(expression)
        else:
            variables = {}
        namespace = self._create_evaluation_namespace(namespace, modules)
        namespace = dict(namespace.items() + variables.items())  #add locals to globals here
        try:
            if not is_string(expression):
                raise TypeError("Expression must be string, got %s."
                                % type_name(expression))
            if not expression:
                raise ValueError("Expression cannot be empty.")
            return eval(expression, namespace)    #ommit locals here
        except:
            raise RuntimeError("Evaluating expression '%s' failed: %s"
                               % (expression, get_error_message()))

Or are there any security issues with this?

@pekkaklarck
Copy link
Member

Does that fix this issue? Have you checked how would it work with the current code? There have been changes.

@MisterChild
Copy link
Contributor

Oh... Yes, sorry. I've seen the implementation has changed (we are using RFW 3.0.4). I'll check with latest RFW later.

@MisterChild
Copy link
Contributor

Ok. I checked in version 3.2b3dev1 and created a namespace dictionary for usage as globals in eval().

def _evaluate(expression, variable_store, modules=None, namespace=None):
    if '$' in expression:
        expression = _decorate_variables(expression, variable_store)
    ns = EvaluationNamespace(variable_store, modules, namespace)
    ns_dict = ns.namespace.copy()
    var_dict = {'RF_VAR_'+k: v for k, v in variable_store.data.items()}
    ns_dict.update(var_dict)
    return eval(expression, ns_dict, ns)

Adding this dictionary as globals and ommiting locals did work for our special dict comprehension case, but ./rundevel.py atest/testdata/standard_libraries/builtin/evaluate.robot failed completely.

Suite setup failed:
Evaluating expression 'sys.path.append(r'/home/johannesk/dev/robotframework/atest/testdata/standard_libraries/builtin')' failed: NameError: name 'sys' is not defined

Adding globals and leaving locals as they are return eval(expression, ns_dict, ns) passed our dict comprehension testcase and passed (and failed the ones that should fail) the atest tests.

In our case, the Evaluation failed with RFW 3.0.4 in Python 2.7.6 as well as RFW 3.2b3dev1 in Python 3.6.7., but the workaround fixed it in 3.2b3dev1.

Test Evaluate
    ${count}    Create Dictionary    n1=${0}    c5=${0}
    :FOR    ${i}    IN RANGE    25
        ${testvar}    Create List    c5 
        ${tmp}    Evaluate    { k: (v+1 if k in $testvar and $i%5==0 else v) for k, v in $count.items() }
        Log to Console    ${tmp}
        ${count}    Create Dictionary    &{tmp}
    END

Maybe you have other examples to check this. And I really don't know if this change will have some negative influences to other things like security or execution speed.

@MisterChild
Copy link
Contributor

MisterChild commented Feb 21, 2020

The example from above did work with the fix.

==============================================================================
[ WARN ] 3.2b3.dev1 (Python 3.6.7 on linux)
[ WARN ] ['ComponentOne', 'ComponentTwo']                                            
==============================================================================
Dict Test :: Handle nested dicts                   
==============================================================================
Access To Nested Dict Values In Evaluate Keyword                      | PASS |
------------------------------------------------------------------------------
Dict Test :: Handle nested dicts           | PASS |
1 critical test, 1 passed, 0 failed
1 test total, 1 passed, 0 failed

@lennartq
Copy link

Any plans to implement the above fix? I've bumped into this bug twice in the last few weeks. A list comprehension is just so convenient and fits on one Evaluate row. OTOH, Collections.Remove Values From List does exactly what I was trying to do.

@pekkaklarck
Copy link
Member

I think the above fix isn't compatible with automatically importing modules. If someone comes up with a fix that doesn't break any existing tests, I'd be happy to review and merge a PR.

@lennartq
Copy link

np, I know what happens now

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants