Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SpecFlow step definitions with parametrized cucumber expressions are not recognized #63

Closed
gasparnagy opened this issue May 30, 2022 · 5 comments
Assignees
Labels

Comments

@gasparnagy
Copy link
Member

I have simple step definitions, like:

[Then(@"there should be {int} pizzas listed")]
public void ThenThereShouldBePizzasListed(int expectedCount)
{
            ...
}

or

[Then(@"the home page main message should be: {string}")]
public void ThenTheHomePageMainMessageShouldBe(string expectedMessage)
{
    ...
}

but the related steps are displayed as undefined. The ones without parameter work...

@Issafalcon Issafalcon self-assigned this Jun 3, 2022
@Issafalcon
Copy link
Contributor

I think I can resolve this one @aslakhellesoy . When I added c-sharp support originally, I wasn't aware that it could support cucumber expressions syntax.

@aslakhellesoy
Copy link
Contributor

The language service scans the file system for glue code and extracts regular expressions and cucumber expressions from source code (using tree-sitter). The extracted expressions are then used to build autocomplete, go to step definition and syntax highlighting.

The language service uses the JavaScript implementation of Cucumber Expressions to parse expressions, regardless of what programming language source code it was extracted from.

The Cucumber Expressions parser will throw an exception when it parses an expression that uses a parameter type that hasn't been defined. Because of this, the language service will first extract all parameter types from the source code, and then try to parse all the Cucumber Expressions.

I've discovered that this poses a challenge with SpecFlow source code because of the way parameter types are defined. Consider this Step Definition:

[When("the client specifies {DateTime} at {TimeSpan} as delivery time")]
public void WhenTheClientSpecifiesDateAtTimeAsDeliveryTime(DateTime deliveryDate, TimeSpan deliveryTime)
{
}

The Cucumber Expression uses two custom parameter types, {DateTime} and {TimeSpan}. These two parameter types must be defined in order for the expression to parse.

With Java syntax, the {DateTime} parameter type would be defined something like this:

@ParameterType(name = "DateTime", value = "\\d{4}-\\d{2}-\\d{2}")
public DateTime convertDateTime(String date) {
  return DateTime.parse(date)
}

The name of the parameter type is extracted from the name attribute of the @ParameterType annotation. The string value that is used to build the date is extracted with the parameter type's regexp (value in the case of Java).
It's similar with Ruby:

ParameterType(
  name:        'DateTime',
  regexp:      /\d{4}-\d{2}-\d{2}/,
  transformer: ->(s) { DateTime.parse(s) }
)

With SpecFlow however, it's different. The .NET implementation does not implement ParameterTypeRegistry or a mechanism to register parameter types. From cucumber/cucumber-expressions#45:

  • The ParameterTypeRegistry is not implemented here, only defines the IParameterTypeRegistry and IParameterType interfaces
  • The matching logic is not implemented in the expression implementation (cucumber, regex). Currently this logic sits in SpecFlow and it is not that easy (or makes sense) to move it out.

SpecFlow has a similar concept to parameter types - step argument transformations. Here is how they work:

[StepArgumentTransformation("today")]
public DateTime ConvertToday()
{
    return DateTime.Today;
}

[StepArgumentTransformation("tomorrow")]
public DateTime ConvertTomorrow()
{
    return DateTime.Today.AddDays(1);
}

[StepArgumentTransformation("(.*) days later")]
public DateTime ConvertDaysLater(int days)
{
    return DateTime.Today.AddDays(days);
}

At first glance, they look similar to the parameter type definitions in Java and Ruby shown above. There are some differences though:

First off, they don't have an explicit name, but they have a type (the return type of the method). They also have something similar to a parameter type's regexp - the expression in the [StepArgumentTransformation] attribute. In the example above these are "today", "tomorrow" and "(.*) days later".

With SpecFlow, it's the type (DateTime) that is used to define the parameter type name. Recall the Cucumber Expression above:

the client specifies {DateTime} at {TimeSpan} as delivery time

This would match the following Gherkin steps in SpecFlow:

When the client specifies tomorrow at noon as delivery time
When the client specifies 4 days later at 23:00 as delivery time
When the client specifies today at 9am as delivery time

In the JavaScript implementation of Cucumber Expressions (which is used in the language server), this becomes a challenge.

How do we define the {DateTime} parameter type so that a Cucumber Expression using that parameter type parses correctly?

This is what I have in mind to make this possible:

  • Extract all the StepArgumentTransformation snippets from the code
  • For each unique return type, define a parameter type that uses all the regexps.

For the example above, this would become (in TypeScript):

const parameterType = new ParameterType(
  'DateTime', // The name
  [/tomorrow/, /today/, /(.*) days later/],
  ...
)

This way the language server would match Gherkin steps in the same way as SpecFlow.

I'd welcome your feedback on this @gasparnagy @SabotageAndi @Issafalcon

@Issafalcon
Copy link
Contributor

This sounds like a perfectly sensible approach to me @aslakhellesoy . Not being as familiar as yourself with this codebase, it sounds like, from your description, that this will work across step definitions in all languages.

One thing to note is the StepArgumentTransformation will be a verbatim string literal when special regex characters are used (i.e. @"regex characters (.*)"). I'm not sure if that impacts the implementation.

@gasparnagy
Copy link
Member Author

@aslakhellesoy Thx for the improvements. I have some notes/feedback.

  • The unnamed parameters can be used with type name as you have observed, but makes sense to mention that this is always the .NET type name, not the c# keyword (Int32 vs int). But the extension defines the most commonly used c# type keywords and use them as an alias for the full type name. So if you make a conversion with decimal return type, you will be able to use it both as {Decimal} (because this is the type name) or {decimal} because this is an alias for {Decimal}.
  • Not with the plugin, but with the final integration, you will be able to make named parameter types in [StepArgumentTransformation("some-regex", Name = "mytypename")].
  • Again in the final version we had to introduce an automatic inclusion of enumeration parameter types. Enums can be converted by default by SpecFlow anyway, so you want to be able to use {Color} (assuming Color is an enum type), without having any StepArgumentTransformation for them.
  • Altogether the parameter matching is fairly complex, because it had to match with the existing parameter type system of SpecFlow, so in the VS extension we simply use .*? for all cucumber expression parameters. I haven't run into conflict so far and generally it is better to show a match that might cause parameter errors runtime than showing it as "unspecified" just because we could not interpret perfectly the parameter rules. I would like to keep highlighting, that many teams use assets (e.g. step argument conversions) from external assemblies, so by only parsing c# it is never going to be perfect. So it is better to prepare for imperfection.
  • In general verbatim and non-verbatim C# strings are interchangeable. So theoretically we should support both in all places where we need strings. Also with new C# you can use interpolated strings in attributes $"hello {foo} bar", but this is fortunately very rarely used. And you can also use constants as well, but that is also rarely used.

@Issafalcon
Copy link
Contributor

The comprehensiveness of @gasparnagy 's reply compared to mine does a good job of highlighting the difference of one who writes the software, and one who simply consumes it 😂

I think I'll defer to @gasparnagy in this case!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants