Parameter count mismatch on optional parameter after migrating from SpecFlow

[BalazsSzaboElypta]

Reqnroll Version

2.0.3

Which test runner are you using?

MSTest

Test Runner Version Number

3.3.1

.NET Implementation

.NET 8.0

Test Execution Method

Visual Studio Test Explorer

Content of reqnroll.json configuration file

No response

Issue Description

Before migrating to Reqnroll from Specflow a step definition using a regular expression with optional parameter (see example below) used to be working, but after migrating I get Reqnroll.BindingException: Parameter count mismatch! error if I don't provide the optional parameter.

I didn't find any breaking change related to this in the migration documentation, so I'd expect it to be working the same (i.e. for the option parameter to be null if I don't provide a value for it in the feature file)

Steps to Reproduce

Use this step definition:

[StepDefinition(@"^Do something (\d+) times(?: as user '(.+)')?$")]
public void DoSomething(string times, string userString)
{
    string user = userString ?? "defaultUser";

    handleDoSomething(times, user);
}

And these test cases:

# Throws Reqnroll.BindingException: Parameter count mismatch!
Scenario: Test
	When Do something 5 times

# Executes fine
Scenario: Test2
	When Do something 5 times as user 'user1'

Link to Repro Project

No response

[gasparnagy]

@BalazsSzaboElypta Wow, that is strange. Regarding my knowledge we never (officially) supported optional parameters with SpecFlow so I think it was just a lucky coincidence that that was working. Which version of SpecFlow you are migrating from?

[BalazsSzaboElypta]

From 3.9.74 so I guess it's possible that it's a breaking change from V3 -> V4

[gasparnagy]

Probably. But as I said, this was just working accidentally, so it was not a goal to keep it. Do you have many of them? Would it be possible to refactor them to ones without optional parameters?

Because to be honest a proper support for that is not super easy and we anyway migrate people from regex to cucumber expressions (where such thing is not even possible), so implementing a support for optional parameters is not a strategic goal right now.

[BalazsSzaboElypta]

I understand, we have around a dozen or two, so it's not the end of the world to refactor them. Probably would make the code a bit more readably as well since the regex expressions tend to get a bit complex with a lot of optional parameters.

Thanks for the clarification!

[gasparnagy]

@BalazsSzaboElypta thx for the understanding. Yes, one of the motivation behind the whole cucumber expression initiative was (by the cucumber community), that the regexes could become quite complex and therefore harder to maintain.

I close this issue now, but if anything comes up, feel free to reopen.

[drosenba]

This stackoverflow was posted/answered over 10 years ago, and then it showed that using RegEx could support optional parameters:
https://stackoverflow.com/questions/18346348/optional-parameter-in-cucumber

[drosenba]

Support for RegEx optional parameters seems to just be a matter of "allowing" the Match Groups regardless of the Success property. I've made these changes and did a few tests.

(The commented out line "//!!!" is the original, and it's followed by the line with my change.)

...\Infrastructure\MatchArgumentCalculator.cs

public object[] CalculateArguments(Match match, StepInstance stepInstance, IStepDefinitionBinding stepDefinitionBinding)
{
    //!!! var regexArgs = match.Groups.Cast<Group>().Skip(1).Where(g => g.Success).Select(g => g.Value);
    var regexArgs = match.Groups.Cast<Group>().Skip(1).Select(g => g.Value);
    var arguments = regexArgs.Cast<object>().ToList();
    if (stepInstance.MultilineTextArgument != null)
        arguments.Add(stepInstance.MultilineTextArgument);
    if (stepInstance.TableArgument != null)
        arguments.Add(stepInstance.TableArgument);

    return arguments.ToArray();
}

...\Infrastructure\TestExecutionEngine.cs

private async Task<object[]> GetExecuteArgumentsAsync(BindingMatch match)
{
    var bindingParameters = match.StepBinding.Method.Parameters.ToArray();
    //!!! if (match.Arguments.Length != bindingParameters.Length)
    if (match.Arguments.Length > bindingParameters.Length)
        throw _errorProvider.GetParameterCountError(match, match.Arguments.Length);

    //!!! var arguments = new object[match.Arguments.Length];
    var arguments = new object[bindingParameters.Length];

    for (var i = 0; i < match.Arguments.Length; i++)
    {
        arguments[i] = await ConvertArg(match.Arguments[i], bindingParameters[i].Type);
    }

    return arguments;
}

Both of these work (as reported here https://github.com/orgs/reqnroll/discussions/288

Feature: BasicTest

Scenario: Basic Test0a
    Given I test 'variable' as 'nameOrValue1-A'
     Then I should pass

Scenario: Basic Test0b
    Given I test 'variable' as 'nameOrValue1-A' plus 'nameOrValue2-B'
     Then I should pass

        [Given(@"I\s*test\s+'([^']+)'\s+as\s+'([^']+)'(?:\s+(plus|minus)\s+'([^']+)')?")]
        public void ITest2(string label, string nameOrValue1, string deviateTypeKeyword, string nameOrValue2)
        {
        }

(And, I tested more complex cases having several optional parameters and excluding ones "in the middle")

[gasparnagy]

@drosenba See discussion at #238. Based on that @olegKoshmeliuk has made the interface to override the behavior per-project (to support legacy / migration cases).

Could you please try if overriding that integrace and implementing the more SpecFlow-compatibility handling way of regex groups would work for you? If yes, we could add it as a sample solution to https://docs.reqnroll.net/latest/guides/how-to-configure-cucumber-expression-behavior.html

[drosenba]

@gasparnagy I implemented this plugin and tested it in a small standalone app. It does work fine for the test, but the "red error squiggle" still appears in the IDE .feature file. indicating parameter count mismatch. I guess that's what you alluded to in #238 "... fix this in Visual Studio extension (code dupl!) ..."

using Reqnroll.Bindings;
using Reqnroll.Infrastructure;
using Reqnroll.Plugins;
using Reqnroll.UnitTestProvider;
using ReqnrollProject1.Plugins;
using System.Text.RegularExpressions;


[assembly: RuntimePlugin(typeof(CustomPlugin))]

namespace ReqnrollProject1.Plugins;

public class CustomArgumentCalculator : IMatchArgumentCalculator
{
    public object[] CalculateArguments(Match match, StepInstance stepInstance, IStepDefinitionBinding stepDefinitionBinding)
    {
        var regexArgs = match.Groups.Cast<Group>().Skip(1).Select(g => g.Value);
        var arguments = regexArgs.Cast<object>().ToList();
        if (stepInstance.MultilineTextArgument != null)
            arguments.Add(stepInstance.MultilineTextArgument);
        if (stepInstance.TableArgument != null)
            arguments.Add(stepInstance.TableArgument);

        return arguments.ToArray();
    }
}

public class CustomPlugin : IRuntimePlugin
{

    public void Initialize(RuntimePluginEvents runtimePluginEvents, RuntimePluginParameters runtimePluginParameters, UnitTestProviderConfiguration unitTestProviderConfiguration)
    {
        runtimePluginEvents.CustomizeGlobalDependencies += (sender, args) =>
        {
            args.ObjectContainer.RegisterTypeAs<CustomArgumentCalculator, IMatchArgumentCalculator>();
        };
    }
}

[drosenba]

My CustomArgumentCalculator above worked in the "standalone" project. But when I put it in my "SpecFlow v3 to ReqnRoll conversion" project, it's not working. It never gets to the Initialize() method. (I put in a breakpoint.) The difference between the standalone and the new / converted app is that the new app has a separate assembly that contains the Steps, Hooks, etc. and that's where I put the above CustomArgumentCalculator with the [assembly: RuntimePlugin(typeof(CustomPlugin))].

I know that my assembly is working for everything else. This is my reqnroll.json:

{
  "$schema": "https://schemas.reqnroll.net/reqnroll-config-latest.json",

  "language": {
    "feature": "en-us"
  },

  "bindingAssemblies": [
    { "assembly": "MyProjectSpecFlow" }
  ],
}

[drosenba]

FYI, I got the above CustomArgumentCalculator plugin to work in my converted app. I was incorrect to say "difference between the standalone and the new / converted app is that the new app has a separate assembly that contains the Steps, Hooks, etc.", because the new / converted app also had the separate assembly. The difference was that the standalone app had its .feature files in that same separate assembly (class project) whereas the converted app had its .feature files in the "main program". In the end, I created a separate project/assembly just for the plugin (and named it xxxxx.ReqnrollPlugin so that it emitted xxxxx.ReqnrollPlugin.dll) and the main program has dependency on both projects (one with the Steps & Hooks, and the other the plugin.)