cucumber / cucumber-expressions

Human friendly alternative to Regular Expressions
MIT License
152 stars 51 forks source link

Add the ability to ignore a cucumber expression parameter just like a non capturing regex #222

Open FredVaugeois opened 1 year ago

FredVaugeois commented 1 year ago

🤔 What's the problem you're trying to solve?

Currently, there is no way to ignore a cucumber expression parameter, even though it is possible to do so in a normal regex. Thus, I think it would be nice to be able to have access to that feature in cucmber expressions as well.

✨ What's your proposed solution?

My proposed solution is simply to add the same mechanism as with a non-capturing regex group (for example (?:.*?)) like this, for example:

@Given( "{string} Database Entry with {?string} {string} has {string} = {string}" )
public void databaseEntryWithIdHasString( String tableName, String id, String columnName, String value) {
    ...
}

As you can see in my snippet, the cucumber expression {?string} is ignored, as it is just a string that will be used to define the name of the id. It's just a way to add more clarity to my features without the need of creating a new step definition for each and every way we can describe our column id.

Currently, I need to add it to my method signature like the snippet below and get warnings that my parameter idName is not used, which requires me to suppress the warning (we are using SonarQube and this kind of warning decreases our code quality):

@SuppressWarnings( "unused" )
@Given( "{string} Database Entry with {string} {string} has {string} = {string}" )
public void databaseEntryWithIdHasString( String tableName, String idName, String id, String columnName, String value) {
    ...
}

I think that in the parser that you are using to convert cucumber expressions to their actual parameter value, you can simply filter out the ones that start with a question mark, as simple as that.

⛏ Have you considered any alternatives or workarounds?

Yes, I have considered using a regex instead, but I feel like that adding the feature would be pretty low-effort, and I'd rather keep using cucumber expressions, since the reason why they exist is to make the step definitions more readable (I want to keep my step definitions readable).

đź“š Any additional context?

FredVaugeois commented 1 year ago

I would also like to add that I was using version 6.2.2 before and it was possible to combine regular expressions AND cucumber expressions (so that need was fixed by using the regex non-capturing group), but when I updated to version 7.11.1, it stopped working. Here is what I had before upgrading:

@Given( "{string} Database Entry with \"(?[a-zA-Z0-9 _]*)\" {string} has {string} = {string}" )
public void databaseEntryWithIdHasString( String tableName, String id, String columnName, String value ) {
    ...
}
mpkorstanje commented 1 year ago

What would be a reason to mention the idName in a feature file if it is ignored? Currently it seems like an incidental detail.

FredVaugeois commented 1 year ago

Ok so basically I am using step definitions to update data in a database. Some tables have a multi-column primary key. So my point is I don't want my step defintion to be like:

@Given( "{string} Database Entry with Id1 {string} and Id2 {string} has {string} = {string}" )
public void databaseEntryWithIdsHasString( String tableName, String id1Value, String id2Value, String columnName, String value ) {
    ...
}

because it's hard to maintain and hard to understand what Id I'm talking about. Thus, I want to have the ability to say "you can write any {idName} to define your primary key column names e.g.

Given "Foo" Database Entry with Foo Id "foo" and Bar Id "bar" has "foo.bar" = "foobar"

My {idName} is actually a custom parameter type defined like this:

@ParameterType( "([a-zA-Z0-9 _]*)" )
public String idName( String idName )
{
    return idName;
}

My point is, if it's possible to ignore a capture group with regexes, why is not allowed with cucumber expressions? The idea being that if it was created for regexes, it's because it was a requested/useful feature that can be as useful in our cucumber expressions case.

FredVaugeois commented 1 year ago

Furthermore, I was totally happy with my original solution (with version 6.2.2) where I could combine regex and cucumber expression. I lost that ability by upgrading the package, which is why I'm requesting this feature.

@Given( "{string} Database Entry with (?[a-zA-Z0-9 _]*) {string} has {string} = {string}" )
public void databaseEntryWithIdHasString( String tableName, String id, String columnName, String value ) {
    ...
}
mpkorstanje commented 1 year ago

My point is, if it's possible to ignore a capture group with regexes, why is not allowed with cucumber expressions? The idea being that if it was created for regexes, it's because it was a requested/useful feature that can be as useful in our cucumber expressions case.

Cucumber expressions are intended to be simple, extremely simple even. We could make them more complex but at some point you may aswell use a regular expression. Where that point exactly is, is ofcourse rather hard to say.

That it was possible to mix regular expressions was a bug caused by a rather naive implementation. It's not a feature we intended to support.

