pester / Pester

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

Cannot verify invocation of mocked function if it throws an exception #2283

Closed amdreallyfast closed 1 year ago

amdreallyfast commented 1 year ago

Checklist

What is the issue?

I am trying to test that a function under test catches an exception. My end use case is checking that my script is properly handling a network SocketException that might be thrown by an Azure PowerShell function. For this bug, I've reproduced the basic issue by using a modified version of the "Mocking Functions" -> "Example" code on the pester website (https://pester.dev/docs/usage/mocking).

I modified it so that the mock of Get-Version throws an exception, and I modified BuildIfChanged to catch that specific exception and handle it. Then I wrote tests to make sure that (1) Get-Version is called once and only once, (2) an error code is returned, and (3) that the BuildIfChanged function is not called.

Tests (2) and (3) pass, but test (1) fails due to throwing a SocketException. This is puzzling. I want to check that the function was called. I am not attempting to verify that the exception was thrown. I explicitly made the mock do that, so I am well aware that this is what the function does. I am trying to make sure that the exception is handled.

Expected Behavior

Test 1: Verify that Get-Version was called once Pass Test 2: Verify that error code was returned Pass Test: Verify that BuildIfChanged was not called Pass

Expected output (from my Write-Host calls):

Calling the test function
caught you
result: '-1'
should call Get-Version once:
should return -1:
should not build:
Tests completed in <number>ms
Tests Passed: 3, Failed: 0, Skipped: 0 NotRun: 0

Steps To Reproduce

Code:

BeforeAll {
    function Build ($version) {
        Write-Host "a build was run for version: $version"
    }

    function Get-Version {
        return 'Version'
    }

    function Get-NextVersion {
        return 'NextVersion'
    }

    function BuildIfChanged {
        try {
            $thisVersion = Get-Version
        }
        catch [System.Net.Sockets.SocketException] {
            Write-Host "caught you"
            $Error | Out-Null
            return -1
        }
        catch {
            Write-Host "didn't catch it"
            return -2
        }

        Write-Host "should not get here"
        $nextVersion = Get-NextVersion
        if ($thisVersion -ne $nextVersion) { Build $nextVersion }
        return $nextVersion
    }
}

Describe "BuildIfChanged" {
    Context "could not connect to server" {
        BeforeAll {
            Mock Get-Version {
                throw New-Object System.Net.Sockets.SocketException
            }
            Mock Get-NextVersion { return 1.2 }
            Mock Build {} -Verifiable -ParameterFilter { $version -eq 1.2 }

            Write-Host "Calling the test function"
            $result = BuildIfChanged
            Write-Host "result: '$result'"
        }

        It "should call Get-Version once" {
            Write-Host "should call Get-Version once:"
            Should -Invoke -CommandName Get-Version -Times 1 
        }

        It "should return -1" {
            Write-Host "should return -1:"
            $result | Should -Be -1
        }

        It "should not build" {
            Write-Host "should not build:"
            Should -Invoke -CommandName Build -Times 0
        }
    }
}

Output (verbosity = 'Detailed'): Note: Unable to reproduce the Detailed output anymore. I'm still seeing the errors, but at the 'None' verbosity. I changed VSCode's Pester verbosity to a less-detailed setting when trying to simplify the expected output, and then I set it back to 'Diagnostic', but the setting now appears to be ignored and the actual output verbosity remains stuck at None. The following Detailed verbose output was the last that I was able to get.

Pester v5.3.3
Discovery: Starting test discovery in 1 test containers. 

Starting discovery in 1 files.
Discovery: Discovering tests in C:\CodeTesting\PowerShell\MyPesterTests.Tests.ps1 
Discovery: Found 3 tests in 15 ms 
Discovery: Processing discovery result objects, to set root, parents, filters etc. 
Discovery found 3 tests in 18ms.
Discovery: Test discovery finished. 
Running tests.

Running tests from 'C:\CodeTesting\PowerShell\MyPesterTests.Tests.ps1'
Mock: Setting up default mock for Get-Version. 
Mock: We are in a block, one time setup or similar. Returning mock table from test block.                                                                                                                                                                         
Mock: Resolving command Get-Version.                                                                                                                                                                                                                              
Mock: Searching for command Get-Version in the script scope.                                                                                                                                                                                                      
Mock: Found the command Get-Version in the script scope.                                                                                                                                                                                                          
Mock: Mock does not have a hook yet, creating a new one.                                                                                                                                                                                                          
Mock: Defined new hook with bootstrap function PesterMock_script_Get-Version_ba5d8dee-3040-41c1-bb22-baa29ee6c3fd and aliases Get-Version.                                                                                                                        
Mock: Adding a new default behavior to Get-Version.                                                                                                                                                                                                               
Mock: Setting up default mock for Get-NextVersion.                                                                                                                                                                                                                
Mock: We are in a block, one time setup or similar. Returning mock table from test block.                                                                                                                                                                         
Mock: Resolving command Get-NextVersion.                                                                                                                                                                                                                          
Mock: Searching for command Get-NextVersion in the script scope.                                                                                                                                                                                                  
Mock: Found the command Get-NextVersion in the script scope.                                                                                                                                                                                                      
Mock: Mock does not have a hook yet, creating a new one.                                                                                                                                                                                                          
Mock: Defined new hook with bootstrap function PesterMock_script_Get-NextVersion_c33aa024-1b15-4416-85e5-18acea9767ce and aliases Get-NextVersion.                                                                                                                
Mock: Adding a new default behavior to Get-NextVersion.                                                                                                                                                                                                           
Mock: Setting up parametrized mock for Build.                                                                                                                                                                                                                     
Mock: We are in a block, one time setup or similar. Returning mock table from test block.                                                                                                                                                                         
Mock: Resolving command Build.                                                                                                                                                                                                                                    
Mock: Searching for command Build in the script scope.                                                                                                                                                                                                            
Mock: Found the command Build in the script scope.                                                                                                                                                                                                                
Mock: Mock does not have a hook yet, creating a new one.                                                                                                                                                                                                          
Mock: Defined new hook with bootstrap function PesterMock_script_Build_37aa099d-f5c3-4f66-99e3-939b31e79036 and aliases Build.                                                                                                                                    
Mock: Adding a new parametrized behavior to Build.                                                                                                                                                                                                                
Calling the test function
Mock: Mock for Get-Version was invoked from block Process, resolving call history and behaviors. 
Mock: Getting all defined mock behaviors in this and parent scopes for command Get-Version.                                                                                                                                                                       
Mock: Finding all behaviors in this block and parent blocks.                                                                                                                                                                                                      
Mock: Found behaviors for 'Get-Version' in 'could not connect to server'.                                                                                                                                                                                         
Mock: Filtering behaviors for command Get-Version, for target module $null (Showing all behaviors for this command, actual filtered list is further in the log, look for 'Filtered parametrized behaviors:' and 'Filtered default behaviors:'):                   
Mock: Behavior is a default behavior from script scope, saving it:                                                                                                                                                                                                
     Target module: $null                                                                                                                                                                                                                                         
     Body: { throw New-Object System.Net.Sockets.SocketException }                                                                                                                                                                                                
     Filter: $null                                                                                                                                                                                                                                                
     Default: $true                                                                                                                                                                                                                                               
     Verifiable: $false                                                                                                                                                                                                                                           
Mock: We are in a block, one time setup or similar. Returning mock table from test block.                                                                                                                                                                         
Mock: Filtered parametrized behaviors:                                                                                                                                                                                                                            
$null                                                                                                                                                                                                                                                             
Mock: Filtered default behavior:                                                                                                                                                                                                                                  
     Target module: $null                                                                                                                                                                                                                                         
     Body: { throw New-Object System.Net.Sockets.SocketException }                                                                                                                                                                                                
     Filter: $null                                                                                                                                                                                                                                                
     Default: $true                                                                                                                                                                                                                                               
     Verifiable: $false                                                                                                                                                                                                                                           
Mock: Finding behavior to use, one that passes filter or a default:                                                                                                                                                                                               
Mock: {                                                                                                                                                                                                                                                           
                throw New-Object System.Net.Sockets.SocketException                                                                                                                                                                                               
             } is a default behavior and will be used for the mock call.                                                                                                                                                                                          
Mock: Executing mock behavior for mock Get-Version.                                                                                                                                                                                                               
caught you
result: '-1'
Describing BuildIfChanged
 Context could not connect to server
should call Get-Version once:
Mock: Resolving command Get-Version. 
Mock: Searching for command Get-Version in the script scope.                                                                                                                                                                                                      
Mock: Found the command Get-Version in the script scope and it resolved to PesterMock_script_Get-Version_ba5d8dee-3040-41c1-bb22-baa29ee6c3fd.                                                                                                                    
   [-] should call Get-Version once 9ms (6ms|2ms)                                                                                                                                                                                                                 
    Expected Get-Version to be called at least 1 times but was called 0 times
    at Should -Invoke -CommandName Get-Version -Times 1, C:\CodeTesting\PowerShell\MyPesterTests.Tests.ps1:51
    at <ScriptBlock>, C:\CodeTesting\PowerShell\MyPesterTests.Tests.ps1:51
