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

fix(cypress-tags.js): scenario outlines with examples were not correctly filtered by tags #609

Merged

Conversation

rkrisztian
Copy link
Contributor

Now the script supports filtering examples of scenario outlines too. I have removed the optimization
that we should check scenario tags if there are any, because they do not work for negated tags (e.g.
not @tag1). Additionally, just checking the feature tag itself is also wrong, because a feature
with tag @feature having a scenario with tag @scenario would incorrectly run when we filter for
not @scenario. So the best is to leave the tag checking for "leaf nodes", i.e. scenarios or
examples. Also consider the corner case where a feature has no scenarios or scenario outlines at
all, i.e. again we would not run such a feature at all.

Fixes #196.

…tly filtered by tags

Now the script supports filtering examples of scenario outlines too. I have removed the optimization
that we should check scenario tags if there are any, because they do not work for negated tags (e.g.
`not @tag1`). Additionally, just checking the feature tag itself is also wrong, because a feature
with tag `@feature` having a scenario with tag `@scenario` would incorrectly run when we filter for
`not @scenario`. So the best is to leave the tag checking for "leaf nodes", i.e. scenarios or
examples. Also consider the corner case where a feature has no scenarios or scenario outlines at
all, i.e. again we would not run such a feature at all.

Fixes #196.
@jime2003
Copy link

jime2003 commented Aug 12, 2021

Now the script supports filtering examples of scenario outlines too. I have removed the optimization
that we should check scenario tags if there are any, because they do not work for negated tags (e.g.
not @tag1). Additionally, just checking the feature tag itself is also wrong, because a feature
with tag @feature having a scenario with tag @scenario would incorrectly run when we filter for
not @scenario. So the best is to leave the tag checking for "leaf nodes", i.e. scenarios or
examples. Also consider the corner case where a feature has no scenarios or scenario outlines at
all, i.e. again we would not run such a feature at all.

Fixes #196.

Is anyone looking into this one as we are not able to use example tables tags to run them in different environments?

@rkrisztian
Copy link
Contributor Author

rkrisztian commented Aug 12, 2021

Is anyone looking into this one as we are not able to use example tables tags to run them in different environments?

@jime2003, The authors are on vacation, we have to wait.

cypress-tags.js Outdated
scenario.examples
? scenario.examples.some((example) =>
shouldProceedCurrentStep(
[...example.tags, ...scenario.tags, ...feature.tags],
Copy link
Contributor Author

@rkrisztian rkrisztian Sep 2, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I discovered today in another project that null checking section.tags is necessary. I'll update the PR in a few days.

Edit: Done.

@dankopetrovic
Copy link

Any updates on this?!

@rkrisztian
Copy link
Contributor Author

@MateuszNowosad , @lgandecki, for how long are you guys on vacation? People need the fix...

@lgandecki
Copy link
Collaborator

I can merge this in without testing it - are you actively using this version of the code in a project? If I was to test it, are there any things that come to your mind would be worth double-checking?

@rkrisztian
Copy link
Contributor Author

@lgandecki , although I did some careful testing to make sure I didn't break anything, I do not actively use cypress-tags.js in my projects yet due to (in my case) expecting diminishing returns over time. I did use it once where I discovered the bug I fixed above, but that's about it.

@rkrisztian
Copy link
Contributor Author

If I was to test it, are there any things that come to your mind would be worth double-checking?

Good question. What I basically did was:

  • Regression test features and scenarios without examples:
    • no tags added to any line
    • tags placed in various lines (Feature/Scenario)
    • using tags on multiple levels
  • Check if examples can be filtered by tags:
    • no tags added to any line
    • tags placed in various lines (Feature/Scenario Outline/Examples)
    • using tags on multiple levels
    • using multiple Examples tables <-- the point of this PR

Ideally, all these are tested automatically, but no test was written from the beginning, so I also suggest some future refactoring to make a good portion of the script easily testable...

@jime2003
Copy link

If I was to test it, are there any things that come to your mind would be worth double-checking?

Good question. What I basically did was:

  • Regression test features and scenarios without examples:

    • no tags added to any line
    • tags placed in various lines (Feature/Scenario)
    • using tags on multiple levels
  • Check if examples can be filtered by tags:

    • no tags added to any line
    • tags placed in various lines (Feature/Scenario Outline/Examples)
    • using tags on multiple levels
    • using multiple Examples tables <-- the point of this PR

Ideally, all these are tested automatically, but no test was written from the beginning, so I also suggest some future refactoring to make a good portion of the script easily testable...

Is it ready to use or not yet?

@rkrisztian
Copy link
Contributor Author

Is it ready to use or not yet?

I think it is. I did all the manual testing I could, and found no problems.

@lgandecki lgandecki merged commit f177e54 into badeball:master Oct 4, 2021
@lgandecki
Copy link
Collaborator

There is something weird here, since I'm pretty sure we did have some tests for the cypress-tags, @MateuszNowosad don't you remember that as well?
Anyway, as I don't really have time to investigate what happened to the tests or to verify this manually I'll just merge it and hope for the best..
The problem is that I don't personally use this project anymore, I reached out to Cypress to help with the maintenance, they've started a conversation with me but said they need time to think and it's been months since any information about this. Basically I feel like I'm doing someone else's work for free - people "vote with their feet", 10% of cypress users use this plugin, so clearly there is a demand for cypress + cucumber. More than working for free - I actually paid @MateuszNowosad from my own pocket to help with this project. But then there is so many negativity and unpleasant comments here that I'm not really motivated anymore.

@lgandecki
Copy link
Collaborator

🎉 This PR is included in version 4.3.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@MateuszNowosad
Copy link
Contributor

MateuszNowosad commented Oct 4, 2021

Sadly I don't remember we ever had unit tests for cypress-tags.js file directly. We have some tests for helpers used inside, for example for mine cypressTagsHelpers. As is this file will need a rewrite down the line and tests.

@badeball badeball mentioned this pull request Oct 4, 2021
@rdroog
Copy link

rdroog commented Oct 5, 2021

@lgandecki: not sure if this is the best place to respond, but if you feel like you shouldn't be one of the maintainers of this project anymore, I think it would be better to hand it over to other people. Currently it feels like it is in a limbo state, and that is both bad for you, and for the project itself. If people would ask me now, I would advice against using this project, even though I know of its value and use it myself.

I don't have any open source experience, so I'm not sure how these things normally go though.

@lgandecki
Copy link
Collaborator

@rdroog
I did want to hand it over (fully or partially) to Cypress which would be the way to go, I hope they will still help at some point. They do make money out of this, so I wish people send their complaints their way :)

But to your point "project feels like it is in a limbo state" - We had 9 releases in the last 6 months, and comments like your is exactly what makes me less willing to work on this :) It's just unfair. I had a month vacation after a couple years of not taking any, and after that I had to focus on my professional and personal responsibilities first - that piled up. Do you think anyone else will work much more on this for free?

Besides the releases, there is quite a bit of support from me, @MateuszNowosad and @badeball which also is very different than what you get with projects in a "limbo state"

Another point - do you think it's that easy to find someone to take a project over? I wouldn't want to give it to someone that has not been actively working on this, because how do I know they won't just take it and never work on it? And if we are limiting to the people that do work on this repo, we narrow down the candidates significantly.

Yet another point - some of my clients do use this project, on some older, fixed version, it works great for them and they are not complaining. They don't care about what's going on in the issues section on github.

@rdroog
Copy link

rdroog commented Oct 5, 2021

I understand that it is your free time and you don't get anything in return, which is how it mostly works in the open source community. Not that it is the best situation though. I'd love for Cypress to provide you with something, either money or their time to work on the project.

My experience is about the PR/issue that I've created (#591), for which I have patiently waited on a review, and after waiting for an extended amount of time as I saw that both you and MateuszNowosad's were temporarily unavailable, I asked for how I could get it to move further. Still, I didn't get a response. I'm not sure what people who would like to contribute to the project could do more than that, but I'd love to know if there are any other opportunities to help you out in some form or way. For me it seems like I am trying to contribute, but am not actually able to contribute.

Of course, n=1, and this is my first experience with the project. I don't know if I'm the unlucky one or not, but seeing other comments on several PRs my assumption would be I'm not. And this is not me complaining about getting no response on my PR, feel free to reject it or leave it alone as long as you wish.

I understand people have vacations or are temporarily less interested - that's life. It would be good to somehow make that clear, so that people know what to expect. Settings expectations helps people understand the current situation and lets them know what to expect for the future.

