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

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

Open
FredVaugeois opened this issue Apr 26, 2023 · 12 comments

Comments

@FredVaugeois
Copy link

FredVaugeois commented Apr 26, 2023

🤔 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
Copy link
Author

FredVaugeois commented Apr 26, 2023

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
Copy link
Contributor

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
Copy link
Author

FredVaugeois commented Apr 27, 2023

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
Copy link
Author

FredVaugeois commented Apr 27, 2023

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
Copy link
Contributor

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
Copy link
Author

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
Copy link
Contributor

mpkorstanje commented Apr 27, 2023

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
Copy link
Contributor

mpkorstanje commented Apr 27, 2023

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
Copy link
Author

FredVaugeois commented Apr 27, 2023

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
Copy link
Contributor

mpkorstanje commented Apr 27, 2023

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
Copy link
Author

FredVaugeois commented Apr 27, 2023 via email

@luke-hill
Copy link
Contributor

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.

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

No branches or pull requests

3 participants