cucumber / common

A home for issues that are common to multiple cucumber repositories
https://cucumber.io/docs
MIT License
3.36k stars 694 forks source link

New tag expression `@^foo` — select all tags that start with 'foo' #1668

Closed customcommander closed 3 years ago

customcommander commented 3 years ago

I'm sure it must have been suggested before but I cannot find a similar ticket in this repo. Apologies in advance if this discussion has already been settled.


Is your feature request related to a problem? Please describe.

We have naming convention for tags e.g.

@featureAAA
@featureBBB
@featureCCC
@regressionAAA
@regressionBBB
@regressionCCC

We apply these tags on scenarios across many files. We can't have all the features scenarios in one directory and all the regressions scenarios in another. We need to be able to mix and match.

With such a setup the only way to run either all features or all regressions scenarios is to apply a general tag @feature and @scenario as well.

Describe the solution you'd like

Would we nice if we could have a tag expression such as @^feature which means "all tags that starts with feature"

Describe alternatives you've considered

The alternative is the apply a global tag as well e.g.

@featureAAA @feature
Scenario: …
  Given …
  When …
  Then …
aurelien-reeves commented 3 years ago

May I ask, why applying several tags instead of one composed tag does not fit your needs?

In your examples, I would apply the following tags:

@feature @AAA
@feature @BBB
@feature @CCC
@regression @AAA
@regression @BBB
@regression @CCC

That way you could even have more possibilities when properly using tag-expressions like things like @feature or @AAA, or @AAA or (@BBB and not @regression)

You are actually not limited into the number of tags you can have on features, rules, or examples

customcommander commented 3 years ago

Hey thanks for the quick reply.

Sorry my example above should have been: (so each tag is unique)

@featureAAA
@featureBBB
@featureCCC
@regressionDDD
@regressionEEE
@regressionFFF

(We need unique tags because of integration with other tools.)

aurelien-reeves commented 3 years ago

(We need unique tags because of integration with other tools.)

Ok, thanks for the precision

mattwynne commented 3 years ago

So you can't add a @feature tag as well @customcommander?

e.g.

@feature
@featureAAA
@featureBBB
@featureCCC
@regression
@regressionDDD
@regressionEEE
@regressionFFF

I can see how what you're suggesting is useful, but it's always worth pushing back a little to see if we can work with what we have before we add more complexity to the code.

customcommander commented 3 years ago

Hey @mattwynne absolutely right I understand and I realise that my initial post is not great actually.

So IRL it would look something like this:

dir1/file4.feature

@featureAAA
Scenario: …
  Given …

dir2/file5.feature

@featureBBB @regressionDDD
Scenario: …
  Given …

dir3/file6.feature

@regressionEEE
Scenario: …
  Given …

Notes:

We absolutely could do this:

dir1/file4.feature

@feature
@featureAAA
Scenario: …
  Given …

dir2/file5.feature

@feature
@featureBBB
@regression
@regressionDDD
Scenario: …
  Given …

dir3/file6.feature

@regression
@regressionEEE
Scenario: …
  Given …

But we would like to avoid ;)

In reality the set of files, scenarios and tags is much much larger so relying on engineers to "do the right thing" is risky :-)

mpkorstanje commented 3 years ago

@customcommander

So if I understand it correctly for your developers you currently have the following rules:

Then @mattwynne and @aurelien-reeves are suggesting that you use these rules:

However with the latter you are concerned that it will harder to get right.

I think this is a problem that could be solved by a linter. Unfortunately Gherkin currently doesn't have one, but I've been looking for use cases to motivate people to create the necessary infrastructure.

customcommander commented 3 years ago

@mpkorstanje As I said earlier we could absolutely add a @feature tag next to each @featureXYZ tag; it just seems both redundant and fragile.

mpkorstanje commented 3 years ago

Overal I can see a few problems with expanding the tag expression language.

  1. There is no room in the reserved character set to expand it without a breaking change.

  2. It only covers a relatively small number of use cases that don't align with the purpose of tags (they are supposed to be composed).

  3. For more advanced filtering needs a solution that enables programmatic selection of the scenarios to execute would be more effective.

    Such a solution would be able to handle any arbitrary filtering criteria. It is a bit more difficult to use then a tag expression but that is not unreasonable given the more esoteric needs it covers.

customcommander commented 3 years ago

Sure. Feel free to close.

mpkorstanje commented 3 years ago

I'm not too familiar with the api of cucumber-js, it may not be possible yet, so I was waiting for feedback from others.

aurelien-reeves commented 3 years ago

