Pester icon indicating copy to clipboard operation
Pester copied to clipboard

Should BeOfType incorrectly asserts type and inconsistently processes type information

Open dfch opened this issue 9 years ago • 13 comments

Issue #327 added a new option Should BeOfType to assert the type of objects under test. However it seems that the assertion logic is not consistent and does not work with all types as expected.

Observation

  • Testing for an (empty) list does not correctly assert the type
    $arrayList = New-Object System.Collections.ArrayList;
    $result | Should BeOfType [System.Collections.ArrayList];
  • When asserting lists/arrays the behavior is different depending on the type of array elements. If all elements are of the same type then the type of the elements will be asserted (instead of the type of the list/array).
    Context "Test-MockBeOfTypeMixedArrayList" {
        It "TypeArrayList-ShouldBeOfTypeArrayListButIncorrectlyAssertsTypeString" -Test {

            # Arrange
            $message = "meaningless-string";
            $arrayList = New-Object System.Collections.ArrayList;
            $null = $arrayList.Add($message);
            $null = $arrayList.Add($message);

            # Act
            $result = $arrayList;

            # Assert
            # will pass but should fail
            # this will only succeed if all items are of the same type
            # still this is WRONG
            $result | Should BeOfType String;
        }
  • Whereas $result -is [SomeType] | Should Be $true always works as expected.

  • Should Not BeOfType cannot handle the type String unless the type is defined as String or ([String]) ([String] does NOT work)

    This will throw the following error: Expected: meaningless-string to be of any type except [[String]], but unable to find type [[String]]. Make sure that the assembly that contains that type is loaded.

        It "TypeArrayList-ShouldNotBeOfTypeStringWithBrackets" -Test {

        # Arrange
        $message = "meaningless-string";
        $arrayList = New-Object System.Collections.ArrayList;
        $null = $arrayList.Add($message);
        $null = $arrayList.Add($message);
            # Act
            $result = $arrayList;

            # Assert
            # WRONG: this should succeed but fails as the type in the Should Not BeOfType
            # clause is incorrectly determined
            $result | Should Not BeOfType [String];
        }

Example Code

See MockBeOfType.Tests.ps1 for the complete example.

Test Results

Describing Test-MockTimesExactly.ps1 (regarding issue #327)
   Context Test-MockBeOfTypeString
    [+] TypeString-ShouldAssertCorrectType 92ms
    [+] TypeString-ShouldAssertCorrectTypeWithBrackets 34ms
    [+] TypeString-ShouldAssertCorrectTypeWithExpressionAndBrackets 31ms
   Context Test-MockBeOfTypeInt
    [+] TypeInt-ShouldAssertIncorrectType 82ms
    [+] TypeInt-ShouldAssertIncorrectTypeWithBrackets 33ms
    [+] TypeInt-ShouldAssertIncorrectTypeWithExpressionAndBrackets 33ms
   Context Test-MockBeOfTypeMixedArrayList
    [+] TypeArrayList-ShouldBeOfTypeArrayListButIncorrectlyAssertsTypeString 80ms
    [+] TypeArrayList-ShouldBeOfTypeArrayListButIncorrectlyAssertsTypeInt 36ms
   Context Test-MockBeOfTypeArrayList
    [+] TypeArrayList-IsArrayListShouldBeTrue 76ms
    [+] TypeArrayList-CountShouldBeTwo 31ms
    [-] TypeArrayList-ShouldBeOfTypeArrayList 32ms
      Expected: meaningless-string to be of type [System.Collections.ArrayList]
      at line: 133 in C:\Github\biz.dfch.PS.Pester.Testing-5b1b64afd0be13f4c4e5\MockBeOfType.Tests.ps1
      133:                      $result | Should BeOfType System.Collections.ArrayList;
    [+] TypeArrayList-ListItemShouldBeOfTypeString 42ms
    [-] TypeArrayList-ShouldNotBeOfTypeString 35ms
      Expected: {meaningless-string} to be of any type except [string], but it's a [string]
      at line: 151 in C:\Github\biz.dfch.PS.Pester.Testing-5b1b64afd0be13f4c4e5\MockBeOfType.Tests.ps1
      151:                      $result | Should Not BeOfType String;
    [-] TypeArrayList-ShouldNotBeOfTypeStringWithBrackets 44ms
      Expected: meaningless-string to be of any type except [[String]], but unable to find type [[String]]. Make sure that the assembly that contains that type
is loaded.
      at line: 161 in C:\Github\biz.dfch.PS.Pester.Testing-5b1b64afd0be13f4c4e5\MockBeOfType.Tests.ps1
      161:                      $result | Should Not BeOfType [String];
    [-] TypeArrayList-ShouldNotBeOfTypeStringWithExpressionAndBrackets 47ms
      Expected: {meaningless-string} to be of any type except [string], but it's a [string]
      at line: 171 in C:\Github\biz.dfch.PS.Pester.Testing-5b1b64afd0be13f4c4e5\MockBeOfType.Tests.ps1
      171:                      $result | Should Not BeOfType ([String]);
   Context Test-MockBeOfTypeArray
    [+] TypeArray-IsarrayShouldBeTrue 84ms
    [+] TypeArray-CountShouldBeTwo 31ms
    [-] TypeArray-ShouldBeOfTypeArray 35ms
      Expected: meaningless-string to be of type [array]
      at line: 203 in C:\Github\biz.dfch.PS.Pester.Testing-5b1b64afd0be13f4c4e5\MockBeOfType.Tests.ps1
      203:                      $result | Should BeOfType Array;
    [+] TypeArray-ListItemShouldBeOfTypeString 43ms
    [-] TypeArray-ShouldNotBeOfTypeString 33ms
      Expected: {meaningless-string} to be of any type except [string], but it's a [string]
      at line: 221 in C:\Github\biz.dfch.PS.Pester.Testing-5b1b64afd0be13f4c4e5\MockBeOfType.Tests.ps1
      221:                      $result | Should Not BeOfType String;
    [-] TypeArray-ShouldNotBeOfTypeStringWithBrackets 47ms
      Expected: meaningless-string to be of any type except [[String]], but unable to find type [[String]]. Make sure that the assembly that contains that type
is loaded.
      at line: 231 in C:\Github\biz.dfch.PS.Pester.Testing-5b1b64afd0be13f4c4e5\MockBeOfType.Tests.ps1
      231:                      $result | Should Not BeOfType [String];
    [-] TypeArray-ShouldNotBeOfTypeStringWithExpressionAndBrackets 46ms
      Expected: {meaningless-string} to be of any type except [string], but it's a [string]
      at line: 241 in C:\Github\biz.dfch.PS.Pester.Testing-5b1b64afd0be13f4c4e5\MockBeOfType.Tests.ps1
      241:                      $result | Should Not BeOfType ([String]);
Tests completed in 1.06s
Passed: 14 Failed: 8 Skipped: 0 Pending: 0

Conclusion

  • I think the behaviour of BeOfType should be the same as the use of the -is operator
  • the inconsistent treatment of brackets in the Not version of BeOfType is not intuitive and should be the same as with the -isnot operator.

Thanks for having a look at it. Regards, Ronald

dfch avatar Jul 13 '15 05:07 dfch

The behavior you've observed with collections has to do with the fact that we're using the pipeline to send input to the Should command (which causes me no end of headaches, but that's the way it is.) When you pipe input to a command, PowerShell "unrolls" the collection and pipes in one element at a time. When you want the entire collection to be piped in at once, you need to use PowerShell's unary comma operator to wrap the collection in a one-element array, like so:

$arrayList = New-Object System.Collections.ArrayList;
,$arrayList | Should BeOfType System.Collections.ArrayList

The second thing you observed has to do with PowerShell's parsing behavior. When you run this:

$whatever | Should BeOfType [String]

The token [String] is treated as text, not as an expression. The square bracket is not one of the characters that forces you into expression parsing mode. (See Get-Help about_Parsing for more information on that.) The -is operator, on the other hand, doesn't put PowerShell into argument parsing mode the way a command does, so [string] is always treated as an expression there, rather than as text.

That said, we can and should probably tweak that operator to detect if the input is a string, and if it is enclosed in square brackets. If so, we can just remove them and avoid the error.

dlwyatt avatar Jul 13 '15 05:07 dlwyatt

Note: In PowerShell 3.0 and later, rather than the unary comma operator, you can also use the -NoEnumerate parameter of Write-Output to achieve the same effect. However, if you want PowerShell v2 compatibility, you're stuck with the comma.

$arrayList = New-Object System.Collections.ArrayList;
Write-Output -NoEnumerate $arrayList | Should BeOfType System.Collections.ArrayList

dlwyatt avatar Jul 13 '15 05:07 dlwyatt

ok, I got this. Stil I personally do not find this very intuitive.

My thoughts on this:

  • If there is a special BeOfType operator than this should just work (and not need special precautions).
  • If this is not possible then the use of $data -is [SomeType] | Should Be $true is more intuitive/consistent.
  • The case with Should Not BeOfType ([String]) is even more counter-intuitive (as without Not I do not need to specify the () brackets but MAY use [] brackets)

I would rather prefer a solid feature instead of some partial/special implementation.

No offense, just thinking out loud. Or maybe I am missing something here? Ronald

dfch avatar Jul 13 '15 07:07 dfch

And of course, if we can change BeOfType to something consistent (with or without brackets) and with a clear error message when used in the wrong way), I would really appreciate and support that.

dfch avatar Jul 13 '15 07:07 dfch

In principle, I don't disagree with you, but Pester is written in PowerShell, and sometimes we just have to accept the way that PowerShell works. If we fundamentally change the syntax for Should in a way that isn't backward-compatible, then every Pester test script out there needs to be updated. We work pretty hard to avoid that sort of thing.

dlwyatt avatar Jul 13 '15 16:07 dlwyatt

Also, looks like Should BeOfType (with or without Not) already handles stripping off the square brackets, if needed. The only problems you were running into had to do with the behavior of the pipeline and collections.

dlwyatt avatar Jul 13 '15 17:07 dlwyatt

Thanks, that's ok for me. I will then just stick to the "safer" $result -is [SomeType] | Should Be $true method of asserting.

dfch avatar Jul 14 '15 23:07 dfch

The same issue exists for a DataTable:

$DataTable = New-Object Data.DataTable
$DataColumn = New-Object Data.DataColumn
$DataColumn.ColumnName = "Name"
$DataTable.Columns.Add($DataColumn)
$DataRow = $DataTable.NewRow()
$DataRow.Item("Name") = "Test"
$DataTable.Rows.Add($DataRow)

$DataTable | Should -BeOfType [System.Data.DataTable]

Results in:

Expected: {System.Data.DataRow} to be of type [System.Data.DataTable]
At C:\Program Files\WindowsPowerShell\Modules\Pester\4.1.1\Functions\Assertions\Should.ps1:209 char:9
+         throw ( New-ShouldErrorRecord -Message $testResult.FailureMes ...
+         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    + CategoryInfo          : InvalidResult: (System.Collections.Hashtable:Hashtable) [], Exception
    + FullyQualifiedErrorId : PesterAssertionFailed

Work around (as above): ,$DataTable | Should -BeOfType [System.Data.DataTable]

I guess there would be more situations where Should should cover the whole $Input object rather then each $Value. Maybe a -NoEnumerate switch for Should itself could clear this.

iRon7 avatar Jan 03 '18 16:01 iRon7

The issue exists for every type that enumerates when piped through the pipeline. The proposed switch is a nice idea, but the problem is how would you implement it? Should is invoked after the collection is enumerated, and even if it wasn't (say if we implemented begin {} in should), is there a way to get the type of the collection that $DataTable is? cc @alx9r

nohwnd avatar Feb 08 '18 10:02 nohwnd

:S You're right, this idea was too short-sighted

iRon7 avatar Feb 08 '18 17:02 iRon7

is there a way to get the type of the collection that $DataTable is?

@nohwnd Per PowerShell/PowerShell#5578 (comment) I'm fairly confident there is no supported way do that.

alx9r avatar Feb 08 '18 18:02 alx9r

@whiggs You definitely can write your tests in a way that works correctly. All you need to do is give up the nice pipeline syntax and use the parameters of the Should function.

# works because the collection is not enumerated by any pipeline
Should -HaveType ([object[]]) -ActualValue @(1,2,3)
# throws: expected [System.Object[]], but got [int]
@(1,2,3) | Should -HaveType ([object[]])

Alternatively you can apply countermeasures by wrapping collections into extra collections, for example by a function like this:

# This is not extensively tested, use at your own risk :)
function AsIs ($InputObject) {
    if ([object]::ReferenceEquals($InputObject, $($InputObject)))
    {
        $InputObject
    }
    else
    {
        ,$InputObject
    }
}

Other than that we cannot do anything about it, that is how the language works, and there is no known way of knowing which type of collection is before the pipeline.

nohwnd avatar Nov 20 '18 23:11 nohwnd

There is nothing to solve here, other than documenting the behavior and being consistent (which will happen as part of the add-assert ps). Pipelines will enumerate and we cannot do anything about it.

nohwnd avatar May 18 '24 12:05 nohwnd