selenoid-ui icon indicating copy to clipboard operation
selenoid-ui copied to clipboard

Clean up capabilities of non-W3C capability names

Open emilorol opened this issue 5 years ago • 14 comments

This change will allow the manual start for all major browsers including IE 11 and Edge 18.

Ideally you will keep the alwaysMatch, but then Edge will shock on it.

Note: I also update the CURL sample code to display the change

selenoid-ui-manual-sessions

emilorol avatar Nov 01 '19 16:11 emilorol

https://developer.mozilla.org/en-US/docs/Web/WebDriver/Capabilities

can you point to the document which stands that removed caps are not compliant?

More than that it explicitly saying

The first thing you need to know is that alwaysMatch/firstMatch is always wrapped inside a capabilities JSON Object, whereas desiredCapabilities/requiredCapabilities exists at the top level

lanwen avatar Nov 16 '19 23:11 lanwen

I was following another documentation and also the Selenium-Webdriver implementation.

An even the mozilla mention the list of as:

  • acceptInsecureCerts
  • browserName
  • browserVersion
  • platformName
  • pageLoadStrategy
  • proxy
  • setWindowRect
  • timeouts
  • unhandledPromptBehavior

Therefore limiting the capabilities just to the allowed properties should not brake anything. I test it locally with out issues. Also while adding firstMatch or alwaysMatch is accepted it does create an issue for Microsoft Edge. The selenoid:options are still passed, but in the desiredCapabilities without having the use the reserve name selenoid:options.

emilorol avatar Nov 17 '19 01:11 emilorol

I think you misread something in the spec. Spec says exactly the same - and firstMatch or alwaysMatch are required inside of the capabilities node. (Even in the implementation code)

https://www.w3.org/TR/webdriver/#dfn-capabilities-processing

Also, there are extensions to the standard caps which are defined in the form of <vendor>:<namespace>.

Most probably you're facing the the Edge/IE issue not implementing this spec properly and responding to the desiredCapabilities only, failing to parse capabilities according to the spec.

So there are 2 ways of fixing this: having a dedicated request for edge/ie, or delegate that job to selenoid. Summon @vania-pooh here to ask if the selenoid should ever think about vendor-specific hacks. Also, the spec says that intermediate node (selenoid in our case) should exclude their custom caps (selenoid:options in our case) and do not forward them to the node (written in the create session section). Maybe that could be an issue as well.

lanwen avatar Nov 17 '19 23:11 lanwen

So following w3c logic only

                "enableVNC": true,
                "name": "this.test.is.launched.by.curl",
                "sessionTimeout": "120s"

are non-w3c-compliant and should not be in the always/first match nodes on the top level (and they are not there)

lanwen avatar Nov 17 '19 23:11 lanwen

I was trying to avoid a conditional logic just to satisfy Edge issues with firstMatch or alwaysMatch and the current proposed solution works with all. Since you are still passing the Solenoid options in the desiredCapabilities, you are really not missing them.

emilorol avatar Nov 18 '19 01:11 emilorol

No, we are not going to maintain vendor-specific hacks. There is nothing in the spec saying that vendor-specific capabilities for intermediary node (i.e. Selenoid) should not be present in the request to endpoint node (i.e. webdriver binary).

vania-pooh avatar Nov 18 '19 03:11 vania-pooh

Hmm image

lanwen avatar Nov 18 '19 09:11 lanwen

okay, so I would try to fix that in selenoid first and then let's see what happens

lanwen avatar Nov 18 '19 11:11 lanwen

fix for https://github.com/aerokube/selenoid/issues/771 was released, it could help, can you verify that?

lanwen avatar Jan 17 '20 09:01 lanwen

I was not aware of that issue. How can I help?

emilorol avatar Jan 17 '20 14:01 emilorol

basically all we discussed here was fixed in the latest selenoid release

lanwen avatar Jan 17 '20 15:01 lanwen

I was able to test, and I can not start IE or Edge manual sessions.

Here are the errors I am getting:

