Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow combining assertion failures into one #19

Open
nohwnd opened this issue Aug 18, 2017 · 5 comments
Open

Allow combining assertion failures into one #19

nohwnd opened this issue Aug 18, 2017 · 5 comments
Labels

Comments

@nohwnd
Copy link
Owner

nohwnd commented Aug 18, 2017

The first assertion to fail will fail the whole test. This leaves you with partial infomation, about why your test failed. Have a test containing these assertions:

$actual = Get-WebRequest
$actual.StatusCode | Assert-Equal 200
$actual.StatusDescription | Assert-Equal 'OK'
$actual.Content | Assert-Equal 'Hello'

Your test initally fails with "StatusCode is '400' but expected '200'.". You fix the call in Get-WebRequest and try again. Your test now fails with "Content is "abc" but expected 'Hello'".

Multiply this by tens of tests that change failure messages as you progress fixing your code, and you have to work very hard to understand what you need to do to fix your code.

Compare that with this failure message:

StatusCode is '400' but expected '200'.
StatusDescription is 'BadRequest', but I expected 'OK'.
Content is 'Id is empty.', but I expected 'Hello'.

This message contains all the failure messages combined, and tells you all the things that are wrong with your code. This allows for simpler overview of failures and easier development.

It is also extremely useful on build servers where you want to collect as much information about the failure as you can, to avoid fixing the code over multiple builds which can take many minutes to finish.

# this is all code that would be hidden in module, scroll down :) 
$script:scope = ""
$script:results = @{}
$AssertionActionPreference = 'Stop'

function Get-Scope () { $script:scope }

function Set-Scope ($Name) {
    $script:scope = $Name
}

function Add-Result ($Result, $Scope = (Get-Scope)) { 
    if ($script:results.ContainsKey($Scope))
    {
        $script:results[$scope] += $Result
    }
    else
    {
       $script:results[$scope] = @($Result)
    }
}

function Test-Result ($Scope) { 
    $script:results.ContainsKey($Scope)
}

function Get-Result ($Scope) { 
    $script:results[$Scope]
}

function Fail-DelayedAssertions ($Scope) { 
    if ((Test-Result $Scope))
    {
        $fails = (Get-Result $scope)
        $r = "Failed with $($Fails.count) errors:`n$( $fails-join "`n")"
        Write-error $r -ErrorAction 'stop'
    }
}

function Combine-Assert ($ScriptBlock) {
    $AssertionActionPreference = 'Continue'
    $scope = [guid]::NewGuid().ToString('N')
    Set-Scope $scope
    &$ScriptBlock
    Fail-DelayedAssertions $scope
}    


# scroll even further
function Assert-Equal {
    param (
        $Expected, 
        [Parameter(ValueFromPipeline)]
        $Actual, 
        [Management.Automation.ActionPreference]$AssertionAction = $AssertionActionPreference 
    ) 
    
    # assertions need to support delaying failures
    $isDelayed = $true
    if ($AssertionAction -eq [System.Management.Automation.ActionPreference]::Stop)
    {
        $isDelayed = $false    
    }

    if ($Expected -ne $Actual) {
         $result = "Actual is '$Actual', but I expected '$Expected'."
         if (-not $isDelayed)
         {
            throw $result
         }
         else
         {
            Add-Result $result
         }
    }
}

function Assert-NotNull {
    param (
        $Expected, 
        [parameter(ValueFromPipeline)]
        $Actual, 
        [Management.Automation.ActionPreference]$AssertionAction = $AssertionActionPreference 
    ) 
    
    $isDelayed = $true
    if ($AssertionAction -eq [System.Management.Automation.ActionPreference]::Stop)
    {
        $isDelayed = $false    
    }

    if ($null -eq $Actual) {
         $result = "Actual is '`$null', but I expected value that is not null."
         if (-not $isDelayed)
         {
            throw $result
         }
         else
         {
            Add-Result $result
         }
    }
}