I share your opinion @mpkorstanje

jenisys commented 3 years ago

I suggested such a tag-matching mechanism with wildcards (like: tag-starts-with-prefix, tag-ends-with-suffix, tag-contains-part) for cucumber tag expressions a while ago (after providing the functionality in the Python implementation of cucumber tag-expressions). Because the idea was not accepted at the time, I moved the functionality to a slightly extended variant of tag-expressions in behave.

The extra functionality is not that big (but may need some minor refactoring in other language implementations).

RELATED: Tag-Expression parser and model extensions for providing tag-matching with wildcards

mattwynne commented 3 years ago

I like your implementation @jenisys - it strikes the balance of simplicity in both the problem and solution domain nicely.

mattwynne commented 3 years ago

@customcommander despite the objections you've heard, I think it would be interesting to see a PR for this, to bring the same functionality Jens has added to Behave into Cucumber. It might turn out that it's not as complicated as people fear. Would you be interested in giving it a go?

customcommander commented 3 years ago

@mattwynne Oh nice! I'd like to give it a go but:

  1. ETA for a PR: ± end of August at best.
  2. I can only do JS (or TS) so I wouldn't be able to replicate for different languages.

Is that ok?

mpkorstanje commented 3 years ago

@mattwynne it's not about the complexity of the solution, it's about the relative complexity of the solution vs. the gains it provides vs the complexity of alternative solutions.

For example achieving these things with JUnit 5, is easy:

private static class MyPostDiscoveryFilter implements PostDiscoveryFilter {
    @Override
    public FilterResult apply(TestDescriptor testDescriptor) {
        return FilterResult.includedIf(
                testDescriptor.isTest() &&
                        testDescriptor.getTags().stream()
                                .map(TestTag::getName)
                                .anyMatch(tag -> tag.startsWith("prefix-")));
    }
}

As is it with TestNG

    @Test(groups = "cucumber", description = "Runs Cucumber Scenarios", dataProvider = "scenarios")
    public void runScenario(PickleWrapper pickleWrapper, FeatureWrapper featureWrapper) {
        if (pickleWrapper.getPickle().getTags().stream().noneMatch(tag -> tag.startsWith("prefix-"))) {
            throw new SkipException("Tags didn't match");
        }

        testNGCucumberRunner.runScenario(pickleWrapper.getPickle());
    }

Cucumber is huge multi-language project already. We should not reinvent the wheel when the test frameworks we integrate with already provide functionality that allows us to do these things.

Now perhaps our integration with javascript test frameworks is such that we currently can't do this programmatically, but I then first suggest looking into improving those integrations rather then adding more functionality to Cucumber. There is no point in reinventing the wheel, just because it was not invented here and we didn't bother to look elsewhere.

mattwynne commented 3 years ago

Hey @customcommander,

We talked about this at the weekly community Zoom today, and there's still some concern that the benefit of this feature won't justify the ongoing maintenance cost, especially given that this is in a shared component where we'd need to implement in multiple langauges.

With that said, I personally think this conversation could be moved forward by looking at some code, or at least fleshing out some test cases / examples to understand the details of how it would behave in edge-cases if we started supporting some kind of pattern matching (or full blown-regexen).

If you have the energy to do that, given the risk that we might still not achieve consensus to merge this behaviour in, then yes we can of course wait until September or even longer 😄. I think that, if we regard this as a prototype, then implementing in one language first makes sense anyway, so you only being able to implement in JS/TS is fine for now.

customcommander commented 3 years ago

@mattwynne Sorry for the late reply; holidays and stuff ;)

Thanks for the transparency here but I'm bit reluctant to do this to convince a steering committee.

Please don't take this the wrong way!

You're all doing a fantastic job in maintaining and coordinating this awesome piece of software (I am a big fan!) but from my perspective if a feature has value or merit and is supported by a large enough group of users and/or realistic use cases then the next step is simply to "make it happen" (I know it's easier said than done ;)

I do accept that an investigation of the cost & feasibility of a feature may lead to it being rejected or put on the back burner (and that's absolutely fine) but it feels like this is not where we are.

Firstly we don't know if other users would find this useful and secondly it doesn't seem like this feature is particularly popular among the maintainers. I do agree that writing some code and test cases is definitely the best way to move this discussion forward but from my point of view doing it now would be premature.

What about we wait for more users and use cases for now? If there's enough support for this feature I'd be more than happy to give it a go.

mattwynne commented 3 years ago

Understood @customcommander I'll close the issue for now but if we hear more feedback we can re-open it. Thanks for your engaged discussion on this.