ExpectedMessage comparison fails

[indented-automation]

General summary of the issue

Error message comparison fails.

Describe your environment

Pester version     : 5.1.0-beta1 C:\Users\Chris\Documents\PowerShell\Modules\Pester\5.1.0\Pester.psm1
PowerShell version : 7.1.0
OS version         : Microsoft Windows NT 10.0.18363.0

Steps to reproduce

{ @(1, 2) - 1 } | Should -Throw -ExpectedMessage "Method invocation failed because [System.Object[]] does not contain a method named 'op_Subtraction'."

Expected Behavior

The assertion should be successful.

Using try / catch to perform the same comparison is successful:

try {
    @(1, 2) - 1
} catch {
    $_.Exception.Message | Should -Be "Method invocation failed because [System.Object[]] does not contain a method named 'op_Subtraction'."
}

Current Behavior

The assertion fails.

[nohwnd]

Maybe because the comparison is using -like and [] is some kind of wildcard?

[indented-automation]

Yep that's the one.

{ @(1, 2) - 1 } | Should -Throw -ExpectedMessage "Method invocation failed because ``[System.Object``[``]``] does not contain a method named 'op_Subtraction'."

Hmm.

[aolszowka]

FWIW this is a breaking change between Pester v4 and Pester v5. Might be worth calling out more specifically than just the blurb of:

Should -Throw is using -like to match the exception message instead of .Contains.
Use * or any of the other -like wildcard to match only part of the message.

That already exists on the breaking changes page here: https://pester-docs.netlify.app/docs/migrations/breaking-changes-in-v5

This is the root cause of this person's issue on StackOverflow: https://stackoverflow.com/questions/65219744/pester-v5-1-should-throw-with-message-containing-square-brackets-fails

Pester v4

Import-Module Pester -MaximumVersion '4.10.1'

function Test-Throw {
    throw "I should throw a [message]"
}

Describe 'PesterThrowBug' {
    It 'Should Throw a Message' {
        { Test-Throw } | Should -Throw -ExpectedMessage 'I should throw a [message]'
    }
}

Output:

  [+] Should Throw a Message 2.14s

Pester v5

Import-Module Pester -MinimumVersion '5.0.0'

function Test-Throw {
    throw "I should throw a [message]"
}

Describe 'PesterThrowBug' {
    It 'Should Throw a Message' {
        { Test-Throw } | Should -Throw -ExpectedMessage 'I should throw a [message]'
    }
}

Output:

Starting discovery in 1 files.
Discovery finished in 8ms.
Running tests.
[-] PesterThrowBug.Should Throw a Message 10ms (9ms|1ms)
 Expected an exception, with message 'I should throw a [message]' to be thrown, but the message was 'I should throw a [message]'. from S:\Git\SimpleExamples\Pester\PesterThrowBug.ps1:5 char:5
     +     throw "I should throw a [message]"
     +     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 at { Test-Throw } | Should -Throw -ExpectedMessage 'I should throw a [message]', S:\Git\SimpleExamples\Pester\PesterThrowBug.ps1:10
 at <ScriptBlock>, S:\Git\SimpleExamples\Pester\PesterThrowBug.ps1:10
Tests completed in 98ms
Tests Passed: 0, Failed: 1, Skipped: 0 NotRun: 0

The error message is particularly unhelpful because the expected and but message was is identical.

Work Around

As mentioned above escaping the [] with the backtick works, but this seems weird especially if you're not using string interpolation:

Import-Module Pester -MinimumVersion '5.0.0'

function Test-Throw {
    throw "I should throw a [message]"
}

Describe 'PesterThrowBug' {
    It 'Should Throw a Message' {
        { Test-Throw } | Should -Throw -ExpectedMessage 'I should throw a `[message`]'
    }
}

Output:

Starting discovery in 1 files.
Discovery finished in 10ms.
Running tests.
[+] S:\Git\SimpleExamples\Pester\PesterThrowBug.ps1 103ms (7ms|88ms)
Tests completed in 107ms
Tests Passed: 1, Failed: 0, Skipped: 0 NotRun: 0

[aolszowka]

Here's a search I wrote up to identify affected locations that might be useful to someone else looking to perform a lift:

