pester / Pester

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

Missing Parameter Docs in V5 #2128

Closed LethiferousMoose closed 1 year ago

LethiferousMoose commented 2 years ago

Location

General summary of the issue

Placeholder text is still being used for parameter documentation.

fflaten commented 2 years ago

Thanks for the report. This is due to the use of dynamic parameters in both functions + lots of common parameters, but we might be able to do something.

  1. Add a reminder in the Description for Should to use ex. Get-ShouldOperator -Name Be for operator-specific help. We link to the assertions-page on pester.dev as a related link, but I see that it's hidden all the way at the bottom. 😞

  2. We should be able to fix -Name in Get-ShouldOperator, so I'm keeping this open for that as a minimum.

  3. Should is a bit more difficult. It has two types of parameters:

    • Operators name/alias: The name of the operator to use. Can only chose one at a time (as it defines the parameter set)
      • We could possibly include the synopsis for the operator-name, example:

    -Be

    Compares one object with another for equality and throws if the two objects are not the same.

    Type: SwitchParameter
    Parameter Sets: Be
    Aliases: EQ
    Accepted values: 
    
    Required: True (None) False (Be)
    Position: Named
    Default value: 
    Accept pipeline input: False
    Accept wildcard characters: False
    DontShow: False

    -Exist

    Does not perform any comparison, but checks if the object calling Exist is present in a PS Provider. The object must have valid path syntax. It essentially must pass a Test-Path call.

    Type: SwitchParameter
    Parameter Sets: Exist
    Aliases: 
    Accepted values: 
    
    Required: True (None) False (Exist)
    Position: Named
    Default value: 
    Accept pipeline input: False
    Accept wildcard characters: False
    DontShow: False
    • Operator input: Parameters like -ExpectedValue, -Times, -Exactly, -ErrorId etc. which could have different meanings for each operator.
      • It looks like PowerShell will select the last helpmessage for a parameter used in multiple parameter sets (operators in our case), so it would just show a random text that only fits one operator.
      • Might be better to just leave it blank or hardcode a message like:

    -ExpectedValue

    Depends on operator being used. See Get-ShouldOperator -Name <Operator> for help.

    Type: Object
    Parameter Sets: Be, BeExactly, BeGreaterThan, BeLessOrEqual, BeIn, BeLessThan, BeGreaterOrEqual, BeLike, BeLikeExactly, Contain, HaveCount
    Aliases: 
    Accepted values: 
    
    Required: True (None) False (Be, BeExactly, BeGreaterThan, BeLessOrEqual, BeIn, BeLessThan, BeGreaterOrEqual, BeLike, BeLikeExactly, Contain, HaveCount)
    Position: 1
    Default value: 
    Accept pipeline input: False
    Accept wildcard characters: False
    DontShow: False

Thoughts? Need to make sure it works all the way back to PSv3, so can't promise anything yet.

LethiferousMoose commented 2 years ago

How do you create your docs? Do you use platyPS? Does it pull straight from the PowerShell code or is the markdown the source of truth? If the markdown was the source of truth you could just update that file could you not? It clearly generated placeholder text. I was less concerned about the Get-Help in actual PowerShell and more concerned the actual docs website is missing information.

https://pester-docs.netlify.app/docs/commands/Should#parameters https://pester-docs.netlify.app/docs/commands/Get-ShouldOperator#parameters

-Alias​
{{ Fill Alias Description }}
fflaten commented 2 years ago

We use platyPS with code as truth. It's done through a Docusaurus helper-module which always use new-markdownhelp, so we'd want to always get the parameter help from code which is fine.

I was mostly thinking about description examples. Synopsis for operators can be rephrased. However I wouldn't personally like to see 10 lines for description of -ExpectedValue, -Because etc. because we had to include it's every operator's variant of it's help (since the usage could vary worst-case). So some type of static text is probably best for the non-operator/input parameters.

AFAIK get-help doesn't work with dynamic parameters, so web only for parameters.

LethiferousMoose commented 2 years ago

Okay, well if you're always using New-MarkdownHelp it looks like it will pull off the HelpMessage from a runtime parameter. I just did a test and that will pre-populate the {{ }} placeholders. I'm not sure how Pester creates it's runtime parameters, but you could just add some general descriptive text in there. Does the usage vary that much? When it comes down to do it, they are fairly general assertions.

Also Get-Help "works" for dynamic parameters, but it's super brittle and breaks when comment help is added.

fflaten commented 2 years ago

it will pull off the HelpMessage from a runtime parameter.

Yeah that's the plan πŸ‘

Does the usage vary that much?

Don't remember all in my head, but can't ignore the possibility. The issue is still that since it uses the last attribute's helpmessage, it could also break in the future when new operators are added or code is reordered.

Maybe I'm overthinking this. Currently no parameter help in operators so it all has to be written too. Was hoping to simplify that part by referencing the official assertion help-pages. πŸ™‚

LethiferousMoose commented 2 years ago

Oh right, I forgot the parameter sets are all in different attributes. I see what you're getting at now. That could be a pain to maintain... Could just save it off per parameter and add it to each set, then order doesn't matter... Obviously that would be bad if a certain set has a very specific usage, but you also only get one help message, so that's kind of irrelevant.

fflaten commented 1 year ago

Should be fixed after next update if docs when 5.4 is released.