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

Include and Exclude tags don't consider scenarios #31

Closed
brentleyjones opened this issue Nov 29, 2016 · 9 comments
Closed

Include and Exclude tags don't consider scenarios #31

brentleyjones opened this issue Nov 29, 2016 · 9 comments

Comments

@brentleyjones
Copy link
Collaborator

brentleyjones commented Nov 29, 2016

parserFeaturesInDirectory:fromBundle:includeTags:excludeTags: only looks at the tags at the feature level. The correct behavior should be that it looks at all of the scenarios, since a tag at the feature level is just shorthand for applying the tag to all of the scenarios under it.

Currently, if a tag only exists on a scenario, like @Skip, it's not considered.

@Ahmed-Ali
Copy link
Owner

Ahmed-Ali commented Dec 1, 2016

mmm, interesting one!
You are right that adding a tag to a feature, is as adding it to every scenario of that feature. I believe this is the current behaviour.

The other one you are suggesting is, let's say I have the following feature
(I am trying to think out loudly with the following and to make sure we are on the same page for the desired behaviour)

Feature: My Feature
@current
Scenario: My Scenario
#scenario steps...

If I try to execute the tag @current, then 'My Scenario' should be executed. I agree this is a proper and clear behaviour.

However, this is also suggests if I have the following

@skip
Feature: My Feature
@current
Scenario: My Scenario
#scenario steps...

Now if I try to execute @current tag but exclude @Skip tag, then 'My Scenario' will not be executed, since now it does have two conflicting tags: the explicit tag, and the inherited one.
However, if I don't exclude @Skip tag, then the scenario will be executed normally as the first one.

Please share your thoughts, and if you can think if any other scenarios that should have a defined behaviour

@brentleyjones
Copy link
Collaborator Author

In your second example the scenario has @skip and @current. Per the rules of include and exclude, it will first include it per the @current, then it will remove it per the @skip. I see no ambiguity. Right now that scenario does have both (since you correctly give all scenarios the tags of the feature), the filtering just doesn't work on it yet.

So, to recap, the only defined behavior would be: use the tags of the scenarios, not the feature, when determining include and exclude tags. Same should be for before and after hooks, which it might already do, but just brining it up in case that is also based on the feature.

@Ahmed-Ali
Copy link
Owner

I agree with that, going to get it done as soon as possible

@Ahmed-Ali
Copy link
Owner

The changes has been pushed to develop branch.
I have tested all the scenarios I can think of, it would be nice if you have sometime to check it out and tell if you found something that seems wrong.

In short the changes will apply the exclusion on the feature level (since all scenarios will inherit the excluded tags anyway), but in case we are trying to match the including tags, we will apply this on the scenario level first, and if all the feature scenarios do not have a tag that should be included, then the whole feature will not be included. Otherwise, the feature with the included scenarios only will be executed.

@brentleyjones
Copy link
Collaborator Author

But what if my feature doesn't have @skip but my scenario does have @skip, will all the scenarios under the feature except the one with @skip be run? That's why even exclusion needs to happen at the scenario level. We can use the feature level to do a quick exclusion, but it needs to do a second pass on the scenario level as well.

I'll take a look at what you have and test it as well.

@brentleyjones
Copy link
Collaborator Author

Seems you are doing what I said above, I just read you wrong. I'll get to testing it now, thanks!

@brentleyjones
Copy link
Collaborator Author

CCIFeaturesManager.m:113 should be NSMutableArray * matchingScnarios = [NSMutableArray new];.

@Ahmed-Ali
Copy link
Owner

Great catch! Fixed.

@Ahmed-Ali
Copy link
Owner

I have covered this behaviour in the CucumberishTest target and pushed it to master.
I will close the issue for now, and if you found any reason to re-open it, please do.

Thanks a lot @brentleyjones for the help!

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

2 participants