When generating step definition regex for int argument, use (\d+) instead of (.*)

[LirazShay]

Currently, if I have step like this:

When I type 20 into the calculator

Then the suggested step definition is:

[When(@"I type (.*) into the calculator")] public void WhenITypeIntoTheCalculator(int p0) { ScenarioContext.Current.Pending(); }

This regex will match every set of characters, not only numbers for example the step:

When I type dasdsadas into the calculator

will also be mapped to that method and will crash on runtime

I know that you might think that it is better to allow by default also strings as parameter and then parse them at run time, But as I see from my usage, I never, ever used this feature and passed a word as argument, even if someone has steps like this:

When I wait 20ms

Will be generated to

[When(@"I wait (.*)ms")] public void WhenIWaitMs(int p0) { ScenarioContext.Current.Pending(); }

That will also work with this regex:

[When(@"I wait (\d+)ms")]

I think that most of the users in 99% of the scenarios when they want to pass a number as argument it is better to create a regex that will only match number and will help much more by showing the error before runtime.

Currently I'm doing it manually every time I have a step with number I change it to (\d+), I now that most of the pro users of SpecFlow also doing that https://www.stevefenton.co.uk/2015/01/useful-specflow-regular-expressions And I'm tired of doing this so @gasparnagy, please find us a solution

[dirkrombauts]

It's been my experience that when I write When I type dasdsadas into the calculator with this binding:

[When(@"I type (.*) into the calculator")] public void WhenITypeIntoTheCalculator(int p0) { ScenarioContext.Current.Pending(); }

Then the error information is more useful ("cannot convert to int" or words to that effect) than the error information I receive when using (\d+) in the binding ("no binding found" or similar). What is your experience here?

I know it a binding with (.*) will match anything, including asdf, but I put this question to you: have you ever encountered a situation where someone actually wrote a text string instead of a number?

[SabotageAndi]

@LirazShay The code for this generation is here: https://github.com/techtalk/SpecFlow/blob/master/TechTalk.SpecFlow/BindingSkeletons/StepDefinitionSkeletonProvider.cs But the SpecFlow Visual Studio Integration is using the SpecFlow 1.9 implementation of this class. Until this is updated (which is a huge work) the behavior of the generation will not change.

[gasparnagy]

@LirazShay As @dirkrombauts mentioned, in the majority of the cases, using the general regex (.*) makes the mistakes better discoverable. Also if you use the general regex, you can later on enhance the conversion with [StepArgumentTransformation], e.g. you can support values like "I wait for one second".

Please be careful. The regular expression \d+ is not a proper a representation of an int. It also allows very long numbers that cannot be represented by int, but it does not support negative numbers or thousand separators.

We have discussed this topic several times and it was a deliberate decision to stick to the general regexes. Using these makes you a real pro. ;-)

[LirazShay]

@dirkrombauts Maybe you right that the error information is more useful ("cannot convert to int" instead of "no binding found") but why to see that error only at runtime, not when writing the tests? if the regex include (\d+) then you will see it when you write the step and like compile time error that saves a lot of time by showing you the errors before running the code. So even it seems more reasonable to see the error when you write the tests (in the Visual Studio with the great extension of SpecFlow) and not wait until getting the error at runtime. Dirk, you asked if I ever encountered a situation where someone actually wrote a text string instead of a number? So, I think it is more helpful to make the default generated regex with (\d+) because when someone use your step he will see that he can only pass number to that step and not string.

Dear @gasparnagy , I think that 99% of the users do not have [StepArgumentTransformation] to convert 'one' to 1 I agree that if someone wants to use [StepArgumentTransformation] in order to use this step "I wait for one second" then he will need to change it manually like I do now, from (\d+) to (.*) But for 99% of the cases, it is more accurate the write the regex only for int and not for every string because in most cases you don't have StepArgumentTransformation so it will fall at runtime.

Gaspar, You said that the regular expression \d+ is not a proper a representation of an int:

1. Because very long numbers that cannot be represented by int
2. It does not support negative numbers or thousand separators My answer is: Currently in the current behaviour, those 2 issues still exist. I mean, if you have a step: [Given(@"I have entered (.) into the calculator")] And you pass '2.0' or '2,000' to (.) then will get exception: An exception is thrown System.FormatException : Input string was not in a correct format. Or long numbers to (.*) An exception is thrown System.OverflowException : Value was either too large or too small for an Int32. If someone wants to handle it he will need to add [StepArgumentTransformation] to parse those values

This means, the if you change the default representation for int in the suggested regex it will play well for all scenarios that are working now with (.*). And you will also show the users at compile time that only number will be matched And about negative numbers, of course the auto generated regex should be (-?\d+)

I can suggest another way to handle it, by allowing the users to configure the default regex in the options of the SpecFlow extension or something like this. or allow to override it by creating a template

[gasparnagy]

@LirazShay regarding the error message, the best would be to have an error message that tells which step definition is applied and failed editing-time. This is not possible currently, so you have to make a compromise. Either 1) you get the error message runtime or 2) you get a very generic error message (no matching step definition, without even a guess to the related step def) editing-time. Since BDD is a test-driven method, so you will run your tests regularly, altogether 1) is a better choice in the majority of cases. (Still, it can happen that for you the other would be more convenient.)

For the thousand separator: as you can see, if you use .*, and realize that these are not supported, you can still solve it globally with 4 lines [StepArgumentTransformation], instead of checking all the hundreds of regular expressions where you have used int parameter and replace the regex with a more complex one. The way how different domain concepts, like a number is represented is a global concern, not local.

There is an interface in specflow (IStepTextAnalyzer) that you could override with a plugin. But currently as @SabotageAndi mentioned, this only changes the step definition skeletons in the test output, but not the skeletons generated from Visual Studio "Generate step definitions" command. So currently there is no simple way to customize it unfortunately.

[LirazShay]

@gasparnagy Thanks for your answer! I'm sorry, but I still don't understand. If someone is writing the Features in another tool on in Visual Studio, then the only way you find out if the scenario is OK is by running the tests so the current message is better. But we're talking when writing the scenarios in Visual Studio with your great extension, so I and most of the users use the auto-completion feature to complete their step, let's talk about the first scenario, when the step does not exist, so if it will suggest skeleton for numbers only, and the user will copy and use this suggestion, I'm talking about the next time I want to write scenario and use this step. If I can pass string into this scenario I /Another tester will see the error only when running the test (OK, all tests together). But If the step regex is more concrete, then in editing time, I will see with the auto-completion feature that I can only pass number as argument. I won't need to run the tests at all in order to find out how what's the problem, in general, I prefer to write the regex in a way that will help the user to understand what arguments he can pass.

About the thousand separator, I agree with you, so if you find that the step contains number with thousand separator (1,000) than in the skeleton generated put (*.), but if step contains number without any separator then you can put (\d+) in the regex and no one will be hurt from this, but I can understand what you said that number representation is a global concern.

Thanks again!