Issue with -Be assertion operator after adding custom

claudiospizzi commented 5 years ago

1. General summary of the issue

When we add custom assertion operators to Pester with Add-AssertionOperator, after adding some of them, we start getting issues with the built-in operator -Be. The following assertion will not work anymore:

1 | Should -Be 1

It will throw the following error:

Should : Cannot bind positional parameters because no names were given.

The error is an issue with the parameter binding on Should. The root case looks like that the last added custom assertion operator is now a mandatory parameter in the parameter set of Be.

2. Describe Your Environment

Pester version :4.8.1 C:\Users\Claudio\Documents\WindowsPowerShell\Modules\Pester\4.8.1\Pester.psd1 PowerShell version :5.1.18362.145 OS version :Microsoft Windows NT 10.0.18362.0

OR

Pester version :4.8.1 C:\Users\Claudio\Documents\PowerShell\Modules\Pester\4.8.1\Pester.psd1 PowerShell version :6.2.0 OS version :Microsoft Windows NT 10.0.18362.0

3. Expected Behavior

The custom assertion operator should not polluting the default -Be operator.

4.Current Behavior

Internally, we have a module with custom assertion operator, to test IP Addresses, Windows Services etc. But it is reproducable with a simple script adding the same operator 10 time, after the last one the Should -Be is starting to get the issue:

function Should-BeDemo0 { [CmdletBinding()] param ( [Parameter(Mandatory = $true, ValueFromPipeline = $true)] [System.Object] $ActualValue, [System.String] $DemoA, [switch] $Negate, [System.String] $Because ) [PSCustomObject] @{ Succeeded = $true; FailureMessage = $null } }
function Should-BeDemo1 { [CmdletBinding()] param ( [Parameter(Mandatory = $true, ValueFromPipeline = $true)] [System.Object] $ActualValue, [System.String] $DemoB, [switch] $Negate, [System.String] $Because ) [PSCustomObject] @{ Succeeded = $true; FailureMessage = $null } }
function Should-BeDemo2 { [CmdletBinding()] param ( [Parameter(Mandatory = $true, ValueFromPipeline = $true)] [System.Object] $ActualValue, [System.String] $DemoC, [switch] $Negate, [System.String] $Because ) [PSCustomObject] @{ Succeeded = $true; FailureMessage = $null } }
function Should-BeDemo3 { [CmdletBinding()] param ( [Parameter(Mandatory = $true, ValueFromPipeline = $true)] [System.Object] $ActualValue, [System.String] $DemoD, [switch] $Negate, [System.String] $Because ) [PSCustomObject] @{ Succeeded = $true; FailureMessage = $null } }
function Should-BeDemo4 { [CmdletBinding()] param ( [Parameter(Mandatory = $true, ValueFromPipeline = $true)] [System.Object] $ActualValue, [System.String] $DemoE, [switch] $Negate, [System.String] $Because ) [PSCustomObject] @{ Succeeded = $true; FailureMessage = $null } }
function Should-BeDemo5 { [CmdletBinding()] param ( [Parameter(Mandatory = $true, ValueFromPipeline = $true)] [System.Object] $ActualValue, [System.String] $DemoF, [switch] $Negate, [System.String] $Because ) [PSCustomObject] @{ Succeeded = $true; FailureMessage = $null } }
function Should-BeDemo6 { [CmdletBinding()] param ( [Parameter(Mandatory = $true, ValueFromPipeline = $true)] [System.Object] $ActualValue, [System.String] $DemoG, [switch] $Negate, [System.String] $Because ) [PSCustomObject] @{ Succeeded = $true; FailureMessage = $null } }
function Should-BeDemo7 { [CmdletBinding()] param ( [Parameter(Mandatory = $true, ValueFromPipeline = $true)] [System.Object] $ActualValue, [System.String] $DemoH, [switch] $Negate, [System.String] $Because ) [PSCustomObject] @{ Succeeded = $true; FailureMessage = $null } }
function Should-BeDemo8 { [CmdletBinding()] param ( [Parameter(Mandatory = $true, ValueFromPipeline = $true)] [System.Object] $ActualValue, [System.String] $DemoI, [switch] $Negate, [System.String] $Because ) [PSCustomObject] @{ Succeeded = $true; FailureMessage = $null } }
function Should-BeDemo9 { [CmdletBinding()] param ( [Parameter(Mandatory = $true, ValueFromPipeline = $true)] [System.Object] $ActualValue, [System.String] $DemoJ, [switch] $Negate, [System.String] $Because ) [PSCustomObject] @{ Succeeded = $true; FailureMessage = $null } }

# Use an array list to store the parameter sets of Should after every assertion registration
$p = [System.Collections.ArrayList]::new(10)

for ($i = 0; $i -lt 10; $i++)
{
    Add-AssertionOperator -Name (New-Guid).Guid -Test (Get-Item -LiteralPath "Microsoft.PowerShell.Core\Function::Should-BeDemo$i").ScriptBlock
    $p.Add((Get-Command Should | % ParameterSets))
    1 | Should -Be 1
}

# $p[8] | clip
# $p[9] | clip

5. Possible Solution

None

6. Context

We are writing a module with custom assertions to optimize the internals operational validation tests.

nohwnd commented 5 years ago

Can replicate this, but I don't yet know what the problem is. I thought there might be an issue with the 1..10 where 1 and the first number of 10 are the same and make this ambiguous for the parameter parser (just a wild guess), but it is reproducible also with using guids exactly after 10th try. Adding many operators with different scriptblock also seems to work, so this seems to be limited to registering the same scriptblock with many names. Not sure right now, need to have a look at this later. But maybe it gives you some starting point :)

claudiospizzi commented 5 years ago

I've some internal code with multiple different assertions, non of them are numbered from 1..10 nor have the same script block - updated the initial demo on this issue with guid-named assertions and different script blocks. Same issue.

Also reordering the assertions, removing parameters, etc. - nothing seems to help. Every time I add the 10th assertion, things go wrong in the parameter sets.

Checking the ParameterSets with a diff tool after every assertion registration with the command Get-Command Should | % ParameterSets, everything is okay until the 10th registration. There I can see a second parameter set named Be added to the parameter sets. I thought, the parameter set names should be unique.

claudiospizzi commented 5 years ago

Okay, I did a little bit more troubleshooting around this issue. I've tried to reproduce the issue only using PowerShell but no Pester.

The following things I've found out:

If you don't use the PowerShell advanced function, all is fine. Hunderts of parameter sets can be added. After adding [CmldetBinding()] or [Parameter()], the binding issue starts happening
If an advanced demo function is used (see below), the maximum I've hit is 32 parameter sets, afterwards the issue begins again: Using the first parameter -Demo0, PowerShell prompts for the 33th parameter -Demo32
This matches with the Pester behavior. My 10th custom assertion parameter is the 33th dynamic parameter (Checked with $script:AssertionDynamicParams.Values.Attributes.ParameterSetName | select -unique | measure)

# Up to 32 entries, it works. Starting with 33 it will not work anymore
$count = 32

function Test-DynamicParam
{
    # Remove the cmdlet binding, so not using an advanced funtion, all is fine
    [CmdletBinding()]
    param ()

    dynamicparam
    {
        $parameterDictionary = New-Object System.Management.Automation.RuntimeDefinedParameterDictionary
        for ($i = 0; $i -lt $count; $i++) {
            $name = "Demo$i"
            $attribute = New-Object Management.Automation.ParameterAttribute
            $attribute.ParameterSetName = $name
            $attribute.Position = $i
            $attribute.Mandatory = $true
            $attributeCollection = New-Object System.Collections.ObjectModel.Collection[System.Attribute]
            $attributeCollection.Add($attribute)
            $parameter = New-Object System.Management.Automation.RuntimeDefinedParameter($name, [switch], $attributeCollection)
            $parameterDictionary.Add($name, $parameter)
        }
        return $parameterDictionary
    }
}

for ($i = 0; $i -lt $count; $i++) {
    $parameterSplat = @{
        "Demo$i" = $true
    }
    Write-Host "Test-DynamicParam -Demo$i"
    Test-DynamicParam @parameterSplat
}

claudiospizzi commented 5 years ago

Okay, I guess I've found it here: https://github.com/PowerShell/PowerShell/blob/3943f18c28e9df8369ef17776467c732648747b8/src/System.Management.Automation/engine/runtime/Binding/Binders.cs#L1491

This is a cache object to hold switch parameter bindings with the size of 32 elements.

claudiospizzi commented 5 years ago

Submitted an issue in the PowerShell repo: https://github.com/PowerShell/PowerShell/issues/10447

nohwnd commented 5 years ago

Awesome work :)

claudiospizzi commented 5 years ago

Thx. The problem is, this won't get fixed soon, it's just up for documentation that this is a limit: https://github.com/PowerShell/PowerShell/issues/9372

So we will have this limit and with like 22 built-in Pester parameter sets, only 10 custom user assertions are possible.

nohwnd commented 3 years ago

Yeah not sure how to fix this, but I am thinking about finally adopting Assert module into Pester to have equivalency and per typeclass assertions (e.g. focused specifically on string, or numbers), so I might need to solve this first.

pester / Pester