[web] update chrome version #20470
[web] update chrome version #20470
Conversation
lib/web_ui/dev/driver_manager.dart
Outdated
| 'drivers', | ||
| browser, | ||
| preferredChromeDriverVersion, | ||
| '${browser}driver-${io.Platform.operatingSystem.toString()}')); |
There was a problem hiding this comment.
Why do we need to append the operating system name? Is it possible to have drivers for different operating systems on the same computer? If not, then we can remove this part of the path because the name of the browser is already encoded in the path on line 46.
There was a problem hiding this comment.
We can't change this name. I already created the cipd packages. This subdirectory is in the package.
There was a problem hiding this comment.
I actually copied the naming from Chrome. When you download a chrome from chromium downloads and unzip, you will see the first directory would be chrome-linux or chrome-mac. We are following that naming.
nturgut
force-pushed
the
increase_chrome_version
branch
from
9a95181
to
5967fd7
Compare
| ## Please refer to README's `Upgrade Browser Version` section for more details | ||
| ## on how to update the version number. | ||
| required_driver_version: | ||
| ## Make sure the major version of the binary in `browser_lock.yaml` is the |
There was a problem hiding this comment.
This no longer needs to refer to browser_lock.yaml as everything is in the same file. Instead it should point to the relevant keys in this file that need to be in sync with this value.
| required_driver_version: | ||
| ## Make sure the major version of the binary in `browser_lock.yaml` is the | ||
| ## same for Chrome. | ||
| chrome: '84' |
There was a problem hiding this comment.
For consistency with geckodriver we should either move geckodriver under required_driver_version, or put chromedriver next to geckodriver without required_driver_version. The former is simpler than the latter.
There was a problem hiding this comment.
As discussed offline, reviewer will address the changes in a separate PR.
|
It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat. Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. |
nturgut
added
the
waiting for tree to go green
|
Engine looks green on the dashboard. Merging the pr |
* update chrome version, should be merged after recipe changes * changing directory to use chrome driver in LUCI * changing directory path's order * add cipd packages's chrome version for mac * addressing reviewer comments * more documentation. update readme * remove late since it didn't build. remove distinction in paths for local and LUCI. * addressing reviewer comments. (non-null fields needs rechanging) * addressing reviewer comments. adding 2.6 on files missing the notation
increase_chrome_version, should be merged after recipe changes.
Note: currently used for testing the recipe.
fixes: flutter/flutter#63633