should return -1:
   [+] should return -1 4ms (3ms|1ms)
should not build:
Mock: Resolving command Build. 
Mock: Searching for command Build in the script scope.                                                                                                                                                                                                            
Mock: Found the command Build in the script scope and it resolved to PesterMock_script_Build_37aa099d-f5c3-4f66-99e3-939b31e79036.                                                                                                                                
   [+] should not build 6ms (5ms|1ms)
Mock: Removing function PesterMock_script_Get-Version_ba5d8dee-3040-41c1-bb22-baa29ee6c3fd and aliases Get-Version for Get-Version. 
Mock: Removed function PesterMock_script_Get-Version_ba5d8dee-3040-41c1-bb22-baa29ee6c3fd from script session state.                                                                                                                                              
Mock: Removed alias Get-Version from script session state.                                                                                                                                                                                                        
Mock: Removing function PesterMock_script_Get-NextVersion_c33aa024-1b15-4416-85e5-18acea9767ce and aliases Get-NextVersion for Get-NextVersion.                                                                                                                   
Mock: Removed function PesterMock_script_Get-NextVersion_c33aa024-1b15-4416-85e5-18acea9767ce from script session state.                                                                                                                                          
Mock: Removed alias Get-NextVersion from script session state.                                                                                                                                                                                                    
Mock: Removing function PesterMock_script_Build_37aa099d-f5c3-4f66-99e3-939b31e79036 and aliases Build for Build.                                                                                                                                                 
Mock: Removed function PesterMock_script_Build_37aa099d-f5c3-4f66-99e3-939b31e79036 from script session state.                                                                                                                                                    
Mock: Removed alias Build from script session state.                                                                                                                                                                                                              
Tests completed in 113ms                                                                                                                                                                                                                                          
Tests Passed: 2, Failed: 1, Skipped: 0 NotRun: 0

Describe your environment

Pester version : 5.3.3 C:\Users\coxjohn\Documents\PowerShell\Modules\Pester\5.3.3\Pester.psm1
PowerShell version : 7.3.1 OS version : Microsoft Windows NT 10.0.22621.0

Possible Solution?

Hack workaround: Don't attempt to verify that a mocked function that throws an exception was called. Pull out the old C-style practice of using a unique error code if there was a problem, and adapt that to the present by having the function under test abort and return that unique error code in the exception handling, and then use $result | Should -Be <unique error code> to indirectly verify that the mocked function was called and subsequently handled.

This is not a proper substitute for Should -Invoke -CommandName <command> -Times 1 working as expected, but I can use this hack as a workaround for now.

I'd prefer if Should -Invoke ... did not fail just because the mocked function intentionally threw an exception.

fflaten commented 1 year ago

Hi! Unless I missed something, this seems to work as expected. By default, Should -Invoke checks invocations inside the current test, but you invoked the mock in BeforeAll and that executes in the parent container, Context .. in this sample. (Unlike BeforeEach which would execute per test -> in It-scope)

Try Should -Invoke -CommandName Get-Version -Times 1 -Scope Context and it should pass as expected.

amdreallyfast commented 1 year ago

...BeforeEach worked, but I don't understand why conceptually. I was under the impression that BeforeAll would run once before the tests in the context, and BeforeEach would run every time before the tests in the context, but it sounds like there's a bigger difference. Is this difference documented somewhere? I don't remember coming across it.

fflaten commented 1 year ago

I was under the impression that BeforeAll would run once before the tests in the context

Correct, which is why the mock call is linked to Context-block. Think of *All as block (Describe/Context) setup/teardown. That's why you need to use Should -Invoke ... -Scope Context when you run Should in It, but the execution was done in BeforeAll.

BeforeEach would run every time before the tests in the context, but it sounds like there's a bigger difference.

Also correct. *Each is executed once per test, so any mock calls is linked to that specific It-test. Think of *Each as test setup/teardown.

Is this difference documented somewhere? I don't remember coming across it.

In general, see https://pester.dev/docs/usage/setup-and-teardown. Ex the quote below and the Scoping-chapter:

BeforeAll runs during Run phase and runs only once in the current block.

For Should -Invoke specifically the closest is https://pester.dev/docs/usage/mocking#counting-mocks-depends-on-placement


In your sample where you have different It-blocks to run different assertions on the same function call, then you'd likely want to stick with BeforeAll to only execute once. However then you need to use -Scope Context to count it, ex:

It "should call Get-Version once" {
    Write-Host "should call Get-Version once:"
    Should -Invoke -CommandName Get-Version -Times 1 -Scope Context
}
amdreallyfast commented 1 year ago

That link isn't very clear to me. Why would calling a function BeforeAll result in the invocation count being 0? The documentation in that link doesn't explain (or if it doesn't, I wasn't able to notice it).

That link also doesn't explain the exception thrown in the mocked function caused the test to failed when using BeforeAll, but the same mocked exception throwing did not cause the test to fail when using BeforeEach.

Please explain both. I'm lost here.

fflaten commented 1 year ago

The exception is irrelevant - it is caught by try/catch in BuildIfChanged.

The key point is that when you invoke a mock in BeforeAll, it is done in the scope of Context "could not connect to server", not the It-block.

What counting-mocks-depends-on-placement tries to explain is that in order to count invocations done in the Context-block (incl. BeforeAll), you need to specify -Scope Context in your Should -Invoke command. If not, it will only count any invocations you've done in the current It-block - which is zero in your sample.

Demo:

Describe 'BuildIfChanged' {
    Context 'demo' {
        BeforeAll {
            function Get-Version {
                return 'Version'
            }
            Mock Get-Version {
                throw New-Object System.Net.Sockets.SocketException
            }

            # This invocation is done in BeforeAll = Context-scope
            try { $result = Get-Version } catch { }
        }

        It 'One invocation in Context-scope: BeforeAll' {
            Should -Invoke -CommandName Get-Version -Times 1 -Scope Context
        }

        It 'Zero invocations in current It-scope: Not executed inside It' {
            Should -Invoke -CommandName Get-Version -Times 0
        }

        It 'One invocation in current It-scope: Executed inside It' {
            # This invocation is done inside It, so will count in current scope
            try { $result = Get-Version } catch { }
            Should -Invoke -CommandName Get-Version -Times 1
        }

        It 'Two invocations in Context-scope: BeforeAll and previous It-block' {
            Should -Invoke -CommandName Get-Version -Times 2 -Scope Context
        }
    }
}
amdreallyfast commented 1 year ago

Oh. So invocation count of the context scope includes the invocations of the individual tests, but the invocation count of the individual tests is completely isolated from invocations in the context scope?

Thanks for explaining that. I didn't come to that conclusion in the documentation.

Suggestion for documentation: Can you add a search function to the table of contents?

On the pester.dev website, I've been finding it difficult to find documentation unless I already know where it is, and so, when trying to find coverage of some subject matter on pester.dev and I don't know where it is, I use Google. But that also turns up a lot of other search results that are only mention the subject matter and are not the official documentation. If I want to find out info about "Should", my first instinct is to search the table of contents for "Should" and find a page about the "Should" function. If I want to learn more about the ParameterFilter argument to the mocks, my first instinct is to search the table of contents for "ParameterFilter". But since this search isn't available, I've been having a very difficult time finding the documentation for a particular function and have spent hours thinking I found a bug when in fact the documentation describing the behavior was off in corner that I hadn't know to look at (in this case, I was trying to look on Google for Invoke behavior, but should have been looking for "Counting mocks depends on placement", which I didn't think to look for).

Regardless, thank you for setting me straight.

My problem has been resolved.

fflaten commented 1 year ago

Oh. So invocation count of the context scope includes the invocations of the individual tests, but the invocation count of the individual tests is completely isolated from invocations in the context scope?

Correct. Every block only knows what has happened inside itself or a child block. So It = only itself. Context/Describe = itself or a child block.

Suggestion for documentation: Can you add a search function to the table of contents?

There is a site-wide search field at the top right. Titles for pages, like Should, will usually be the first hit. image image

If you have any specific issues with it, you could submit an issue in https://github.com/pester/docs repo and we might see if it's possible to adjust the indexing.

As you may see on that particular page (Command Reference\Should) though, it isn't great. That's because Should is a wrapper for many commands (assertions) so the generated PowerShell parameter-help can't be made very specific to each individual assertion. So for assertions I usually refer to https://pester.dev/docs/assertions which will point you to the Usage\Mocking-page for Should -Invoke since that is a special one.