ipjohnson / SeleniumFixture

Selenium testing fixture for C#
Microsoft Public License
15 stars 6 forks source link

RemoteDriverAttribute doesn't support collection of flags #5

Open martijnburgers opened 7 years ago

martijnburgers commented 7 years ago

I like the idea of providing a collection of RemoteWebDriverCapability flags to tell you want to run the test on multiple browsers in a Selenium grid. This is however not implemented by SeleniumFixture.

Providing multiple RemoteWebDriverAttribute is also not possible making it less easy to run the tests on multiple browsers in a Selenium grid.

Where there any specific issues implementing this or just not got the time to implement this?

ipjohnson commented 7 years ago

Hi Martin,

What happens when you put multiple remote attributes on a method?

I'll take a look at it today but there shouldn't be any technical problems, I just have personally need to use the remote driver and you're the first person to ask for it.

For the moment I think you can and the flags together to get multiple drivers (I believe as I'm not specifically looking at it right now).

martijnburgers commented 7 years ago

Multiple attributes result in a compiler says no:

error CS0579: Duplicate 'RemoteDriver' attribute

The following code in the RemoteDriverAttribute clarifies it. The GetDrivers method will always yield one driver.

public override IEnumerable<IWebDriver> GetDrivers(MethodInfo testMethod)
{
    yield return CreateWebDriver(testMethod, Capability, Hub);
}

public static IWebDriver CreateWebDriver(MethodInfo testMethod, RemoteWebDriverCapability capability, string hub)
{
    DesiredCapabilities capabilities = null;

    switch (capability)
    {
        case RemoteWebDriverCapability.Andriod:
            capabilities = DesiredCapabilities.Android();
            break;
        case RemoteWebDriverCapability.Chrome:
            capabilities = DesiredCapabilities.Chrome();
            break;
        case RemoteWebDriverCapability.FireFox:
            capabilities = DesiredCapabilities.Firefox();
            break;
        case RemoteWebDriverCapability.HtmlUnit:
            capabilities = DesiredCapabilities.HtmlUnit();
            break;
        case RemoteWebDriverCapability.HtmlUnitWithJS:
            capabilities = DesiredCapabilities.HtmlUnitWithJavaScript();
            break;
        case RemoteWebDriverCapability.InternetExplorer:
            capabilities = DesiredCapabilities.InternetExplorer();
            break;
        case RemoteWebDriverCapability.IPad:
            capabilities = DesiredCapabilities.IPad();
            break;
        case RemoteWebDriverCapability.IPhone:
            capabilities = DesiredCapabilities.IPhone();
            break;
        case RemoteWebDriverCapability.Safari:
            capabilities = DesiredCapabilities.Safari();
            break;
    }

    if (string.IsNullOrEmpty(hub))
    {
        var attr = ReflectionHelper.GetAttribute<RemoteWebDriverHubAddressAttribute>(testMethod);

        if (attr != null)
        {
            hub = attr.Hub;
        }
    }

    return CreateWebDriverInstance(testMethod, hub, capabilities);
}

BTW I am currently working around this by sub classing RemoteDriverAttribute. First I wanted to use the RemoteWebDriverProviderAttribute but that's not supported as well.

ipjohnson commented 7 years ago

Yup that will definitely not work. I think the original intention was to be able to or them together but I obviously didn't end up implement it that way.

That said which would make sense to you, or them together, params array of enum, apply multiple attributes with one or maybe all of the above?

I don't have a selenium grid setup so it will be hard for me to test. I could put out an alpha for you to try if that makes sense?

martijnburgers commented 7 years ago

I like the collection flags approach (OR them together).

Yes I can do that, no problem.

Setting up a grid is super easy though. Download Java, Selenium server and Chromedriver. Start the JAR in hub and node role. Obviously you need to install the browsers as well.

I have one folder (D:\Selenium) with the following files:

nodeConfig.json

{
  "capabilities":
      [
        {
          "browserName": "*firefox",
          "maxInstances": 10,
          "seleniumProtocol": "Selenium"
        },
        {
          "browserName": "*googlechrome",
          "maxInstances": 10,
          "seleniumProtocol": "Selenium"
        },
        {
          "browserName": "*iexplore",
          "maxInstances": 10,
          "seleniumProtocol": "Selenium"
        },
        {
          "browserName": "firefox",
          "maxInstances": 10,
          "seleniumProtocol": "WebDriver"
        },
        {
          "browserName": "chrome",
          "maxInstances": 10,
          "seleniumProtocol": "WebDriver"
        },
        {
          "browserName": "internet explorer",
          "maxInstances": 10,
          "seleniumProtocol": "WebDriver"
        }
      ],
  "configuration":
  {
    "proxy": "org.openqa.grid.selenium.proxy.DefaultRemoteProxy",
    "maxSession": 10,
    "port": 5555,
    "host": ip,
    "register": true,
    "registerCycle": 5000,
    "hubPort": 4444,
    "hubHost": ip
  }
}

hub.bat

java -jar selenium-server-standalone-2.53.1.jar -role hub -log C:\Selenium\gridlog.txt -hub http://localhost:4444/grid/register

node.bat

java -jar selenium-server-standalone-2.53.1.jar -role node -nodeConfig nodeconfig.json -log C:\Selenium\nodelog.txt
martijnburgers commented 7 years ago

One more thing, there is no hurry to fix this for me as I can't use it even when fixed because I have more requirements :-). For example I need a hook to supply additional capabilities to the remote webdriver depending on the browser of choice.

ipjohnson commented 7 years ago

Oddly enough I was just going to ask you about adding the ability to provide more capabilities.

option 1: Add 3 properties for Platform, BrowserName, and Version. They would be optional, as well as I fix the RemoteWebDriverProviderAttribute functionality to be the same as other drivers.

option 2: Add a new property that allows you to add a type for providing extra capabilities (you could inherit from RemoteDriverAttribute to hide the typeof)

[RemoteDriver(RemoteWebDriverCapability.Chrome, ExtraCapabilities = typeof(SomeClass)]

public class SomeClass : IRemoteDriverCapabilityProvider
{
   public DesiredCapabilities ProvideCapabilities(MethodInfo testMethod, DesiredCapabilities currentCapabilities)
   {
    // do stuff
   }
}

option 3: do option 1 and 2.

I'm going to set the attribute usage to allow multiple so you can package them together in many different ways.

martijnburgers commented 7 years ago

Option 4 (similar to 2). Create a new abstract attribute for extra capabilities, more or less like the ChromeOptionsAttribute.

I like option 4 the most because it's aligned with the rest of the usage.

ipjohnson commented 7 years ago

I definitely thought about that the problem I see with that is that the other attributes don't allow multiples so you know the one driver goes with the one driver option attribute.

If I allow multiple remote attributes then there isn't really a way to have separate extra capabilities attributes (which looking at some of the capabilities it's can be very browser specific). By tying the extra capabilities to each attribute then you can do something like apply 3 chrome attributes each with a specific version (say the last 3 releases).

The remote driver is somewhat it's own animal in that you also can't reuse the drivers either where you can with the chrome, firefox, and IE.

martijnburgers commented 7 years ago

I understand.

I have no experience testing on multiple platforms and versions. So I can't say what is the best way for doing is.

How would you test for multiple platforms and versions with option 2? Does option 2 also get's a version and platform property?

ipjohnson commented 7 years ago

For option 2 I think you would define 3 different classes that implement IRemoteDriverCapabilityProvider


[RemoteDriver(RemoteWebDriverCapability.Chrome, ExtraCapabilities = typeof(ChromeCurrentVersion)]
[RemoteDriver(RemoteWebDriverCapability.Chrome, ExtraCapabilities = typeof(ChromeOneVersionBack)]
[RemoteDriver(RemoteWebDriverCapability.Chrome, ExtraCapabilities = typeof(ChromeTwoVersionBack)]
martijnburgers commented 7 years ago

Ok Perfect. Option 2 it is.

ipjohnson commented 7 years ago

And if that get's to unruly to manage there is always the option of inheriting from RemoteDriver and return as many drivers and configurations as needed.

I'll look to implement these changes over the next week, if you find any other requirements you need let me know and I'll see about rolling them in.

martijnburgers commented 7 years ago

So far I found one other minor issue which I will submit tonight or tomorrow.

Another requirement I have is to support dynamic skipping of [SeleniumTheory] just like Xunit.SkippableFact. Currently I am quit busy for a new client but I maybe can create some time to send a PR for this.