-
-
Notifications
You must be signed in to change notification settings - Fork 456
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
allow downloading of recent versions of chrome #537
allow downloading of recent versions of chrome #537
Conversation
need to patch File parsing of response, currently breaking because it's returning a file with .exe in the name |
lets wait to merge, I'm still seeing some fails in CI/CD versions, I've almost resolved |
I've tested my little heart out, and it seems to be working locally and within my CI/CD pipelines. We should run the testing suite to confirm everything is good to go |
i adjusted so that the failing 114.0.5735.198 test pass |
…n/webdriver_manager into de-upgrade-chrome-version
Hi @david-engelmann. Tried out your repo and branch to get around the url issue, But I get this error. Any ideas?
|
@tonycapone I believe that should work now |
Thanks @david-engelmann, could you add a specific test for chrome v115? |
I've merged fixes for chrome. Please test and provide a feedback |
@SergeyPirogov It's unclear how I define test cases for certain chrome versions. the test failing on 114 is because that version (114.0.5735.198) doesn't exist. I was looking at your merge conflicts and I couldn't follow the logic for most of them. In particular, the removal of support for the short form version 115 and the build version 115.0.5735 etc. I can remove that logic but it will require users to use the patched version of their browser which changes frequently. |
@david-engelmann I've fixed this in github actions by installing latest Google Chrome for every os |
@SergeyPirogov Perfect! I adjusted the parameters and os_type logic. The only conflict I left in was the short form support for version xxx and version xxx.x.xxxx. |
@david-engelmann please point to exect lines of code, because its hard to get what do you mean by short version |
|
||
raise Exception(f"No such driver version {browser_version} for {platform}") | ||
def get_latest_release_for_version(self, version): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@SergeyPirogov Here is the function for latest release by short version ie. 115
return v | ||
return None | ||
|
||
def get_latest_patch_version_for_build_version(self, build_version): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@SergeyPirogov Here is the function for latest release by build version ie. 115.0.5735
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So, why do we need it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@SergeyPirogov The old LATEST_RELEASE url supported those style lookups. ie. https://chromedriver.storage.googleapis.com/LATEST_RELEASE_113 and https://chromedriver.storage.googleapis.com/LATEST_RELEASE_114.0.5735. It was suggested we retain that for versions over 115. Without that, users will have to pass in the full browser version ie. version="114.0.5735.90", which can change frequently. Either way, I'm fine including for backwards compatibility, I'm fine with removing for simplicity and add an exception that explains as suggested by you
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd postpone this, because what I see know is that browser version is equal to the one mentioned in their driver website. Moreover I think that fix should be applied in method get_url_for_version_and_platform
Code merged in #539 |
going to close out and submit a new pr for the short version support feature discussed here |
Thanks @david-engelmann for all your hard work on this PR! |
Sorry, Chrome changed driver url path. please re-revise content. |
This is my error. Driver path 115.0.5790.110 is not valid from above URL site. Traceback (most recent call last): |
@grapefruiteater It appears that the version of Chrome you have is not supported by chrome-for-testing |
I am experiencing this issue as well. How can I fix it? |
@Derpitron I would install a version of chrome that matches a version supported by chrome-for-testing. This link shows everything they support. The main issue is the patch version -> 115.0.5790.110. 102 is the last version supported by google-for-testing |
the hosting path for chrome has changed for recent versions