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

CLI: Download WebDriver binary according to version of browser under test #10451

Open
jugglinmike opened this issue Apr 12, 2018 · 19 comments
Open

Comments

@jugglinmike
Copy link
Contributor

Currently, the CLI always downloads the very latest release of the WebDriver
implementation for the browser under test. Because users may specify any
browser at any version, this may not always be appropriate.

The CLI should begin test execution by querying the browser binary for its
version and determining the WebDriver version based on that information. For
recent browser releases, the CLI could continue to dynamically fetch whatever
version happens to considered "latest" at the time of invocation. This would
mainly be a way to prevent older browsers from being tested with
known-incompatible releases (e.g. Firefox 54 and Geckodriver 20).

@jgraham does that sound logical to you?

@jugglinmike
Copy link
Contributor Author

I'm also curious to know what @kereliuk thinks

@jgraham
Copy link
Contributor

jgraham commented Apr 12, 2018

For geckodriver this seems like unnecessary complexity. If for some reason you want to test a Firefox older than latest ESR then you are pretty much on your own. That seems fine; I don't think we should spend effort supporting niche use cases like that.

Chromedriver ,may be a different matter since it seems to suffer from frequent changes to CDP (if I understand correctly, which I may not). How do users know which Chromedriver version they should use with each Chrome release?

@foolip
Copy link
Member

foolip commented Apr 12, 2018

The CLI should begin test execution by querying the browser binary for its
version

This is valuable in its own right, and I think it'd be a good idea to include browser and driver version in results reports, or at least as sideband information to @Hexcles' new results receiver.

@Hexcles
Copy link
Member

Hexcles commented Apr 12, 2018

ChromeDriver seems to have a narrower window of supported Chrome versions, @kereliuk ?

@kereliuk
Copy link
Contributor

ChromeDriver does historically have a pretty narrow window (especially recently). We can easily create a mapping between ChromeDriver versions and Chrome versions from the documentation on the releases so this doesn't seem to hard to support for ChromeDriver at least (I'm not sure how often we will run into this though).

I agree @foolip in that I think the browser and driver version should be included in the results report. This is probably a good place to start since we need to grab these to dynamically download the WebDriver binaries.

Currently we are trying to to plan changes around this and have a more reliable release pattern for ChromeDriver, possibly packaging it with Chrome itself in the regular release channels. I can update once we have more concrete plans :)

@jgraham
Copy link
Contributor

jgraham commented Apr 13, 2018

FWIW if we want to log the browser and driver version, the right place to put that is in the run_info dict; it's logged as part of the test_start action and is used to do things like set the right expectation data where that's being used.

That data isn't currently in the wptreport.json files but there's no reason it couldn't be.

@gsnedders
Copy link
Member

The narrowest case is Edge, where MicrosoftWebDriver is build specific.

I remember talking to @InstyleVII about that this at TPAC, and about providing some machine-readable index as https://developer.microsoft.com/en-us/microsoft-edge/tools/webdriver/ does. The awkward case here is this only lists "Insiders" supporting "Current Insiders Fast Ring Build".

@InstyleVII
Copy link
Contributor

InstyleVII commented Apr 18, 2018

@gsnedders is correct, it can be build specific (but not always). It's a good rule of thumb to match the build/branch/flavor of Windows and MicrosoftWebDriver.exe.

That said in our next release (RS5) which is due sometime this fall we are changing how we ship WebDriver (more detailed message here).

The gist is that MicrosoftWebDriver.exe will now be a Windows Feature on Demand (FoD). The downside is that we'll no longer be hosting a standalone download of the binary. Due to the build matching requirement allowing users to download builds and attempt to match on their own has lead to a lot of incoming bugs where users are simply using mismatched versions. The upside however is that FoD's can match build/branch/flavor and are updating automatically when the Windows build is updated (so between major releases for end users) and can be installed via dism or via the UI.

