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

Execute JavaScript converts arguments to strings with robot==6.1 #1843

Closed
ikseek opened this issue Jun 28, 2023 · 4 comments
Closed

Execute JavaScript converts arguments to strings with robot==6.1 #1843

ikseek opened this issue Jun 28, 2023 · 4 comments
Labels
acknowledge To be acknowledged in release notes bug priority: high rc 1
Milestone

Comments

@ikseek
Copy link

ikseek commented Jun 28, 2023

Steps to reproduce the issue

*** Settings ***
Library    SeleniumLibrary

*** Test Cases ***
Test Print Dictionary Value
    Open Browser        browser=chrome
    &{ARGS}=            Create Dictionary  key=value
    Execute JavaScript  alert(JSON.stringify(arguments))    ARGUMENTS    ${ARGS}
    Sleep    5

Expected behavior and actual behavior

When I run this robot script using robot==6.1 I see the browser window with alert message box saying:
{"0":"{'key': 'value'}"}

I was expecting to see a dictionary value that seleniumlibrary with robot==6.0.2 produced
{"0":{"key":"value"}}

Notice that currently I see a string with python representation of the dict and previously it was a dict itself.

Hint: this happens because the type hint in this line insists on WebElements and strings:

def execute_javascript(self, *code: Union[WebElement, str]) -> Any:

Robot 6.1 seems to use type hints for automatic type conversions.

Environment

Browser: Chrome (latest)
Operating System: Mac OS 13.4.1
Libraries

  • Robot Framework: 6.1
  • Selenium: 4.9.1
  • SeleniumLibrary: 6.1
  • Interpreter: cpython 3.11
@Snooz82
Copy link
Member

Snooz82 commented Jul 18, 2023

Without testing it, but reading the type hint of that keyword and its code, i would say, that the type hint of WebElement | str is simply wrong.

Python Selenium docs do say for execute_async_script for argument *args:

*args: Any applicable arguments for your JavaScript.

So there should not be any type hint it can be Union[Any, str] or so.

Ps: i am again really disappointed by the docs of Selenium itself. selenium.dev does not mention how to use args at all and even readthedocs is absolutely too lean here.
And by reading the Selenium code that is implemeting this, i must say, that it is weird too. They have a weird special handling if one of the args is a list or tuple and contains EventFiringWebElement.

However: in the end they are just calling json.dumps on whatever you hand over to *args.

So if you hand over a string, it stays a string and if you hand over any json serializable object, like a dictionary in this example it will work.

Therefore the type hint must say, that args can be anything that is json-serializable.
If SeleniumLibrary would require at least Robot Framework 5.0, you could introduce a new type hint with a custom converter.

That custom converter would try a json.dumps() on whatever it gets. and if it fails it would return a meaningful error that whatever is handed over could not be converted to a json object and therefor not be digested by Selenium.

See supported-conversions

class JsonSerializable(str):
    @classmethod
    def to_json_serializable(cls, value):
      """Any object that can be string serialised by `json.dumps()` function can be accepted.

      This may be one of `str, int, float, bool, None, Dict, List, Tuple` or a compound of those, like a list of int.
      Also custom types are possible as long as they are compatible with `json.dumps()` function."""
      try:
        json.dumps(value)
      except:
        raise ValueError(f"The given argument could not be serialized to json. Argument value: '{value}'")
      return cls(value)

ROBOT_LIBRARY_CONVERTERS = {JsonSerializable: JsonSerializable.to_json_serializable}

with this converter the type hint could just be JsonSerializable

@pekkaklarck
Copy link
Member

As the mentioned already in the description, this keyword has wrong type hints. The reason it causes issues in RF 6.1 but not earlier is:
robotframework/robotframework#4648

Using Any would probably be the easiest fix. A custom converter that would report an error if args cannot be converted to JSON would be nice but I'm not sure would it be worth the effort.

@Snooz82
Copy link
Member

Snooz82 commented Jul 22, 2023

Its not only the error message, but more importantly the documentation of the types accepted.

@emanlove emanlove added this to the v6.1.4 milestone Nov 18, 2023
@emanlove emanlove added bug priority: high acknowledge To be acknowledged in release notes and removed needs review labels Nov 18, 2023
@emanlove
Copy link
Member

Fixed with #1866.

@emanlove emanlove modified the milestones: v6.1.4, v6.2.0 Nov 19, 2023
@emanlove emanlove added the rc 1 label Nov 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
acknowledge To be acknowledged in release notes bug priority: high rc 1
Projects
None yet
Development

No branches or pull requests

4 participants