[py] Fix converting list of tuples to str in send_keys

[GeyseR]

Description

Fixes a bug in converting a tuple of values passed to WebElement.send_keys method

Motivation and Context

Detecting local files is broken in the 4.0.0.b2.post1 version

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)

[GeyseR]

Should I write a regression test for this PR?

[AutomatedTester]

Please add a regression test for this please.

…

On Mar 29, 2021, 11:22 AM +0100, Sergey Fursov ***@***.***>, wrote: Should I write a regression test for this PR? — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or unsubscribe.

[GeyseR]

hi @AutomatedTester, actually tests for the remote webdriver fail with the actual implementation and my update fixes them, they just are not running in GH actions.

P.S. I found it too difficult to run the project's test suite locally. I ended up running pytest manually without using bazel at all. Did you consider writing an instruction on how to run tests properly using bazel?

[AutomatedTester]

The tests are passing, what do you mean they aren't? I just looked at https://github.com/SeleniumHQ/selenium/runs/2318807884?check_suite_focus=true#step:9:121 and it's passing when I run locally.

davidburns in ~/development/selenium on trunk λ bazel test //py:test-remote-test/selenium/webdriver/common/upload_tests.py
INFO: Invocation ID: 41001f34-9660-4861-a7bb-cbdde03f4f74
INFO: Analyzed target //py:test-remote-test/selenium/webdriver/common/upload_tests.py (1 packages loaded, 15 targets configured).
INFO: Found 1 test target...
Target //py:test-remote-test/selenium/webdriver/common/upload_tests.py up-to-date:
  bazel-bin/py/test-remote-test/selenium/webdriver/common/upload_tests.py-runner.py
  bazel-bin/py/test-remote-test/selenium/webdriver/common/upload_tests.py
INFO: Elapsed time: 11.927s, Critical Path: 10.44s
INFO: 2 processes: 2 local.
INFO: Build completed successfully, 2 total actions
//py:test-remote-test/selenium/webdriver/common/upload_tests.py          PASSED in 9.9s

Executed 1 out of 1 test: 1 test passes.
There were tests whose specified size is too big. Use the --test_verbose_time
INFO: Build completed successfully, 2 total actions

The same tests are passing with this patch too so there is clearly a difference in the way you're using it which is why another test would be awesome

[GeyseR]

hmm, could it be some cache issue or something with the bazel build/test workflow? I guess you already have locally copied files locally that are not cleared during the tests or something like this.

here is what I see in debug mode in IDE:

1 - the code is in its initial state
2 - value is a tuple as we define it with the def send_keys(*value): statement
3 - converting a tuple to the str results to obviously wrong string-in-parenthesis value
4 - as a result, None is in the list of local files and copying file to remote doesn't happen

Finally, I have the following error in two upload tests:

upload_tests.py::test_can_upload_file[remote] FAILED                     [ 50%]
test/selenium/webdriver/common/upload_tests.py:23 (test_can_upload_file[remote])
driver = <selenium.webdriver.remote.webdriver.WebDriver (session="9e86a7d6-a0e1-4a97-874e-8bc250f315e1")>
pages = <conftest.pages.<locals>.Pages object at 0x10a5fdbb0>

    def test_can_upload_file(driver, pages):
    
        pages.load("upload.html")
        current_dir = os.path.dirname(os.path.realpath(__file__))
>       driver.find_element(By.ID, 'upload').send_keys(os.path.join(current_dir, "test_file.txt"))