I've been selfhosting it on internal RS5 builds and it's extremely nice to have it just be updated auotmatically with my build (and also as a side effect of the flavor matching, we're now shipping x64 as well).

@CalebRouleau
Copy link

CalebRouleau commented Oct 23, 2018

Hi, for ChromeDriver we have what I think is a perfect solution for you. See this email for details

https://groups.google.com/forum/#!topic/chromedriver-users/HvlKWqv34es

John Chen is owning this work.

I think we could potentially also backfill ChromeDriver versions for M69, M68, etc simply by copying the ChromeDriver binary that we think is most suitable for those versions over to a properly-named folder. (These builds would not respond properly to chromedriver --version command, but I doubt that is necessary)

Also, note that ChromeDriver is now built and archived alongside Chrome inside gs://chrome-unsigned Google Cloud Storage bucket for every new versioned-numbered build. So if you are already getting Chrome binary from there then you should also get ChromeDriver binary from there. (Note that that bucket has access restrictions that we may need to work out.) This may help for testing M71 and M72 before we publish ChromeDriver versions for those to here https://chromedriver.storage.googleapis.com/index.html

@foolip
Copy link
Member

foolip commented Oct 23, 2018

This is great, thanks @CalebRouleau! The oldest Chrome that we run for wpt.fyi is 70, so no need to backfill for us at least.

@lukebjerring
Copy link
Contributor

While not strictly necessary, having those legacy browsers/drivers, for major releases, could prove useful in weird edge-cases or information hunting.

@foolip
Copy link
Member

foolip commented Jan 24, 2019

Quoting from @JohnChen0 and web-platform-tests/wpt.fyi#406 (comment), there's a way to do this for ChromeDriver now:

@Hexcles
Copy link
Member

Hexcles commented Feb 20, 2019

This is kind of blocked by the lack of ChromeDriver releases for the unstable (dev) channel: https://crbug.com/chromedriver/2656

stephenmcgruer added a commit that referenced this issue Sep 24, 2020
This fixes a long standing annoyance where we will blindly accept any
'chromedriver' binary, even if it isn't compatible with the Chrome
version under test. Instead, check that the versions of Chrome and
ChromeDriver mostly match. ChromeDriver is allowed to lag behind by a
release version, to allow for using a local build of Chromium.

See #10451
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Sep 26, 2020
…accepting found binary, a=testonly

Automatic update from web-platform-tests
[wpt] Check ChromeDriver version before accepting found binary (#25657)

This fixes a long standing annoyance where we will blindly accept any
'chromedriver' binary, even if it isn't compatible with the Chrome
version under test. Instead, check that the versions of Chrome and
ChromeDriver mostly match. ChromeDriver is allowed to lag behind by a
release version, to allow for using a local build of Chromium.

See web-platform-tests/wpt#10451
--

wpt-commits: 28bab3e887e6ea4e4398d8e069fd1f92421f1e95
wpt-pr: 25657
@stephenmcgruer
Copy link
Contributor

I've recently landed code for both Chrome and EdgeChromium to detect when the webdriver version doesn't match the selected browser binary version, and to offer to install the matching webdriver if it doesn't match.

Above @jgraham mentions this isn't relevant for Firefox, and I believe it isn't relevant for Safari either (we seem to pull the browser version from the safaridriver, so they sorta have to match), so I think we can probably close this?

I'll give @Hexcles @jgraham @gsnedders a few days to object, otherwise I'll close this fixed.

@jgraham
Copy link
Contributor

jgraham commented Sep 28, 2020

No objections.

gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this issue Sep 28, 2020
…accepting found binary, a=testonly

Automatic update from web-platform-tests
[wpt] Check ChromeDriver version before accepting found binary (#25657)

This fixes a long standing annoyance where we will blindly accept any
'chromedriver' binary, even if it isn't compatible with the Chrome
version under test. Instead, check that the versions of Chrome and
ChromeDriver mostly match. ChromeDriver is allowed to lag behind by a
release version, to allow for using a local build of Chromium.

See web-platform-tests/wpt#10451
--

wpt-commits: 28bab3e887e6ea4e4398d8e069fd1f92421f1e95
wpt-pr: 25657

UltraBlame original commit: 4b8dfda63eda039224cf964afd32717f71cdd8c8
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this issue Sep 28, 2020
…accepting found binary, a=testonly

Automatic update from web-platform-tests
[wpt] Check ChromeDriver version before accepting found binary (#25657)

This fixes a long standing annoyance where we will blindly accept any
'chromedriver' binary, even if it isn't compatible with the Chrome
version under test. Instead, check that the versions of Chrome and
ChromeDriver mostly match. ChromeDriver is allowed to lag behind by a
release version, to allow for using a local build of Chromium.

See web-platform-tests/wpt#10451
--

wpt-commits: 28bab3e887e6ea4e4398d8e069fd1f92421f1e95
wpt-pr: 25657

UltraBlame original commit: 4b8dfda63eda039224cf964afd32717f71cdd8c8
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this issue Sep 28, 2020
…accepting found binary, a=testonly

Automatic update from web-platform-tests
[wpt] Check ChromeDriver version before accepting found binary (#25657)

This fixes a long standing annoyance where we will blindly accept any
'chromedriver' binary, even if it isn't compatible with the Chrome
version under test. Instead, check that the versions of Chrome and
ChromeDriver mostly match. ChromeDriver is allowed to lag behind by a
release version, to allow for using a local build of Chromium.

See web-platform-tests/wpt#10451
--

wpt-commits: 28bab3e887e6ea4e4398d8e069fd1f92421f1e95
wpt-pr: 25657

UltraBlame original commit: 4b8dfda63eda039224cf964afd32717f71cdd8c8
@Hexcles
Copy link
Member

Hexcles commented Sep 28, 2020

IIUC, Firefox does have the same issue; James is just saying that it's a minor annoyance (apologies if I misunderstood). I'd say the severity is no different from Chrome/Edge's cases, and might in fact be more annoying when compared with Chrome/Edge which can now download the appropriate version of webdriver binaries without having to purge _venv.

Maybe we can leave this open and perhaps add a note at the top saying this no longer affects Chrome/Edge now.

sidvishnoi pushed a commit to sidvishnoi/gecko-webmonetization that referenced this issue Oct 1, 2020
…accepting found binary, a=testonly

Automatic update from web-platform-tests
[wpt] Check ChromeDriver version before accepting found binary (#25657)

This fixes a long standing annoyance where we will blindly accept any
'chromedriver' binary, even if it isn't compatible with the Chrome
version under test. Instead, check that the versions of Chrome and
ChromeDriver mostly match. ChromeDriver is allowed to lag behind by a
release version, to allow for using a local build of Chromium.

See web-platform-tests/wpt#10451
--

wpt-commits: 28bab3e887e6ea4e4398d8e069fd1f92421f1e95
wpt-pr: 25657
@jgraham
Copy link
Contributor

jgraham commented Oct 1, 2020

I think release geckodriver will support all versions of the browser that it's reasonable to test. So from that point of view it's not an issue for Firefox. We have occasionally seen issues where the (quasi-unsupported) marionette library we're using makes a backwards-incompatible change in the way it invokes specific commands, but that's not really solvable in the same way.

@Hexcles
Copy link
Member

Hexcles commented Oct 1, 2020 via email

@gsnedders
Copy link
Member

I believe it isn't relevant for Safari either (we seem to pull the browser version from the safaridriver, so they sorta have to match), so I think we can probably close this?

You have to be using the same safaridriver version as you are Safari, and safaridriver isn't distributed separately to Safari, so there's never anything to download.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests