Should v3 still support user's attributes derived from StepDefinitionBaseAttribute?

[adrichem]

We often need to use the same step from a Given and a When line in a featue file. I use below [GivenWhen(....)] to avoid duplicating the regex/expression. Reqnroll v2 worked fine but 3.0.2 fails with a Reqnroll.MissingStepDefinitionException on these steps. Is this a regression bug or should users now avoid derived attributes on their step definition methods?

public class GivenWhenAttribute : StepDefinitionBaseAttribute
{
    readonly static StepDefinitionType[] types = new[] { StepDefinitionType.Given, StepDefinitionType.When };
    public GivenWhen() : this(null)  {  }
    public GivenWhen(string regex) : base(regex, types ) { }
}

[GivenWhen("I do something")]
public async Task Do() { ... }

[304NotModified]

This should work IMO.

Please post all the stacktrace and full error message. Thanks!

[adrichem]

Thanks for the swift response. Here is the message and stack trace.

  Message: 
Reqnroll.MissingStepDefinitionException : No matching step definition found for one or more steps.

  Stack Trace: 
ErrorProvider.ThrowPendingError(ScenarioExecutionStatus testStatus, String message)
TestExecutionEngine.OnAfterLastStepAsync()
TestRunner.CollectScenarioErrorsAsync()
ReproductionFeature.ScenarioCleanupAsync()
ReproductionFeature.CantFindStep() line 8
GenericAdapter`1.GetResult()
AsyncToSyncAdapter.Await[TResult](TestExecutionContext context, Func`1 invoke)
AsyncToSyncAdapter.Await(TestExecutionContext context, Func`1 invoke)
TestMethodCommand.RunTestMethod(TestExecutionContext context)
TestMethodCommand.Execute(TestExecutionContext context)
<>c__DisplayClass1_0.<Execute>b__0()
DelegatingTestCommand.RunTestMethodInThreadAbortSafeZone(TestExecutionContext context, Action action)

  Standard Output: 
Given this is a normal given step
-> done: StepDefinitions.Given() (0,0s)
When this is a normal when step
-> done: StepDefinitions.When() (0,0s)
And this uses reqnrolls StepDefinition attr
-> done: StepDefinitions.All() (0,0s)
Given this step uses GivenWhenAttribute
-> undefined: No matching step definition found for the step. Use the following code to create one:
        [Given("this step uses GivenWhenAttribute")]
public void GivenThisStepUsesGivenWhenAttribute()
{
    throw new PendingStepException();
}

When this step uses GivenWhenAttribute
-> undefined: No matching step definition found for the step. Use the following code to create one:
        [When("this step uses GivenWhenAttribute")]
public void WhenThisStepUsesGivenWhenAttribute()
{
    throw new PendingStepException();
}

I reproduced it using this feature file:

Feature: Reproduction

Scenario: CantFindStep
	Given this is a normal given step
	When this is a normal when step
	And this uses reqnrolls StepDefinition attr
	Given this step uses GivenWhenAttribute
	When this step uses GivenWhenAttribute

And this binding code:

public class GivenWhenAttribute : StepDefinitionBaseAttribute
{
    readonly static StepDefinitionType[] types = [StepDefinitionType.Given, StepDefinitionType.When];
    public GivenWhenAttribute() : this(null!) { }
    public GivenWhenAttribute(string s) : base(s, types) { }
}


[Binding]
public class StepDefinitions
{
    [Given("this is a normal given step")]
    public void Given() { }

    [When("this is a normal when step")]
    public void When() { }

    [GivenWhen("this step uses GivenWhenAttribute")]
    public void Both() { }

    [StepDefinition("this uses reqnrolls StepDefinition attr")]
    public void All() { }

}

[304NotModified]

Thanks for the details! I try to debug it this week. But its a pretty busy week, so not sure if I could find the time.

[clrudolphi]

I agree with @304NotModified , this should work.
But, I have tried this setup on ALL versions of Reqnroll (v1.0, through 3.0.0) and it won't work on any of them. So while I can say that I can repro the problem, I also cannot repro correct behavior under a prior version.

@adrichem which specific version of Reqnroll 2.x, version of .NET/Framework, and which test framework (and version) are you using?

[Code-Grump]

I think we should be cautious about supporting this. Saying we'll support custom attributes means rather that everywhere step binding attributes are considered, we'd need to change to look for a class hierarchy instead of the well-defined subset we have today. That impacts:

• Runtime step discovery
• All IDE tooling
• The generators and analysers I've been working on

Whilst I'm pretty sure we can handle it for our runtime discovery, I'm more concerned about the other areas, where trying to determine that a user-defined type attribute applied to their steps is one we should be interested in. We can't look for tokens that probably represent interesting attributes and then check the semantic model, instead we'd have to scan the semantic model for any attributes, then determine if they're ones we should be interested in.

I just want to highlight that this might sound simple on the surface, but it has big ripples.

[adrichem]

@clrudolphi We're using these versions with net8.0. Its been working for us even before the migration from SpecFlow to Reqnroll.

<PackageReference Include="Microsoft.NET.Test.Sdk" Version="17.13.0" />
<PackageReference Include="Reqnroll" Version="2.4.0" />
<PackageReference Include="Reqnroll.MsTest" Version="2.4.0" />
<PackageReference Include="MSTest.TestAdapter" Version="3.8.3" />
<PackageReference Include="MSTest.TestFramework" Version="3.8.3" />

[UL-ChrisGlew]

Had a bit of a play around with this today - the issue seems to stem from the name of the input parameter for the expression. If I change the following code:

public class GivenWhenAttribute : StepDefinitionBaseAttribute
{
    readonly static StepDefinitionType[] types = [StepDefinitionType.Given, StepDefinitionType.When];
    public GivenWhenAttribute() : this(null!) { }
    public GivenWhenAttribute(string s) : base(s, types) { }
}

to:

public class GivenWhenAttribute : StepDefinitionBaseAttribute
{
    readonly static StepDefinitionType[] types = [StepDefinitionType.Given, StepDefinitionType.When];
    public GivenWhenAttribute() : this(null!) { }
    public GivenWhenAttribute(string expression) : base(expression, types) { }
}

everything seems to work okay.

TL;DR - replace the s in public GivenWhenAttribute(string s) : base(s, types) { } with expression - everything binds and works as expected.

[clrudolphi]

Yes, this works using Reqnroll v3.0.0 and greater, but fails in any version before that.
Still looking for root cause.

[clrudolphi]

I have what I believe to be a root cause, but not sure how or whether to proceed with any specific change.

The RuntimeBindingRegistryBuilder, when reflecting over an attribute in the CreateAttribute() function (return type BindingSourceAttribute) gathers together two collections. One is all public|instance Fields and Properties (and their values) stored as the NamedAttributeValues dictionary (keyed by field/property Name). The other collection is simply called AttributeValues and is an array that represents the values of the parameters of the most complex constructor of the Attribute. Presumably the most complex constructor is used as it is assumed to be the one that will be used by end-users. The name of each parameter is used to key into the previously constructed NamedAttributeValues dictionary when initializing the AttributeValues array.

It is the first entry in this array that is used as the regex/cucumber expression.

In other words, it is presumed that the names of the parameters given in the constructor match the names of public properties/fields of the attribute type (with allowances given for differences in initial letter capitalization).

When the names don't match, the fallback is to use the method name as the regex expression.

As for how to move forward:

• We could do nothing as this is a narrow corner case and infrequently used extension mechanism.
• We could enhance the documentation to highlight the constraint about naming of parameters of custom Binding attributes. (We have no documentation on the subject of custom binding attributes)
• We could normalize all access to binding attribute values around the use of named Fields/Properties (eliminating the use of AttributeValues array).

[gasparnagy]

Thx for the detail analysis @UL-ChrisGlew and @clrudolphi.

As this being a special case, I would suggest extending the documentation (where?) for now and declaring that if anyone creates derived attributes, they should use the same parameter names as the base class. (It is very unlikely that we will rename them again.)

@adrichem Is it a feasible workaround for you to fix the constructor parameter names accordingly?

[adrichem]

The workaround is fine for me. Should I run into any other problems, I'll raise a new issue.

Thanks to all of you for your hard work on this!