JoshKeegan / xRetry

Retry running tests via Xunit and Specflow
MIT License
46 stars 15 forks source link

Investigate: Refactor SpecFlow retry generator plugin #126

Closed JoshKeegan closed 1 year ago

JoshKeegan commented 2 years ago

Could consider refactoring to use ITestMethodDecorator and possibly ITestMethodTagDecorator to decouple some of the tagging logic from the xunit generator. Currently this sits within the TestGeneratorProvider.

Having said that, I'm not sure if it would offer any real benefit though because there's no common API for offering retry logic between different test frameworks (unless I built one). So however decoupled the xRetry.Specflow code became, we'd still depend on xRetry and therefore xunit...

It may still help make the implementation simpler to understand though. Currently you have to know about the XUnit2TestGeneratorProvider within SpecFlow.xUnit and then how the IUnitTestGeneratorProvider gets called from within Specflow, and in what order calls happen.

Need to look at this and see whether it would work and whether it helps.

JoshKeegan commented 2 years ago

This is trigerred by the changes from #115 - with feature support there is no longer a single method we can override to capture all possible places that a retry can have been added.

JoshKeegan commented 1 year ago

Got basic PoC working for this. Needs more time spending to decide if this is a better route. It allows for feature & scenario level tags to be handled separately however raises some extra questions (left as TODOs & comments in that code). If I do go with this approach the PoC code is currently very WET but with refactoring could be good.

JoshKeegan commented 1 year ago

A little more work done on this & pushed to the branch. The current approach with TestGeneratorProvider requires 3 methods to handle all cases, called when:

This alternate approach requires implementing both ITestMethodDecorator and ITestMethodTagDecorator to handle both scenario & feature level tags, and each interface has multiple methods. It also doesn't achieve much separation of resposibilities, since most of the code is the same. If this moved beyond PoC I'd either extract common methods or just merge the classes...

Overall I think this would only be worthwhile if the library were reworked to support multiple test frameworks. See SpecFlow's IgnoreDecorator as an example of this. There's no need for this though, and I think the purpose of this library should remain focussed on xUnit for now.