Pester icon indicating copy to clipboard operation
Pester copied to clipboard

Missing -Not parameter in new Should- assertions

Open fflaten opened this issue 1 year ago • 1 comments

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 https://github.com/pester/Pester/issues/2533#issuecomment-2252270088

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 https://github.com/pester/Pester/issues/2533#issuecomment-2252338247

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 https://github.com/pester/Pester/issues/2533#issuecomment-2252347148

fflaten avatar Jul 26 '24 11:07 fflaten

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?

fflaten avatar Jul 26 '24 11:07 fflaten

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 :)

nohwnd avatar Jan 30 '25 07:01 nohwnd

I'm with @nohwnd on this. I kind of like the new assertions and I'm not missing the parameter -Not.

With several thousands of tests in one project that currently using Pester v5 (and moving to v6) there are very few occurrences where we used -Not. 99.9% of the test code does not negate tests. The one time we often used negate is Should -Not -Throw which we should not have used anyway. I also found a few Should -Not -BeNullOrEmpty and Should -Not -Contain. The conclusion I see is that Should-Not* commands will be used rarely, so why make the other commands more complex by introducing -Not again. 🤔

johlju avatar Feb 05 '25 06:02 johlju

Thx for the feedback everyone. I am clearly in the minority here, so this issue can be closed.

I think I'll just have to get used to not using not :)

DarkLite1 avatar Feb 05 '25 07:02 DarkLite1

I think I'll just have to get used to not using not :)

Just to be clear, this was not what I was trying to say. I am perfectly fine with using -not when needed, e.g. Should -Not -BeNullOrEmpty. On the other hand such assertions can often be replaced with more concrete examples, e.g. rather than saying should not be empty. We can say: Should-BeEqual "abc" or Should-BeLike "{ "name": "jakub"*"

nohwnd avatar Feb 06 '25 08:02 nohwnd

"What @nohwnd said" | Should -BeTrue :)

DarkLite1 avatar Feb 06 '25 08:02 DarkLite1