pester / Pester

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

Is there a terse way to assert that a mock was called with certain parameters and not otherwise? #315

Closed alx9r closed 9 years ago

alx9r commented 9 years ago

Consider the following files which demonstrate asserting that Write-Error was called only once with a particular parameter and not otherwise. I find this to be a common scenario for testing basic validation logic and error reporting of functions.

Module.Tests.ps1

Import-Module Module

InModuleScope Module{
Describe f1 {
    Mock Write-Error -Verifiable 

    It 'writes exactly "message 1"' {
        f1 one | Should be $false
        Assert-MockCalled Write-Error -Exactly -Times 1 -Scope It -ParameterFilter {
            $Message -eq "message 1"
        }
        Assert-MockCalled Write-Error -Exactly -Times 0 -Scope It -ParameterFilter {
            $Message -ne "message 1"
        }
    }
    It 'writes exactly "message 2"' {
        f1 two | Should be $false
        Assert-MockCalled Write-Error -Exactly -Times 1 -Scope It -ParameterFilter {
            $Message -eq "message 2"
        }
        Assert-MockCalled Write-Error -Exactly -Times 0 -Scope It -ParameterFilter {
            $Message -ne "message 2"
        }
    }
}
}

Module.psm1

This is the module we are working on. It has mistakes and the tests aren't passing yet.

function f1
{
    param($which)

    if ( $which -eq 'two' )
    {
        Write-Error 'message 1'
    }
    if ( $which -eq 'two' )
    {
        Write-Error 'message 2'
        return $false
    }
    return $true
}

Is there a terser way to achieve the same mock-called assertions?

What I don't like about how I am doing this is the repetition in the two calls to Assert-MockCalled in each It block. The first call always has -Exactly -Times 1 parameters and some filter, while the second call always has -Exactly -Times 0 and the inverse of the first calls' filter. I have found that maintaining this duplication is rather time-consuming and error-prone.

Normally I would deal with this kind of duplication with a helper function that makes both calls. I have experimented with this a bit but haven't found an improved result. I ran into two problems:

I find myself wishing that Assert-MockCalled had a -NotOtherwise switch that could be used with -ParameterFilter.

Is there some other approach that achieves a "assert mock called with these parameters and not otherwise" that is shorter and easier to maintain?

nohwnd commented 9 years ago

That is an interesting use case, I am surprised it did not need it before, because it makes a lot of sense. I am not sure about the implications at the moment, so let's talk just about simplifying your script now:

The Assert-MockCalled counts all calls to a given mock in the whole Describe (as you know, you can override that behavior with -Scope It) so let's start by asserting that the mock was called just once in the current scope. That is your "negative" part of the assertion. No inverting of the script block necessary. Since the first failing assertion fails the test we are now sure that the command was in fact called just once. Now we need to make sure the correct parameter was used. For that we use the -ParameterFilter parameter. The code would look like this:

Describe 'f1' {
    It 'Calls Write-Error single time with "message 1" when two is provided' {
        Mock Write-Error {}
        f1 two

        #no inverting filter here, just make 
        #sure the total number of calls was the same as the number of calls for the 
        #filtered mock, in other words the command was called only once
        Assert-MockCalled Write-Error -Exactly 1 -Scope It     
        Assert-MockCalled Write-Error -Exactly 1 -ParameterFilter { $which -eq "message 1" } -Scope It

    }

    It "Returns false for 'two'" {
        f1 two | Should Be $false
    }

    It "Returns true for empty string" {
        f1 "" | should be $true
    }

    It "Returns true for null" {
        f1 | should be $true
    }

    It "Returns true for one string" {
        f1 "one" | should be $true
    }
} 

Keep in mind that it is important to avoid assertion roulette, that is not being sure which assertion failed and why. For that reason I'd recommend to order the assertions in such order that the next assertion only refines the results of the first assertion.

Using single assertion would be preferable of course, but it is not possible with the current implementation.

This will only work if you assert exact number of calls. If you needed to make sure the mock was called at least once with the parameter and never without it you'd really need to invert the script block. In that case a special parameter would likely be warranted.

alx9r commented 9 years ago

Oh right. Your two-call method should work just fine for asserting calls to Write-Verbose, -Warning, and -Error. I also see the advantages of avoiding ambiguity by performing one logical test per It block.

Thanks for your help @nohwnd.

dlwyatt commented 9 years ago

I like the idea. I'm not sure what the switch should be named; -NotOtherwise or something else. Either way, it should be a pretty simple thing for us to implement, since we're already filtering the call history anyway. We just need to maintain two lists; the calls that matched the filter, and the calls that did not. If this new switch is used, the "did not" list has to be 0.

nohwnd commented 9 years ago

@dlwyatt I'd take a look on how classic suites like xUnit approach that problem. Maybe there is some nice and tested solution. Maybe the -Exactly on Mock should mean Exactly for any mock. Or maybe something totally different.