function Get-ExpectedMessageUsage {
    [CmdletBinding()]
    param (
        [Parameter(ValueFromPipeline)]
        $Path
    )
    process {
        $scriptContent = Get-Content -Path $Path -Raw
        $scriptAst = [ScriptBlock]::Create($scriptContent).Ast
        $possibleAffectedLocations = $scriptAst.FindAll({ $args[0] -is [System.Management.Automation.Language.CommandAst] }, $true) |
            Where-Object { $_.GetCommandName() -eq 'Should' } |
            Where-Object { $null -ne ($_ | Select-Object -ExpandProperty CommandElements | Select-Object -ExpandProperty ParameterName -ErrorAction SilentlyContinue | Where-Object { $_ -eq 'ExpectedMessage' }) }
        # It is only an affected location if it contains a Wild Card Character
        # as defined here: https://docs.microsoft.com/en-us/powershell/module/microsoft.powershell.core/about/about_wildcards?view=powershell-7.1
        # that is not escaped.
        $affectedLocations = $possibleAffectedLocations |
            Select-Object -ExpandProperty CommandElements |
            Where-Object { ($_.StringConstantType -eq 'DoubleQuoted' -or $_.StringConstantType -eq 'SingleQuoted') -and ([Regex]::Match($($_.Value), '[^`][\[\]\*\?]')) }
        [PSCustomObject]@{
            Path = $Path
            AffectedCount = ($affectedLocations | Measure-Object).Count
        }
    }
}

[nohwnd]

Reopening this because there are multiple comments, and I don't think I can move them all into a new comment.

There are few options we have:

• In 5.4 I would like to add diagnostic logging to Should and it's assertions, we could check the message for [ ] and some other problematic -like wildcards if there are any (e.g. ? is imho also a wild card, but because hello? will match hello. people are unlikely to see it a error.), and log that into the diag log with the suggested solution. This is how we do it in the Mock, and it gives a lot of flexibility, because we can be very verbose in that log, and we also have the luxury of doing more expensive checks (e.g. checking if the string contains [.

• Add -ExpectedMessageLike alias to -ExpectedMessage, and also -ExpectedMessageEq parameter that would do the -eq check. Or more in line with -Be, we would add -ExpectedMessageExactly instead of -ExpectedMessageEq. But this has discoverability problem I think.

[aolszowka]

Reopening this because there are multiple comments, and I don't think I can move them all into a new comment.

My bad :( I waffled back and forth between opening a new issue or not.

I would like to add diagnostic logging to Should and it's assertions

Sounds great to me.

? is imho also a wild card, but because hello? will match hello.

I completely agree with this; even more so for * because it will match both 2*2 and 2*2*2 the same way. The AST search above checks for all Wildcard characters to cover this.

Add -ExpectedMessageLike alias to -ExpectedMessage, and also -ExpectedMessageEq parameter that would do the -eq check. Or more in line with -Be, we would add -ExpectedMessageExactly instead of -ExpectedMessageEq. But this has discoverability problem I think.

I would vote for this personally, while more verbose also avoids the footgun.

[fflaten]

• In 5.4 I would like to add diagnostic logging to Should and it's assertions, we could check the message for [ ] and some other problematic -like wildcards if there are any (e.g. ? is imho also a wild card, but because hello? will match hello.

Correct, []*?. There's also a helper-method to detect it [System.Management.Automation.WildcardPattern]::ContainsWildcardCharacters('your?string') (need full type-name for PSv3)

Add -ExpectedMessageLike alias to -ExpectedMessage,..

~If this road is chosen we'll have to add aliases to operator-parameters. For some reason we check that parameter-aliases don't conflict with other operators, but never add the parameter alias so kinda wasted. 🤷‍♂️ (Would have to extend that test for parameter-aliases)~

One disadvantage with parameter-alias in our case is that they're not linked to parameter-sets, so they would also apply for other operators using the same ExpectedMessage parameter-name. Only -Throw using it in Pester, but possibly custom operators.

I'd personally vote for the wildcardpattern-check + diag-log mentioning it and how to escape []*? with backtick for now and revisit ExpectedMessageEq etc with next-gen assertions.

Update: Or add a -UseExactMessage/NoWildcard/SimpleMatch switch to use -eq.