fixed IndexOutOfBoundsException in gherkin java support

[trfiala]

This bug manifested itself in cucumber-jvm when running against a feature with no steps implemented yet. It looked like this in the maven output:

Scenario: Say hello # cucumber/examples/java/helloworld/helloworld.feature:3 Given I have a hello app with "Howdy" When I ask it to say hi When I ask it to say hiTests run: 3, Failures: 0, Errors: 2, Skipped: 1, Time elapsed: 0.56 sec <<< FAILURE! Feature: Hello World Time elapsed: 0.014 sec <<< ERROR! java.lang.IndexOutOfBoundsException: Index: 0, Size: 0 at java.util.ArrayList.rangeCheck(ArrayList.java:604) at java.util.ArrayList.get(ArrayList.java:382) at gherkin.formatter.PrettyFormatter.indentedLocation(PrettyFormatter.java:142) at gherkin.formatter.PrettyFormatter.printStep(PrettyFormatter.java:255) at gherkin.formatter.PrettyFormatter.match(PrettyFormatter.java:179) ...

[os97673]

please add test for your changes

[os97673]

btw looks like it is the same problem as in #246

[aslakhellesoy]

I think this PR would solve the problems with the pretty formatter. It's a bit of a hack, but I don't blame @trfiala for it - the code is gnarly to begin with. I say just apply it. I have a pending push to cucumber-jvm that trickles this bug and that is fixed by this change. That's good enough. Keep in mind that our next goal is Gherkin3 and new and simpler formatter which should make this bug go away.

[trfiala]

I was going to put a unit test in there for it as well today when I got a moment. I just needed to track down exactly how cucumber-jvm was triggering it with the setup I had (example HelloWorld feature test from cucumber-jvm without the steps defined).

-Todd

On Apr 23, 2013, at 3:15 PM, Aslak Hellesøy notifications@github.com wrote:

I think this PR would solve the problems with the pretty formatter. It's a bit of a hack, but I don't blame @trfiala for it - the code is gnarly to begin with. I say just apply it. I have a pending push to cucumber-jvm that trickles this bug and that is fixed by this change. That's good enough. Keep in mind that our next goal is Gherkin3 and new and simpler formatter which should make this bug go away.

— Reply to this email directly or view it on GitHub.

[ilanpillemer]

I get this error for any feature I have tried with when you use the pretty formatter (and you have step definitions) via the command line options. For example java-calculator. The reason its being triggered with hello world maven build I think is because it uses the ant build that has pretty turned on?

[brasmusson]

What seems to have happened in Cucumber-JVM 1.1.3 is that the order of calls to formatters (interface Formatter and Reporter) have changed. Previously the step() method was called for all steps in a scenario, before any call to match() and result(). In 1.1.3 the calls to match() (and then to result(), follows each call to step(). The first call to match() triggers the calculation of indentations (space between the step text and the step definition location) for the whole scenario, that is the scenario title and the first step (in 1.1.3), and the printing of the scenario title and the first step. Then when the second call to match() and result() occurs, the list of indentation is emtpy, since step() had not been called for the second step when the indentations were calculated, and an out of bounds exception occurs. (so this issue is not limited to features with no stepts implemented yet, where it manifested itself for @trfiala) This fix will inhibit the exception, but the output will not be that pretty any longer. The space between all but the first step in the scenarios and the step definition location will be one space, and the step definition location text will not be aligned. In terms of unit tests, scenario for the usage of Cucumber-JVM whould be a sequence of calls like: ... formatter.scenario() formatter.step() formatter.match() formatter.result() formatter.step() formatter.match() formatter.result() ... To me it is not clear if a formatter in the gherkin library also should support the usage scenario in Cucumber-JVM 1.1.2 and earlier: ... formatter.scenario() formatter.step() formatter.step() formatter.step() formatter.match() formatter.result() formatter.match() formatter.result() ...

[trfiala]

Thanks for tracking down more specifics. That's exactly why I didn't keep pursuing the fix on the gherkin side --- my output from Cucumber, while no longer crashing, was certainly incorrect. I saw doublings of gherkin lines, misclassification of color, and (IIRC) lack of generation of file/line number info. It wasn't clear to me which combination of gherkin/cucumber needed a change.

On May 21, 2013, at 4:45 AM, Björn Rasmusson notifications@github.com wrote:

What seems to have happened in Cucumber-JVM 1.1.3 is that the order of calls to formatters (interface Formatter and Reporter) have changed. Previously the step() method was called for all steps in a scenario, before any call to match() and result(). In 1.1.3 the calls to match() (and then to result(), follows each call to step(). The first call to match() triggers the calculation of indentations (space between the step text and the step definition location) for the whole scenario, that is the scenario title and the first step (in 1.1.3), and the printing of the scenario title and the first step. Then when the second call to match() and result() occurs, the list of indentation is emtpy, since step() had not been called for the second step when the indentations were calculated, and an out of bounds exception occurs. (so this issue is not limited to features with no stepts implemented yet, where it manifested itself for @trfiala) This fix will inhibit the exception, but the output will not be that pretty any longer. The space between all but the first step in the scenarios and the step definition location will be one space, and the step definition location text will not be aligned. In terms of unit tests, scenario for the usage of Cucumber-JVM whould be a sequence of calls like: ... formatter.scenario() formatter.step() formatter.match() formatter.result() formatter.step() formatter.match() formatter.result() ... To me it is not clear if a formatter in the gherkin library also should support the usage scenario in Cucumber-JVM 1.1.2 and earlier: ... formatter.scenario() formatter.step() formatter.step() formatter.step() formatter.match() formatter.result() formatter.match() formatter.result() ...

— Reply to this email directly or view it on GitHub.

[dkowis]

Running rake test on this, and everything passes. I can't see any reason not to make a release for it on its own to get past the problems people are having with other projects? If there's no complaints, I'll merge this one in, and release it, and then update the dependencies in the cucumber-jvm project.

[aslakhellesoy]

@dkowis LGTM - go ahead and merge it.

[brasmusson]

@dkowis @aslakhellesoy Note however that this PR does not really solves the problem with the Pretty formatter in Cucumber-JVM 1.1.3, it merely hides the problem. Since the underlying miss match remains, the miss match between the expectations of the Pretty formatter of Gherkin that step() is called for all steps in the scenario before any call to match() and the Cucumber-JVM 1.1.3 behavior of calling step(), match() and result() for each step before moving on the next, the Pretty formatter will in Cucumber-JVM will now display something like

  Scenario: Say hello                     # test\resources\cucumber\examples\java\helloworld\helloworld.feature:4
    Given I have a hello app with "Howdy" # HelloStepdefs.I_have_a_hello_app_with(String)
    When I ask it to say hi # HelloStepdefs.I_ask_it_to_say_hi()
    Then it should answer with "Howdy World" # HelloStepdefs.it_should_answer_with(String)

See also https://github.com/cucumber/gherkin/pull/261

In retrospect I should probably should have med a comment on this PR when submitting https://github.com/cucumber/gherkin/pull/261, I'm sorry I missed that

[dkowis]

I still haven't had any luck getting gherkin to build on my linux box, at least for the mono stuff :(

I'd like to get this one shipped since it'll at least resolve https://github.com/cucumber/cucumber-jvm/issues/476 . It might not be the best long-term solution, but I think that gherkin 3 should be the path forward in that direction anyway.

Could someone build/release a new gherkin for all the artifacts?

[aslakhellesoy]

I actually started working on a new gherkin release yesterday. I have a brand new mac, so there is a few things to set up. I also started using rbenv instead of rvm, so the build scripts will need some tweaking. I'm likely to spend several days on this.

[dkowis]

@aslakhellesoy /me is annoying, how is it going? :)

[bsletten]

I am running into this issue now too. Are there any workarounds?

[brasmusson]

@bsletten the workarounds I can think of are: 1) Do not use the pretty formatter with Cucumber-JVM 1.1.3 (replace with the progress formatter for instance) 2) Downgrade to Cucumber-JVM 1.1.2 3) Build and install into your local maven repository (a new version of the 2.12.0 version of) the Gherkin jar from the latest source, and upgrade to Cucumber-JVM 1.1.4-SNAPSHOT (which no longer shades the Gherkin jar as Cucumber-JVM 1.1.3 did, but declare a dependency to it in the pom)

