-
Notifications
You must be signed in to change notification settings - Fork 195
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
Added practical example of how to use capabilities #1218
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the patch. I think there is opportunity to tighten up the wording of this example, otherwise I think it is mostly fine. Perhaps @shs96c can take a cursory look as he is the proponent of the capabilities matching system.
I have a sense that the example guides implementations of intermediary nodes a bit too much. An intermediary node does not have to be implemented like this, even though Selenium Grid is. I would love to see a more generally written example that isn’t so specific, but I wouldn’t call this request a blocker for landing this change.
One final thing: Can you please add your name to the acknowledgement section near the bottom?
@@ -2547,6 +2547,66 @@ <h3>Processing Capabilities</h3> | |||
</ol> | |||
|
|||
</section> <!-- /Processing Capabilities --> | |||
|
|||
<aside class=example> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I feel that this example should go above 7.1 Proxy.
@@ -2547,6 +2547,66 @@ <h3>Processing Capabilities</h3> | |||
</ol> | |||
|
|||
</section> <!-- /Processing Capabilities --> | |||
|
|||
<aside class=example> | |||
<p>Capabilities are meant for defining a failsafe matrix selection of a |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/are meant for/can be used to/
<aside class=example> | ||
<p>Capabilities are meant for defining a failsafe matrix selection of a | ||
browser configuration in a distributed WebDriver environment. When | ||
communicating directly with an endpoint node (i.e. geckodriver), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Drop “(i.e. geckodriver)” and link <a>endpoint node</a>
.
<p>Capabilities are meant for defining a failsafe matrix selection of a | ||
browser configuration in a distributed WebDriver environment. When | ||
communicating directly with an endpoint node (i.e. geckodriver), | ||
<a>capabilities</a> dictionaries are only used for carrying |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/only/primarily/
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
They could also be used by (for example) the geckodriver or the safaridriver to do something like pick a version channel to use ("canary", "release"), but I don't think that any of them actually take advantage of this possibility.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also note that even in the case of an end point node, it might be useful to configure things differently between browser versions (perhaps different versions of plugins that depend on features only exposed/removed in particular browser versions)
browser configuration in a distributed WebDriver environment. When | ||
communicating directly with an endpoint node (i.e. geckodriver), | ||
<a>capabilities</a> dictionaries are only used for carrying | ||
browser-specific configuration (i.e. <var>moz:firefoxOptions</var>); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/(i.e. moz:firefoxOptions
)// as it is not defined in the spec that implementations must supply browser-specific configuration. It just so happens that Chrome and Firefox does, but this is beyond the scope of the spec.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please link to extension capabilities
here.
communicating directly with an endpoint node (i.e. geckodriver), | ||
<a>capabilities</a> dictionaries are only used for carrying | ||
browser-specific configuration (i.e. <var>moz:firefoxOptions</var>); | ||
however, when talking to an <a>intermediary node</a> multiplexer, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
intermediary node … that acts as a multiplexer
are used for selecting a node in this network that matches the capability | ||
requirements.</p> | ||
|
||
<p>If there is a requirement to have a Firefox webdriver, with no specific |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/to have a Firefox webdriver/to purvey a Firefox WebDriver session/
|
||
<p>If Firefosx is still a requirement, but the operating system is also | ||
to be taken into account and it must be either <code>linux</code> or | ||
<code>windows</code>, the requirement object would be: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would skip “If Firefosx is still a requirement” and instead say:
If the operating system is an additional requirement, and it must be either Linux or Windows, this can be defined in the
firstMatch
array, which will be matched iteratively in the defined order:
] | ||
}</pre> | ||
|
||
<p>This system allows complex matrix selections; if the requirements |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Incorrect use of semicolon.
<code>firstMatch</code> entry (that is, either | ||
<code>{ "browserName": "firefox", "setWindowRect": true }</code> or | ||
<code>{ "browserName": "safari" }</code>), but <i>always</i> making sure | ||
that <code>{ "acceptInsecureCerts": true }</code> is satisfied. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would drop this entire paragraph.
however, when talking to an <a>intermediary node</a> multiplexer, | ||
the request will likely be forwarded to another node: <a>capabilities</a> | ||
are used for selecting a node in this network that matches the capability | ||
requirements.</p> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What tends to happen is that the intermediary node expands the matrix, then iterates over the merged capabilities until it finds one that matches. The request itself is seldom sent to the node, but something semantically equivalent to one of the requested capabilities is sent.
requirements in terms of operating system, the requirement object | ||
would be:</p> | ||
|
||
<pre>{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This example is incorrect. Provided the sent payload contains an object as the value of the capabilities
property, the spec says it's fine to do what you'd like. In other words, a remote end can do what it likes when it sees an empty capabilities payload.
} | ||
}</pre> | ||
|
||
<p>If Firefosx is still a requirement, but the operating system is also |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typo
}</pre> | ||
|
||
<p>This system allows complex matrix selections; if the requirements | ||
dictate that any browser picked must supports ignoring <a>insecure TLS |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/must supports/must support/
<p>Capabilities are meant for defining a failsafe matrix selection of a | ||
browser configuration in a distributed WebDriver environment. When | ||
communicating directly with an endpoint node (i.e. geckodriver), | ||
<a>capabilities</a> dictionaries are only used for carrying |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
They could also be used by (for example) the geckodriver or the safaridriver to do something like pick a version channel to use ("canary", "release"), but I don't think that any of them actually take advantage of this possibility.
browser configuration in a distributed WebDriver environment. When | ||
communicating directly with an endpoint node (i.e. geckodriver), | ||
<a>capabilities</a> dictionaries are only used for carrying | ||
browser-specific configuration (i.e. <var>moz:firefoxOptions</var>); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please link to extension capabilities
here.
<p>Capabilities are meant for defining a failsafe matrix selection of a | ||
browser configuration in a distributed WebDriver environment. When | ||
communicating directly with an endpoint node (i.e. geckodriver), | ||
<a>capabilities</a> dictionaries are only used for carrying |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also note that even in the case of an end point node, it might be useful to configure things differently between browser versions (perhaps different versions of plugins that depend on features only exposed/removed in particular browser versions)
I have a huge commitment to wrap up on the 19th of Feb. I can get to this after then. Is that OK? |
@mercmobily there's no rush: take the time you need to do the best you can. We really appreciate the effort. |
In the meantime I’ve taken some time to document WebDriver capabilities on MDN: https://developer.mozilla.org/en-US/docs/Web/WebDriver/Capabilities Do we still think it’s worthwhile adding the examples from this PR to the specification document? If so, I can resurrect the PR if @mercmobily doesn’t have the time to finish it. |
Honest hat on, I haven't worked on webdrivers/etc. since then, and I don't think I'd be able to apply the required changes. |
@mercmobily @andreastt I would be happy to take over the work @mercmobily has started. I think this is valueable. |
Hi,
Thank you Christian -- regardless of whether this gets pushed or not
Merc.
…On Tue, 6 Nov 2018 at 05:33, Christian Bromann ***@***.***> wrote:
@mercmobily <https://github.com/mercmobily> @andreastt
<https://github.com/andreastt> I would be happy to take over the work
@mercmobily <https://github.com/mercmobily> has started. I think this is
valueable.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#1218 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/ACB7Xlf9otb-Y1EeMhOggVgb5xQDqJWMks5usK6bgaJpZM4R2kBC>
.
--
|
I would have loved to have a section such as this one 12 days ago, when I saw the specs for the first time.
It's my first attempt to edit this file... so, there is certainly room for improvement.
This resolves #1215; the text is "heavily borrowed" from that issue, thanks to @andreastt
Preview | Diff