pester / Pester

Pester is the ubiquitous test and mock framework for PowerShell.
https://pester.dev/
Other
3.11k stars 473 forks source link

How do I discover Should operators? #878

Closed alx9r closed 5 years ago

alx9r commented 7 years ago

As I understand it, using Pester 4 the better practice is to use named parameters with Should to compose assert statements. The following is an example of such use:

$r | Should -beOfType ([string])  # <= I think this is the preferred syntax

I believe it has been stated that this is preferable to composition using arguments to should like the following:

$r | Should beOfType ([string]) # <= I think this syntax is deprecated

Because of the move toward operators as parameters, I expected there to be a way to discover the operators as is possible for most other function's parameters. I have not yet found a way to do that. Invoking help should -Parameter *, for example, outputs no useful information. help about_Should outputs a number of useful examples. But that command merely outputs whatever was written. I see no reason to expect that it currently is, or will ever be, reliably comprehensive of all operators.

How do I, as a normal user, discover the Should operators?

it-praktyk commented 7 years ago

My ideas

  1. Read about_Should?
  2. Some kind of auto-completion?

The related issue #875

alx9r commented 7 years ago

It looks like intellisense suggests the operators as parameters but without any explanation of what they do. It seems like if that is working, there ought to be a way to make Get-Help -Parameter * output help for each of those parameters as well.

If that's not possible, perhaps it's worthwhile to implement something like Get-AssertionOperatorHelp which outputs the same types of objects as Get-Help Should -Parameter * except populated with the information from actual operators.

nohwnd commented 7 years ago

This is one of the things I don't like about the current approach. It is a step forward from 'Should Not Be' that is not discoverable at all, but it's still not ideal. I would love all the assertions to take form of Should-Be Should-NotBe, that would make them discoverable by the normal means of command discovery which is Get-Command.

In fact I would rather use Assert-*, but for that we need a new dialect, so maybe in v5 :)

Till then I think adding help to the intellisense (if that is how other commands behave) is the best way forward. I need to have a look as well.

brianbunke commented 6 years ago

When these assertions (-Be, -Match, etc.) moved to dynamic parameters, a side effect is that displaying help -- basically, any generic PowerShell discovery -- becomes very difficult. (more info)

I have submitted a new Show-PesterAssertion prototype function in #1111. It's a little silly, and I won't mind if you reject it.

Solving this problem with a new function is good and bad.

Pros:

Cons:

Given that this problem is likely to stick around for a while, I think it's a decent stopgap solution.

nohwnd commented 6 years ago

@brianbunke Thanks for the PR. I think we should solve this in a more extensible way, because there are not only our assertions, but also custom assertions that can register with should via Add-AssertionOperator.

Could we maybe:

This way we don't have to regex, and we can document the extra parameters we add to each operator close to where it is defined, and others can do the same.

Maybe we should also alias Add-AssertionOperator to Add-ShouldOperator, add Get-ShouldOperator, and call the new function called Get-ShouldHelp / Get-ShouldOperatorHelp ?

brianbunke commented 6 years ago

More than happy to solve this closer to the source.

What exactly do you mean by:

looking the operator up in the assertion table

Want to make sure I'm heading in the right direction when I get back to it. Thanks for taking a look!

nohwnd commented 6 years ago

@brianbunke There is a scoped variable in the Pester module listing all the registered assertion operators. Externally you can view it like this Import-Module Pester ; &(get-module pester){ $script:AssertionOperators }, but now that I am looking at it, it does not link back to the function that implements it, it only uses the definition of the function. So that would need to change a bit.

brianbunke commented 6 years ago

1116 is a WIP example based on that feedback.

image image image

Questions for you:

  1. Is Show-PesterAssertion the best name for this? There is an Add-AssertionOperator; would Show-AssertionOperator or Get-AssertionOperator make more sense?
  2. Given Show-PesterAssertion, I like that all registrations are returned as objects with Name and Alias properties, but the Help is so ugly. Any better ideas there?
  3. Given Show-PesterAssertion -Name Be, should it return the full Name/Alias/Help object, or should I just return the reasonably formatted help?
  4. Adding InternalName allows us to reach back for help on-demand, but PowerShell's built-in help then refers to ShouldBe instead of Be.
    1. Ignore this?
    2. Return only the Description property of help, allowing us to write a couple sentences on what each assertion checks, and then provide freeform examples?
    3. If that ^, go one step further from the source and just start maintaining Be.md, Match.md, etc.? Would enable build aggregation of about_Should.help.txt, as well as further shenanigans (a future GitHub Pages site nobody wants to build?!?).
nohwnd commented 6 years ago

1&2) I'd rename the Add-AssertionOperator to Add-ShouldOperator, and call your function Get-ShouldOperator.

Maybe the -Name parameter could mandatory in a default parameter set and throw the standard validate set exception. (for discoverability). A second non-default param set could be there with -All to be able to get the nice table.

3) I'd show the help directly. we can complicate it later

4) keep in mind that we are building this not only for us, but also for others, so I would go with the most standard solution. frankly I am not sure if there is any login to how the functions are named, I would propose we rename all of them to Should- form, even though I don't see a reason we could not call them with just the operator name. Even function throw () {} is a valid file definition. You cannot call it directly but we don't care about that much. Should-Throw might be simpler though, and less confusing.

brianbunke commented 6 years ago

I imagine the typical user's flow will be:

Get-ShouldOperator

image

Maybe that's what I need? Get-ShouldOperator -Name BeExactly

image

Updated the PR with those changes, plus renaming the internal functions of Be/BeExactly and adding help.

If this works, I'll pursue changing internal names and adding help for the rest of the operators.

Will submit a different PR to propose changing Add-AssertionOperator to Add-ShouldOperator.

nohwnd commented 6 years ago

Yeah that looks good to me. 🙂

Ad Add-AssertionOperator: It should be renamed, but you should also add an alias to avoid making this a breaking change.