2020/01/22 15:12:58 [-] [NEW_REQUEST] [unknown] [192.168.10.154]
2020/01/22 15:12:58 [-] [NEW_REQUEST_ACCEPTED] [unknown] [192.168.10.154]
2020/01/22 15:12:58 [12] [LOCATING_SERVICE] [MicrosoftEdge] [18.0]
2020/01/22 15:12:58 [12] [USING_DOCKER] [MicrosoftEdge] [18.0]
2020/01/22 15:12:58 [12] [CREATING_CONTAINER] [nexus.internal.com:8443/docker/windows:browsers]
2020/01/22 15:12:58 [12] [STARTING_CONTAINER] [nexus.internal.com:8443/docker/windows:browsers] [97e96226b25fc43dccc43c9351de672e7c815e301beb8e65daa02525677a865b]
2020/01/22 15:12:58 [12] [CONTAINER_STARTED] [nexus.internal.com:8443/docker/windows:browsers] [97e96226b25fc43dccc43c9351de672e7c815e301beb8e65daa02525677a865b] [0.13s]
2020/01/22 15:13:06 [12] [SERVICE_STARTED] [nexus.internal.com:8443/docker/windows:browsers] [97e96226b25fc43dccc43c9351de672e7c815e301beb8e65daa02525677a865b] [7.40s]
2020/01/22 15:13:06 [12] [PROXY_TO] [97e96226b25fc43dccc43c9351de672e7c815e301beb8e65daa02525677a865b] [http://172.17.0.4:5555/]
2020/01/22 15:13:06 [12] [SESSION_ATTEMPTED] [http://172.17.0.4:5555/] [1]
2020/01/22 15:13:06 [12] [SESSION_FAILED] [http://172.17.0.4:5555/] [400 Bad Request]
2020/01/22 15:13:06 [12] [REMOVING_CONTAINER] [97e96226b25fc43dccc43c9351de672e7c815e301beb8e65daa02525677a865b]
2020/01/22 15:13:06 [12] [CONTAINER_REMOVED] [97e96226b25fc43dccc43c9351de672e7c815e301beb8e65daa02525677a865b]


2020/01/22 15:13:49 [-] [NEW_REQUEST] [unknown] [192.168.10.154]
2020/01/22 15:13:49 [-] [NEW_REQUEST_ACCEPTED] [unknown] [192.168.10.154]
2020/01/22 15:13:49 [23] [LOCATING_SERVICE] [internet explorer] [11.0]
2020/01/22 15:13:49 [23] [USING_DOCKER] [internet explorer] [11.0]
2020/01/22 15:13:49 [23] [CREATING_CONTAINER] [nexus.internal.com:8443/docker/windows:browsers]
2020/01/22 15:13:49 [23] [STARTING_CONTAINER] [nexus.internal.com:8443/docker/windows:browsers] [9001b195075f12581b063f2d59ed271587cb62db1670384594ccc9bd7756686d]
2020/01/22 15:13:49 [23] [CONTAINER_STARTED] [nexus.internal.com:8443/docker/windows:browsers] [9001b195075f12581b063f2d59ed271587cb62db1670384594ccc9bd7756686d] [0.13s]
2020/01/22 15:13:56 [23] [SERVICE_STARTED] [nexus.internal.com:8443/docker/windows:browsers] [9001b195075f12581b063f2d59ed271587cb62db1670384594ccc9bd7756686d] [7.53s]
2020/01/22 15:13:56 [23] [PROXY_TO] [9001b195075f12581b063f2d59ed271587cb62db1670384594ccc9bd7756686d] [http://172.17.0.4:4444/]
2020/01/22 15:13:56 [23] [SESSION_ATTEMPTED] [http://172.17.0.4:4444/] [1]
2020/01/22 15:13:56 [23] [SESSION_FAILED] [http://172.17.0.4:4444/] [500 Internal Server Error]

This is my browsers file:

    "internet explorer": {
        "default": "11.0",
        "versions": {
            "11.0": {
                "image": "nexus.internal.com:8443/docker/windows:browsers",
                "port": "4444",
                "path": "/",
                "privileged": true,
                "env": ["SCREEN_RESOLUTION=3200x2450x24"]
            }
        }
    },
    "MicrosoftEdge": {
        "default": "18.0",
        "versions": {
            "18.0": {
                "image": "nexus.internal.com:8443/docker/windows:browsers",
                "port": "5555",
                "path": "/",
                "privileged": true,
                "env": ["SCREEN_RESOLUTION=3200x2450x24"]
            }
        }
    },

One more thing. I can start a manual session using CURL with the following body:

{ 
  "desiredCapabilities":{
    "browserName":"MicrosoftEdge", 
    "version": "18.0", 
    "enableVNC": true,
    "name": "Manual session",
    "labels": { "manual": "true"},
    "sessionTimeout": "60m"
  },
  "capabilities": {
    "browserName": "MicrosoftEdge", 
    "browserVersion": "18.0"
  }
}

emilorol avatar Jan 22 '20 15:01 emilorol

@emilorol thx for the extensive answer, we will try to investigate more on the selenoid side

lanwen avatar Jan 22 '20 20:01 lanwen

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 35.15%. Comparing base (120cd42) to head (63df8af). Report is 173 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #253   +/-   ##
=======================================
  Coverage   35.15%   35.15%           
=======================================
  Files          36       36           
  Lines         640      640           
  Branches       98       98           
=======================================
  Hits          225      225           
  Misses        362      362           
  Partials       53       53           
Flag Coverage Δ
go 35.15% <ø> (ø)
ui 25.00% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov[bot] avatar Jan 15 '24 01:01 codecov[bot]