Should -Be gives confusing error message comparing arrays that are not enumerated.

[vexx32]

1. General summary of the issue

In writing tests to ensure the accurate fixing of the issue resolved by this PR, I discovered that Should can and will differentiate between a complete array passed over the pipeline and an enumerated array, and it seems to expect the latter.

Simple repro:

$a = 1, 2, 3
,$a |Should -Be $a

2. Describe Your Environment

Operating System, Pester version, and PowerShell version:

Pester version     : 4.3.1 C:\Program Files\PowerShell\Modules\Pester\4.3.1\Pester.psd1
PowerShell version : 6.1.0
OS version         : Microsoft Windows NT 10.0.17763.0

3. Expected Behavior

Should should more clearly indicate what is happening, with either symbolic representation of the different element that more closely reflects what's happened on the command line, or by explicitly calling out the fact that the two arrays are different in some other way.

Bonus: perhaps an optional switch or operator might be a good idea that doesn't check for this, however this is getting picked up.

4.Current Behavior

Clearly, it is able to distinguish between what is a multidimensional array that has been enumerated, and a flat array. However, its error message is the polar opposite of clear. 😄

Simple example:

PS> $a = 1, 2, 3
PS> ,$a |Should -Be $a
Expected @(1, 2, 3), but got @(1, 2, 3).
At C:\Program Files\PowerShell\Modules\Pester\4.3.1\Functions\Assertions\Should.ps1:206 char:9
+         throw ( New-ShouldErrorRecord -Message $testResult.FailureMes ...
+         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ CategoryInfo          : InvalidResult: (System.Collections.Hashtable:Hashtable) [], Exception
+ FullyQualifiedErrorId : PesterAssertionFailed

6. Context

https://github.com/PowerShell/PowerShell/pull/8407

It was noted that Get-Variable passes collections whole over the pipeline, thus similar to the original example, this too throws a strangely confusing error. This particular case is fixed in the linked PR, however, making the command properly enumerate collections when outputting.

$v = 1, 2, 3
Get-Variable -Name v -ValueOnly | Should -Be $v

[nohwnd]

Agreed, this needs to get better. At the very least the error should show that there is a wrapping array as:

Expected @(1, 2, 3), but got @(@(1, 2, 3)).

Or better:

Expected a collection @(1, 2, 3) with length 3, but got a collection @(@(1, 2, 3)) with length 1.

Problem is finding the right balance where this kind output stays readable even for bigger objects. Probably the best approach here is to list the info that stays short on the top and progress to expanding the objects on the bottom.

Expected a collection with length 3, but got a collection with length 1.
Expected:
Type: [Object[]]
Length:  3
Content: 
@(1, 2, 3)

Actual: 
Type: [Object[]]
Length:  1
Content: 
@(
  @(1, 2, 3)
)

I will start with the simplest fix, and solve it better when Assert merges into Pester, because the formatting there is solved better.

[nohwnd]

This problem should lie in the Format.psm1 module where the formatting is defined.

[fflaten]

Format-Collection works as expected, returning @(@(1,2,3)). However these lines are unrolling the outer array.

function ShouldBeFailureMessage($ActualValue, $ExpectedValue, $Because) {
    # This looks odd; it's to unroll single-element arrays so the "-is [string]" expression works properly.
    $ActualValue = $($ActualValue)
    $ExpectedValue = $($ExpectedValue)

Demo:

    It "ISSUE: single element nested array" {
        $a = 1, 2, 3
        ,$a | Should -Be $a
    }

    It "nested arrays" {
        $a = (1, 2, 3),1
        $a | Should -Be (1, 2, 3)
    }

    [-] ISSUE: single element nested array 83ms
    Expected @(1, 2, 3), but got @(1, 2, 3).
    59:         ,$a |Should -Be $a
    at <ScriptBlock>, C:\Users\frode\Git\Pester\debug.tests.ps1: line 59
    [-] nested arrays 35ms
    Expected @(1, 2, 3), but got @(@(1, 2, 3), 1).
    64:         $a |Should -Be (1, 2, 3)
    at <ScriptBlock>, C:\Users\frode\Git\Pester\debug.tests.ps1: line 64

Something like this might work to fix both this issue and the string-specific error message that the lines were there to fix. Looks kinda ugly though...

if(@($ActualValue).Count -eq 1 -and @($ActualValue)[0] -is [string]) { $ActualValue = $($ActualValue) }
if(@($ExpectedValue).Count -eq 1 -and @($ExpectedValue)[0] -is [string]) { $ExpectedValue = $($ExpectedValue) }

During testing I also noticed that this test is passing, bug in ShouldBe/ShouldBeExactly?

It "single element integer" {
        $a = 1
        ,$a | Should -Be $a
}

[nohwnd]

The problem here is that when you pass an array through the pipeline it enumerates it, but when you wrap the array into an extra array (by using ,) then the pipeline will eat the wrapper array and pass the original array through the pipeline as single item. Inside Should we then check if we got a collection of we got a single item. Non-enumerated array is a single item, so it is not equal to the original array because they have different size.

If we have an array after the pipeline we know that it must have been wrapped (otherwise the pipeline would enumerate it) and so we want to add extra array to the incoming value when it is array and print it via the format collection.

bug in ShouldBe/ShouldBeExactly

The pipeline unrolls the array created by , so single value 1 passes through as single value 1, and 1 is equal to 1. So this works as expected.

1 | foreach { 1 -eq $_ } # -> true 1 = 1
,1 | foreach { 1 -eq $_ } # -> true 1 = 1
,,1 | foreach { 1 -eq $_ } # -> false @(1) != 1