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

Allow override of --tags in profile #966

Closed
tooky opened this issue Oct 30, 2017 · 10 comments
Closed

Allow override of --tags in profile #966

tooky opened this issue Oct 30, 2017 · 10 comments

Comments

@tooky
Copy link
Member

tooky commented Oct 30, 2017

Since merging #940 we are no longer able to override tags that we have specified in our profile.

e.g.

module.exports = {
  default: '--tags "not @wip"'
}

We will routinely tag scenarios that we are currently working on with @wip.

Feature: A feature
  Scenario: A completed scenario
    Given it passes

  @wip
  Scenario: A scenario we're working on
    Given it currently fails

We expect these scenarios to fail and we don't want them to run as part of our build, but we do want to run them in development, e.g.:

$ cucumber -t @wip

This now results in the tag expression not @wip and @wip, and the scenarios don't run.

I realise this is at odds with the goal of #940 so I wanted to open the discussion.

@aslakhellesoy is this better tackled in tag expressions?

Perhaps: not @wip and @wip => @wip and @wip and not @wip => not @wip

@charlierudolph
Copy link
Member

Hmm, I think we need to pick one use case and stick with it. Does it make more sense for tags to be overridable or mergeable? In cucumber-js the other filters (name / file and line number) are mergeable and not overridable. Thus I think mergeable makes more sense.

I personally dislike the wip tag and think no wip scenario should ever be committed to the repo..

For you case you could work around it currently by loading a different profile in order to not load the default profile.

Is this tags option treated differently in other languages?

@mattwynne
Copy link
Member

mattwynne commented Nov 28, 2017

Yeah, we are working around it by having multiple profiles for now, but we're ending up with a lot of them!

I can't decide whether merging or overriding makes the most sense... Maybe we need to look at some more concrete examples.

@charlierudolph can you try and describe an example where merging tags from the CLI with tags from a profile would make sense?

@charlierudolph
Copy link
Member

See #939 for the example. I think tags should actually be merged with OR instead of AND as that is how the other filters works. For the use case presented in #939, I think we would need to split --tags into two options:

  • --tags repeatable merged with OR, which selects scenarios that match the expression
  • --exclude-tags repeatable merged with OR, which rejects scenarios that match the expression

@kozhevnikov @aslakhellesoy thoughts?

@mattwynne
Copy link
Member

+1 @charlierudolph that sounds spot-on.

@kozhevnikov
Copy link
Member

Personally it's difficult for me to visualise having both mergeable tags and exclude-tags that both might contain negations a la not @foo. Would it be implemented in argv_parser at merge stage so we still have single final tag expression at the end?

Couple of other options to throw out

  1. Since cucumber.js is executable default profile can be made conditional on env var and no changes needed, e.g.
module.exports = {
  default: process.env.DEV ? '' : '-t "not @wip"',
};

DEV=1 cucumber.js -t @wip
  1. May be have --tag and --tags options with --tag being repeatable and mergeable as is now and --tags being be all end all overwrite everything at once as before. Should be fairly straight forward to implement and document/understand.

--tag "not @wip" --tag @product => "(not @wip) and (@product)"
--tag "not @wip" --tags "@wip and @product" => "@wip and @product"

@charlierudolph
Copy link
Member

I would hope by having include tags and exclude tags that people would no longer use not. If you want to exclude a particular tag use exclude tags.

I was not thinking of merging the two but having the pickle_filter take in both includeTags and excludeTags.

I don't like having separate options that can override each other as I think that is harder to follow

@kozhevnikov
Copy link
Member

kozhevnikov commented Dec 19, 2017

But how would it work without overrides (sorry, not familiar with pickle_filter what it is/how it works)? If I understand correctly both #939 and #966 would define default profile with --exclude-tags @wip then #966 wants to supply --tags @wip at runtime to override it hence --tags would have to have priority over --exclude-tags, no?

@charlierudolph
Copy link
Member

The two tags don't take any priority over each other. You would no longer be able to remove filters (which is consistent with the other filter options). Instead you would need to use a different profile.

@charlierudolph
Copy link
Member

@mattwynne @tooky I think I'd prefer to go with just the --tags option (and switching it to or)

I think the concept of having --tags 'not @wip' in your default profile is a poor starting point. I think that could be solved another way. @wip scenarios could be skipped using a before hook with a selector for that tag that returns skipped.

@mattwynne
Copy link
Member

@charlierudolph yeah we've tidied up our profiles and only use the not @wip in the tag expression for the specific "stable" profiles. So that all seems to be working OK for us now.

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

No branches or pull requests

5 participants