Skip to content
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

[rust] Selenium Manager processes PATH #11597

Merged
merged 2 commits into from
Feb 1, 2023
Merged

Conversation

bonigarcia
Copy link
Member

@bonigarcia bonigarcia commented Jan 30, 2023

Description

With this PR, Selenium process the PATH, i.e., it tries to find the required driver (e.g., chromedriver 109) in the set of local directories where executable programs are located. If the driver is found but is not compatible, it shows a WARN trace (which can be captured by the bindings using the JSON output format). If found and compatible, it uses it. Here a couple of examples:

  1. Found but incompatible (chromedriver 107 in the PATH in a computer with Chrome 109):
> cargo run -- --browser chrome
	 
WARN    Incompatible release of chromedriver (version 107.0.5304.62) detected in PATH: C:\Users\boni\Documents\bat\chromedriver.exe
INFO    C:\Users\boni\.cache\selenium\chromedriver\win32\109.0.5414.74\chromedriver.exe
  1. Found and compatible (the same situation than before, but forcing to use Chrome 107):
> cargo run -- --browser chrome --browser-version 107

INFO    C:\Users\boni\Documents\bat\chromedriver.exe

Motivation and Context

This PR implements #11356 (and #11373).

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.

@titusfortner
Copy link
Member

titusfortner commented Jan 30, 2023

Excellent, this is going to make a big difference for users.

Right now in the bindings we are skipping calling the selenium manager if users pass in the location of the driver, set it with system property (Java) or on class variable (Ruby), or if the bindings find it on PATH. So with current bindings implementation, this feature won't be used.

Ideally the bindings will be able to offload finding the driver on PATH to the manager entirely. We need to be cautious because this would transition the manager to an "opt-out" solution instead of "opt-in." The main feature we need to implement before we can make this transition is #11599. Though, we may want to avoid making this transition until #11357 even.

Note: none of this conversation prevents merging this code, just commenting that we have to more carefully think through how/when we implement it in bindings.

@bonigarcia bonigarcia merged commit 7de6dec into trunk Feb 1, 2023
@bonigarcia bonigarcia deleted the se_mgr_process_path branch February 1, 2023 13:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants