cypress-io / cypress-grep

Filter tests using substring
137 stars 19 forks source link

Inherit tags between describe(context) and it #53

Closed oririner-wiz closed 3 years ago

oririner-wiz commented 3 years ago

This is a great plugin! I loved mocha's test tagging and glad it has a complementary plugin for cypress 🙏

What I find a bit confusing though is this case:

describe('Screen A', { tags: ['@smoke', '@screen-a'] }, () => {
  it('should navigate to Screen B successfully', { tags: ['@screen-b'] }, () => {
    // do something that eventually sends the page to screen b.
  }); 
}); 

if I run cypress run --env grepTags="@smoke+@screen-b" I'd expect it to run the test, because I assume that describe adds some context to the test, and tags would have been included in this context (similar to how beforeEach of an ancestor describe is run for all of its descendant its ).

However, this didn't work, and looking a bit at the code it looks like tags are calculated separately for describe and it, and only if the describe contains tags matching the current grep it'll try running all of the its inside it.

So, this works:

describe('Screen A', { tags: ['@smoke', '@screen-a'] }, () => {
  it('should navigate to Screen B successfully', { tags: ['@smoke', '@screen-b'] }, () => {
    // do something that eventually sends the page to screen b.
  }); 
}); 

However, it feels odd to me... why would I need to specify @smoke twice? I know it makes things a bit more explict, and explicitness is good, but this is a contrived example. What if I have multiple describes (2-3-4 aren't that far-fetched), and some of them contain tags, I'd then have to specify all the tags, all the way down, everywhere I have at least one additional tag. This makes things harder to maintain if I'm having to always have to keep them in sync.

Changing to this behavior would mean that describes are no longer "skipped" in advance, instead, they're run as is, and their tags would be collected along the way till we get to the it and then calculate if that it should run according to all of the tags.

If this change is desired then I'd be happy to implement it :)

bahmutov commented 3 years ago

Good point, if I have time I would add it...

oririner-wiz commented 3 years ago

Great! 🙏

Also, I just noticed that when doing something like this:

describe('Screen A', { tags: ['@screen-a'] }, () => {
  describe('Smoke', { tags: ['@smoke'] }, () => {
    it('some test', () => {
      // test code
    }); 
  });
}); 

And then run cypress run --env grepTags=@smoke it also doesn't work, because it looks at the topmost describe and checks if it passes the grep, if not it skips that entire block, while actually it shouldn't because the grep matches the inner describe.

My patch will probably solve it since it'll be calculated only at the it level :)

Plus, I saw #5 which similar but applies to the name, not just the tags. I think given all of the above it also needs to aggregate the names, because right now it only greps the name of each it and if you want to grep a higher context it's not possible.

I'll implement both, and it'd be easy enough to drop the name if it's not desired yet.

bahmutov commented 3 years ago

That’s awesome, I will review the PR when I get back, thanks for doing it

Sent from my iPhone

On Jun 26, 2021, at 03:40, oririner-wiz @.***> wrote:

 Great! 🙏

Also, I just noticed that when doing something like this:

describe('Screen A', { tags: @.'] }, () => { describe('Smoke', { tags: @.'] }, () => { it('some test', () => { // test code }); }); }); And then run cypress run --env @.*** it also doesn't work, because it looks at the topmost describe and checks if it passes the grep, if not it skips that entire block, while actually it shouldn't because the grep matches the inner describe.

My patch will probably solve it since it'll be calculated only at the it level :)

Plus, I saw #5 which similar but applies to the name, not just the tags. I think given all of the above it also needs to aggregate the names, because right now it only greps the name of each it and if you want to grep a higher context it's not possible.

I'll implement both, and it'd be easy enough to drop the name if it's not desired yet.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub, or unsubscribe.

github-actions[bot] commented 3 years ago

:tada: This issue has been resolved in version 2.5.2 :tada:

The release is available on:

Your semantic-release bot :package::rocket:

Gooti commented 2 years ago

Hi @bahmutov , @oririner-wiz! I have to admit this is a great feature and I see it's been already released! For testing purposes I implemented the same tags @oririner-wiz provided:

describe('Screen A', { tags: ['@smoke', '@screen-a'] }, () => {
  it('should navigate to Screen B successfully', { tags: ['@screen-b'] }, () => {
    // do something that eventually sends the page to screen b.
  }); 
}); 

Then I tried to run it using yarn cypress run --env grepTags="@smoke+@screen-b" but it didn't grab only that test.

Unfortunately I somehow cannot get it to work. I don't see any instruction in readme on how to run commend in this specific case. Is there anything missing? Is there any specific command I should use?

bahmutov commented 2 years ago

@Gooti I have added your example test case in https://github.com/cypress-io/cypress-grep/commit/c88579a91218c8d469cd995389aac20af28432c6 and it works as expected. Please try running with DEBUG logs (see the readme) to see what is going on. Also I suggest trying the latest version of the plugin.

Gooti commented 2 years ago

@bahmutov thank you so much for clarification! You are correct, I was mistaken, grepTags works fine in this case. I made an assumption that grepFilterSpecs will work in this case, but I am afraid it will not. Correct me if I'm wrong please.

bahmutov commented 2 years ago

Might not. It probably filters very simply. If this is a problem please open a new issue with complete info of what is not working as expected

Sent from my iPhone

On Jan 12, 2022, at 09:02, Gooti @.***> wrote:

 @bahmutov thank you so much for clarification! You are correct, I was mistaken, grepTags works fine in this case. I made an assumption that grepFilterSpecs will work in this case, but I am afraid it will not. Correct me if I'm wrong please.

— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you were mentioned.