ipjohnson / SeleniumFixture

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

RemoteDriverAttribute doesn't support shared drivers. #7

Open martijnburgers opened 7 years ago

martijnburgers commented 7 years ago

The RemoteDriverAttribute doesn't respect the Shared property. It will ignore it by always creating a new driver.

In the code:

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

Unfortunately sub classing it and implementing the methods as follows doesn't work because of the cache implementation. In my scenario I have created a specific attribute for a Chrome remote driver.

public class CustomChromeRemoteDriverAttribute : RemoteDriverAttribute
{
    public CustomChromeRemoteDriverAttribute() : base(RemoteWebDriverCapability.Chrome)
    {
    }

    public override void ReturnDriver(MethodInfo testMethod, IWebDriver driver)
    {
        this.ReturnDriver(testMethod, driver as RemoteWebDriver);
    }

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

    public RemoteWebDriver CreateRemoteWebDriver(MethodInfo testMethod, RemoteWebDriverCapability capability, string hub)
    {
        var capabilities = CustomDesiredCapabilities.Chrome();

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

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

        return CreateWebDriverInstance(testMethod, hub, capabilities);
    }

    public static RemoteWebDriver CreateWebDriverInstance(MethodInfo testMethod, string hub, ICapabilities capabilities)
    {
        var driver = string.IsNullOrEmpty(hub) ? new RemoteWebDriver(capabilities) : new RemoteWebDriver(new Uri(hub), capabilities, TimeSpan.FromMinutes(60));

        InitializeDriver(testMethod, driver);

        return driver;
    }
}

The solution to get it to work with the current version of SeleniumFixture is to subclass RemoteWebDriver as well and change the CustomChromeRemoteDriverAttribute to

public class CustomChromeRemoteDriverAttribute : RemoteDriverAttribute
{
    public CustomChromeRemoteDriverAttribute() : base(RemoteWebDriverCapability.Chrome)
    {
    }

    public override void ReturnDriver(MethodInfo testMethod, IWebDriver driver)
    {
        this.ReturnDriver(testMethod, driver as CustomChromeRemoteDriver);
    }

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

    public CustomChromeRemoteDriver CreateRemoteWebDriver(MethodInfo testMethod, RemoteWebDriverCapability capability, string hub)
    {
        var capabilities = CustomDesiredCapabilities.Chrome();

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

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

        return CreateWebDriverInstance(testMethod, hub, capabilities);
    }

    public static CustomChromeRemoteDriver CreateWebDriverInstance(MethodInfo testMethod, string hub, ICapabilities capabilities)
    {
        var driver = string.IsNullOrEmpty(hub) ? new CustomChromeRemoteDriver(capabilities) : new CustomChromeRemoteDriver(new Uri(hub), capabilities, TimeSpan.FromMinutes(60));

        InitializeDriver(testMethod, driver);

        return driver;
    }
}

The shared instance cache key should not only be the driver type because this will not work in case of multiple remote webdrivers with different capabilities.

ipjohnson commented 7 years ago

Yeah originally my thinking was that sharing wasn't a good idea because the combinations of possible different drivers was large and I didn't know if it was really needed (wasn't sure if it helps with a grid or not).

Since you're the first person to use/ask for features for the remote driver I'll defer to your use cases.

At the moment I'm knee deep in another project, I'm not sure when I will get to this change. If you want to create a PR for it to speed things along that would be cool. If not I'll see what I can do over the next week or two.

martijnburgers commented 7 years ago

No worries.

I am also pretty busy for a new project so I will only be able to file the issues I encounter. We can work from there whenever you or me got more time. I can work around those issues fairly easy so there is no hurry. When I have more time I am happy to help out.

ipjohnson commented 7 years ago

In case you're interested these are the other two projects that I'm currently working.

Grace - Dependency injection container for .net, currently re-writing from the ground up for .netstandard and performance reasons.

SimpleFixture - Data generation library used by SeleniumFixture for creating random data as well as constructing models instances for the PageObject pattern.

martijnburgers commented 7 years ago

I will look into them.