runFor is very flaky

[olivercoad]

Disclaimer: I haven't actually used runFor (or canopy's build-in test runner much at all, for that matter) and just picked up these bugs while reading the implementation for https://github.com/lefthandedgoat/canopy/issues/190#issuecomment-671990433 so I may have a couple things wrong.

Although it seems neat to implement runFor by duplicating the test suites with added suites to start/switch browsers using Once, it actually leads to a number of bugs.

Does not work with runFailedContextsFirst

When runFailedContextsFirst = true, the test suites are not run in order, so the added test suites which start browsers are not run at the right time. This will lead to no browser being open for all the failed tests.

Does not work if there are any WIP tests

If there are any WIP tests, then all test suites not including a WIP test (including the ones to start/switch browser) are not run.

Silent errors starting browsers

If an error occurs while starting a browser or switching browser, it will be silently ignored.

The browser is started or switched to using suite.Once. If an exception occurs, it will be caught and failSuite will be run with the exception to report the error for all tests in the suite. However, because the suite does not have any tests to report the exception for, it won't even get reported. All the tests intended to run on that browser will just silently get run on the previously open browser instead.

This means you can't rely on testing multiple browsers in CI. For example, if you used

runFor (BrowserStartModes [chrome; firefox])

and firefox failed to open in continuous integration for whatever reason, then all the tests would silently be run in chrome (twice). There would be no indication in the logs that this had occurred - in fact the suite contexts would still read "(firefox) ...".

The current implementation of runFor for easy reference https://github.com/lefthandedgoat/canopy/blob/a593649e5995fb0804c5b7c8f9510cd40840a569/src/canopy/runner.fs#L241-L269

Again, I don't actually use runFor myself so feel free to correct me.

A fix

I think it should be simple enough to implement correctly by essentially copying run () but running all the suites for each browser (L207-232). with refactoring

[olivercoad]

Will make a PR if you reckon that would work @lefthandedgoat

[lefthandedgoat]

I will have to look into it more and think on it. Im not sure how many people even use it.