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

Add Should-HaveParameter, to replace Should -HaveParameter #2618

Open
3 tasks done
johlju opened this issue Feb 8, 2025 · 8 comments · May be fixed by #2621
Open
3 tasks done

Add Should-HaveParameter, to replace Should -HaveParameter #2618

johlju opened this issue Feb 8, 2025 · 8 comments · May be fixed by #2621
Labels

Comments

@johlju
Copy link
Contributor

johlju commented Feb 8, 2025

Checklist

Summary of the feature request

Suggestions is to add Should-HaveParameter to Pester 6 assertions to replace Should -HaveParameter.

It was mentioned in the issue title but there are no other mentions of it, so I thought we add a new issue to track it or discuss if it should be or not be.

How should it work?

No response

@johlju johlju added the Feature label Feb 8, 2025
@johlju johlju changed the title Add Should-HaveParameter to Pester 6 assertions to replace Should -HaveParameter Add Should-HaveParameter, to replace Should -HaveParameter Feb 8, 2025
@nohwnd
Copy link
Member

nohwnd commented Feb 8, 2025 via email

@JosephColvin
Copy link

I'm not sure if this has already been talked about but I can see an argument for a general "Should-Have" assert/command vs. granular named asserts/commands.
The general floating idea in my mind is from a hypothetical need to assert against a parameter, variable, and/or attribute:
Should-Have and then via a switch parameter like: -Attribute, -Default, and -Parameter
vs.
Should-HaveAttribute, Should-HaveDefault, and Should-HaveParameter.

@fflaten
Copy link
Collaborator

fflaten commented Feb 14, 2025

Thanks for the feedback!

The general floating idea in my mind is from a hypothetical need to assert against a parameter, variable, and/or attribute

Could you provide a few usecase examples? Only parameters have default values IIRC, so it would likely be Get-Command abc | Should-HaveParameter -Name foo -DefaultValue bar etc.

We currently don't have a generic attribute assertion, so a separate Should-HaveAttribute might be useful on it own. They can be attached to both parameters and function/script param-blocks.

@JosephColvin
Copy link

JosephColvin commented Feb 18, 2025

The general floating idea in my mind is from a hypothetical need to assert against a parameter, variable, and/or attribute

Could you provide a few usecase examples? Only parameters have default values IIRC, so it would likely be Get-Command abc | Should-HaveParameter -Name foo -DefaultValue bar etc.

@fflaten I know this is not where this should be, but if you tell me where I should put all of this, I can do that. thank you for fixing the code formatting.

Actually, after spending time baking the idea, I think having a general Should-Have is not a good idea at all but being more explicit is the correct route. Here are the results from baking the idea longer:

Should-HaveInitValue / Should-HaveDefault
Summary: Asserts that the named variable or parameter with given type has an initial value set, that is the first instance found is being set to the given value or matches the pattern via regular expression or direct context match.
Parameter: InputObject, AST/ScriptBlock/CommandInfo/ParameterInfo, the input to use
Parameter: Name, System.String, the name of the parameter or variable
Parameter: Type, The type of the given variable or parameter
Parameter: Pattern, Pattern can be used when the variable/parameter's value could not be ascertained.
Parameter: StringOnly, switch, prevents the pattern from being used as a regular expression and will be a literal text match instead.
Parameter: Value, system.object, what the actual value of the parameter/variable if deterministic/ascertained
Parameter: Because, System.String, the because message
Parameter: Not, switch, negates the result
Note: This assertion should throw an error if the value was not ascertained, and no pattern was provided
Example:
Advance function "Get-MyGreeting":

function Get-MyGreeting {
    [CmdletBinding()]
    [OutputType([string])]
    param (
        [Parameter(Mandatory, Position = 0, ValueFromPipeline, ValueFromPipelineByPropertyName, HelpMessage = 'Enter in the first name of the person you want a greeting for.')]
        [ValidateNotNullOrEmpty()]
        [string]
        $FirstName,

        [Parameter(Position = 1, ValueFromPipelineByPropertyName)]
        [AllowEmptyString()]
        [ValidateNotNull()]
        [string]
        $LastName = ([string]::Empty),

        [Parameter(ValueFromPipelineByPropertyName, DontShow)]
        [string]
        $Greeting = 'Hello'
    )
    
    begin {
        [string]$GreetingFormat = "{0} {1}$( ([string]::IsNullOrEmpty($LastName)) ? ([string]::Empty) : (', ')){2}"
        [string]$Msg = ([string]::Empty)
    }
    
    process {
        if (-not [string]::IsNullOrEmpty($LastName)) {
            $Msg | Write-Output
        }
        $Msg = $GreetingFormat -f $Greeting, $LastName, $FirstName
        $Msg = $Msg.Trim()
    }
    
    end {
        $Msg | Write-Output
    }

    clean {
        Remove-Variable -Name 'GreetingFormat', 'Msg' -Scope 'Local'
    }
}

Test file:

Describe "Get-MyGreeting" {
Context "Inital setup of function state" {
  It "Should have initial value for internal GreetingFormat as expected" {
    Get-Command "Get-MyGreeting" | Should-HaveInitValue -Name 'GreetingFormt' -Type ([system.string]) -Pattern "{0} {1}$( ([string]::IsNullOrEmpty($LastName)) ? ([string]::Empty) : (', ')){2}" -Value " " -StringOnly
  }
  It "Should have a parameter named Greeting that contains the expected value" {
    Get-Command "Get-MyGreeting" | Should-HaveInitValue -Name 'Greeting' -Type ([system.string]) -Value 'Hello'
  }
} # end context "Initial setup of function state"
} # end describe "Get-MyGreeting"

Should-HaveAttribute
Summary: Asserts for existence of an attribute attached to the supplied object of the supplied type and attribute values
Parameter: ActuleValue, System.Object, the input object via pipeline or named parameter
Parameter: Type, System.Attribute, the type of the attribute
Parameter: Value, System.Collections.Hashtable, key/value pair where key maps to a property/field found on the provided type and the value component is what the instance of the property/field's value should be.
Parameter: Because, System.String, the because message
Parameter: Not, switch, negates the result
Examples:
Test file:

Describe "Get-MyGreeting" {
Context "Advance script function parameters" {
  It "Should have ValidateNotNullOrEmptyAttribute applied to parameter 'Foo'" {
    (Get-Command Get-MyGreeting).Parameters['FirstName'] | Should-HaveAttribute ([System.Management.Automation.ValidateNotNullOrEmptyAttribute])
  }
  It "Should have a mandatory pipeline enabled parameter 'foo'" {
    (Get-Command Get-MyGreeting).Parameters['FirstName'] | Should-HaveAttribute ([System.Management.Automation.ParameterAttribute]) -Value @{Mandatory=$true; ValueFromPipeline=$true; ValueFromPipelineByPropertyName=$true;}
  }
} # end context "Advance script function parameters" 
Context "Output" {
  It "Should return a greeting of type string" {
    Get-MyGreeting -FirstName 'John' | Should-Be ([string])
  }
  It "Should return a greeting with a foobar attribute" {
    Get-MyGreeting -FirstName 'John' | Should-HaveAttribute -Not [FoobarAttribute] @{Status='SNAFU'}
  }
} # end context "Output"
} # end describe "Get-MyGreeting"

Should-HaveMember
Summary: Asserts if a given member signature exists on the input object or not, does not evaluate the value of member.
Parameter: MemberType, System.Management.Automation.PSMemberTypes, The member types to search for with default to 'All'
Parameter: Name, system.string, The name of the member
Parameter: Count, system.int32, the number of members that match the name - might be a wildcard name match and should default to one
Parameter: ParameterTypes, array of system.type, used for argument type signatures of methods and parameterized properties matches with default of an empty array
Parameter: ReturnType, system.type, the type of the member's value should be
Parameter: ReadOnly, switch, ensures only a 'get' method/operator exists for this member
Parameter: WriteOnly, switch, ensures only a 'set' method/operator exists for this member
Parameter: Not, switch, negates the result
Example:
Advance Function "Get-InterfaceObject" file: - I have no real clue, sorry and this is already a long enough post.
Test file:

Describe "Get-InterfaceObject" {
Context "Output" {
  It "Should return an interface object" {
    Get-InterfaceObject -ID 'Foo' | Should-Be -Type ([system.object])
  }
  It "Should have 10 properties" {
    Get-InterfaceObject -ID 'Foo' | Should-HaveMember -MemberType 'Property' -Name '*' -Count 10
  }
  It "Should have a read only property named GUID of type GUID" {
    Get-InterfaceObject -ID 'Foo' | Should-HaveMember -MemberType 'Property' -Name 'GUID' -ReturnType ([System.Guid]) -ReadOnly
  }
 It "Should have an expected value for the GUID on the 'Foo' interface" {
   (Get-InterfaceObject -ID 'Foo').GUID | Should-Be '00000000-0000-0000-0000-000000000000'
 }
 It "Should have a Port([int], [int]) property" {
   Get-InterfaceObject -ID 'Foo' | Should-HaveMember -MemberType 'Property' -Name 'Port' -PatameterTypes @( ([System.Int32]), ([System.Int32]) )
 }
 It "Should have 2 Send methods (SendNumber, SendText) and both return boolean values" {
   Get-InterfaceObject -ID 'Foo' | Should-HaveMember -MemberType 'Method' -Name 'Send*' -Count 2 -ReturnType ([System.Boolean]) -Because "need a way to send numbers and text (converted to ascii codes internally) out the interface and have a return of true/false value to determine success"
 }
 It "Should have a write only property named OutBuffer of type Object" {
   Get-InterfaceObject -ID 'Foo' | Should-HaveMember -MemberType 'Property' -Name 'OutBuffer' -Count 1 -ReturnType ([system.object]) -WriteOnly
 }
} # end context "Output"
} # end describe "Get-InterfaceObject"

@nohwnd
Copy link
Member

nohwnd commented Feb 19, 2025

Reading the comment above I have hard time understanding what the proposal actually is. I think keeping the assertions specific to one area is important, rather than having Should-Have that can act both on paramters and members. (And you seem to agree above).

I am not opposed to specific Should-HaveMember assertion, but it seems quite easy to replace with separate calls to Should-Be, especially if we made Should-Be more strict by adding a switch to e.g. force the actual and expected to have the same type. Then you'd be able to check e.g.

$myObject.GUID | Should-Be  ([System.Guid]"00000000-0000-0000-0000-000000000000") -EnsureSameType

And having a dedicated Should-HaveMember assertion falls apart if we would want to encode all the different value checks into it. e.g. checking if the value is a string, that starts with a given string. In that case we are better served by dedicated Should-BeLikeString.

But I do still see some value in it, to check if members are present. Please make a new issue with this request if you'd like.

@JosephColvin
Copy link

Thank you @nohwnd, I agree 100% and I do realize that one could do the following for a 'member exists' assertion, which is probably how it should be left:

(-Not $null -eq ($Contact | Get-Member 'Name')) | Should-Be $true

Sorry about the confusion. To clear up confusion, it really was just a few things that I came up with while trying to make a concrete argument for a generalized Should-Have. After which I found that I couldn't argue for it but in the process found the listed commands candidates for possible value. I posted them here as I initially didn't know if @fflaten still wanted to "see" what I was thinking. I soon realized that it just made it more confusing and muddied the waters.

Though I can only argue somewhat for a Should-HaveAttribute command, the others I have already realized are of no value that I could argue.

Feel free to remove any comments to clean up the issue tracker.

@nohwnd
Copy link
Member

nohwnd commented Feb 20, 2025

Brainstorming here is perfectly fine, thanks for your ideas. :)

@nohwnd nohwnd linked a pull request Feb 20, 2025 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants