[rust] Support for beta/dev/canary browser version detection with Selenium Manager (#11239) #11334

bonigarcia · 2022-11-29T16:52:05Z

Description

This PR include support for browser version detection, considering not only stable versions, but also beta/canary/dev for Chrome, Firefox, Edge.

Motivation and Context

As discussed in TLC, the detection of non-prod browsers will be a feature that needs to be explicitly requested, e.g.:

./selenium-manager --browser chrome --browser-version beta
./selenium-manager --browser firefox --browser-version dev
./selenium-manager --browser edge --browser-version canary

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)

Checklist

I have read the contributing document.
My change requires a change to the documentation.
I have updated the documentation accordingly.
I have added tests to cover my changes.
All new and existing tests passed.

…enium Manager

titusfortner · 2022-12-01T17:09:43Z

I'm not getting what I expect from this:

SL-1495:se-mgr titusfortner$ ./selenium-manager --browser edge --browser-version canary
INFO	/Users/titusfortner/.cache/selenium/msedgedriver/mac64/107.0.1418.62/msedgedriver
SL-1495:se-mgr titusfortner$ ./selenium-manager --browser firefox --browser-version dev
INFO	/Users/titusfortner/.cache/selenium/geckodriver/mac64/0.32.0/geckodriver
SL-1495:se-mgr titusfortner$ ./selenium-manager --browser chrome --browser-version beta
INFO	/Users/titusfortner/.cache/selenium/chromedriver/mac64/108.0.5359.71/chromedriver
SL-1495:se-mgr titusfortner$ ./selenium-manager --browser chrome --browser-version dev
ERROR	Wrong browser/driver version
SL-1495:se-mgr titusfortner$ ./selenium-manager --browser chrome --browser-version canary
INFO	/Users/titusfortner/.cache/selenium/chromedriver/mac64/108.0.5359.71/chromedriver
SL-1495:se-mgr titusfortner$ ./selenium-manager --browser firefox --browser-version nightly
INFO	/Users/titusfortner/.cache/selenium/geckodriver/mac64/0.32.0/geckodriver
SL-1495:se-mgr titusfortner$ ./selenium-manager --browser edge --browser-version beta
INFO	/Users/titusfortner/.cache/selenium/msedgedriver/mac64/107.0.1418.62/msedgedriver
SL-1495:se-mgr titusfortner$ ./selenium-manager --browser edge
INFO	/Users/titusfortner/.cache/selenium/msedgedriver/mac64/107.0.1418.62/msedgedriver

bonigarcia · 2022-12-01T17:33:58Z

I'm not sure what you were expecting, but this PR includes extra commands to discover the version of non-prod browsers. A good way to understand a bit better what is happening is running the commands using --debug.

titusfortner · 2022-12-01T18:50:29Z

I'm expecting the driver version to be the correct one for the browser version I'm requesting.
I have Chrome Canary installed on my machine, it is not v108. Is it not supposed to get a compliant driver? Also it does not support dev channel.

titusfortner · 2022-12-01T19:05:24Z

Oh, except I renamed it so manager wouldn't be able to find it, lol.
Also looks like beta channel hasn't updated to 109. Ok

If I'm specifying a version and it doesn't exist, I expect to get an error, not a fall-back.
Should support dev channel?
This is what I'm getting with Canary in the right place:

L-1495:macos titusfortner$ ./selenium-manager --browser chrome --browser-version canary --debug
DEBUG	Using shell command to find out chrome version
DEBUG	Running sh command: "/Applications/Google\\ Chrome\\ Canary.app/Contents/MacOS/Google\\ Chrome\\ Canary --version"
DEBUG	Output { status: ExitStatus(unix_wait_status(0)), stdout: "Google Chrome 110.0.5451.0 canary\n", stderr: "" }
DEBUG	The version of chrome is 110.0.5451.0
DEBUG	Detected browser: chrome 110
DEBUG	Reading chromedriver version from https://chromedriver.storage.googleapis.com/LATEST_RELEASE_110
DEBUG	starting new connection: https://chromedriver.storage.googleapis.com/
DEBUG	No cached session for DnsName(DnsName(DnsName("chromedriver.storage.googleapis.com")))
DEBUG	Not resuming any session
DEBUG	Using ciphersuite TLS13_AES_256_GCM_SHA384
DEBUG	Not resuming
DEBUG	TLS1.3 encrypted extensions: [Protocols([6832])]
DEBUG	ALPN protocol is Some(b"h2")
DEBUG	Ticket saved
DEBUG	Ticket saved
ERROR	Wrong browser/driver version

chromedriver storage will never have canary, you need to get the driver from chromium storage: https://chromedriver.chromium.org/chromedriver-canary

bonigarcia · 2022-12-02T09:29:35Z

Regarding 1, in my opinion, Selenium Manager should make the best effort to provide the best driver possible. Throwing an error because the browser detection is failing is not the best choice, IMO, since the LATEST driver is likely to work with the underlying browser. In the worst case, the test will fail due to browser-driver incompatibility, but that's the fail I would expect (since the important thing is the users want to driver browsers, not the driver version itself). What we can do to improve this is to write a WARN log with Selenium Manager (e.g., the browser version cannot be detected, trying with the latest driver version or similar), and capture this log in the bindings and re-log it to the user.