[dkowis]

I can build everything but the Mono parts of gherkin, is that acceptable for releasing this guy? If so, I can do it this weekend.

[aslakhellesoy]

@dkowis that would be great!

[brasmusson]

@dkowis I give you a head up: I just discovered that with the very latest version in the cucumber repository (https://github.com/cucumber/cucumber/commit/2794e3b4fa7e1182eb037313b08a8c52ad672d99) the gherkin tests will fail the scenarios:

Failing Scenarios:
cucumber features/pretty_formatter.feature:6 # Scenario: Parse all the features in Cucumber
cucumber features/pretty_formatter.feature:13 # Scenario: Parse all the features in Cucumber with JSON

with the error (in an new example file for greek):

Parse error at .../gherkin/features/step_definitions/../../../cucumber/examples/i18n/el/features/addition.feature:9.
Found feature when expecting one of: comment, step, tag. (Current state: scenario_outline).

[brasmusson]

@dkowis To be more specific, if executing the cucumber tests in gherkin with:

bundle exec cucumber --profile default

The two scenarios fails on the new example file. However, I now realized that the builds on Travis uses the profile "travis", which does not include the test of the example files, so Travis builds for gherkin will pass also with the latest version in the cucumber repository. Therefore will not the immediate issues I foresee with releasing the gherkin library surface.

[dkowis]

Well, I can't build and install all of the things needed anyway. I don't have any Mac computers in my house, and I can't get the dependencies to build.

https://github.com/cucumber/gherkin#javascript I can't get kelbt to build:

14:02:17|dkowis@raziel kelbt-0.15  ruby-1.9.3-p429@cucumber
$ make
make[1]: Entering directory `/home/dkowis/temporary/kelbt-0.15/kelbt'
g++ -c -g -Wall  -I../aapl -o main.o main.cpp
In file included from ../aapl/bstmap.h:78:0,
                 from parsedata.h:29,
                 from main.cpp:32:
../aapl/bstcommon.h: In instantiation of ‘BstMapEl<Key, Value>* BstMap<Key, Value, Compare, Resize>::find(const Key&, BstMapEl<Key, Value>**) const [with Key = long int; Value = TransAp*; Compare = CmpOrd<long int>; Resize = ResizeExpn]’:
fsmgraph.h:320:47:   required from here
../aapl/bstcommon.h:422:43: error: ‘compare’ was not declared in this scope, and no declarations were found by argument-dependent lookup at the point of instantiation [-fpermissive]
   keyRelation = compare(key, GET_KEY(*mid));
                                           ^
../aapl/bstcommon.h:422:43: note: declarations in dependent base ‘CmpOrd<long int>’ are not found by unqualified lookup
../aapl/bstcommon.h:422:43: note: use ‘this->compare’ instead
make[1]: *** [main.o] Error 1
make[1]: Leaving directory `/home/dkowis/temporary/kelbt-0.15/kelbt'
make: *** [all] Error 2

@aslakhellesoy @jbpros @mattwynne Maybe one of you can do this? I don't have any apples, and I can't get the stuff to cooperate with me for release.

I suppose I could just release the java artifact alone, but I would think that'd be a bad thing?

What do you guys think?

@brasmusson I'll get to those after I get a released version of gherkin out there to get a hack around the pretty formatter blowing up for pretty much everyone that uses cucumber-jvm.

[kharsh]

still getting the same Exception for gherkin 2.11.8