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

Compatibility layer for cucumber/gherkin #253

Open
wants to merge 11 commits into
base: master
Choose a base branch
from

Conversation

ciaranmcnulty
Copy link
Contributor

@ciaranmcnulty ciaranmcnulty commented Jul 10, 2022

This adds a compatibility layer that can be used when cucumber/gherkin is also installed, that uses that parser instead and then maps the result back to our 'regular' AST for Behat (and maybe Codeception?) to consume.

This means that when parsing edge cases we will conform much more closely with the cucumber core implementations. It also means newer syntax items will be ignored / handled more cleanly rather than triggering odd parsing errors

To avoid bumping the minimum PHP version we support, this does not depend directly on cucumber/gherkin; instead there is a static CucumberGherkinLoader::isAvailable() method that can be called to see if the CucumberGherkinLoader can be used instead of the regular one, e.g.

if (
  class_exists(CucumberGherkinLoader::class) // if supporting older behat/gherkin
  && CucumberGherkinLoader::isAvailable()
) {
   $parser = new CucumberGherkinLoader(...)
} 
else {
   $parser = new GherkinFileLoader(...)
}

An alternative option to this would be to merge this into cucumber/gherkin 5.0.0 and let that require cucumber/gherkin (and PHP 8.1), then the version detection can move to Behat - I'm not sure that's warranted yet

Motivation

Moving to the cucumber parser entirely will reduce the maintenance effort on Behat, and unlock newer features we didn't support yet.

That's a big refactor though, and this goes someway towards it by enabling Behat to introduce the cucumber parser partially as an experimental feature (behind a config flag) to detect feature files in the wild that are unparsable, or parsed in unexpected ways.

Compatibility

  • Passes the behat/gherkin etalon tests (tests in PR) with one exception (see Known incompatibilities)
  • Passes the entire behat/behat test suite (manually tested by me)

Mapping notes

The majority of the mapping is simple from one tree to another, aside from a few places where the Behat AST doesn't support a feature:

Scenarios and Scenario Outlines

Cucumber no longer differentiates between these. To retain compatibility we map any Scenarios that have Examples attached as a Scenario Outline

Multi-line scenario/background/outline names

Scenario: Something awesome
   Something amazing

Cucumber sees this as Scenario with name: 'Something awesome' and description: 'Something amazing'. For compatibility we map this as a multi-line scenario name (which cucumber does not support but Behat does) so title: "Something awesome\nSomething amazing"

Descriptions trimming

Cucumber preserves whitespaces in descriptions, Behat right and left-trims them based on how indented the previous keyword is. This logic is retained to be as like Behat as possible.

Rule support

Cucumber supports scenarios inside tagged Rules:

Feature:

    @foo
    Scenario:

    @bar
    Rule:

        @baz
        Scenario:

Because the Behat AST doesn't support Rule, this will be flattened into the same AST as this feature:

Feature:

    @foo
    Scenario:


   

        @bar @baz
        Scenario:

This will enable implementations to parse files with Rule, and to use tags from Rule keywords when filtering examples

Known incompatibilities

There will doubtless be incompatibilities in the wild, but most will be around 'odd' Gherkin. The following are known:

Comments at the end of Feature description no longer work

Cucumber has a bug about this so one of the test files has had to be excluded.

Backgrounds inside Rules do not work

There's no sensible way to map Background up into the Feature level and only have it apply to some scenarios, therefore we throw a parser error if it happens.

This syntax is discouraged by Cucumber anyhow

@ciaranmcnulty
Copy link
Contributor Author

@Naktibalda any comments from a codeception POV?

@ciaranmcnulty ciaranmcnulty requested review from stof and pamil July 10, 2022 16:05
src/Behat/Gherkin/Loader/CucumberGherkinLoader.php Outdated Show resolved Hide resolved
test.feature Outdated Show resolved Hide resolved
@Naktibalda
Copy link

@Naktibalda any comments from a codeception POV?

This is great idea. Codeception can't migrate to cucumber/gherkin right away, but it can certainly support it as an optional dependency via this compatibility layer.

@ciaranmcnulty
Copy link
Contributor Author

Another option for support would be to:

  1. Add the CucumberGherkinLoader with its ::isAvailable method returning false, and make everything else a no-op
  2. Release the real implementation as behat/gherkin 5.0 that explicitly requires cucumber/gherkin and PHP 8.1

Is that cleaner? Behat/Codeception could rely on ^4.9 || ^5.0 I think without breaking support for anyone?

@ciaranmcnulty ciaranmcnulty force-pushed the cucumber-gherkin branch 2 times, most recently from 594897c to 8295b48 Compare July 17, 2022 07:11
@ciaranmcnulty
Copy link
Contributor Author

I want to redo this on top of cucumber/common#254 when that's merged

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants