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

Missing -Not parameter in new Should- assertions #2550

Open
fflaten opened this issue Jul 26, 2024 · 2 comments
Open

Missing -Not parameter in new Should- assertions #2550

fflaten opened this issue Jul 26, 2024 · 2 comments

Comments

@fflaten
Copy link
Collaborator

fflaten commented Jul 26, 2024

In Pester v5 I really loved the simplicity of negating a statement:

.. | Should -Invoke Get-Process
.. | Should -Not -Invoke Get-Process

I guess the -Not switch will no longer be used in the future? Really loved that feature though..

Originally posted by @DarkLite1 in #2533 (comment)

The v6 style is currently

# invoke is not implemented yet
.. | Should-Invoke Get-Process
.. | Should-NotInvoke Get-Process

Is that worse? As we're using standalone functions to overcome parameter set max limitation the alternative would be:

.. | Should-Invoke Get-Process
.. | Should-Invoke -Not Get-Process
.. | Should-Invoke Get-Process -Not

The parameter doesn't flow/read as well imo.

Originally posted by @fflaten in #2533 (comment)

I hear what you say, it does read easier but for splatting purposes, the separate -Not argument would be easier. I can imagine TestCases or ForEach where a decision is made to either call the mocked function or not.

Also for inteliSense it would be easier to use one CmdLet name instead of having to type Should-Not.

Originally posted by @DarkLite1 in #2533 (comment)

@fflaten
Copy link
Collaborator Author

fflaten commented Jul 26, 2024

I don't disagree on the function bloat and 90% help duplication etc. though we have to find a balance.

Personally I'm struggling a bit with testcases modifying the assertion behavior like Should .. -Not:$SomeTestParam. Maybe it's my personal style, but I'd always have two separate tests for working vs failing, mocked vs not etc. for easier troubleshooting and test naming.

@nohwnd What's your thoughts on the current design for Not/Negate?

@nohwnd
Copy link
Member

nohwnd commented Jan 30, 2025

I have the same opinion as you @fflaten. controlling whether the assertion will or will not be negated from the test seems like a code smell. A code smell that is sometimes necessary and makes stuff easier, but that should be (and from experience is) an exceptional case. And probably should not drive our design.

I also wanted to avoid adding an implicit -Not operator to every single assertion (even though that is not what this issue is asking for), and documenting the negative assertions as: "this is the same as the positive case, but opposite." Because that statement was not true in many cases.

I find it better to have a special function that has its own help and examples for the negative case, because often the negative assertion is either not needed, or much simpler than the positive one. (e.g. GreaterOrEqualThan vs LessThan, but not GreaterOrEqualThan vs NotGreaterOrEqualThan, and Should-HaveParameter -Name abc -Mandatory, vs Should-NotHaveParameter abc (and no additional checks like is mandatory, and so on, because we primarily check that the parameter is not present)).

This of course can be to some extent modelled by not forcing the parameter on the assertion, and using parameters sets, but to me it reads worse with -Not parameter, and so I decided to take the other route.

We are still in alpha though, so let's discuss further :)

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