This fixes an issues where negative numbers were not passed correctly… #623
Conversation
… as arguments when using regexless methods. The underlyinh problem was that when using regexless parameters a regex is generated, and this regex was using '\W' between any words in the method name. this matches anything which is not a-zA-Z0-9_, which includes the '-' character which is part of the negative number. As I think that we are really matching gaps between words I have changed this to use '\s' which simply matches any whitespace characters. We could instead explictily use the non-word match and add the '-' character if we change it to '[^a-zA-Z0-9-]'. Both make the test pass, but the withspace one seems better to me. Any input appreciated. This area does not contain any unit tests, but I have added an integration test which checks this, and based it on the scenario in the issue which this fixes, SpecFlowOSS#246 I might be able to add some unit tests in the future, but this seems sufficient for now.
|
bah, broke another test. Will investigate and fix. |
|
is this test a situation we should be supporting: should bind to [When]
public void When_WHO_does_WHAT_with(string who, string what, Table table)
{
if (what != "something") throw new Exception("invalid parameter: " + what);
}this feels like its wrong to me, like we are supporting allowing random punctuation to be included in the scenario definition for no apparent reason. Generating a regex which allows this but doesn't also consume the '-' character for negative numbers is going to be more tricky I expect |
|
Ah I've woken up understanding the use case for this test. its not a random bit of punctuation - its a sentence with a dash used as a separator, as I've just done. I'll fix the generated regex to cope with this |
…enerated Regex to be '\s+(?:-(?!\d)\s?)?' which copes with all the current test cases. This pattern basically says between the words we can have at least one white space character which can optionally be followed by a '-' (as long as that is not immediately followed by a number) which can optionally be followed by any number of which space characters.
|
Ok I think I have modified the regex to make all the tests pass, but I'm wondering if there are sufficient test cases. It copes with the ' - ' used as a separator, but won't cope with other things like semi colons, colons, currency symbols etc. Ideally we want to say 'consume all non word characters unless its a dash followed by a number'. My regex isn't all that but I'll keep looking at this and see if I can come up with something better, and try and add a few more test cases around this to cope with currency values in particular |
…s one or more non whitespace chars until it finds one which is a follwed by a number which is preceeded by a minus sign
|
I think this is fixed now. All test cases pass now, including addtional ones for currency and negative numbers |
|
Ok, thx! |
… as arguments when using regexless methods. The underlyinh problem was that when using regexless parameters a regex is generated, and this regex was using '\W' between any words in the method name. this matches anything which is not a-zA-Z0-9_, which includes the '-' character which is part of the negative number. As I think that we are really matching gaps between words I have changed this to use '\s' which simply matches any whitespace characters. We could instead explictily use the non-word match and add the '-' character if we change it to '[^a-zA-Z0-9-]'. Both make the test pass, but the withspace one seems better to me. Any input appreciated.
This area does not contain any unit tests, but I have added an integration test which checks this, and based it on the scenario in the issue which this fixes, #246
I might be able to add some unit tests in the future, but this seems sufficient for now.