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

Tags with whitespace are deprecated and may be removed #600

Closed
andriokha opened this issue Nov 14, 2021 · 7 comments · Fixed by #601
Closed

Tags with whitespace are deprecated and may be removed #600

andriokha opened this issue Nov 14, 2021 · 7 comments · Fixed by #601

Comments

@andriokha
Copy link
Contributor

Since Gherkin 4.9.0 (issue, PR) tags with whitespace are deprecated, and I'm seeing failures due to \Drupal\DrupalExtension\Context\MailContext with @BeforeScenario @mail @email (and the after scenario's the same).

  16384: Tags with whitespace are deprecated and may be removed in a future version in /var/www/html/vendor/behat/gherkin/src/Behat/Gherkin/Filter/TagFilter.php line 38  

I can suppress deprecations with the following in behat.yml as an overly-broad bandaid:

default:
  calls:
    error_reporting: 16383

(Thanks to https://stackoverflow.com/a/31059663)

And I think the fix would just be to use a comma, which functions as an OR operator, see https://behat.org/en/latest/user_guide/context/hooks.html#tagged-hooks.

Thanks!

andriokha added a commit to andriokha/drupalextension that referenced this issue Nov 14, 2021
Since Gherkin 4.9.0 tags with whitespace are deprecated: use a comma as
an explicit OR operator.

See
- Behat/Gherkin#213
- Behat/Gherkin#215
- https://behat.org/en/latest/user_guide/context/hooks.html#tagged-hooks

Closes jhedstrom#600
@pfrenssen
Copy link
Collaborator

pfrenssen commented Nov 17, 2021

Can you clarify with an example where it is actually failing? There is a difference between tags that include whitespace, and a list of different tags that are separated by whitespace.

Note the difference in the usage of the @ symbols in the example you give (three different tags: @BeforeScenario @mail @email), and in the example given in the issue you link to (one single tag with whitespace in it: @foo bar baz).

@pfrenssen
Copy link
Collaborator

I just checked the reference implementation and having multiple tags on a single line separated by whitespace (e.g. @BeforeScenario @mail @email) is allowed. I could not find one example using commas as separators.

See the examples using tags on https://github.com/cucumber/common/tree/main/gherkin

@andriokha
Copy link
Contributor Author

Hi, thanks for your time!

Can you clarify with an example where it is actually failing?

For me, just having the mail context enabled and running a test is sufficient IIUC: as part of dispatching the before scenario hooks it ends up parsing the context's tag(s).

There is a difference between tags that include whitespace, and a list of different tags that are separated by whitespace.

Yeah, and that tripped me for a while, as I couldn't find the tag with whitespace in it through visual inspection! It could be that the issue is with Gherkin, but certainly the $filterString that gets inspected for whitespace in \Behat\Gherkin\Filter\TagFilter::__construct() is @mail @email which triggers the deprecation. The "tag" gets parsed by \Behat\Gherkin\Filter\TagFilter::isTagsMatchCondition() which looks like it supports &&, , and ~.

Thanks again!

@Berdir
Copy link
Contributor

Berdir commented Nov 19, 2021

You might be mixing up tag annotations on tests and the tag filter argument on the behat CLI.. only there tag operators for AND/OR/NOT make sense.

@andriokha
Copy link
Contributor Author

You might be mixing up tag annotations on tests and the tag filter argument on the behat CLI.. only there tag operators for AND/OR/NOT make sense.

I mean I just updated behat and gherkin, reran a test and it spat out a deprecation due to the mail context (:

It looks like currently Behat uses the same class (\Behat\Gherkin\Filter\TagFilter) to filter CLI and hook tags, see:

Although there is a slight difference in that it's possible to specify multiple arguments to the CLI with --tags=foo --tags=bar whereas it looks like a hook just has a single 'filter string'.

Interestlingly my read of cucumber (which the gherkin change was made to match) is that tag expressions can have spaces (and don't use symbols for logical operators), so I guess I'm really missing something. (:

Thanks both!

@Berdir
Copy link
Contributor

Berdir commented Nov 19, 2021

Yes, I'm not sure what's exactly going on, what I means is that @foo and --tags=foo are very different things and some of the code and documentation you have been quoting is from the TagFilter class, which is concerned with validating the --tags=foo CLI input, what's specified there has very different rules than the annotations themself.

@kerasai
Copy link

kerasai commented Feb 23, 2022

I've just started running into this as well.

I've hacked at the MailContext's code a bit and it's certainly due to the @BeforeScenario @mail @email in the annotation on MailContext::collectMail and @AfterScenario @mail @email on MailContext::stopCollectingMail.

However I'm not clear on what the proper fix may be. In terms of avoiding the error, comma-separating the tags as well as putting each tag an a new line both work but I'm not sure which provides the intended behavior.

I'm working around this for now by patching using the fix from #601.

Berdir pushed a commit to Berdir/drupalextension that referenced this issue Apr 24, 2022
Since Gherkin 4.9.0 tags with whitespace are deprecated: use a comma as
an explicit OR operator.

See
- Behat/Gherkin#213
- Behat/Gherkin#215
- https://behat.org/en/latest/user_guide/context/hooks.html#tagged-hooks

Closes jhedstrom#600
jhedstrom added a commit that referenced this issue Apr 26, 2022
xurizaemon pushed a commit to xurizaemon/drupalextension that referenced this issue Nov 28, 2022
Since Gherkin 4.9.0 tags with whitespace are deprecated: use a comma as
an explicit OR operator.

See
- Behat/Gherkin#213
- Behat/Gherkin#215
- https://behat.org/en/latest/user_guide/context/hooks.html#tagged-hooks

Closes jhedstrom#600
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 a pull request may close this issue.

4 participants