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

Using $PSBoundParameters does not work in ParameterFilter for Should -Invoke #1542

Closed
johlju opened this issue May 15, 2020 · 12 comments · Fixed by #1916
Closed

Using $PSBoundParameters does not work in ParameterFilter for Should -Invoke #1542

johlju opened this issue May 15, 2020 · 12 comments · Fixed by #1916
Milestone

Comments

@johlju
Copy link
Contributor

johlju commented May 15, 2020

1. General summary of the issue

Using $PSBoundParameters in a the ParameterFilter for a Should -Invoke does not work.

2. Describe Your Environment

Using Pester 5 RC8

Pester version     : 5.0.0 C:\source\SqlServerDsc\output\RequiredModules\Pester\5.0.0\Pester.psd1
PowerShell version : 7.0.1
OS version         : Microsoft Windows NT 10.0.19041.0

3. Expected Behavior

Able to evaluate the parameters that was bound to the call.

4.Current Behavior

Running this example it fails even when the parameter was actually passed that was evaluated in the ParameterFilter.

BeforeAll {
    function Get-Something {
        [CmdletBinding()]
        param
        (
            [Parameter()]
            $MyParam1,

            [Parameter()]
            $MyParam2
        )
    }

    function Set-Something {
        [CmdletBinding()]
        param ( )

        Get-Something -MyParam1 SomeValue
    }
}

Describe 'MyTest2' {
    Context 'MyContext' {
        BeforeAll {
            Mock Get-Something
        }

        It 'MyIt' {
            { Set-Something } | Should -Not -Throw

            Should -Invoke -CommandName Get-Something
            Should -Invoke -CommandName Get-Something -ParameterFilter {
                #Write-Host $MyInvocation.Line
                #$MyInvocation | ConvertTo-Json | Write-Host
                #$MyInvocation.BoundParameters.ContainsKey('MyParam1') -eq $true
                $PSBoundParameters.ContainsKey('MyParam1') -eq $true
            } -Exactly -Times 1 -Scope It
        }
    }
}

5. Possible Solution

No, sorry :/

6. Context

Converting Pester 4 tests to Pester 5. This method has been a normal way, although not heavy used, of separating different asserts for the same cmdlet. This is used instead of splitting up into several Mock's which use different ParameterFilter.

@nohwnd nohwnd added this to the 5.0 milestone May 15, 2020
@nohwnd nohwnd modified the milestones: 5.0, 5.1 May 26, 2020
@nohwnd nohwnd modified the milestones: 5.1, 5.2 Oct 25, 2020
@johlju
Copy link
Contributor Author

johlju commented Dec 16, 2020

I worked around this by using the following. But it is not the best workaround since it is not as explicit as $PSBoundParameters.ContainsKey().

Should -Invoke -CommandName Get-Something -ParameterFilter {
    # Intead of $PSBoundParameters.ContainsKey('MyParam1') -eq $true
    $null -ne $MyParam1
} -Exactly -Times 1 -Scope It

Should -Invoke -CommandName Get-Something -ParameterFilter {
    # Intead of $PSBoundParameters.ContainsKey('MyParam1') -eq $false
    $null -eq $MyParam1
} -Exactly -Times 1 -Scope It

@DarkLite1
Copy link
Contributor

DarkLite1 commented Mar 17, 2021

@johlju you cannot be 100% certain that $MyParam1 represents the parameter used in the function. It can also be a variable used within the test script or a variable set in the it clause. More info here.

This will always pass for example:

it 'test' {
   $MyParam1 = $true

   Should -Invoke -CommandName Get-Something -ParameterFilter {
       # Intead of $PSBoundParameters.ContainsKey('MyParam1') -eq $false
       $null -ne $MyParam1
   } -Exactly -Times 1 -Scope It
}

@johlju
Copy link
Contributor Author

johlju commented Mar 17, 2021

Agree. For variables in test scripts I try to prefix them with 'mock', e.g. $mockMyParam1 so they do not affect the code being tested, especially when using InModuleScope. But it could definitely use a variable from the parent scope (for example when using -ModuleName too). Rather see $PSBoundParameters to work again or some other variable name that returns all bound parameter names and values for the mock.

@DarkLite1
Copy link
Contributor

Rather see $PSBoundParameters to work again or some other variable name that returns all bound parameter names and values for the mock.

That's something I would like to see too. A dedicated way to address the parameters of a mocked function. @nohwnd just tagging you as an fyi.

@nohwnd
Copy link
Member

nohwnd commented Apr 7, 2021

So the problem is that users don't define the param block in mocks, and before we defined it for them, but this prevented them from debugging the mock code.

I removed binding the params because when you don't have param block then $args will contain the params and then the args. This is change of behavior from pester 4, so I just did not bind the params.

I don't know about any way to define the params on your behalf without detaching the scriptblock from the original scriptblock.

I can mitigate by binding the params as second, which will go to the end of $args in the case when you don't provide param block (e.g. 99.9% of the time.) Which won't be consistent with how powershell does it, but allows you to specify cmdletbinding and param manually.

Imho this is not practical because you have to be familiar with the types of parameters the mocked command uses and specify them explicitly.

So I guess the best way would be to add a variable such as $BoundParameters (or $PesterBoundParameters) and populate it with the actually bound parameters, and document this limitation.

@DarkLite1
Copy link
Contributor

So I guess the best way would be to add a variable such as $BoundParameters (or $PesterBoundParameters) and populate it with the actually bound parameters, and document this limitation.

This would allow a user to be explicit about what he/she is asserting. The best way would indeed be a predefined hashtable like $pesterBoundParameters or something similar that would allow the user to explicitly check the value provided to a parameter of a mocked function.

@johlju
Copy link
Contributor Author

johlju commented Apr 8, 2021

I'm good with having a variable that holds the bound parameters, but maybe we can call it $MockBoundParameters?

@nohwnd
Copy link
Member

nohwnd commented Apr 8, 2021

There is at least one issue complaining about missing the bound parameters It and other blocks, so I was after a standardized name that could be used in other places as well. Personally I like $BoundParameters the most, but it seems to be easily conflicting.

@johlju
Copy link
Contributor Author

johlju commented Apr 8, 2021

PesterBoundParameters might be best then, to visibly separate it clearly from PSBoundParameters. Can’t come up with something better. 😄

@fflaten
Copy link
Collaborator

fflaten commented Apr 8, 2021

There is at least one issue complaining about missing the bound parameters It and other blocks

Got link or more details? Is this about providing a "$pesterboundparameters"-dictionary for values like tags, expandedname etc. for the it/block-node?

@nohwnd
Copy link
Member

nohwnd commented Apr 9, 2021

#1828

@DarkLite1
Copy link
Contributor

I came back to this issue because we had trouble migrating to PowerShell 7, where the ActiveDirectory module is deserialised.

This worked in PowerShell 5.1

Mock Get-ADGroupMember {
    throw 'Group members cannot be retrieved'
} -ParameterFilter {
    'Group1' -eq $Identity.SamAccountName
} 

Working code in PowerShell 7:

Mock Get-ADGroupMember {
    throw 'Group members cannot be retrieved'
} -ParameterFilter {
    'Group1' -eq $PipelineVariable.SamAccountName
} 

To figure out the parameter name used by Pester in the Mock function we used the following code:

Mock Get-ADGroupMember {
    foreach ($param in $PesterBoundParameters.GetEnumerator()) {
        Write-Warning "Key={0} Value={1}" -f $param.Key, $param.Value
    }
}

Hopefully this might help others struggling to find the correct parameter name in a Pester Mock function. I could also be helpful to mention this in the docs.

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.

4 participants