Regarding 2, I don't know if it is worth it, since driving a canary browser is something very few people will do.

Regarding 3, Selenium Manager is throwing an error since chromedriver 110 (stable) has yet to be released. But to be coherent with 1), I think this case should fallback to LATEST, instead of throwing that error. I can implement this for the next release.

diemol · 2022-12-02T10:26:09Z

Regarding 1, in my opinion, Selenium Manager should make the best effort to provide the best driver possible. Throwing an error because the browser detection is failing is not the best choice, IMO, since the LATEST driver is likely to work with the underlying browser. In the worst case, the test will fail due to browser-driver incompatibility, but that's the fail I would expect (since the important thing is the users want to driver browsers, not the driver version itself). What we can do to improve this is to write a WARN log with Selenium Manager (e.g., the browser version cannot be detected, trying with the latest driver version or similar), and capture this log in the bindings and re-log it to the user.

Regarding 2, I don't know if it is worth it, since driving a canary browser is something very few people will do.

Regarding 3, Selenium Manager is throwing an error since chromedriver 110 (stable) has yet to be released. But to be coherent with 1), I think this case should fallback to LATEST, instead of throwing that error. I can implement this for the next release.

I agree on points 1 and 3. But maybe we should look into the Canary thing. I see more people wanting to test with unreleased versions to detect bugs early.

bonigarcia · 2022-12-02T10:29:47Z

I agree on points 1 and 3. But maybe we should look into the Canary thing. I see more people wanting to test with unreleased versions to detect bugs early.

No problem, I can take a look on how to support the chromedriver canary for the next release.

titusfortner · 2022-12-02T13:58:03Z

Selenium Manager should make the best effort to provide the best driver possible

Only by default. If a user *specifies something and we can't provide that, it needs to error. This is just like with Capabilities class. If a person thinks they are getting one thing but are actually getting another, you end up with Type II errors, which are a bigger deal.

As far as Canary goes, I don't think it ever gets released, just beta & dev channels. Canary requires using a nightly build. I think we probably want to support beta and dev and not canary right now.

diemol · 2022-12-02T15:21:46Z

Chrome Canary can be downloaded https://www.google.com/chrome/canary/, and IIRC, it updates itself.

diemol · 2022-12-02T15:28:07Z

Another option, to reduce the pain of figuring out which build of the ChromeDriver is needed to work with the Canary browser, we could pass --disable-build-check to the latest ChromeDriver. I guess this needs to be done in the bindings?

titusfortner · 2022-12-02T16:14:08Z

Yeah, Canary download is easy. Canary *drivers are hard. Canary browser with Dev channel drivers works, but I'm not sure we want to just do that.

--disable-build-check would need to be set by the user in service options, when using the driver.

If Selenium Manager knew it was being set, though, it could do things like:

Download & use Dev driver when Canary requested (if we don't support canary natively)
Use closest driver found without updating it. (though this might also not do what the user is thinking)

Might just make more sense to require user to be explicit in the code:

DriverService ds = new ChromeDriverService.Builder()
  .withBrowserVersion("canary")
  .withDriverVersion("dev")
  .build();
Chromedriver driver = new ChromeDriver(ds, new ChromeOptions());

or

ChromeOptions() options = new ChromeOptions() ;
options.addArgument("--disable-build-check");

DriverService ds = new ChromeDriverService.Builder()
  .withDriverVersion("104")
  .build();
Chromedriver driver = new ChromeDriver(ds, options);

titusfortner · 2022-12-02T22:01:41Z

I take it back, Selenium Manager should throw the error if user provides incompatible versions *unless --disable-build-check is passed. I think that makes the implementation more consistent as well?

bonigarcia · 2022-12-06T09:43:41Z

If we decide to error when browser detection does not find the browser version (e.g., beta), it is not difficult to do it in Rust. Maybe it can be decided in the next TLC meeting, together with the rest of issues that needs a decision.

In any case, to progress in the development, I would merge this PR (and #11371, which has been created over this one).

titusfortner

When I execute this line:

./selenium-manager --browser chrome --browser-version dev

If dev doesn't exist on the machine, it returns the location of prod driver.
If dev does exist on the machine, it errors because there isn't a v110 driver released.

In the first case it should error because the browser version doesn't exist.

In the second case, I actually think it should include a warning and return v109 driver. That would mean specifically checking to see if we get No such object: chromedriver/LATEST_RELEASE_110.

diemol · 2022-12-07T10:23:28Z

Why does the PR have changes that mention IEDriver too? Wasn't that already merged?

bonigarcia · 2022-12-07T11:52:10Z

Why does the PR have changes that mention IEDriver too? Wasn't that already merged?

The changes related to IEDriverServer is just in the README, since I forgot to include it in #11279 (and the current PR updates a bit the README).

diemol · 2022-12-07T12:19:50Z

Why does the PR have changes that mention IEDriver too? Wasn't that already merged?

The changes related to IEDriverServer is just in the README, since I forgot to include it in #11279 (and the current PR updates a bit the README).

But this is making more changes than the README https://github.com/SeleniumHQ/selenium/pull/11334/files#diff-2166be5cf60b67e7f0945ccac14fbe86ae137fe7fb71914482031de897dc70f3

diemol · 2022-12-07T12:58:30Z

When I execute this line:
./selenium-manager --browser chrome --browser-version dev
If dev doesn't exist on the machine, it returns the location of prod driver. If dev does exist on the machine, it errors because there isn't a v110 driver released.

In the first case it should error because the browser version doesn't exist.

In the second case, I actually think it should include a warning and return v109 driver. That would mean specifically checking to see if we get No such object: chromedriver/LATEST_RELEASE_110.

@bonigarcia this makes sense. So, either we work on supporting dev properly or we take it out of this PR and do it in a future PR.

bonigarcia · 2022-12-07T13:02:30Z

In the second case, I actually think it should include a warning and return v109 driver. That would mean specifically checking to see if we get No such object: chromedriver/LATEST_RELEASE_110.

Yes, it is a good idea.

To simplify the development on my side, I would prefer to merge this PR as it is now and #11371 (which has been created over this one). Then, I will make the new changes over #11371 (since it includes a relevant refactoring in the Rust code).

diemol · 2022-12-07T13:08:44Z

OK, then let's merge this and follow up in the next one.

titusfortner · 2022-12-07T13:59:46Z

Can we open a new issue to track what needs to be done?

bonigarcia · 2022-12-08T09:54:29Z

Can we open a new issue to track what needs to be done?

I have just created #11382, #11383, and #11384.

[rust] Support for beta/dev/canary browser version detection with Sel…

a9fa5ff

…enium Manager

bonigarcia force-pushed the se_mgr_beta branch from 3daf3e3 to a9fa5ff Compare November 29, 2022 17:35

titusfortner requested changes Dec 6, 2022

View reviewed changes

diemol approved these changes Dec 7, 2022

View reviewed changes

bonigarcia merged commit aa8d6cf into trunk Dec 7, 2022

bonigarcia deleted the se_mgr_beta branch December 7, 2022 13:16

bonigarcia mentioned this pull request Jan 9, 2023

[🚀 Feature]: Support for browser beta/canary/dev versions in Selenium Manager #11239

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[rust] Support for beta/dev/canary browser version detection with Selenium Manager (#11239) #11334

[rust] Support for beta/dev/canary browser version detection with Selenium Manager (#11239) #11334

bonigarcia commented Nov 29, 2022 •

edited

Loading

titusfortner commented Dec 1, 2022

bonigarcia commented Dec 1, 2022

titusfortner commented Dec 1, 2022

titusfortner commented Dec 1, 2022 •

edited

Loading

bonigarcia commented Dec 2, 2022

diemol commented Dec 2, 2022

bonigarcia commented Dec 2, 2022

titusfortner commented Dec 2, 2022

diemol commented Dec 2, 2022

diemol commented Dec 2, 2022

titusfortner commented Dec 2, 2022

titusfortner commented Dec 2, 2022

bonigarcia commented Dec 6, 2022

titusfortner left a comment

diemol commented Dec 7, 2022

bonigarcia commented Dec 7, 2022 •

edited

Loading

diemol commented Dec 7, 2022

diemol commented Dec 7, 2022

bonigarcia commented Dec 7, 2022

diemol commented Dec 7, 2022

titusfortner commented Dec 7, 2022

bonigarcia commented Dec 8, 2022

[rust] Support for beta/dev/canary browser version detection with Selenium Manager (#11239) #11334

[rust] Support for beta/dev/canary browser version detection with Selenium Manager (#11239) #11334

Conversation

bonigarcia commented Nov 29, 2022 • edited Loading

Description

Motivation and Context

Types of changes

Checklist

titusfortner commented Dec 1, 2022

bonigarcia commented Dec 1, 2022

titusfortner commented Dec 1, 2022

titusfortner commented Dec 1, 2022 • edited Loading

bonigarcia commented Dec 2, 2022

diemol commented Dec 2, 2022

bonigarcia commented Dec 2, 2022

titusfortner commented Dec 2, 2022

diemol commented Dec 2, 2022

diemol commented Dec 2, 2022

titusfortner commented Dec 2, 2022

titusfortner commented Dec 2, 2022

bonigarcia commented Dec 6, 2022

titusfortner left a comment

Choose a reason for hiding this comment

diemol commented Dec 7, 2022

bonigarcia commented Dec 7, 2022 • edited Loading

diemol commented Dec 7, 2022

diemol commented Dec 7, 2022

bonigarcia commented Dec 7, 2022

diemol commented Dec 7, 2022

titusfortner commented Dec 7, 2022

bonigarcia commented Dec 8, 2022

bonigarcia commented Nov 29, 2022 •

edited

Loading

titusfortner commented Dec 1, 2022 •

edited

Loading

bonigarcia commented Dec 7, 2022 •

edited

Loading