Using timeout from FirefoxBinary inside GeckoDriverService, XpiDriver…#6117
Using timeout from FirefoxBinary inside GeckoDriverService, XpiDriver…#6117grigaman wants to merge 27 commits intoSeleniumHQ:masterfrom
Conversation
shs96c
left a comment
shs96c
left a comment
There was a problem hiding this comment.
It would be nice to get a test that shows the timeout is being passed to the constructor.
| ImmutableMap<String, String> environment) { | ||
| try { | ||
| GeckoDriverService service = new GeckoDriverService(exe, port, args, environment); | ||
| Duration timeout = (firefoxBinary == null) ? Duration.ofSeconds(45) : Duration.ofMillis(firefoxBinary.getTimeout()); |
There was a problem hiding this comment.
You change the default timeout from 20 to 45 seconds here. I realise that the XpiDriverService uses the timeout from the binary (which is 45), but it's preferable to not change behaviour here.
| ImmutableMap<String, String> environment) { | ||
| try { | ||
| GeckoDriverService service = new GeckoDriverService(exe, port, args, environment); | ||
| Duration timeout = (firefoxBinary == null) ? Duration.ofSeconds(45) : Duration.ofMillis(firefoxBinary.getTimeout()); |
There was a problem hiding this comment.
Why not calculate the timeout when someone calls usingFirefoxBinary on the builder?
| * @param timeout The timeout for connecting to the browser. | ||
| * @throws IOException If an I/O error occurs. | ||
| */ | ||
| public GeckoDriverService( |
There was a problem hiding this comment.
Changing the public constructor breaks compatibility. Please add a second, and have the original constructor delegate to that. You may mark the older constructor as deprecated so we can then get back to just one in the release after 3.14.
|
I like the idea of this. Thank you for taking the time to prepare the PR. Looking forward to merging it in. |
This lays more of the groundwork for the core package to no longer be split (required for Java 9 modules) without us having to do a lot of additional work or move classes around.
…to old IE versions.
Fix test to work with newer byte-buddy Unblocks byte-buddy update from 1.8.3 to 1.8.12+. 1.8.12 (at least) requires the generated class has access to its super-interfaces, and will otherwise fail with messages like: java.lang.IllegalAccessError: class org.openqa.selenium.remote.RemoteWebDriver$ByteBuddy$CuMcRkYl cannot access its superinterface org.openqa.selenium.remote.BaseAugmenterTest$MagicNumberHolder
Currently the end-point /grid/api/hub/status
does not provide information about the nodes
attached to it and also the busy/free status
per browser flavor on each of the nodes.
Enriching the end-point to provide node info
when invoked with the option
/grid/api/hub/status?configuration=nodes
Sample payload as below:
{
"nodes": [
{
"Id": "http://192.168.1.6:5555",
"browsers": [
{
"browser": "safari",
"slots": {
"busy": 0,
"total": 1
}
},
{
"browser": "chrome",
"slots": {
"busy": 0,
"total": 5
}
},
{
"browser": "firefox",
"slots": {
"busy": 0,
"total": 5
}
}
]
}
],
"success": true
}
This is a change from previous behavior, where such dialogs were handled via the standard alert handling commands. The W3C WebDriver Specification demands that onBeforeUnload dialogs be dismissed automatically, with no input allowed for the user. This may be a breaking change for some users who are currently relying on the alert-handling commands to handle these alerts; however, not implementing this behavior makes the driver not compliant with the specificiation. Users in that state will have no choice but to modify their code to accomodate the new, spec-compliant behavior.
Fixes SeleniumHQ#6064 Signed-off-by: Alex Rodionov <p0deje@gmail.com>
…piDriverService" This reverts commit 1b23277
|
I thought on your comment and found better solution to customize timeout. I updated DriverService.Builder. Could you please look at PR #6151. If you think that this way also better. reject this PR. |
|
Marking as "blocked on external" until we land #6025 |
|
I like the idea to make this timeout configurable. But I feel discomport about changing GeckoDriverService public constructor signature. I'd prefer implementing private setTimeout() method to call from service builder. |
|
Alexei it was created the new pull request #6151 for updating all Driver service to support configurable timeout. I think this is better solution then this one. |
|
Closing in favour of #6151 |
9 participants
Fixing method waitUntilAvailable in org.openqa.selenium.firefox.XpiDriverService and in org.openqa.selenium.firefox.GeckoDriverService (get timeout from FirefoxBinary)
Xin the preceding checkbox, I verify that I have signed the Contributor License Agreement