Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

StringIndexOutOfBoundsException when optional argument not present #394

Closed
wketting opened this Issue · 6 comments

4 participants

@wketting

With cucumber-jvm version 1.0.14:

Given the following step:
Then I should see "Running version"

picked up by this stepdefinition:
@Then("^(?:|I )should see (.)(?:| within (.))$")

Gives StringIndexOutOfBoundsException.

Cause is because of the missing optional last argument in the step in JdkPatternArgumentMatcher:22 =>
arguments.add(new Argument(matcher.start(i), matcher.group(i)));
the new Argument object is created with contructor args (-1, null). (matcher.start(i) returns -1)
But it should be created with arguments (null, null), since in e.g. StepPrinter:10 the check is:
if (argument.getOffset() != null)
Now this gives StringIndexOutOfBoundsException on StepPrinter:11 =>
String text = stepName.substring(textStart, argument.getOffset());

Possible solution would be to adjust JdkPatternArgumentMatcher to give proper contructor arguments to Argument when the option does not exist.

Finally the complete stack of the exception:

java.lang.StringIndexOutOfBoundsException: String index out of range: -30
at java.lang.String.substring(String.java:1937)
at gherkin.formatter.StepPrinter.writeStep(StepPrinter.java:11)
at gherkin.formatter.PrettyFormatter.printStep(PrettyFormatter.java:254)
at gherkin.formatter.PrettyFormatter.match(PrettyFormatter.java:179)
at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39)
at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)
at cucumber.runtime.Utils$1.call(Utils.java:40)
at cucumber.runtime.Timeout.timeout(Timeout.java:12)
at cucumber.runtime.Utils.invoke(Utils.java:36)
at cucumber.runtime.RuntimeOptions$2.invoke(RuntimeOptions.java:119)
at $Proxy15.match(Unknown Source)
at cucumber.junit.JUnitReporter.match(JUnitReporter.java:60)
at cucumber.runtime.Runtime.runStep(Runtime.java:247)
at cucumber.runtime.model.StepContainer.runStep(StepContainer.java:44)
at cucumber.runtime.model.StepContainer.runSteps(StepContainer.java:39)
at cucumber.runtime.model.CucumberScenario.run(CucumberScenario.java:36)
at cucumber.junit.ExecutionUnitRunner.run(ExecutionUnitRunner.java:76)
at cucumber.junit.FeatureRunner.runChild(FeatureRunner.java:65)
at cucumber.junit.FeatureRunner.runChild(FeatureRunner.java:20)
at org.junit.runners.ParentRunner$3.run(ParentRunner.java:231)
at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:60)
at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:229)
at org.junit.runners.ParentRunner.access$000(ParentRunner.java:50)
at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:222)
at org.junit.runners.ParentRunner.run(ParentRunner.java:292)
at cucumber.junit.FeatureRunner.run(FeatureRunner.java:72)
at cucumber.junit.Cucumber.runChild(Cucumber.java:76)
at cucumber.junit.Cucumber.runChild(Cucumber.java:36)
at org.junit.runners.ParentRunner$3.run(ParentRunner.java:231)
at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:60)
at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:229)
at org.junit.runners.ParentRunner.access$000(ParentRunner.java:50)
at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:222)
at org.junit.runners.ParentRunner.run(ParentRunner.java:292)
at cucumber.junit.Cucumber.run(Cucumber.java:81)
at org.junit.runner.JUnitCore.run(JUnitCore.java:157)
at com.intellij.rt.execution.junit.JUnitStarter.main(JUnitStarter.java:63)

@theaberrant

This appears to be the same issue I'm dealing with. My workaround was to use the HTML reporter, but this is not optimal.

My case is with a step definition:
When(~"(\w+)?I press (\w+)") { String nothing, String opname ->
result = calc."$opname"()
}

I branched the calc Groovy example:
theaberrant/cucumber-jvm-groovy-example@3f012c6

Hope that helps - I'll see if I can find a solution, but I'm still learning.

@guyburton

I came up against this today- confusing for a little while. Possible options for improvement are:

  • throw an exception when the case of an 'optional' argument is detected
  • pass null for the argument
  • handle 'optional' arguments as either varargs or List in some way (and throw if signature doesn't support it)

I would be in favour of throwing in this situation- you should split into two seperate step defs if you want this behaviour I think.

I'm happy to make the changes and submit a pull request if we can agree on one of those approaches.

@aslakhellesoy

I think optional arguments in stepdefs are a smell since it blurs the ubiquitous language, so I think I like the first solution best - throw an exception with an explanation.

Could you send a pull request for that @guyburton?

@theaberrant

I think optional arguments in stepdefs are a smell since it blurs the ubiquitous language, so I think I like the first solution best - throw an exception with an explanation.

I use optional arguments in steps to modify something before it is created (say, an account) that isn't a normal case. Often times these items cannot be modified after the item is created, due to the limits of our system.

This issue only occurs with the PrettyPrint formatter, not with the HTML or jUnit formatters. Also, this only occurs if the optional argument is the first - another workaround is to add any text before the optional argument. From my previous example, adding 'this' before the first group doesn't throw the error.

When(~"This (\w+)?I press (\w+)") { String nothing, String opname ->
result = calc."$opname"()
}

I'm not sure why we would want to limit functionality for one formatter but not the others. Just my two cents though - I'm not really aware of the underlying code yet, so apologies if this doesn't make much sense. I'll update with a scenario if I can think of one - although I can't post work code.

Thanks for looking into it :)

@guyburton

@theaberrant I don't know what you mean when you say you use optional arguments to modify something before it is created. If you mean you are creating immutable objects and may want to specify different properties then you could use one of the following options:

  • use the Builder pattern to create your immutable objects. have one step def per property.
  • have one stepdef per set of configurations (equivalent to multiple static constructors)

After looking into capture group semantics, I may have changed my opinion and agree with you.

The regex engine numbers them from 'left to right' so they will always correspond to java arguments in the same way. I had assumed that they would be evaluated as 'encountered' when evaluating which wouldn't be useful for method parameters, because you would never know which bit of the regex was parsed. Thats not a good explanation, hopefully this example will demonstrate:

"^I wait (.* )?for (.) seconds$|^Someone like me waits for (.)$"
a) "I wait for 30 seconds" -> [null, 30, null]
b) "Someone like me waits for 30s" -> [null, null, 30s]

So for b) the regex engine is evaluating the right hand branch of the OR, but the first capture group it encounters is still numbered 3.

Using the OR operator like this is clearly a bit silly- you are writing two stepdefs in one method. In your example however, the first arg will (correctly) be null and the second will be correct. I can see that to be useful in some circumstances and as you point out, only in the formatter will there be an issue.

In conclusion I think the best thing is just to fix StepPrinter to handle an Argument index of -1 or construct the Argument with null for index in JdkArgumentMatcher (either of which is a trivial fix).

@aslakhellesoy Apologies for the confusion. The options I gave before were a bit misleading. I agree it's a smell for stepdefs but didn't realise that using optional capture groups actually could/does work sensibly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.