Handle interleaved calls to step, match and result in the java PrettyFormatter #261
Conversation
The java PrettyFormatter has relied on that the call to step() for all steps in a scenario, come before any call to match() for any step in the scenario. In Cucumber-JVM version 1.1.3 the call sequence was changed so this assumption is not true anymore. To handle interleaved calls to step(), match() and result(), the java PrettyFormatter is changed to also store the received match and result data in a list until the end of the scenario. Then the indentations for the step definition locations are calculated, and the whole scenario is printed at once. The final formatted result is not changed, but for long running scenarios the delay of printing until the end of the scenario will be notable. Since the printing is deleyed until the end of the scenario, the printing of the current step as executing is also removed.
|
Tests failed :( Please fix them. |
|
Yep, I have seen the failed tests. I was aware - and not very happy about - that I had not executed other tests that the java tests for the change. But I gave up after struggling with making it through 'rake spec' on Ruby 1.9.3 on Windows. When I got an "expecting '...\r\n...' got '...\n...'" from a spec that did not seem related at all to the PrettyFormatter, I just gave up. On the other hand, if I had succeeded with running the tests on Ruby 1.9.3, I would likely have been so happy that is would not occur to me to rerun test test on JRuby. There are two problems to be handled in the failing spec:
I have made the changes in the spec and tested them on Ruby 1.9.3, but have not yet succeeded with running that spec on JRuby (JRuby 1.7.4 on Windows - RVM is not available on Windows) with the gherkin jar-file I have build (with maven). Currently I am stuck on the error: Chances are that I see no other option than to push up those changes without succeeding to have tested them locally, and I do not like that option at all. |
Update the rspec spec for the pretty formatter to explicitly mark the end of the scenarios with a call to Formatter.eof(). Relax the spec to allow for but not require the "executing" output for the currently executing step. The java Pretty Formatter is not printing those, since it needs to wait to the end of the scenario to be able to calculate the indentation for the step definition locations for the scenario.
|
JRuby, Windows and myself do not go together very well. But finally I have managed to execute the updated failing spec both on the Ruby side and the JRuby+Java side. A new commit is coming soon... |
|
Now that cucumber/cucumber-jvm#557 has been merged into Cucumber-JVM, this PR is practice not needed anymore. Unless it is important that the java PrettyFormatter handles both "all steps first" and "one step at the time" execution. Given that Gherkin3 is coming, it is probably not that important. |
|
The decision to do
was probably not a good decision, so I'm happy to see this fix. It'll make it easier to revert to the old
|
The java PrettyFormatter has relied on that the call to step() for all the steps in a scenario, come before any call to match() for any step in the scenario. When the call to formatter.step() in cucumber.runtime.model.StepContainer was moved from the format() method to runStep() method in the commit cucumber/cucumber-jvm@4fba53c this assumption is not true anymore. Therefore IndexOutOfBoundsException is thrown from the PrettyFormatter in Cucumber-JVM version 1.1.3 on the second step of any scenario. This has been noted several times, for instance in cucumber/gherkin#252 and cucumber/cucumber-jvm#491, but proposed solutions have masked the problem rather than adapting to the root cause of the problem.
To solve the root cause of the exception, the java PrettyFormatter in this pull request is changed to handle the interleaved sequence of calls to step(), match() and result() that Cucumber-JVM version 1.1.3 is issuing. The solution in the java PrettyFormatter is to also store the received match and result data in a list until the end of the scenario. Then the indentations for the step definition locations are calculated, and the whole scenario is printed at once.
The final formatted result is not changed, but for long running scenarios the delay of printing until the end of the scenario will be notable. Since the printing is delayed until the end of the scenario, the printing of the current step as executing is also removed. This has the side effect that most of the ANSI up cursor magic mentioned in cucumber/gherkin#187 is removed.
Note, for this pull request the java code has been tested through mvn install in the java directory, with the test passed. Other tests, for instance through rake, have not been performed.