This all doesn't mean we're not taking new features requests. And broadly speaking feature requests are evaluated by their utility. So a concrete real world example of your actual problemen would help.

So far I get the impression that your suggestion doesn't have much utility. Because the parameters goes unused these two steps are equivalent:

Given "Foo" Database Entry with "arbitrary irrelevant text" has "foo.bar" = "foobar"
Given "Foo" Database Entry has "foo.bar" = "foobar"

By all appearances the phrase with "arbitrary irrelevant text" is an incidental detail that can be removed from the feature file and so you should not have a problem in your step definitions. And if it does matter to the reader, then it seems it should also matter in the code.

Now personally I think I'm probably not understanding your problem quite right. So I think a concrete real world example would help.

FredVaugeois commented 1 year ago

I agree that maybe I should just use regexe's. My use case is mainly for reporting purposes since these testing reports are sent to some non technical people. So it's way easier for them to understand:

 Given "Product" Database Entry with Product Name "Product", Product Type "Food" and Product Subtype "Dairy" has "price_column" = 300.00

than

 Given "Product" Database Entry with Id1 "Product", Id2 "Food" and Id3 "Dairy" has "price_column" = 300.00

The reason why I don't need the actual "description" of the id is because it's my internal java logic that knows which id is what.

I agree that it needs to be as simple as possible, that's why my solutions was quite simple by simply marking it as optional with a "?" juste like regexes. You can also see it as being the same use case as the optional text. Why was that implemented if it's as irrelevant as my "id description". I agree that it is a "small detail" but it's the same basic idea of "Sometimes I want to have an "s" at the end and sometimes I don't". Sometimes I want to describe more clearly my id, sometimes I don't.

If that makes more sense than having a "?" like I proposed, it could be implemented with parentheses just like it is with optional text. Again, with the alternative text. It is allowed to use different words for the same step definition, since stomach and belly are synonyms. Same thing for me: my id names are all "synonyms" in the sense that they all represent an Id, but I still would like to be able to define them differently in my feature without creating another step definition. I could even use alternative text, but the problem is that I have hundreds of different names for my ids in my features, so that would just be a huge list for no reason.

So basically, the reason why I'm requesting the feature is the same reason why alternative text and optional text exists: sometimes, the same statement corresponds to the same step definition, but written in an alternative way.

Does that make more sense? :)

mpkorstanje commented 1 year ago

I still get the feeling something is missing.

Your first example roughly speaking defines a map, while the second one defines a list. Yet it seems you're able to ignore the field names because you are using the order implicitly in the first example.

So what happens if I write the IDs and field names in a different order in your first example?

mpkorstanje commented 1 year ago

In essence these should be equivalent

Given "Product" Database Entry with Product Name "Product", Product Type "Food" and Product Subtype "Dairy" has "price_column" = 300.00
Given "Product" Database Entry with Product Subtype "Dairy", Product Name "Product" and Product Type "Food" has "price_column" = 300.00

But I don't see how they can be without using the field name somehow.

FredVaugeois commented 1 year ago

The order indeed is defined implicitly. You are right in assuming changing the order would not work. Again, we add the "Id names" to describe what is the id since the order is, again, implicit and not clear on a feature level. Thus, wether or not I am describing a map, the point is, the text can be seen as either optional or alternative text.

Which is already partly supported by cucumber. So my feature request is simply to add the ability to have optional "words/objects" in your step definition instead of just characters. Basically "combining" alternative and optional text.

mpkorstanje commented 1 year ago

It doesn't really matter how "easy" a feature is to add. Rather, there should be a use for it. Currently it seems to me your step definitions have a bug and with that there is presently no use for the feature. This may change in the future, if and when some one runs into case where it is useful. But right now there seems to be none.

Now of-course, you can work around it. Nothing stops you from using a regular expression. But you should know better. The most pragmatic and short term thing you can probably do is to suppress the warning and add a comment that explains the order is implicit, not explicit.

FredVaugeois commented 1 year ago

What do you mean by "your step definition has a bug"? It doesn't have a bug. It works fine, I just have to suppress my warning.

The use for my feature is the same use as someone who uses alternative text or optional text, I don't understand why you are saying it has no use case. Forget my actual implementation, just focus on the idea of providing a way to extend these features.

The use case for "belly/stomach" was already proven to be of value. Same for "cucumber(s)". How does providing a way to have an optional "Cucumber expression capture group" not fit the same exact motivation of creating these two features in the first place?

luke-hill commented 1 year ago

There "is" a way to kind of ignore it, you can have the transformer return a nil or language equivalent. I appreciate this will still throw your indices out of arity captures, but it does work - I have used it.