Make `Mock` learn `Assert` parameter

[johlju]

Summary of the feature request

As a contributor or maintainer of a project I'd like to be able to help other contributors to correctly mock functions by failing a test if a mock does not exist. This would be very useful in situations when there are very complex scripts that handle a lot of scenarios and using a lot of external helper functions.

The ask would be to add a new parameter to Mock that set up a default mock that fails the test if no other mock have overridden it. The parameter name could be Assert, MustMock, Override, or MustBeOverridden (or any other name that is better).

Mock -Assert -CommandName Get-Something2

How should it work? (optional)

Take this example as a "complex script", let us say a contributor added the second Context-block for a new scenario. The contributor missed to add a mock for the function Get-Something2, and the maintainer missed it in the review. The script luckily works, but when the mock was missed the test runs much slower than everyone expects, and does not easily show in the pipeline.

BeforeAll {
    function Get-Something1
    {
        Start-Sleep -Seconds 3
    }

    function Get-Something2
    {
        Start-Sleep -Seconds 3
    }

    function BigScriptWithALotOfMocks
    {
        Get-Something1
        Get-Something2

        return $true
    }
}

Describe 'BigScriptWithALotOfMocks' {
    Context 'When testing scenario 1' {
        BeforeAll {
            Mock -CommandName Get-Something1
            Mock -CommandName Get-Something2
        }

        It 'Should return $true' {
            BigScriptWithALotOfMocks | Should -BeTrue
        }
    }

    Context 'When testing scenario 2' {
        BeforeAll {
            Mock -CommandName Get-Something1
        }

        It 'Should return $true' {
            BigScriptWithALotOfMocks | Should -BeTrue
        }
    }
}

This result is a test that runs "slow" (3.45 seconds):

Starting discovery in 1 files.
Discovery found 2 tests in 409ms.
Running tests.
[+] C:\Users\johlju\OneDrive\Desktop\PesterExampleMustMock.Tests.ps1 3.44s (3.13s|178ms)
Tests completed in 3.45s
Tests Passed: 2, Failed: 0, Skipped: 0 NotRun: 0

We could help prevent this today by adding a default mock for each command that needs to be mocked by contributors under the Describe-block. The default mock throws an error.

BeforeAll {
    function Get-Something1
    {
        Start-Sleep -Seconds 3
    }

    function Get-Something2
    {
        Start-Sleep -Seconds 3
    }

    function BigScriptWithALotOfMocks
    {
        Get-Something1
        Get-Something2

        return $true
    }
}

Describe 'BigScriptWithALotOfMocks' {
    BeforeAll {
        Mock -CommandName Get-Something1 -MockWith {
            throw "The default mock for 'Get-Something1' was not overridden."
        }

        Mock -CommandName Get-Something2 -MockWith {
            throw "The default mock for 'Get-Something1' was not overridden."
        }
    }

    Context 'When testing scenario 1' {
        BeforeAll {
            Mock -CommandName Get-Something1
            Mock -CommandName Get-Something2
        }

        It 'Should return $true' {
            BigScriptWithALotOfMocks | Should -BeTrue
        }
    }

    Context 'When testing scenario 2' {
        BeforeAll {
            Mock -CommandName Get-Something1
        }

        It 'Should return $true' {
            BigScriptWithALotOfMocks | Should -BeTrue
        }
    }
}

This result is:

Starting discovery in 1 files.
Discovery found 2 tests in 931ms.
Running tests.
[-] BigScriptWithALotOfMocks.When testing scenario 2.Should return $true 145ms (143ms|3ms)
 RuntimeException: The default mock for 'Get-Something1' was not overridden.
 at <ScriptBlock>, C:\Users\johlju\Desktop\PesterExampleMustMock.Tests.ps1:29

The contributor adds the missing mock and the tests passes and runs as quick one would expect.

BeforeAll {
    function Get-Something1
    {
        Start-Sleep -Seconds 3
    }

    function Get-Something2
    {
        Start-Sleep -Seconds 3
    }

    function BigScriptWithALotOfMocks
    {
        Get-Something1
        Get-Something2

        return $true
    }
}

Describe 'BigScriptWithALotOfMocks' {
    BeforeAll {
        Mock -CommandName Get-Something1 -MockWith {
            throw "The default mock for 'Get-Something1' was not overridden."
        }

        Mock -CommandName Get-Something2 -MockWith {
            throw "The default mock for 'Get-Something1' was not overridden."
        }
    }

    Context 'When testing scenario 1' {
        BeforeAll {
            Mock -CommandName Get-Something1
            Mock -CommandName Get-Something2
        }

        It 'Should return $true' {
            BigScriptWithALotOfMocks | Should -BeTrue
        }
    }

    Context 'When testing scenario 2' {
        BeforeAll {
            Mock -CommandName Get-Something1
            Mock -CommandName Get-Something2
        }

        It 'Should return $true' {
            BigScriptWithALotOfMocks | Should -BeTrue
        }
    }
}

The result is faster execution:

Starting discovery in 1 files.
Discovery found 2 tests in 499ms.
Running tests.
[+] C:\Users\johlju\OneDrive\Desktop\PesterExampleMustMock.Tests.ps1 906ms (456ms|242ms)
Tests completed in 1.01s
Tests Passed: 2, Failed: 0, Skipped: 0 NotRun: 0

Instead of setting a default mock that throws I would like to see this works natively with a command. So instead of this:

    BeforeAll {
        Mock -CommandName Get-Something1 -MockWith {
            throw "The default mock for 'Get-Something1' was not overridden."
        }

        Mock -CommandName Get-Something2 -MockWith {
            throw "The default mock for 'Get-Something1' was not overridden."
        }
    }

This would look nicer and be simple if we could simply replace the BeforeAll-block with this:

    BeforeAll {
        Mock -Assert -CommandName Get-Something1
        Mock -Assert -CommandName Get-Something2
    }

[nohwnd]

Maybe Mock Get-Something1 -Throw? But let's think about how it would grow. -Throw as a switch is nice, and probably what 90% of the usages would use that. But it would also be nice to be able provide a custom message, and letting -Throw to take $null sucks, because then it will eat your command name when you do Mock -Throw Get-Something. So maybe another parameter -ThrowMessage?. Or -Because?

[fflaten]

Mock ... -Throw -Because "we're consistent" 👍

Should this also work with non-default mocks? Like a direct alternative to -MockWith { throw 'invalid parameters used'}?

[nohwnd]

Yeah, why not, I don't see myself using it, but I also don't see a reason why not implement it.

[nohwnd]

Personally I would like to see parametrized mocks failing / doing nothing, rather than failling back to calling the real command when there is no default mock.

[johlju]

I like -Throw and -Throw -Because for a custom message. But then it would also be great if we could output the command name, e.g. if we have -Throw -Because 'must be overridden with a template' it would output something like because command Get-Somthing must be overridden with a template, or if we could use <_> in the string to get the command name at that location. 🤔

[fflaten]

👍 The command name would be part of static error message. -Because usually appends an optional message somewhere in the error message. Ex.

Default mock for Get-Something should never be called, because ...

[nohwnd]

Yes, that. Make it work like assertions. "Default mock for Get-Something was called, <optional because message>,but it should have never been called.". If you want fully custom message, then throw yourself from the mock body.

[nohwnd]

The implementer is expected to add the -Throw parameter to Mock, and throw when the parameter is provided, and the mock is invoked.

It will probably need to be stored as two new fields on the internal mock object, that is stored in mock table. There are no changes to the complicated mock execution part that I can think of, so this change should be rather simple.

[nohwnd]

Moved to 6, where we want to throw by default, so the requested functionality will be done by default.

[fflaten]

Moved to 6, where we want to throw by default, so the requested functionality will be done by default.

@nohwnd: Need more details to implement this. See a concern I have here: https://github.com/pester/Pester/issues/2178#issuecomment-1124679601