# ----> start here, this is what the real code would mostly look like
function Get-WebResponse { 
    "{ StatusCode: 400, StatusDescription : 'BadRequest', Content : 'Id is empty.'  }" `
        | ConvertFrom-Json 
}


Describe "Get-WebResponse" {
    # out usual test. Fails on the first assertion,
    # which means that we only see we got 400 but not 
    # the content of the response, which usually contains validation 
    # errors
    It "Fails on first failed assertion" {
        $actual = Get-WebResponse

        $actual.StatusCode | Assert-Equal 200
        $actual.StatusDescription | Assert-Equal 'OK'
        $actual.Content | Assert-Equal 'Hello'
    }


    # assertions in the combine assert block are all run 
    # and produce single comprehensive failure
    It "Fails after testing all three properties, because we put assertions in Combine-Assert block" {
        $actual = Get-WebResponse

        Combine-Assert {
            $actual.StatusCode | Assert-Equal 200
            $actual.StatusDescription | Assert-Equal 'OK'
            $actual.Content | Assert-Equal 'Hello'
        }
    }

    # here we do the same thing, but in a way that would require extra
    # support from Pester. 
    # Pester would manage scopes and verify delayed assertions. The code
    # on top and bottom would be automatic, user would only write the  code
    # in the middle
    It "Fails after testing all three properties, because we specify the AssertionAction paramater" {
        # scope would be set by pester if we add support for this
        Set-Scope "1"
        ###

        $actual = Get-WebResponse

        $actual.StatusCode | Assert-Equal 200 -AssertionAction Continue
        $actual.StatusDescription | Assert-Equal 'OK' -AssertionAction Continue
        $actual.Content | Assert-Equal 'Hello' -AssertionAction Continue

        # validation would be done by pester if we add support for this
        Fail-DelayedAssertions -Scope "1"
        ###
    }

    # here again we require extra support from pester, 
    # but rather than telling every assertion to delay explicitly
    # we use AssertionActionPreference to do it for every assertion in the test
    # or potentialy for every assertion in the file or even test suite.
    It "Fails after testing all three properties, because we use AssertionActionPreference" {
        # preference and scope would be set by pester if we add support for this
        Set-Scope "2"
        ###

        $AssertionActionPreference = 'Continue'
        $actual = Get-WebResponse

        $actual.StatusCode | Assert-Equal 200
        $actual.StatusDescription | Assert-Equal 'OK'
        $actual.Content | Assert-Equal 'Hello'

        # preference and validation would be done by pester if we add support for this
        Fail-DelayedAssertions -Scope "2"
        ###
    }
}
Describing Get-WebResponse
  [-] Fails on first failed assertion 19ms
    RuntimeException: Actual is '400', but I expected '200'.
    at Assert-Equal, <No file>: line 68
    at <ScriptBlock>, <No file>: line 120
  [-] Fails after testing all three properties, because we put assertions in Combine-Assert block 22ms
    WriteErrorException: Failed with 3 errors:
    Actual is '400', but I expected '200'.
    Actual is 'BadRequest', but I expected 'OK'.
    Actual is 'Id is empty.', but I expected 'Hello'.
    at Fail-DelayedAssertions, <No file>: line 36
    at Combine-Assert, <No file>: line 45
    at <ScriptBlock>, <No file>: line 131
  [-] Fails after testing all three properties, because we specify the AssertionAction paramater 19ms
    WriteErrorException: Failed with 3 errors:
    Actual is '400', but I expected '200'.
    Actual is 'BadRequest', but I expected 'OK'.
    Actual is 'Id is empty.', but I expected 'Hello'.
    at Fail-DelayedAssertions, <No file>: line 36
    at <ScriptBlock>, <No file>: line 155
  [-] Fails after testing all three properties, because we use AssertionActionPreference 17ms
    WriteErrorException: Failed with 3 errors:
    Actual is '400', but I expected '200'.
    Actual is 'BadRequest', but I expected 'OK'.
    Actual is 'Id is empty.', but I expected 'Hello'.
    at Fail-DelayedAssertions, <No file>: line 36
    at <ScriptBlock>, <No file>: line 176
@nohwnd
Copy link
Owner Author

nohwnd commented Nov 15, 2017

Some more concepts, probably the syntax with -Property parameter would be the best. Lot of this stuff can be done more easily when Assert-Equivalent is done. Then we can just give it simple object with five properties, allow ignoring missing properties on the actual object and we're done.

@alx9r
Copy link

alx9r commented Nov 19, 2017

@nohwnd I think I've got a pattern that might work for simultaneous reporting of failed assertions in this usage, but without needing CombineAssert:

image

I'd rather not duplicate your work so I'm wondering if you've already got a solution for that.

@alx9r
Copy link

alx9r commented Nov 20, 2017

Here's a gist of the chaining pattern I was thinking of. If each Assert- function follows the pattern, I think you can get the desired output without coupling to Pester.

Describe 'chained asserts' {
    It 'fails some of these' {
        24 |
            Assert-Something {-not ($_ % 3)} 'divisible by 3' |
            Assert-Something {-not ($_ % 4)} 'divisible by 4' |
            Assert-Something {-not ($_ % 5)} 'divisible by 5' |
            Assert-Something { $_ -gt 30 }   'greater than 30' |
            Assert-Something { $_ -ge 0 }    'not negative'
    }
}

outputs

image

@nohwnd
Copy link
Owner Author

nohwnd commented Nov 21, 2017

That's pretty clever, propagating the value throught the pipeline and failing the last assertion first. Problem is that the assertions are collecting input in the process block, and then asserting on it in the end block. Is there any way we could use this?

@alx9r
Copy link

alx9r commented Nov 21, 2017

You're right about this technique not working for exceptions in the end block. I've got a more elaborate pattern that looks promising for Assert-Any and any other assertion that needs to operate on the entire contents of collections. But it depends on more explicitly stating intentions around testing collections versus collection contents. That's the discussion in #29.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants