dotnet-client icon indicating copy to clipboard operation
dotnet-client copied to clipboard

Provide a better integration with the DriverOptions mechanism of Selenium

Open hexawyz opened this issue 5 years ago • 12 comments

Description

In the current beta (4.0.0.6), Appium provides an AppiumOptions class extending DriverOptions, but overrides all its behavior in unexpected ways. (From a user perspective)

Especially, setting properties of DriverOptions will have no effect, and using DriverOptions instances that are not AppiumOptions (e.g. ChromeOptions), may fail to work.

Environment

  • Appium.WebDriver 4.0.0.6-beta, Selenium.WebDriver 3.141.0

Details

DriverOptions provide properties such as BrowserName, BrowserVersion, PlatformName & others, that will map internally to capabilities. (see here)

Classes derived from DriverOptions are expected to add their own properties (and the associated "known capability" mapping) in order to provide a clean object model to users of the type. Then, when calling ToCapabilities(), both known capabilities and additional capabilities are merged.

The choice made by Selenium authors was that "known capabilities" should be set via their C# property, and not via calls to AddAdditionalCapability. Thus, trying to call AddAdditionalCapability with a "known capability" name, could raise an error, such as in the case of ChromeOptions. (see here) We could debate whether or not this implementation is legitimate, but that is how things are today. Currently, this is implementation specific, but the validation mechanism will be directly provided by DriverOptions in Selenium 4.

Today, AppiumOptions doesn't expose any additional properties, and will ignore values set on inherited properties, as PlatformName. Considering that, AppiumDriver will set PlatformName using the AddAdditionalCapability API instead of simply setting DriverOptions.PlatformName (see here), which will with break other DriverOptions implementation, such as ChromeOptions. (see #311)

Proposal

  • Implement a more complete AppiumOptions class, that will take the base DriverOptions implementation into account.

  • Update AppiumDriver to use the PlatformName property instead of setting it via AddAdditionalCapability here.

  • Ideally, provide expose more Appium-specific capabilities as properties on AppiumOptions. This will have the benefit of making the options more discoverable, and to add strong typing.

  • Possibly, provide specifc subclasses of AppiumOptions for Adroid & iOS. e.g. AndoidOptions, IOSOptions, exposing OS-specific options as properties.

Code To Reproduce Issue

See issue #311 for an example of inconveniences

For what it's worth, I wrote a dumb DriverOptions class for the project I'm working on. This could be used as a starting base for a minimalistic implementation of DriverOptions.

public sealed class RawOptions : DriverOptions
{
    private readonly Dictionary<string, object> _additionalCapabilities = new Dictionary<string, object>();

    public RawOptions(string browserName)
    {
        BrowserName = browserName;
    }

    public override void AddAdditionalCapability(string capabilityName, object capabilityValue)
    {
        if (string.IsNullOrEmpty(capabilityName))
        {
            throw new ArgumentException("Capability name may not be null an empty string.", nameof(capabilityName));
        }

        if (IsKnownCapabilityName(capabilityName))
        {
            throw new ArgumentException(FormattableString.Invariant($"There is already an option for the {capabilityName} capability. Please use the {GetTypeSafeOptionName(capabilityName)} instead."), nameof(capabilityName));
        }

        _additionalCapabilities.Add(capabilityName, capabilityValue);
    }

    public override ICapabilities ToCapabilities()
    {
        var desiredCapabilities = GenerateDesiredCapabilities(false);

        if (GenerateLoggingPreferencesDictionary() is object loggingPreferences)
        {
            desiredCapabilities.SetCapability(CapabilityType.LoggingPreferences, loggingPreferences);
        }

        foreach (var additionalCapability in _additionalCapabilities)
        {
            desiredCapabilities.SetCapability(additionalCapability.Key, additionalCapability.Value);
        }

        return desiredCapabilities;
    }

Also, InternetExplorerOptions is a good example of a quite complete DriverOptions implementation.

hexawyz avatar Jun 18 '19 16:06 hexawyz

Until this or a similar approach is adopted and released by the Appium .NET bindings project, Appium cannot be used with the 4.0 alpha and later releases of the Selenium .NET bindings. The change that breaks the Appium .NET bindings was committed in November, 2018. The Selenium developers are ready and eager to consult on how this implementation can be achieved.

@GoldenCrystal The implication that the current DriverOptions implementation in Selenium is “illegitimate” is disappointing. The Selenium .NET maintainers are more than willing to explain the reasons behind the design choices at length and in exhaustive detail at your (or anyone else’s) discretion. Do feel free to join the Selenium IRC or Slack channel to engage in that discussion.

jimevans avatar Apr 07 '20 12:04 jimevans

@akinsolb I've just had this bug happen to me. Are there any updates to this bug? How can help fix this?

This seems like a massive blocker to Appium adoption with Selenium 4?

matthew-horrocks avatar May 18 '21 05:05 matthew-horrocks

@akinsolb hey, sorry to bug you, but I'm keen to get support for Selenium 4 added to Appium.

Does there need to be a roadmap item added somewhere? Is there discord channel or something that I can use for dev support?

matthew-horrocks avatar May 28 '21 14:05 matthew-horrocks

@matthew-horrocks I've been occupied with native mobile projects as of late. I'll look at providing a solution by the end of next week. You're more than welcome to provide a PR in the meantime.

laolubenson avatar Jun 02 '21 16:06 laolubenson

@akinsolb I'm happy to help... I just don't know where to start with this :/

matthew-horrocks avatar Jun 08 '21 08:06 matthew-horrocks

Hi @matthew-horrocks @akinsolb , were you able to make any progress? this looks like a blocker if we try to use selenium 4(I'm using beta2) together with Appium/WinAppDriver which uses appiumoptions to define capabilities.

pradeipp avatar Sep 15 '21 08:09 pradeipp

Hi @matthew-horrocks @akinsolb , were you able to make any progress? this looks like a blocker if we try to use selenium 4(I'm using beta2) together with Appium/WinAppDriver which uses appiumoptions to define capabilities.

hey :) no, I didn't make any progress with this, though it looks like work is being done under #460 :)

matthew-horrocks avatar Sep 19 '21 19:09 matthew-horrocks

Is there at least a workaround? I've just updated my Selenium packages from 3.141.0 to the Release 4.0 and got blocked by this issue. Or the only solution is to roll back to Selenium 3.x?

alazurkevych avatar Oct 14 '21 12:10 alazurkevych

@alazurkevych will see what can be done before v5 is released

laolubenson avatar Oct 16 '21 11:10 laolubenson

Any work around of this?

evilgenius786 avatar Dec 02 '21 05:12 evilgenius786

Any update?

albert-asin avatar Jan 12 '22 10:01 albert-asin

@albert-asin @evilgenius786 please check Appium v5.0.0 Beta01 - https://github.com/appium/appium-dotnet-driver/releases

pradeipp avatar Mar 08 '22 13:03 pradeipp

@GoldenCrystal, We are in the advanced stages of Appium 5.x Beta. The latest version can be found here: https://github.com/appium/dotnet-client/releases/tag/v5.0.0-beta02 I'm closing this issue, feel free to open a new issue in case you find any with the new version.

Dor-bl avatar Nov 15 '22 05:11 Dor-bl