The client implementation for Chromedriver storage #131
Conversation
|
@mykola-mokhnach. This is cool. I would also have a reference of the new storage client as part of install.js to fetch all required versions, as well as within chromedriver.js which would choose the appropriate binary to launch based on the chrome version / android system webview version within automation |
mykola-mokhnach
force-pushed
the
cd_download
branch
from
d464645 to
559b083
Compare
|
@dpgraham @imurchie @jlipps @KazuCocoa What do you think would be the best place/strategy to invoke chromedriver sync. @umutuzgur proposed to add additional server arguments, so if Appium is started with these then the sync is executed. My idea was to trigger that automatically in case the driver cannot match existing chromedriver executables to the webview version. I would be happy to consider your proposals |
|
I vote for not putting it behind a flag. |
|
Automatically, mykola's idea, sounds good to me, but I believe some cloud providers would like to disable the downloading feature. |
|
@KazuCocoa That was my main reason as well. Cloud providers usually have control over Appium's behaviour usually over the server flags, e.g |
|
ok, I will consider automagical driver(s) download in case there is no match for the current webview/chrome version AND if relaxed security in general (or particularly this feature) is allowed. Any objections? |
|
|
||
| const log = logger.getLogger('ChromedriverStorageClient'); | ||
|
|
||
| async function walkDir (dir) { |
There was a problem hiding this comment.
this seems like the kind of thing that should already exist in the node std lib or if not, that we could put into appium-support/fs
There was a problem hiding this comment.
might be, I'll do it later
| * @param {string} content - Release notes of the corresponding chromedriver | ||
| * @returns {AdditionalDriverDetails} | ||
| */ | ||
| parseNotes (content) { |
There was a problem hiding this comment.
let's hope they don't change their notes format 🤞
sounds like a good approach to me. |
mykola-mokhnach
changed the title
|
Ok, added the final changes. The fact that the corresponding server feature is enabled will be checked in the corresponding driver (aka BaseDriver instance). Then it will only be necessary to set the |
| } | ||
| const workingCds = cds.filter((cd) => { | ||
| const versionObj = semver.coerce(cd.minChromeVersion); | ||
| return versionObj && chromeVersion.major === versionObj.major; |
There was a problem hiding this comment.
why use ”==“ ，the chromedriver ability to support multiple versions,A range should be set
There was a problem hiding this comment.
Since Chrome 73 the versions of Chrome and Chromedriver have been aligned. Chrome 74.x.x requires Chromedriver 74.x.x. Chrome 76.x.x requires Chromedriver 76.x.x
This is the POC implementation of Chromedriver storage client. The client is able to retrieve all chromedrivers from the storage, which match to the server OS and architecture. It is also possible to limit the drivers selection by particular chromedriver version or by minimum browser version.
I'm still not quite sure on where to invoke chromedrivers synchronisation, but any feedback on the current implementation or feature requests are welcome