Automatic retry on failed steps

[adrichem]

I was wondering if you see value in a plugin that automatically initiates retries on failed steps instead of complete scenarios? I have a small Proof Of Concept that might be useful for decoupling the logic of a step definition from its need to be retried.

Why retry steps?

My System Under Test (SUT) runs asynchronously and needs time to reach the correct state. During this time the various Then steps may not fail the scenario. While the scenario is executing, it may even contain steps to change the grace period.

How the plugin is implemented

The plugin registers a custom ITestExecutionEngine. This custom engine derives from the default Reqnroll TestExecutionEngine but overrides the ExecuteStepMatchAsync method to:

1. Retrieve the IRetryStepPluginConfiguration from the ScenarioContainer and ask if the current BindingMatch should be retried.
2. If retry is needed, it retrieves an IAsyncRetryer from the ScenarioContainer to perform the retry logic on the invocation of the binding like this

await Retryer.RetryAsync(async () => await _bindingInvoker.InvokeBindingAsync(match.StepBinding, _contextManager, arguments, _testTracer, durationHolder));

How the author provides the logic and decisions needed to retry

The author of the step definition registers a IRetryStepPluginConfiguration in the scenario container. This object has 1 method to decide if a BindingMatch should be retried or not.

They also register a IAsyncRetryer having the Task RetryAsync(Func<Task> action) method. This object is responsible for doing the retry logic. For example:

[BeforeScenario]
public static void BeforeScenario(IObjectContainer c)
{
    c.RegisterInstanceAs<IRetryPolicy>(new DefaultRetryPolicy());
    c.RegisterInstanceAs<IAsyncRetryer>(new SimpleRetryer(c.Resolve<IRetryPolicy>()));
    c.RegisterInstanceAs<IRetryStepPluginConfiguration>(new AttributeBasedRetryer());
}

public ConstructorOfBindingClass(DefaultRetryPolicy P)
{
    Policy = P;
    Policy.RetryForExceptionTypes.Add(typeof(InvalidOperationException));
}

[Given(@"I set the max number of retries to ([+-]{0,1}[1-9][0-9]*)")]
public void SetRetryLimit(int n) => Policy.MaxRetries = n;

[Retry]
[Then("some then step must pass within the defined retry policy settings")
public void DoCheck() => MySystem.State.Should().BeOK();

[Code-Grump]

I don't object to this, but I would be interested to understand why it might be better than using a library like Polly. Have you tried using a transient-fault handling library to achieve retries (or any other transient-fault handling pattern?)

[adrichem]

Thanks for your time so far! The plugin helps to integrate transient failure libraries. Its not intended to replace them. Instead of writing many step-definitions like this:

[Then("state must be ok")]
public void CheckState() 
=> new ResiliencePipelineBuilder().AddRetry(Options) .Build() .Execute(() => MySystem.State.Should().BeTrue());

They write many like this, the plugin kicks-off the logic to deal with transient failures:

[Then("state must be ok")]
public void CheckState() 
=> MySystem.State.Should().BeTrue();

Here's a demo with Polly:

Feature: PollyAsRetryFramework
As author of step definitions, I want freedom to use my preferred framework for handling retries.

Scenario: ProofOfConcept
	
	Given the amount of transient errors is 5
	When I set the max number of retries to 5
	Then these transient failures may not cause test failure

    public class PollyRetryer : IAsyncRetryer
    {
        public RetryStrategyOptions Options { get; set; } = new RetryStrategyOptions();
        public async Task RetryAsync(Func<Task> action) => await new ResiliencePipelineBuilder()
            .AddRetry(Options)
            .Build()
            .ExecuteAsync(async _ => { await action(); });
    }

    public class RetryThen : IRetryStepPluginConfiguration
    {
        public virtual bool RetryEnabledFor(BindingMatch match)
        => match.StepBinding.StepDefinitionType == StepDefinitionType.Then;
    }

    [Binding]
    public class PollyDemoStepDefinitions
    {
        readonly PollyRetryer Retryer;
        int nTimesToFail;

        [BeforeScenario]
        public static void BeforeScenario(IObjectContainer c)
        {    
            c.RegisterInstanceAs<IRetryStepPluginConfiguration>(new RetryThen());
            c.RegisterInstanceAs<IAsyncRetryer>(new PollyRetryer());
        }

        public PollyDemoStepDefinitions(PollyRetryer P) => Retryer = P;

        [Given(@"the amount of transient errors is {int}")]
        public void SetNumberOfFails(int n) => nTimesToFail = n;

        [When(@"I set the max number of retries to {int}")]
        public void SetRetryLimit(int n) => Retryer.Options.MaxRetryAttempts = n;

        [Then(@"these transient failures may not cause test failure")]
        public void SimulateFlakyness()
        {
            if (nTimesToFail >= 1)
            {
                nTimesToFail--;
                Console.WriteLine($"Flaky action is throwing.");
                throw new InvalidOperationException();
            }
            Console.WriteLine("Flaky action is passing.");
        }
    }
}

[gasparnagy]

Seeing this I wonder if this should be an infrastructure of Reqnroll itself and not a plugin. Maybe this IAsyncRetryer interface in your concept is the missing link I was looking for to provide execution flow modifications without the need of overriding ITestExecutionEngine...

[adrichem]

That would be awesome @gasparnagy I think the idea of the IRetryStepPluginConfiguration is needed as test projects need to control which step-definitions enter the retry logic/pipelines. Some examples:

1. Apply retry when step def is a Then.
2. Opt-in when step-def method has some attribute.
3. Opt-out when methods have some attribute.

[gasparnagy]

@adrichem Do you have the POC code available somewhere, so that I can look and think it over?

[adrichem]

@gasparnagy https://github.com/adrichem/ReqnrollRetryPlugin

[gasparnagy]

I also don't object (the idea with Polly by @Code-Grump is pretty interesting though).

I have some generic comments / thoughts based on this.

• I don't think there is a better way right now, but overriding the ITestExecutionEngine is not practical, because it is not composable with other plugins wanting to override the ITestExecutionEngine as well. We should think what design would allow better the creation of plugins that influence the execution flow. Any ideas are welcome.

• It seems using some sort of configuration is also a common need for plugins (mentioned also in Revise Reqnroll "Actions" concept #34), so we should consider making Reqnroll's own config system more extensible. (As an alternative of any code-configuration that is obviously still valid and useful.)

• You haven't mentioned @adrichem whether you would like to maintain this plugin in your own repo or would you be interested in including it into the main repo, but it made me think that in case of the latter, how do we decide whether something should be a Reqnroll-maintained plugin (in the main repo) or a feature built-in to the product.

[ajeckmans]

I'd go one step further and use names policies akin to how authorization policies work in asp.net where you define named policies and then later in the attribute specify which policy is to be used.
That would eliminate the need to introduce constructors in step binding files, which sometimes are not necessary.