And yes, finding someone to take over is probably not easy. As said before, I don't have any open source experience, so I'm not sure what the usual methods of doing such a thing would be.

@lgandecki
Copy link
Collaborator

Just to be clear about one thing, I don't mind that much that it's my free time and I don't get anything in return.
The problem happens when you add the third element - demands, complaints, or sometimes straight-forward offensive remarks ( as here: #386 where a guy that didn't even install a package came to complain in github issues that it didn't work, he was creating one issue after another which pretty much was unpaid hand-holding/consulting and after I helped him in a few of those issues and then suggested he should move to stack overflow he started throwing ugly remarks towards me). This work is frequently less pleasant and more demanding than what I do for my clients, that treat me with respect and pay very well :-)

Take a look at some commercial repositories, like Cypress. I have an open and acknowledged issue there, with quite a few of likes, for soon to be three years - cypress-io/cypress#3323 . there is a ton of opened issues and PRs - would you say that's also "limbo"? ;) Remember - there are many full-time employees backed by investors money..

I agree it would be nice to somehow make things clear to the users, but as you said - it's Open Source, it comes as is, the license states: "THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND". Of course if we decide to abandon the project we would let the community know, but so far it's definitely not abandoned, any significant bugs will be fixed, and any necessary updates will be performed. cypress-tags is of a lower priority since it's just a helper that's not required to use the package, and in addition to that it has no test coverage which makes it low priority and hard to change at the same time.

@rdroog
Copy link

rdroog commented Oct 7, 2021

I holy agree with you that demands and complaints as like the issue you mentioned are not good and I can understand it makes work far less pleasant. And I know that actual paying clients are usually very willing to work together as they (and their money) are in it as well, which is great.

Maybe my expectations are/were too high. There is though a slight difference between yours and mine: I created a fix, so if it would add value to the project, it could get reviewed and merged, and if it doesn't, it could get rejected and we can move on and think about alternatives. I don't really mind issues to be open for a longer time (that's how it works if the priority isn't high enough), as long as there is some sort of communication about it.

All in all, I really appreciate you working on the project. Without it we might not have chosen for Cypress, and I personally love Cypress (and BDD) :)

@jime2003
Copy link

jime2003 commented Oct 8, 2021

@lgandecki - I still see one issue here -

Below is an example:

@smoke
Scenario Outline: Test example
Given Step with
When Step with
Then Result is

@qa
Examples:
| param1 | param2 | result |
| foo | bar | foobar |
| 1 | 2 | 3 |

@uat
Examples:
| param1 | param2 | result |
| foo | bar | foobar |
| 1 | 2 | 3 |

When I use "npx cypress-tags run -e TAGS="@uat" --browser electron" - worked fine

but when I try to switch environment at the run time like below:
npx cypress-tags run -e TAGS="@uat" --browser electron --headed --env env=stage - It is switching over to correct environment as expected but again it runs all the scenarios including @qa even though in the tag I have only mentioned"@uat"

@dankopetrovic
Copy link

@lgandecki - I still see one issue here -

Below is an example:

@smoke Scenario Outline: Test example Given Step with When Step with Then Result is

@qa Examples: | param1 | param2 | result | | foo | bar | foobar | | 1 | 2 | 3 |

@uat Examples: | param1 | param2 | result | | foo | bar | foobar | | 1 | 2 | 3 |

When I use "npx cypress-tags run -e TAGS="@uat" --browser electron" - worked fine

but when I try to switch environment at the run time like below: npx cypress-tags run -e TAGS="@uat" --browser electron --headed --env env=stage - It is switching over to correct environment as expected but again it runs all the scenarios including @qa even though in the tag I have only mentioned"@uat"

@jime2003 I believe you need to specify the TAGS and env together, separated with comma, like this; -e TAGS="@uat",env=stage

@rkrisztian
Copy link
Contributor Author

rkrisztian commented Oct 8, 2021

Sorry, but I find it impractical that we are doing discussions in a merged PR:

  • The future of this project is off-topic,
  • Issues should be reported in issue tickets.

If we don't organize our discussions, it will be too easy to get lost in the chaos.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[feature request] use tags for example tables for scenarios with more example tables
6 participants