pester / Pester

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

[Pester 5] Original function silently executed if parameter validation fails in mock #2178

Open bozho opened 2 years ago

bozho commented 2 years ago

General summary of the issue

When mock function fails parameter validation, the original function is executed with no warnings/errors reported.

Describe your environment

Pester version : 5.3.0 F:\Users\bozho.powershell\modules\Pester\5.3.0\Pester.psm1 PowerShell version : 5.1.19041.1645 OS version : Microsoft Windows NT 10.0.19044.0

Steps to reproduce

I have a module function that looks something like this:

function Get-SomeRestResource {
    [CmdletBinding(PositionalBinding = $false)]
    param(
        [parameter()]
        [ValidateNotNull()]
        [string] $Server = $script:DefaultServer
    )

    try {
        $headers = @{
            Authorization = "...."
            "Content-Type" = "application/json"
        }

        $response = Invoke-RestMethod -Uri "https://$Server/api/some/resource" -Method Get -Headers $headers
    }
    catch {
        ....
    }
}

$script:DefaultServer is a module variable.

~I am mocking both Get-SomeRestResource and Invoke-RestMethod.~ I am mocking Invoke-RestMethod. Mocked Invoke-RestMethod simply returns a constant JSON object (doesn't reference $Server parameter at all).

I have a test where I simply execute Get-SomeRestResource with no parameters, so $Server is supposed to pick up the default server string.

I had a bug where $script:DefaultServer module variable was not defined (I forgot to initialise it in the module), which resulted in $Server parameter being an empty string.

~This in turn resulted in the original Get-SomeRestResource being executed - I've confirmed that both using VS Code's test debugging and the fact that the original Invoke-RestMethod was complaining about invalid URL (https:///api/some/resource).~

This in turn resulted in the original Invoke-RestMethod being executed - I've confirmed that both using VS Code's test debugging and the fact that the original Invoke-RestMethod was complaining about invalid URL (https:///api/some/resource).

The only change I had to make is to actually declare $script:DefaultServer module variable, with no other changes to the module or test code and everything worked as expected.

Expected Behavior

Parameter validation error being raised.

nohwnd commented 2 years ago

This is "by design" when there is no default mock the real command is called when no parametrized mock matches. It is like that since Mocks were introduced into Pester in v2.

But I agree that behavior is rarely useful and probably source of grief for many, and would like to make it parametrized, with default being not falling through.

@fflaten what do you think? Does it make sense that once there is any mock in scope, the real function is not called?

fflaten commented 2 years ago

So Invoke-RestMethod was only mocked with a parameterfilter, which didn't match the call, correct? (Repro is missing the Mock-commands)

@fflaten what do you think? Does it make sense that once there is any mock in scope, the real function is not called?

Wouldn't that block someone intentionally mocking Invoke-RestMethod against a specific service while using real calls for the rest? We already have #2166 which would allow you to quickly add a default mock to throw. That would solve this issue, wouldn't it?

bozho commented 2 years ago

@fflaten Sorry, rereading my original question, I see I messed up the explanation a bit: I am only mocking Invoke-RestMethod in this example.

My test does simply something like this:

It "..." {
   Get-RestResource
}

When my module being tested does not define $script:DefaultServer (used as a default for the mandatory $Server parameter, the original Invoke-RestMethod is executed from Get-RestResource.

When $script:DefaultServer module variable is defined, Get-RestResource executes the mocked Invoke-RestMethod.

fflaten commented 2 years ago

🙂 It was the Mock -CommandName Invoke-RestMethod ...... I was missing.