Optional text not working as expected

[spatel1009]

Summary

When using optional text in the stepdef, it is not being read by the Gherkin as expected

Expected Behavior

Would like to use optional word where it is applicable to all alternative words following and not just the first one listed. Currently the third option below is what we are using since in that scenario only call needs the word test in front of it, but for future use may be instances were multiple alternative text have that optional word in front and would like to have possibility of being interchangeable.

Current Behavior

the( test) chat/email/call interaction is not visible
only reads: "And the test chat/email/call interaction is not visible"
Will not read: "And the chat interaction is not visible"
Nor singular: "And the test chat interaction is not visible"
Summary: Will only read where "test" is used and not optional with all 3 alternative words also included in the Gherkin

the (test) chat/email/call interaction is not visible
Will read "And the test call interaction is not visible"
But not "And the call interaction is not visible".
Summary: Will read only when test is put in front of the 3 options, not without. Basically renders "test" not optional

"the (test )call/email/chat interaction is not visible"
Will read "And the test call interaction is not visible"
And also: "And the chat interaction is not visible"
But not "And the test chat interaction is not visible"
Summary: Will read the use of test when in front of call only, not for email or chat.

Steps to Reproduce (for bugs)

Unable to provide company code, however, creating a stepdef that needs optional words in front of 2 out of 3 alternative text directly following it will help recreate issue.

Context & Motivation

This issue is for non-capture groups so this allows for more clear readable gherkin linking to same test for multiple portions of app.

Your Environment

Java

[luke-hill]

Just for benefit (Although you have half done it), you "could" reproduce the failing code. As you could create a feature that just printed nil to stdout for each step def or a blank string.

I'll try reproduce this issue, but as I'm a rubyist, this may be a java-centric problem. I don't know without a full repro scenario.

The best type of reproduction scenario is a failing test against our CI. See if you can do that, using our code. It could be your environment is causing some issue. There are lots of factors here.

[aslakhellesoy]

@luke-hill if this really is a bug, it's likely present in all implementations (including Ruby). The easiest way to reproduce it is to write a failing test in CucumberExpressionTest

[spatel1009]

Hey not sure if this is what you meant but I added a couple tests in how we are trying to use it?

master...spatel1009:patch-1

[luke-hill]

That's exactly the kind of thing that's brilliant thanks. Although obviously those use cases don't cover the full extent of ones you mentioned.

If you could add the ones you are physically using so we have a close repro scenario and then make it as a PR, it'll run through our CI and it'll automatically flag whether they fail or not. Giving us a much easier time to triage.

Thanks for your swift response.

[aslakhellesoy]

@spatel1009 considering this:

assertEquals(emptyList(), match("three (brown )mice/rat/easter", "three brown mice"));

I think the cucumber expression parser gets confused here.

Is )mice/rat/easter interpreted as )mice or rat or easter? Perhaps we should disallow {}() as part of / alternation and throw a better error message? I'm not sure.

Another way to approach this is to write a slightly lower level test in CucumberExpressionPatternTest and assert what regexp you expect this to be translated into.

[spatel1009]

That's exactly the kind of thing that's brilliant thanks. Although obviously those use cases don't cover the full extent of ones you mentioned.

If you could add the ones you are physically using so we have a close repro scenario and then make it as a PR, it'll run through our CI and it'll automatically flag whether they fail or not. Giving us a much easier time to triage.

Thanks for your swift response.

Add a few of our expected stepdefs to the CucumberExpressionTest or create a separate repo with a drawn out stepdef/feature file flow?

[spatel1009]

@spatel1009 considering this:

assertEquals(emptyList(), match("three (brown )mice/rat/easter", "three brown mice"));

I think the cucumber expression parser gets confused here.

Is )mice/rat/easter interpreted as )mice or rat or easter? Perhaps we should disallow {}() as part of / alternation and throw a better error message? I'm not sure.

Another way to approach this is to write a slightly lower level test in CucumberExpressionPatternTest and assert what regexp you expect this to be translated into.

I have added a test to this with the expected regex.
#770

[mpkorstanje]

Is )mice/rat/easter interpreted as )mice or rat or easter? Perhaps we should disallow {}() as part of / alternation and throw a better error message? I'm not sure.

There seems to be quite some ambiguity we'd have to resolve first I think.

What is the meaning of (brown )mice/rat?

• matches brown mice or brown rat
• matches brown mice or rat

If the latter we'd have to make the rewrite rules include optionals when rewriting alternatives.

If the former we we'd have to stop matching alternatives at whitespace, ) and } on the left or whitespace, ( and { on the right.

[spatel1009]

Sure so we don't have many cases of this but the way the regexTest I wrote may explain better but the regex version allows for brown mice, brown rat, mice, rat. Basically all 4 variations and we we attempting to do that with cucumber expressions.

[mpkorstanje]

Cucumber expressions are intended to be a simpler alternative to regular expressions. So if you are trying to do all that with cucumber expressions you may want to consider defining multiple step definitions or sticking with regular expressions.

Still there is a bug here.

[spatel1009]

Yes we are definitely leaning towards breaking them out since we want to move away from regex completely and utilize cucumber expressions solely.

[mpkorstanje]

Some more interesting cases:

• three (brown/black) mice
• three (brown)/(black) mice

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in a week if no further activity occurs.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in a week if no further activity occurs.

[stale]

This issue has been automatically closed because of inactivity. You can support the Cucumber core team on opencollective.