jhedstrom / drupalextension

An integration layer between Behat, Mink Extension, and Drupal.
GNU General Public License v2.0
209 stars 192 forks source link

Tags with whitespace are deprecated and may be removed #600

Closed andriokha closed 2 years ago

andriokha commented 2 years ago

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!

pfrenssen commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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.