Use Gherkin v4 #1035

brasmusson · 2016-08-04T13:49:11Z

This PR updates Cucumber-JVM to use the Gherkin v4 parser and compiler.

Notes:

Since this change contains non-backward compatible changes, the version is updated to 2.0.0-SNAPSHOT.
Since Gherkin in v3/v4 has changed the group id from info.cukes to io.cucumber, the group id of Cucumber-JVM is also changed to io.cucumber.
The formatter interface is re-worked completely (it had to it was previously defined in Gherkin v2), and is not based on events that are fired from inside Cucumber-JVM and that the formatters can register listeners for.
Now when compiling features to pickles, and the to test cases, the step keyword is not available when running the test cases (basically the scenarios), there for the generated snippets always use the first step keyword of the language, and the keyword is not used when reporting the steps.
When using notifications schemas for other testing frameworks (for instance JUnit), Feature: <feature name> have often been used as "class name", now the feature file path is used instead (as the feature AST is not available after compiling the feature to pickles).
Previously the line filtering has been quite flexible, for instance if specifying the line of a step - then the scenario that the step is part of has been executed, or is specifying the line of an example table - then all scenarios from that table have been executed. Now the pickle only contain information of the line of the Scenario (or the line of the Scenario Outline and the example row that created the pickle), so that is the only line(s) that will execute the test case of the pickle.
The Pickle class in Gherkin does not have a getter for the tags of the pickle, therefore reflection has to be used to access the pickle tags.
The GherkinDialect class in Gherkin does not have a getter for each keyword, therefore reflections is used to implement the --i18n option.
To not lose the support to run Cucumber-JVM in OSGi containers, Gherkin need to define the Java jar as a bundle (chore(deps): update cucumber/action-publish-pypi action to v3 gherkin#221).

All in all, I do not see any blocker for merging this and creating a 2.0.0-SNAPSHOT. Before releasing a 2.0.0 version some more things (pretty and html formatters, OSGi support) are needed though.

aslakhellesoy · 2016-09-06T20:35:48Z

You are a machine @brasmusson ! I'll do everything I can to cut a new release this week. Thanks for your awesome work.

mattwynne · 2016-11-23T09:23:20Z

@aslakhellesoy are you the only person who can make releases? Is there anyone else with the experience / authority to merge this PR? Seems a shame for this to get stuck for so long just because you're busy.

Use the new tag-expressions library, but support both the use of the new tag expression syntax and the old tag expression syntax.

mpkorstanje

Reviewed up to 51c8656

Might be worth putting this on the backlog. Maven Relocation
Is there a reason not to match the package structure to the groupId? E.g. io.cucumber.api rather then cucumber.api?

mpkorstanje

Reviewed the [JUnit] changes

Would suggest moving contents of cucumber.runtime.junit into cucumber.api.junit so all classes aside from Cucumber can be package private.

PickleWrapper and PickleStepWrapper implement Serializable but their contents is not serializable (infact, their contents is not needed at all).

I don't quite understand SanityChecker. Should probably be in test sources.

Submitted nitpicks as #1122. No point in writing them out.

mpkorstanje · 2017-05-13T23:01:17Z

junit/src/main/java/cucumber/runtime/junit/ExecutionUnitRunner.java


-    public ExecutionUnitRunner(Runtime runtime, CucumberScenario cucumberScenario, JUnitReporter jUnitReporter) throws InitializationError {
+    public ExecutionUnitRunner(Runner runner, PickleEvent pickleEvent, JUnitReporter jUnitReporter) throws InitializationError {
        super(ExecutionUnitRunner.class);


As far as I can tell this should be super(null). There is no reason to scan this class for annotations.

mpkorstanje · 2017-05-14T20:01:14Z

core/src/main/java/cucumber/api/TestCase.java

+        ScenarioImpl scenarioResult = new ScenarioImpl(bus, pickleEvent.pickle);
+        for (TestStep step : testSteps) {
+            Result stepResult = step.run(bus, pickleEvent.pickle.getLanguage(), scenarioResult, skipNextStep);
+            if (stepResult.getStatus() != Result.PASSED) {


I guess you intended this to be an enum comparison not a string?

This is also causing problems in TestNgReporter#result. It compares strings against objects for Result.UNDEFINED.

Now that the Result class has moved to this project from gherkin, there is an opportunity improve things like the definition of status codes. I'll look into that.

By the introduction of an enum for the result statuses in #1133, this kind of problem should be eliminated.

mpkorstanje · 2017-05-14T20:12:04Z

junit/src/main/java/cucumber/api/junit/Cucumber.java

@@ -57,10 +62,10 @@ public Cucumber(Class clazz) throws InitializationError, IOException {

        ResourceLoader resourceLoader = new MultiLoader(classLoader);
        runtime = createRuntime(resourceLoader, classLoader, runtimeOptions);
-
+        formatter = runtimeOptions.formatter(classLoader);


I don't understand this. Is this needed? This also happens in Runner#buildBackendWorlds which calls runtimeOptions.getPlugins();

mpkorstanje · 2017-05-14T21:01:56Z

core/src/main/java/cucumber/runtime/formatter/TestSourcesModel.java

+import java.util.HashMap;
+import java.util.Map;
+
+public class TestSourcesModel {


I don't think TestSourcesModel is part of the public API but I can't tell for sure. Whole thing should probably be package private.

mpkorstanje · 2017-05-14T21:25:02Z

core/src/main/java/cucumber/runtime/model/CucumberFeature.java

@@ -1,83 +1,99 @@
 package cucumber.runtime.model;


This package has become quite anemic. All closely tied to RuntimeOptions. Might want to consolidate this, RuntimeOptions and friends into a options package.

mpkorstanje · 2017-05-17T20:51:02Z

core/src/main/java/cucumber/runtime/table/DataTableDiff.java

+import java.util.ArrayList;
+import java.util.List;
+
+public class DataTableDiff extends DataTable {


This and other classes are all public but don't need to be. They're not part of the public API as far as I can see.

mpkorstanje · 2017-05-17T21:09:35Z

core/src/main/java/cucumber/runtime/LinePredicate.java

+    public boolean apply(PickleEvent pickleEvent) {
+        String picklePath = pickleEvent.uri;
+        if (!lineFilters.containsKey(picklePath)) {
+            return true;


I think this should be false. If a Pickle isn't referenced in the rerun file it shouldn't be added to the list of pickles to execute.

Unit tests confirm the behavior is as intended, but I can't find any thing to explain this behavior there either.

I finally remembered why it should be true, see the (new) explaining comment in the test case

That makes sense. The runner presents each file[:line] arguments as having or semantics. But internally it first finds all feature files by looking at the file part and then reduces the set of tests with the line predicate.

mpkorstanje · 2017-05-17T21:42:08Z

Reviewed the [Core] changes

Took me a while to do this, this change is somewhat voluminous but I found nothing major. This is mostly a straightforward rewrite against a changed API. There are some TODOs present and a few disabled tests but they don't seem to matter too much.

Pushed more more nitpicks to #1122. No point in writing them out.

brasmusson · 2017-05-18T18:21:08Z

@mpkorstanje Your effort is much appreciated.

brasmusson · 2017-05-18T18:26:58Z

Is there a reason not to match the package structure to the groupId? E.g. io.cucumber.api rather then cucumber.api?

I do not know, when I got involved the groupId did correspond to the web site (http://cukes.info/), but the package structure was cucumber.<x>, gherkin.<x>. Now then the web site has moved to http://cucumber.io, the goupdId changes seems reasonable (and the newer gherkin release has changed groupId).

mattwynne · 2017-05-18T20:25:35Z

@mpkorstanje Your effort is much appreciated.

Amen! 👏

mpkorstanje · 2017-05-22T20:22:44Z

testng/src/main/java/cucumber/api/testng/FeatureResultListener.java

-
-    @Override
-    public void embedding(String mimeType, byte[] data) {
-        reporter.embedding(mimeType, data);


I can not find where this method has been replaced with an event listener. From what I can see this is not needed as the Runtime takes care of this now and will inform the reporter from the pllugins. The TestNgReporter itself prints to sysout and doesn't care about embeddings.

So no problem.

mpkorstanje · 2017-05-22T20:23:07Z

Reviewed the [TestNG] changes

9021844

Nothing major.

Pushed a fix for the string vs non-string comparison in the TestNgReporter to #1122.

Pushed more more nitpicks to #1122.

aslakhellesoy · 2017-05-25T14:46:16Z

Hey @brasmusson and @mpkorstanje - awesome work on this and sorry it took so long!

brasmusson · 2017-05-26T12:02:24Z

@mpkorstanje

I don't quite understand SanityChecker. Should probably be in test sources.

Yes, it is test related, but it is also used in the test of picocontainer. AFAIK it needs to be in the src/main part to be usable from the picocontainer module (on the other hand all of that testing maybe should be moved to the junit module).

lock · 2018-10-25T02:13:20Z

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

brasmusson mentioned this pull request Aug 11, 2016

Handle doc string and data tables in a single step #911

Closed

This was referenced Sep 1, 2016

CucumberOptions: Tags and name do not work well together #976

Closed

Tags at the examples block are not treated as actual tags in scenario #849

Closed

Pickle filters cucumber-attic/gherkin#229

Closed

brasmusson force-pushed the use-gherkin4 branch from 95b15a2 to ecfc554 Compare September 4, 2016 17:42

brasmusson mentioned this pull request Oct 27, 2016

Initialise parser in arbitrary state cucumber-attic/gherkin#109

Closed

brasmusson force-pushed the use-gherkin4 branch from e14c980 to b339d0c Compare November 20, 2016 11:42

Update versions to 2.0.0-SNAPSHOT and gherkin to 4.0.0.

5f704b9

brasmusson force-pushed the use-gherkin4 branch from b339d0c to 832c378 Compare November 20, 2016 12:09

Change the maven groupId to io.cucumber.

51c8656

brasmusson force-pushed the use-gherkin4 branch from 832c378 to 1031db6 Compare November 20, 2016 12:40

brasmusson mentioned this pull request Nov 23, 2016

Support concurrent executions of scenarios #630

Closed

brasmusson added 13 commits December 18, 2016 19:40

[Core] Updated core for the use of Gherkin v4.0.0.

aca512d

[JUnit] Updated junit for the use of Gherkin v4.0.0.

5df2263

[Java] Updated java for the use of Gherkin v4.0.0.

41c169a

[Java8] Updated java8 for the use of Gherkin v4.0.0.

d66b876

[Android] Updated android for the use of Gherkin v4.0.0.

de5fe34

[Clojure] Updated clojure for the use of Gherkin v4.0.0.

c9af7f5

[Groovy] Udated groovy for the use of Gherkin v4.0.0.

46a35ba

[JRuby] Updated jruby for the use of Gherkin v4.0.0.

7c88881

[Jython] Updated jython for the use of Gherkin v4.0.0.

7841adf

[Rhino] Updated rhino for the use of Gherkin v4.0.0.

d50a9d9

[Scala] Updated scala for the use of Gherkin v4.0.0.

90ce0a3

[TestNG] Updated testng for the use of Gherkin v4.0.0.

9021844

Use the tag-expressions library.

1af20c5

Use the new tag-expressions library, but support both the use of the new tag expression syntax and the old tag expression syntax.

brasmusson force-pushed the use-gherkin4 branch from 1031db6 to 1af20c5 Compare December 18, 2016 18:44

This was referenced Jan 1, 2017

NullPointerException from @Before tag #701

Closed

JsonFormatter attach text to last step when sceneario.write is invoked from after hook #1080

Closed

brasmusson added 2 commits March 18, 2017 17:39

[Java] Do not pass the language outside the pickle.

4243175

[Junit] Do not pass the language outside the pickle.

fd1b645

brasmusson mentioned this pull request May 1, 2017

Run each Scenario as a TestNG @Test. #1118

Closed

6 tasks

mpkorstanje mentioned this pull request May 13, 2017

How do we rerun failed scenarios temyers/cucumber-jvm-parallel-plugin#31

Closed

mpkorstanje reviewed May 13, 2017

View reviewed changes

mpkorstanje reviewed May 14, 2017

View reviewed changes

mpkorstanje reviewed May 17, 2017

View reviewed changes

mpkorstanje mentioned this pull request May 19, 2017

Generic reruner temyers/cucumber-jvm-parallel-plugin#89

Closed

mpkorstanje reviewed May 22, 2017

View reviewed changes

aslakhellesoy merged commit fd1b645 into master May 25, 2017

aslakhellesoy added a commit that referenced this pull request May 25, 2017

Attribution for #1035

df3be33

brasmusson mentioned this pull request May 25, 2017

Use an enum for result status. #1133

Merged

6 tasks

mpkorstanje mentioned this pull request May 25, 2017

Use deterministic unique ids in Descriptions #1121

Closed

2 tasks

mpkorstanje mentioned this pull request May 28, 2017

Proposal to fix Issue577 -- improved junit runner #588

Closed

brasmusson added a commit that referenced this pull request Jul 1, 2017

Update History.md, add #638 as fixed by the merge of #1035.

28aeffe

lock bot locked as resolved and limited conversation to collaborators Oct 25, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use Gherkin v4 #1035

Use Gherkin v4 #1035

brasmusson commented Aug 4, 2016 •

edited

Loading

aslakhellesoy commented Sep 6, 2016

mattwynne commented Nov 23, 2016

mpkorstanje left a comment •

edited

Loading

mpkorstanje left a comment •

edited

Loading

mpkorstanje May 13, 2017

mpkorstanje May 14, 2017 •

edited

Loading

mpkorstanje May 14, 2017

brasmusson May 18, 2017

brasmusson May 25, 2017

mpkorstanje May 14, 2017

mpkorstanje May 14, 2017

mpkorstanje May 14, 2017

mpkorstanje May 17, 2017

mpkorstanje May 17, 2017 •

edited

Loading

brasmusson May 26, 2017

mpkorstanje May 26, 2017

mpkorstanje commented May 17, 2017 •

edited

Loading

brasmusson commented May 18, 2017

brasmusson commented May 18, 2017

mattwynne commented May 18, 2017 •

edited

Loading

mpkorstanje May 22, 2017 •

edited

Loading

mpkorstanje commented May 22, 2017

aslakhellesoy commented May 25, 2017

brasmusson commented May 26, 2017

lock bot commented Oct 25, 2018

Use Gherkin v4 #1035

Use Gherkin v4 #1035

Conversation

brasmusson commented Aug 4, 2016 • edited Loading

aslakhellesoy commented Sep 6, 2016

mattwynne commented Nov 23, 2016

mpkorstanje left a comment • edited Loading

Choose a reason for hiding this comment

mpkorstanje left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mpkorstanje May 14, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mpkorstanje May 17, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mpkorstanje commented May 17, 2017 • edited Loading

brasmusson commented May 18, 2017

brasmusson commented May 18, 2017

mattwynne commented May 18, 2017 • edited Loading

mpkorstanje May 22, 2017 • edited Loading

Choose a reason for hiding this comment

mpkorstanje commented May 22, 2017

aslakhellesoy commented May 25, 2017

brasmusson commented May 26, 2017

lock bot commented Oct 25, 2018

brasmusson commented Aug 4, 2016 •

edited

Loading

mpkorstanje left a comment •

edited

Loading

mpkorstanje left a comment •

edited

Loading

mpkorstanje May 14, 2017 •

edited

Loading

mpkorstanje May 17, 2017 •

edited

Loading

mpkorstanje commented May 17, 2017 •

edited

Loading

mattwynne commented May 18, 2017 •

edited

Loading

mpkorstanje May 22, 2017 •

edited

Loading