Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a feature with an empty scenario in the compatibility kit #1498

Closed
wants to merge 12 commits into from

Conversation

aurelien-reeves
Copy link
Contributor

@aurelien-reeves aurelien-reeves commented Apr 27, 2021

Summary

Following cucumber/gherkin#11, the gherkin parser is considering empty scenarios, but cucumber implementation are not yet align on how to process empty scenarios.

Details

An empty-scenario has been added to the compatibility kit.

cucumber-ruby is already managing empty scenarios well so fake-cucumber has been updated in order to have the same behavior as cucumber-ruby.

That way, everything remains green without a lot of effort.

No behavior has been changed in any other module. Gherkin remain unchanged.

Checklist:

  • Add a dedicated feature in the compatibility kit
  • Implement the desired behavior in fake-cucumber
  • Add the corresponding ndjson to the compatibility kit
  • Port to the ruby implementation of the cck
  • I have updated the CHANGELOG accordingly.
  • Fix the acceptance tests of the json-formatter (cf. comment here)
  • Fix cucumber-ruby to make the CI green? (cf. comment here)

@aurelien-reeves aurelien-reeves added ⚡ enhancement Request for new functionality library: fake-cucumber library: compatibility-kit 🥒 core team Candidate for going onto the Cucumber Open Board: https://github.com/orgs/cucumber/projects/8 labels Apr 27, 2021
@aurelien-reeves
Copy link
Contributor Author

aurelien-reeves commented Apr 29, 2021

The current issue with the CI is:

  • the json-formatter uses cucumber-ruby 3 to generate a reference json
  • cucumber-ruby 3 did not managed empty scenarios properly. cucumber-ruby 4 would
  • cucumber-ruby 4 resulting json has some data in the match.location attribute that cannot be used for reference

In order to go further with that PR, I suggest:

  • we update cucumber-ruby in json-formatter/ruby-testdata to at least version 4
  • we update the sanitizer scripts to make the resulting json looks like we want to

Any other idea?

Note: the extra data in match.location are those multiple line locations which comes from example tables:

"match": {
  "location": "../../compatibility-kit/javascript/features/examples-tables/examples-tables.feature:27:9"
},

Option 2: fixing the json-formatter. The line of the output of the ruby json formatter in recent version seems legit actually.

@aurelien-reeves
Copy link
Contributor Author

aurelien-reeves commented Apr 30, 2021

The json report from cucumber-ruby seems actually wrong. Instead of resulting in an empty scenario, it results in a background

[
  {
    "description": "  Scenario can be empty.\n  Background and hooks are not executed.\n  The state of the scenario is `undefined`",
    "elements": [
      {
        "description": "",
        "keyword": "Background",
        "line": 6,
        "name": "",
        "type": "background"
      }
    ],
    "id": "minimal",
    "keyword": "Feature",
    "line": 1,
    "name": "minimal",
    "uri": "features/empty-scenario/empty-scenario.feature"
  }
]

The result of the json-formatter when converting messages from fake-cucumber looks pretty good to me:

[
  {
    "description": "  Scenario can be empty.\n  Background and hooks are not executed.\n  The state of the scenario is `undefined`",
    "elements": [
      {
        "id": "minimal;empty",
        "keyword": "Scenario",
        "line": 9,
        "name": "empty",
        "steps": [],
        "type": "scenario"
      }
    ],
    "id": "minimal",
    "keyword": "Feature",
    "line": 1,
    "name": "minimal",
    "uri": "features/empty-scenario/empty-scenario.feature"
  }
]

@aurelien-reeves aurelien-reeves marked this pull request as ready for review June 25, 2021 14:55
@aurelien-reeves aurelien-reeves marked this pull request as draft July 7, 2021 12:25
@aurelien-reeves
Copy link
Contributor Author

Back to draft as there is no consensus regarding empty scenarios

@mattwynne mattwynne removed this from In Progress in Cucumber Open Sep 29, 2021
@aurelien-reeves
Copy link
Contributor Author

Due to lack of consensus, I close this PR.

We actually noticed that such behavior could differ depending the ecosystem it runs on, so I guess each implementation could deal with empty scenarios on their own.

As far as I can tell, cucumber-ruby and cucumber-jvm are already managing empty scenarios. However cucumber-js may still have unhandled exceptions with those.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🥒 core team Candidate for going onto the Cucumber Open Board: https://github.com/orgs/cucumber/projects/8 library: compatibility-kit library: fake-cucumber ⚡ enhancement Request for new functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants