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

Filter what Context and It blocks run #1374

Conversation

splatteredbits
Copy link
Contributor

@splatteredbits splatteredbits commented Sep 24, 2019

Pester wants test fixtures to have one (or just a few) Describe blocks with many It blocks representing specific tests. Unfortunately, the TestName parameter only applies to Describe blocks. If you have all your It blocks in a single Describe (as most everybody and Pester examples do), then you can't run individual tests. If these It blocks take a long time and one of them is failing, it can be very painful to debug them. You can comment out the tests you don't want to run, or re-arrange, or add return statements. Or, you use a pattern where you have a single It block in each Describe block.

As a developer, I'd like parameters on Invoke-Pester that allow me to filter It blocks so I don't have to go through all the steps I currently have to just to run a single It block.

This PR adds ContextFilter and ItFilter parameters to Invoke-Pester that allows this level of control.

This should address issue #292, although without the special formatting to the TestName parameter described in that issue.

I would also like to rename the TestName parameter to DescribeFilter (with a backwards-compatible alias) along with renaming internal parameters and Pester state property names. That's a big refactor and didn't want to do it without permission.

Also, maybe in Pester 5, we can make ItFilter the second positional parameter instead of TestName because it just makes more sense to filter It blocks my default than Describe blocks.

@nohwnd
Copy link
Member

nohwnd commented Oct 3, 2019

Thanks for this PR. I see that my comment from last week was not posted. Not sure what I did wrong.

This can theoretically work, but there are a lot of limitations and edge cases, and things that the user needs to understand for this implementation to work in his favor. I was achieving this in v5 and it took a lot of figuring out to provide a good solution that is easy to understand and has less edge cases.

I'd like to finish v5 soon and release it and I don't think it makes sense to merge this into v4 as it would add parameters that don't have behavior that is easy to understand and require a lot from the user to actually do what they are supposed to do.

I am not sure if I am describing this in a way that can be understood, so please ask questions.

@splatteredbits
Copy link
Contributor Author

I'm curious to know more about the limitations and edge cases. It seems pretty straight-forward to me.

Getting this functionality into Pester is critical for our team.

Do you have documentation on how the v5 functionality works?

Also, I disagree that these parameters are not easy to understand. ItFilter seems pretty obvious.

I could be convinced to drop the ContextFilter. Contexts behave weirdly.

@nohwnd
Copy link
Member

nohwnd commented Oct 5, 2019

I'm curious to know more about the limitations and edge cases. It seems pretty straight-forward to me.

The filter promises to run just the single test, but it will run the test and every setup and teardown in every file that is included in the run. It is the same situation for Describe/Context. To enable the parameter to work correctly the test author would need to put all code in Pester controlled blocks so the execution of the code can be postponed till appropriate time, or skipped entirely (as in v5).

The thread you recently commented in sums up my thinking about this in a more complete manner I hope.

Another limitation is that once you add anything dynamic to the name of the test you won't be able to filter it anymore because the strings won't match and you will have to resort to using wildcards. A better way to solve this is for example using the position of the opening curly brace of the test because that won't change during runtime.

Pretty much I am reluctant to merge this because the promises the parameter names give are hard to keep, the limitations of the functionality is hard to sum up, and I am pretty sure that when v4 will be put to maintenance mode those parameters would be a source of confusion for the years to come. ☹️

@ErikUmhoe
Copy link

Any plans to get this merged in?

@nohwnd
Copy link
Member

nohwnd commented May 15, 2021

Not merging this because of the reasons outlined above. Thanks for posting the PR.

@nohwnd nohwnd closed this May 15, 2021
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

Successfully merging this pull request may close these issues.

3 participants