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 Parameter Docs in V5 #2128

Closed
LethiferousMoose opened this issue Jan 31, 2022 · 7 comments · Fixed by pester/docs#247
Closed

Missing Parameter Docs in V5 #2128

LethiferousMoose opened this issue Jan 31, 2022 · 7 comments · Fixed by pester/docs#247
Assignees

Comments

@LethiferousMoose
Copy link

Location

  • Pester version: v5
  • Functions: Should, Get-ShouldOperator

General summary of the issue

Placeholder text is still being used for parameter documentation.

@fflaten
Copy link
Collaborator

fflaten commented Jan 31, 2022

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
Copy link
Author

LethiferousMoose commented Feb 2, 2022

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
Copy link
Collaborator

fflaten commented Feb 2, 2022

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
Copy link
Author

LethiferousMoose commented Feb 2, 2022

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
Copy link
Collaborator

fflaten commented Feb 2, 2022

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
Copy link
Author

LethiferousMoose commented Feb 2, 2022

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
Copy link
Collaborator

fflaten commented Dec 27, 2022

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants