You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Add support for setting specific version of chromedriver, including unit tests.
Existing code path would GetUrl FromChromeForTestingApi because _chromeVersionInfo was null at the time of URL construction. This updated version allows for a place in ChromeConfig to select the correct Chrome Version Info based on the incoming explicit version.
@breed052 Thanks for request. I'm not really sure about GetMatchingExplicitRequest method in general because of partially logic duplication in chrome config. Right now I'm thinking about dropping ChromeStorage using and focus only on ChromeForTestingApi. The safe solution is move work logic with chrome < 113 version to separate config, this case external library API won't change. Something like LegacyChromeConfig like it was make for LegacyEdgeConfig some time ago.
@breed052 Thanks for request. I'm not really sure about GetMatchingExplicitRequest method in general because of partially logic duplication in chrome config. Right now I'm thinking about dropping ChromeStorage using and focus only on ChromeForTestingApi. The safe solution is move work logic with chrome < 113 version to separate config, this case external library API won't change. Something like LegacyChromeConfig like it was make for LegacyEdgeConfig some time ago.
I understand the desire to split the config to a Legacy and Chrome config to simplify the logic, but I think the additional method in the interface will still be needed:
version = GetVersionToDownload(config, version);
var url = architecture.Equals(Architecture.X32) ? config.GetUrl32() : config.GetUrl64();
ChromeConfig and ChromeLegacyConfig rely on _chromeVersion or _chromeVersionInfo to build the Url either in GetVersionFromChromeForTestingApi OR in GetUrlFromChromeStorage for OSX, but in GetVersionToDownload, if the version specified is explicit, it is not set in the ChromeConfig object, so OSX LegacyChrome would fail on specific version without these changes.
Would you be ok with LegacyChromeConfig and ChromeConfig that still have the GetMatchingExplicitRequest method?
In scope of #352 I get rid of using of this variables
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Existing code path would GetUrl FromChromeForTestingApi because _chromeVersionInfo was null at the time of URL construction. This updated version allows for a place in ChromeConfig to select the correct Chrome Version Info based on the incoming explicit version.