xRetry.Reqnroll: Don't Retry Scenarios that Fail with PENDING, UNDEFINED, or AMBIGUOUS exceptions

[clrudolphi]

There is no point in retrying a Scenario that has no hope of returning a PASSED execution status, such as when a step is either PENDING, UNDEFINED, or AMBIGUOUS. Reqnroll returns a specific exception in each of these cases and should cause the xRetry.Reqnroll plugin to skip the retry.

Context: I am working on enhancements to Reqnroll to support Cucumber Messages output, which can then feed existing Cucumber reporting solutions. The Cucumber Messages spec requires Cucumber implementations to not retry failed Scenarios in these types of situations. Please help Reqnroll maintain compliance with Cucumber by properly handling these situations.

[JoshKeegan]

Sounds like this would be a good addition 👍 Adding this should also be fairly simple if Reqnroll has exceptions defined for this, as we already support SkipException.

Does the current retry behaviour cause a specific problem, or is it just that these retries are unnecessary?

[clrudolphi]

The existing behavior does not cause an execution problem.

However, the extra retries cause the Reqnroll implementation of Cucumber Messages to be non-compliant with the Cucumber Project's specification of how Cucumber implementations should behave. (See the Cucumber Compatibility Kit for the test-cases currently defined for how implementations should behave with Retry).

Since Reqnroll does not have Retry behavior built-in, it depends upon cooperation with those plugins that are providing Retry to help it stay in compliance with Cucumber expectations.

(edit): These conditions (IIRC) result in Reqnroll throwing an exception back to the test runner (xUnit in this case). The exceptions would include:

• PENDING: PendingStepException
• UNDEFINED: MissingStepDefinitionException
• AMBIGUOUS: Reqnroll currently throws a BindingException but when support for Cucumber Messages rolls out this will change to AmbiquousBindingException.

[clrudolphi]

I was curious how well this idea would work, so I've tried modifying the TestGeneratorProvider to include the PendingStepException in the list of SkipExceptions with this code:

            // Always skip on Xunit.SkipException (from Xunit.SkippableFact) which is used by Reqnroll.xUnit to implement
            //  dynamic test skipping. This way we can intercept the exception that is already thrown without also having
            //  our own runtime plugin.
            retryAttribute.Arguments.Add(new CodeAttributeArgument(
                new CodeArrayCreateExpression(new CodeTypeReference(typeof(Type)),
                    new CodeExpression[]
                    {
                        new CodeTypeOfExpression(typeof(Xunit.SkipException)),
                        new CodeTypeOfExpression(new CodeTypeReference(typeof(PendingStepException), CodeTypeReferenceOptions.GlobalReference))
                    })));

Would that be the correct way to go about it?

It doesn't seem to work in my test case; perhaps I'm doing something wrong. I modified the RetrySteps.cs in the UnitTests.Reqnroll project to include the following step definition:

        [When("I increment the retry count, it should never be more than 1")]
        public void LookForUnWantedRetry()
        {
            WhenIIncrementTheRetryCount();
            ThenTheResultShouldBe(1);
        }

with the intention that this step would be called first, followed by a step that returns a PendingStep exception. The intent of the scenario would be to prove that the scenario was only ever executed a single time.

@retry
Scenario: Scenario with a Pending Step should not be retried
	When I increment the retry count, it should never be more than 1
	Then a pending step

Which resulted in this code in the generated code-behind (which looks correct to me?):

        [Xunit.TraitAttribute("FeatureTitle", "SkipRetryOfBrokenFeatures")]
        [Xunit.TraitAttribute("Description", "Scenario with a Pending Step should not be retried")]
        [Xunit.TraitAttribute("Category", "retry")]
        [xRetry.RetryFact(3, 0, new System.Type[] {
                typeof(Xunit.SkipException),
                typeof(global::Reqnroll.PendingStepException)}, DisplayName="Scenario with a Pending Step should not be retried")]
        public async System.Threading.Tasks.Task ScenarioWithAPendingStepShouldNotBeRetried()

But when executed it does seem to retry the default 3 times:

 Scenario with a Pending Step should not be retried
   Source: SkipRetryOfBrokenFeatures.feature line 7
   Duration: < 1 ms

  Message: 
Assert.Equal() Failure: Values differ
Expected: 1
Actual:   3

  Stack Trace: 
RetrySteps.ThenTheResultShouldBe(Int32 expected) line 29
InvokeStub_Action`1.Invoke(Object, Span`1)
MethodBaseInvoker.InvokeWithOneArg(Object obj, BindingFlags invokeAttr, Binder binder, Object[] parameters, CultureInfo culture)
--- End of stack trace from previous location ---
BindingInvoker.InvokeBindingAsync(IBinding binding, IContextManager contextManager, Object[] arguments, ITestTracer testTracer, DurationHolder durationHolder)
TestExecutionEngine.ExecuteStepMatchAsync(BindingMatch match, Object[] arguments, DurationHolder durationHolder)
TestExecutionEngine.ExecuteStepAsync(IContextManager contextManager, StepInstance stepInstance)
TestExecutionEngine.OnAfterLastStepAsync()
TestRunner.CollectScenarioErrorsAsync()
SkipRetryOfBrokenFeaturesFeature.ScenarioCleanupAsync()
SkipRetryOfBrokenFeaturesFeature.ScenarioWithAPendingStepShouldNotBeRetried() line 9
--- End of stack trace from previous location ---

  Standard Output: 
When I increment the retry count, it should never be more than 1
-> error: Assert.Equal() Failure: Values differ
Expected: 1
Actual:   3 (0.0s)
Then a pending step
-> skipped because of previous errors

Any ideas for things I should be checking?

[JoshKeegan]

I'd expect what you've done to cause that test to be skipped if everything was working. Maybe PendingStepException isn't getting thrown?

I don't thinkwould be correct behaviour though, as the test result will be skipped and you'd still want it to fail in these cases (just without retries). To implement this behaviour, you'd first need to implement a way of excluding some exceptions from retries in xRetry. Then you could update xRetry.Reqnroll to specify PendingStepException to be excluded from retries like you've done to make it a skipped exception.

If you want to implement exceptions excluded from retries, see:

• RetryTestCaseRunner
• MessageTransformer - I don't think you'll need to edit this, but it shows how to get the test failure exception types since this is how skipped exceptions works
• IRetryableTestCase & its implementations - how you could get the exceptions to exclude into the runner