pester / Pester

Pester is the ubiquitous test and mock framework for PowerShell.
https://pester.dev/
Other
3.11k stars 473 forks source link

`Should -Be` doesn't enumerate some collections? #1200

Open vexx32 opened 5 years ago

vexx32 commented 5 years ago

1. General summary of the issue

Easiest shown by demonstration:

$a = @{ One = 1; Two = 2 }
$b = $a.Keys
$b | Should -Be $a.Keys

Note that $a.Keys is of type [System.Collections.Hashtable+KeyCollection] When done with an [ordered] hashtable, the keys property is of type [System.Collections.Specialized.OrderedDictionary+OrderedDictionaryKeyValueCollection] but the same issue is reflected.

$a = [ordered]@{ One = 1; Two = 2 }
$b = $a.Keys
$b | Should -Be $a.Keys
@( 'One', 'Two' ) | Should -Be $a.Keys

2. Describe Your Environment

Pester version     : 4.4.0 \\hct-byn-fs01\FolderRedirections\Joel\My Documents\WindowsPowerShell\Modules\Pester\4.4.0\Pester.psd1
PowerShell version : 5.1.17134.407
OS version         : Microsoft Windows NT 10.0.17134.0

3. Expected Behavior

In general use, these collections behave essentially as arrays with string contents. I'm sure the difference is an implementation detail in .NET / .NET Core.

I would generally expect flat collections like this to be enumerated in a similar manner to true arrays to retrieve their contents, at least in the absence of the -BeExactly switch. (Such a switch should probably attempt to verify the type of the original collection where possible, but that may not be easily accessible due to the pipeline enumeration of collections.)

4.Current Behavior

Assertion fails, displaying the following error:

Expected @('One', 'Two'), but got @('One', 'Two').
At \\hct-byn-fs01\FolderRedirections\Joel\My
Documents\WindowsPowerShell\Modules\Pester\4.4.0\Functions\Assertions\Should.ps1:206 char:9
+         throw ( New-ShouldErrorRecord -Message $testResult.FailureMes ...
+         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    + CategoryInfo          : InvalidResult: (System.Collections.Hashtable:Hashtable) [], Exception
    + FullyQualifiedErrorId : PesterAssertionFailed

Looks very similar to #1154, but stems from a different (but possibly related) cause, I think.

5. Possible Solution

In the absence of -BeExactly or some such similar specific switch, it should enumerate the collection and compare the contents, I would expect, based on its behaviour of similar flat collections.

6. Context

https://github.com/vexx32/PSKoans/issues/124

fflaten commented 5 years ago

The behaviour is expected, but the error message is unfortunate.

$a.Keys is a KeyCollection with System.Object as basetype, not an array. $b however is converted into an array when it is enumerated as part of the pipeline to Should. When the expected value ($a.Keys) is sent to the assertion-function ArraysAreEqual, the param is expected as type [object[]], which turns your KeyCollection into an array with one item: a KeyCollection

@('One','Two') != @([KeyCollection])

Try: $b | Should -Be ($a.Keys.GetEnumerator() | % { $_ })

vexx32 commented 5 years ago

Yeah, a simpler solution is $b | Should -Be $a.Keys.ForEach{$_}

I was hoping that we could attempt to expect the pipeline enumeration behaviour on such objects and account for it like how Should -Be already does with standard arrays. :smile:

But in either case, that error message is very difficult to make sense of! :grin:

nohwnd commented 5 years ago

From slack:

you are almost right. Keys implements IEnumerable $hashtable.keys -ie [Collections.IEnumerable] = true , but Pester does not identify collections by that, instead it uses [Array] which the collection is not $hashtable.keys -is [Array] = false https://github.com/pester/Pester/blob/af720f98a8e19ccdf2ad6229afcebe1ce02b1515/Functions/Assertions/Be.ps1#L296. Because should be does not identify the keys as collection it does not compare them both as collections, so instead of comparing each item ("One" -eq "$hashtable.Keys[0]; "Two" -eq "$hashtable.Keys[0]; etc. ) it compares each item to the whole collection ("One" -eq $hashtable.Keys;) which is never true. Enumerating the collection yourself into an array makes it work, because then $ks is an array: $Hashtable = [ordered]@{One = 1; Two = 2; Three = 3; Four = 4} ; $ks = $Hashtable.Keys | % {$_}; @('One', 'Two', 'Three', 'Four') | Should -Be $ks. Which is also the reason why it works when you put hashtable.keys before the pipeline, because then the keys get enumerated through the pipeline and collected into an array on the other side, and the right hand side already is an array so the comparison works. I won't be able to fix this in v4 but I think identifying the collection like this instead should work much better: https://github.com/pester/Pester/blob/b7904b0aa6c0a1444c904b4b1ec5bdbf9a425545/Dependencies/TypeClass/TypeClass.psm1#L6

This should be tried before releasing v5.

vexx32 commented 5 years ago

I think there's something in the binders or LanguagePrimitives that can conclusively tell you if an object is considered by PowerShell to be typically enumerable (PS itself have some exceptions, e.g., it ignores that strings are IEnumerable)

@seeminglyscience knows more than I there.

SeeminglyScience commented 5 years ago

I think there's something in the binders or LanguagePrimitives that can conclusively

Yep that's it :)

Easiest way to tell is something like this

function IsPSEnumerable($obj) {
    $enumerator = [System.Management.Automation.LanguagePrimitives]::GetEnumerator($obj)
    return $null -ne $enumerator
}

Which calls out to this binder.

vexx32 commented 5 years ago

I think I need to save that for the future. 👍

nohwnd commented 5 years ago

@SeeminglyScience very useful, as always, thanks. :)

nohwnd commented 4 years ago

Won't change the behavior in v5, but might add a whole new collection of assertions later side-by-side, where this would be tackled. Moving to Better Should milestone