pester / Pester

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

Arrays are silently truncated to 10 elements when printed by Should -Be #1668

Closed michaelrgibbs closed 3 years ago

michaelrgibbs commented 4 years ago

1. General summary of the issue

When comparing two arrays with "Should -Be", in the output the arrays are truncated to the first 10 elements, but it isn't obvious this is happening.

2. Describe Your Environment

Pester version : 5.0.3 PowerShell version : 5.1.18362.752 OS version : Microsoft Windows NT 10.0.18362.0

3. Expected Behavior

At minimum, show that there is truncation happening.

e.g.

@(1,2,3,4,5,6,7,8,9,10,11) | Should -Be @(1,2,3,4,5,6,7,8,9,10,12)
Expected @(1, 2, 3, 4, 5, 6, 7, 8, 9, 10, ...), but got @(1, 2, 3, 4, 5, 6, 7, 8, 9, 10, ...).

Some way of showing at which element they differ would also be nice, like with strings

 Expected strings to be the same, but they were different.
 String lengths are both 45.
 Strings differ at index 19.
 Expected: '...aaaaabaaaa...'
 But was:  '...aaaaaaaaaa...'
 at "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" | Should -Be "aaaaaaaaaaaaaaaaaaabaaaaaaaaaaaaaaaaaaaaaaaaa"

4. Current Behavior

@(1,2,3,4,5,6,7,8,9,10,11) | Should -Be @(1,2,3,4,5,6,7,8,9,10,12)
Expected @(1, 2, 3, 4, 5, 6, 7, 8, 9, 10), but got @(1, 2, 3, 4, 5, 6, 7, 8, 9, 10).
asears commented 4 years ago

This caught me a couple of times too. I ended up comparing by string length because the result were not showing properly and couldn't figure out why.

The problems are with hard-coded values in Get-CompareStringMessage and Format-AsExcerpt in Be.ps1

 $ellipsis = "..."
        $excerptSize = 5;
        "Expected: '{0}'" -f (  Expand-SpecialCharacters -InputObject (Format-AsExcerpt -InputObject $ExpectedValue -startIndex $differenceIndex -excerptSize $excerptSize  -excerptMarker $ellipsis) )
        "But was:  '{0}'" -f ( Expand-SpecialCharacters -InputObject (Format-AsExcerpt -InputObject $actual -startIndex $differenceIndex -excerptSize $excerptSize -excerptMarker $ellipsis ) )
 $InputObjectDisplay = [string]::Empty
    $displayDifferenceIndex = $startIndex - $excerptSize
    $maximumStringLength = 40
    $maximumSubstringLength = $excerptSize * 2
    $substringLength = $InputObject.Length - $displayDifferenceIndex

The problem could be fixed by a warning message on maximum string length being hit and optional parameters for the length. It probably affects all comparison functions.

nohwnd commented 4 years ago

This would be problem in Format-Nicely, elipsis is already implemented there, but commented out.

https://github.com/pester/Pester/blob/47c3c138d2bf1d87d5283365a8975a8f31b6b13c/src/Format.psm1#L5-L14

Maybe we should make it more obvious what it means:

function Format-Collection ($Value, [switch]$Pretty) {
    $Limit = 10
    $separator = ', '
    if ($Pretty) {
        $separator = ",`n"
    }
    $count = $Value.Count
    $trimmed = $count  -gt $Limit
    '@(' + (($Value | Select -First $Limit | % {  $_  }) -join $separator ) + $(if ($trimmed) {", +$($count-$limit) more"}) + ')'
}

format-collection (1..1100)
nohwnd commented 4 years ago

Or maybe the ... ellipsis is enough.

asears commented 4 years ago

The message may not be silent, but it sure is quiet!

I would suggest that the expected and actual message be shown in full, or truncated to the amount specified by the user, at least as an option. Personally, I use the result messages to "fix" some of my initial test expected values, with a write failing test that eventually succeeds pattern.

The ellipsis is a hint yet not a constructive means to suggest that it is truncated, or how to fix the issue.

Is there a document on getting local dev environment & tests setup with Pester, similar to what is executing in the build pipelines of Devops?

Was trying to setup a few local tests for this. Getting this error. Tried looking at the Devops pipeline steps but have no access.

System.Management.Automation.CommandNotFoundException: The term 'InPesterModuleScope' is not recognized as the name of a cmdlet, function, script file, or operable program. Check the spelling of the name, or if a path was included, verify that the path is correct and try again.
   at System.Management.Automation.ExceptionHandlingOps.CheckActionPreference(FunctionContext funcContext, Exception exception)
   at System.Management.Automation.Interpreter.ActionCallInstruction`2.Run(InterpretedFrame frame)
   at System.Management.Automation.Interpreter.EnterTryCatchFinallyInstruction.Run(InterpretedFrame frame)
   at System.Management.Automation.Interpreter.EnterTryCatchFinallyInstruction.Run(InterpretedFrame frame)
   at System.Management.Automation.Interpreter.Interpreter.Run(InterpretedFrame frame)
   at System.Management.Automation.Interpreter.LightLambda.RunVoid1[T0](T0 arg0)
   at System.Management.Automation.PSScriptCmdlet.RunClause(Action`1 clause, Object dollarUnderbar, Object inputToProcess)
   at System.Management.Automation.PSScriptCmdlet.DoEndProcessing()
   at System.Management.Automation.CommandProcessorBase.Complete()
at <ScriptBlock>, D:\projects\powershell\Pester\tst\Pester.Tests.ps1: line 98
at <ScriptBlock>, C:\Program Files\WindowsPowerShell\Modules\Pester\5.0.4\Pester.psm1: line 2719
at Invoke-File, C:\Program Files\WindowsPowerShell\Modules\Pester\5.0.4\Pester.psm1: line 2728
at Invoke-BlockContainer, C:\Program Files\WindowsPowerShell\Modules\Pester\5.0.4\Pester.psm1: line 2657
at Discover-Test, C:\Program Files\WindowsPowerShell\Modules\Pester\5.0.4\Pester.psm1: line 1291
at Invoke-Test, C:\Program Files\WindowsPowerShell\Modules\Pester\5.0.4\Pester.psm1: line 2190
at Invoke-Pester<End>, C:\Program Files\WindowsPowerShell\Modules\Pester\5.0.4\Pester.psm1: line 4196
at <ScriptBlock>, <No file>: line 1
nohwnd commented 4 years ago

https://github.com/pester/Pester/blob/v5.0/azure-pipelines.yml#L121 this is all it does.

You need to run test.ps1 first, it defines the function, and some types. After that you can run the tests normally. Or you can pass the file to test.ps1 and optionally -skipPTests to skip the .ts.ps1 files.

As for the actual functionality, I find the new should -be string behavior with elipsis to be unhelpful, and liked the one with the arrow better. I have a branch reverting it somewhere I think. And I think it would be nice to do the same for collections. With pointing out what is the value and on which index it is.

asears commented 4 years ago

Thanks! Going to open an issue and could help with PR for documenting the test parameters and other contributor documentation for setup & tests.

Something funky going on with my setup. Both these tests succeed when added to Be.Tests.ps1. I can break other tests.

        It "Does not silently truncate arrays"
        {
            $a1 = @(1,2,3,4,5,6,7,8,9,10,11,12)
            $a2 = @(1,2,3,4,5,6,7,8,9)

            { $a1 | Should -be $a2 }

        }
        It "Repros the error"
        {
            @(1,2,3,4,5,6,7,8,9,10,11) | Should -Be @(1,2,3,4,5,6,7,8,9,10,12)

        }
nohwnd commented 4 years ago

The scriptblocks are on the next line, not on the same line. So I guess the It throws because it is missing the Test parameter, and fails the whole describe?

nohwnd commented 4 years ago

The .ts.ps1 tests are brittle in some ways. I should have rather write some custom assertions to find the test in the object, and check it's result, instead of indexing into array. This way when you fail unexpectedly, e.g. killing the whole container or block, it won't find the test results and report expected Passed but got $null.

Instead an assertion should check if the whole run passed, and then find the given test and assert on it.

nohwnd commented 4 years ago

I am adding the ellipsis to get this fix into next 5.0.x release, while we figure out a better way to present this.

fflaten commented 3 years ago

Not sure if this is considered closed by #1961. Latest 5.3.0 alpha shows the number of truncated items, ex. image

Ideally it would focus on where the difference is like string, but that might be a different/new issue.