lefthandedgoat / canopy

f# web automation and testing library, built on top of Selenium (friendly to c# also)
http://lefthandedgoat.github.io/canopy/
MIT License
506 stars 115 forks source link

Is there a reason `runFor browsers` takes in a `'a` and then boxes it? #478

Closed kaeedo closed 4 years ago

kaeedo commented 4 years ago

https://github.com/lefthandedgoat/canopy/blob/master/src/canopy/runner.fs#L241

It seems that it would be simpler and more correct to have browsers be of a concrete type that accepts either BrowserStartModes or WebDrivers. This would give compile time safety to what is being passed in.

Something like this:

type Browsers =
| BrowserStartModes of canopy.types.BrowserStartMode seq
| WebDrivers of IWebDriver seq

let runFor (browsers: Browsers) =
match browsers with
    | BrowserStartModes browsers -> () // do stuff
    | WebDrivers browsers -> () // do stuff

Usage form C# would also remain simple:

classic.Browsers br = classic.Browsers.NewBrowserStartModes(new List<types.BrowserStartMode>
        {
            types.BrowserStartMode.ChromeHeadless,
            types.BrowserStartMode.FirefoxHeadless
        });

        // or

        br = classic.Browsers.NewWebDrivers(new List<IWebDriver>
        {
            new ChromeDriver(),
            new FirefoxDriver()
        });

        _.runFor(br);
lefthandedgoat commented 4 years ago

Sorry, for some reason I dont always get notifications on my phone of new issues, so I never saw this.

Honestly I added that so long ago that I can't tell you why it is the way it is. I don't even know if people use it, someone asked for it so I added it.

I probably thought the syntax of runFor [chrome; firefox; ie] was clean.

kaeedo commented 4 years ago

Ok. In that case I'd like to make a Pull Request with the changes I proposed. The usage syntax would remain the same, but will become Type Safe