upload_tests.py:28: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
../../../../selenium/webdriver/remote/webelement.py:539: in send_keys
    self._execute(Command.SEND_KEYS_TO_ELEMENT,
../../../../selenium/webdriver/remote/webelement.py:693: in _execute
    return self._parent.execute(command, params)
../../../../selenium/webdriver/remote/webdriver.py:369: in execute
    self.error_handler.check_response(response)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

self = <selenium.webdriver.remote.errorhandler.ErrorHandler object at 0x10a5fdb50>
response = {'status': 400, 'value': '{"value":{"error":"invalid argument","message":"File not found: /Users/geyser/coding/opensou...//marionette/content/error.js:310:5\\ninteraction.uploadFiles@chrome://marionette/content/interaction.js:541:13\\n"}}'}

    def check_response(self, response):
        """
        Checks that a JSON response from the WebDriver does not have an error.
    
        :Args:
         - response - The JSON response from the WebDriver server as a dictionary
           object.
    
        :Raises: If the response contains an error message.
        """
        status = response.get('status', None)
        if not status or status == ErrorCode.SUCCESS:
            return
        value = None
        message = response.get("message", "")
        if isinstance(status, int):
            value_json = response.get('value', None)
            if value_json and isinstance(value_json, str):
                import json
                try:
                    value = json.loads(value_json)
                    if len(value.keys()) == 1:
                        value = value['value']
                    status = value.get('error', None)
                    if not status:
                        status = value["status"]
                        message = value["value"]
                        if not isinstance(message, str):
                            value = message
                            message = message.get('message')
                    else:
                        message = value.get('message', None)
                except ValueError:
                    pass
    
        if status in ErrorCode.NO_SUCH_ELEMENT:
            exception_class = NoSuchElementException
        elif status in ErrorCode.NO_SUCH_FRAME:
            exception_class = NoSuchFrameException
        elif status in ErrorCode.NO_SUCH_WINDOW:
            exception_class = NoSuchWindowException
        elif status in ErrorCode.STALE_ELEMENT_REFERENCE:
            exception_class = StaleElementReferenceException
        elif status in ErrorCode.ELEMENT_NOT_VISIBLE:
            exception_class = ElementNotVisibleException
        elif status in ErrorCode.INVALID_ELEMENT_STATE:
            exception_class = InvalidElementStateException
        elif status in ErrorCode.INVALID_SELECTOR \
                or status in ErrorCode.INVALID_XPATH_SELECTOR \
                or status in ErrorCode.INVALID_XPATH_SELECTOR_RETURN_TYPER:
            exception_class = InvalidSelectorException
        elif status in ErrorCode.ELEMENT_IS_NOT_SELECTABLE:
            exception_class = ElementNotSelectableException
        elif status in ErrorCode.ELEMENT_NOT_INTERACTABLE:
            exception_class = ElementNotInteractableException
        elif status in ErrorCode.INVALID_COOKIE_DOMAIN:
            exception_class = InvalidCookieDomainException
        elif status in ErrorCode.UNABLE_TO_SET_COOKIE:
            exception_class = UnableToSetCookieException
        elif status in ErrorCode.TIMEOUT:
            exception_class = TimeoutException
        elif status in ErrorCode.SCRIPT_TIMEOUT:
            exception_class = TimeoutException
        elif status in ErrorCode.UNKNOWN_ERROR:
            exception_class = WebDriverException
        elif status in ErrorCode.UNEXPECTED_ALERT_OPEN:
            exception_class = UnexpectedAlertPresentException
        elif status in ErrorCode.NO_ALERT_OPEN:
            exception_class = NoAlertPresentException
        elif status in ErrorCode.IME_NOT_AVAILABLE:
            exception_class = ImeNotAvailableException
        elif status in ErrorCode.IME_ENGINE_ACTIVATION_FAILED:
            exception_class = ImeActivationFailedException
        elif status in ErrorCode.MOVE_TARGET_OUT_OF_BOUNDS:
            exception_class = MoveTargetOutOfBoundsException
        elif status in ErrorCode.JAVASCRIPT_ERROR:
            exception_class = JavascriptException
        elif status in ErrorCode.SESSION_NOT_CREATED:
            exception_class = SessionNotCreatedException
        elif status in ErrorCode.INVALID_ARGUMENT:
            exception_class = InvalidArgumentException
        elif status in ErrorCode.NO_SUCH_COOKIE:
            exception_class = NoSuchCookieException
        elif status in ErrorCode.UNABLE_TO_CAPTURE_SCREEN:
            exception_class = ScreenshotException
        elif status in ErrorCode.ELEMENT_CLICK_INTERCEPTED:
            exception_class = ElementClickInterceptedException
        elif status in ErrorCode.INSECURE_CERTIFICATE:
            exception_class = InsecureCertificateException
        elif status in ErrorCode.INVALID_COORDINATES:
            exception_class = InvalidCoordinatesException
        elif status in ErrorCode.INVALID_SESSION_ID:
            exception_class = InvalidSessionIdException
        elif status in ErrorCode.UNKNOWN_METHOD:
            exception_class = UnknownMethodException
        else:
            exception_class = WebDriverException
        if not value:
            value = response['value']
        if isinstance(value, str):
            raise exception_class(value)
        if message == "" and 'message' in value:
            message = value['message']
    
        screen = None
        if 'screen' in value:
            screen = value['screen']
    
        stacktrace = None
        st_value = value.get('stackTrace') or value.get('stacktrace')
        if st_value:
            if isinstance(st_value, str):
                stacktrace = st_value.split('\n')
            else:
                stacktrace = []
                try:
                    for frame in st_value:
                        line = self._value_or_default(frame, 'lineNumber', '')
                        file = self._value_or_default(frame, 'fileName', '<anonymous>')
                        if line:
                            file = "%s:%s" % (file, line)
                        meth = self._value_or_default(frame, 'methodName', '<anonymous>')
                        if 'className' in frame:
                            meth = "%s.%s" % (frame['className'], meth)
                        msg = "    at %s (%s)"
                        msg = msg % (meth, file)
                        stacktrace.append(msg)
                except TypeError:
                    pass
        if exception_class == UnexpectedAlertPresentException:
            alert_text = None
            if 'data' in value:
                alert_text = value['data'].get('text')
            elif 'alert' in value:
                alert_text = value['alert'].get('text')
            raise exception_class(message, screen, stacktrace, alert_text)
>       raise exception_class(message, screen, stacktrace)
E       selenium.common.exceptions.InvalidArgumentException: Message: File not found: /Users/geyser/coding/opensource/selenium/py/test/selenium/webdriver/common/test_file.txt
E       Stacktrace:
E       WebDriverError@chrome://marionette/content/error.js:181:5
E       InvalidArgumentError@chrome://marionette/content/error.js:310:5
E       interaction.uploadFiles@chrome://marionette/content/interaction.js:541:13

../../../../selenium/webdriver/remote/errorhandler.py:234: InvalidArgumentException

It would be interesting to know what do you have locally in the variables from my screenshot? Maybe adding some debug output in code.

[AutomatedTester]

The reason we use Bazel is to prevent "it works on my machine" errors. It only allows specific env vars to be used. You can see them here https://github.com/SeleniumHQ/selenium/blob/trunk/.bazelrc#L33-L42

The error you're getting is that Marionette can't find the file. Is this happening with bazel or via a different method? It's not saying the wrong type or anything is being passed it, Firefox is saying that it can't find the file at all in your last comment.

[GeyseR]

The reason we use Bazel is to prevent "it works on my machine" errors.

How do you handle problems like "Bazel" doesn't work on my machine"? :)
I've already spent several hours trying to run it but I don't like to spend more my and probably someone else time to run it...

I use a different method, running pytest directly and connecting to ready-to-use selenium/standalone-firefox-debug docker image instead of building it locally.
The test doesn't work only with the old code, my update fixes the code and test passes.
I still think that it is something wrong with running tests using your method - marionette always able to find the file, because it is running locally and it always has access to the local filesystem. Using my method - selenium is running using the "real" remote installation inside docker and not copying file inside the send_keys method breaks the test as it should. Could you please debug the test and make sure that it works as intended?

[diemol]

FWIW, selenium/standalone-firefox-debug is using Grid 3. IIRC, remote tests in Python with Bazel will actually build the Grid 4 and run tests against it.

[GeyseR]

IIRC, remote tests in Python with Bazel will actually build the Grid 4 and run tests against it.

but it still has access to the local filesystem, right?

[diemol]

IIRC, remote tests in Python with Bazel will actually build the Grid 4 and run tests against it.

but it still has access to the local filesystem, right?

Yes, everything is running locally, no docker containers are involved.

[AutomatedTester]

As @diemol says, we build and give the python tests to the server that is built as part of the task.

You can see the bazel target in https://github.com/SeleniumHQ/selenium/blob/trunk/py/BUILD.bazel#L347-L369

The data part that has the string starting with // means build the this target and put it into the build directory so it can be used.

This can all be solved by you adding a failing test so that we make sure that we never regress this code. You will see it start failing on github actions too then we definitely know that it's not a "works on my machine" type problem

[GeyseR]

This can all be solved by you adding a failing test so that we make sure that we never regress this code. You will see it start failing on github actions too then we definitely know that it's not a "works on my machine" type problem

What I'm trying to say, is that the current test suite already catches the bug in code and it fails when the "real" remote webdriver is used for testing. Let me try to think how to assert this in tests without reworking the tests running method...

[GeyseR]

hey guys, I've added an additional test for checking that the actual file upload happened to the machine with the remote webdriver. Could you please check?

[sonarqubecloud]

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
No Duplication information