search

Behat / Gherkin

Gherkin parser, written in PHP for Behat project
MIT License
1.05k stars 92 forks source link

Fix issue 187 "Comment between tags and scenario" #189

Closed phil-davis closed 3 years ago

phil-davis commented 3 years ago

Fixes issue #187

1) New scenarios from PR #188 2) Add another outline to tags_sample.feature ready for more failing tests (this outline passes fine) 3) Add failing test case to tags_sample.feature - this has, after a Scenario Outline, some tags, then a comment, then a Scenario.

That last commit fails with:

There was 1 error:

1) Tests\Behat\Gherkin\ParserTest::testParser with data set "tags_sample" ('tags_sample')
Behat\Gherkin\Exception\ParserException: Expected Step or Examples table, but got Scenario on line: 61 in file: /home/runner/work/Gherkin/Gherkin/tests/Behat/Gherkin/Fixtures/features/tags_sample.feature

/home/runner/work/Gherkin/Gherkin/src/Behat/Gherkin/Parser.php:478
/home/runner/work/Gherkin/Gherkin/src/Behat/Gherkin/Parser.php:194
/home/runner/work/Gherkin/Gherkin/src/Behat/Gherkin/Parser.php:245
/home/runner/work/Gherkin/Gherkin/src/Behat/Gherkin/Parser.php:188
/home/runner/work/Gherkin/Gherkin/src/Behat/Gherkin/Parser.php:608
/home/runner/work/Gherkin/Gherkin/src/Behat/Gherkin/Parser.php:208
/home/runner/work/Gherkin/Gherkin/src/Behat/Gherkin/Parser.php:83
/home/runner/work/Gherkin/Gherkin/tests/Behat/Gherkin/ParserTest.php:145
/home/runner/work/Gherkin/Gherkin/tests/Behat/Gherkin/ParserTest.php:39

The problem is that Parser.php parseOutline() does not know how to nicely stop consuming lines of the feature file after it finds more tags. It expects that there can be tags, and Examples table, more tags, another Examples table... After it finds the tags line, it then does predictTokenType() and finds a comment, so there could be another Examples table coming after the comment. So it does parseExpression() and sadly, rather than returning a comment string, it returns the next Scenario, and the code raises a ParserException.

The problem does not happen when the tags and scenario are not separated by a comment, because the while loop ends up being exited.

The code needs to "see what is next" after a comment, without actually "gobbling up" the next thing.

4) Explicitly skip comments when parsing a scenario outline - this avoids calling parseExpression() to "see what is next". The while loop will "see what is next" using predictTokenType() and that will correctly detect the upcoming Scenario and exit the while loop. The upcoming Scenario will be sitting in the Lexer as the stashedToken and will be happily found by the higher-level processing of the Parser.

phil-davis commented 3 years ago

@ciaranmcnulty this fixes the problem. Please review.

After looking at the code, it seemed quite unfortunate that when sitting at a comment and doing parseExpression() it ends up "eating" the comment and returning the next token after the comment. That puts the Lexer past both the comment and "the next token". It seemed useful to be able to tell the Lexer to "just skip" the current token so that the code can look at "the next token" and decide what to do without actually "eating" the token. In this case I skip any comments.

phil-davis commented 3 years ago

@stof @pamil @weaverryan @everzet (all the members of https://github.com/orgs/Behat/people

Is anyone available to review this, merge if OK, and make release 4.7.1 ?

ciaranmcnulty commented 3 years ago

Can you resolve the conflict?

phil-davis commented 3 years ago

Can you resolve the conflict?

@ciaranmcnulty done

ciaranmcnulty commented 3 years ago

Thanks @phil-davis

phil-davis commented 3 years ago

No problem - it was a weird edge-case in code that we contributed to in the first place!