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.

Sign up for GitHub

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

WIP - Block original command by default when parameterized mocks exist #2547

Draft
wants to merge 7 commits into
base: main
Choose a base branch
from
Draft

WIP - Block original command by default when parameterized mocks exist #2547

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
6f724b4
Add initial tests
fflaten Jul 14, 2024
d48aedf
Update tests for new behavior
fflaten Jul 14, 2024
de75df7
Initial draft
fflaten Jul 14, 2024
8cb47e5
Fix fallback for affected Mock tests
fflaten Jul 14, 2024
c101907
Fix typo in debug output
fflaten Jul 14, 2024
31f34bf
WIP tests
fflaten Jul 14, 2024
22ef754
Support nested runs
fflaten Jul 14, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 24 additions & 11 deletions src/functions/Mock.ps1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,20 +28,22 @@
[Parameter(Mandatory)]
$Hook,
[string[]]$RemoveParameterType,
[string[]]$RemoveParameterValidation
[string[]]$RemoveParameterValidation,

Check warning

Code scanning / PSScriptAnalyzer

The parameter 'RemoveParameterValidation' has been declared but not used. Warning

The parameter 'RemoveParameterValidation' has been declared but not used.
[switch]$AllowFallback
)

[PSCustomObject] @{
CommandName = $ContextInfo.Command.Name
ModuleName = $ContextInfo.TargetModule
Filter = $ParameterFilter
IsDefault = $null -eq $ParameterFilter
IsInModule = -not [string]::IsNullOrEmpty($ContextInfo.TargetModule)
Verifiable = $Verifiable
Executed = $false
ScriptBlock = $MockWith
Hook = $Hook
PSTypeName = 'MockBehavior'
CommandName = $ContextInfo.Command.Name
ModuleName = $ContextInfo.TargetModule
Filter = $ParameterFilter
AllowFallback = $AllowFallback
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Name suggestions for parameter and property? Fallback is also used for "fallback to default mock". AllowOriginal could give expectation of blocking stuff like & (Get-Command Get-ChildItem -CommandType Cmdlet). AllowPassthrough is unclear. Other short suggestions?

Focus on other review comments first in case this design is discarded.

IsDefault = $null -eq $ParameterFilter
IsInModule = -not [string]::IsNullOrEmpty($ContextInfo.TargetModule)
Verifiable = $Verifiable
Executed = $false
ScriptBlock = $MockWith
Hook = $Hook
PSTypeName = 'MockBehavior'
}
}

Expand Down Expand Up @@ -982,6 +984,10 @@

$foundDefaultBehavior = $false
$defaultBehavior = $null
# A mock exists so we do not allow original command to be called. Either:
# - A parameterized mock explicitly allows it below
# - Or a default mock exists which takes precedence either way
$fallbackAllowed = $false
foreach ($b in $Behaviors) {

if ($b.IsDefault -and -not $foundDefaultBehavior) {
Expand All @@ -1005,6 +1011,8 @@
}
return $b
}

if ($b.AllowFallback) { $fallbackAllowed = $true }
}
}

Expand All @@ -1018,6 +1026,11 @@
if ($PesterPreference.Debug.WriteDebugMessages.Value) {
Write-PesterDebugMessage -Scope Mock "No parametrized or default behaviors were found filter."
}

if (-not $fallbackAllowed) {
throw 'Not implemented. None of the parameterized mocks match the call or accept fallback to original command.'
}

return $null
}

Expand Down
19 changes: 15 additions & 4 deletions src/functions/Pester.SessionState.Mock.ps1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,10 @@ function Mock {
validation requirements, and allows functions that are strict about their
parameter validation to be mocked more easily.

.PARAMETER AllowFallback
Allows original command to execute when only mocks with -ParameterFilter exists and the call does not match any of them.
Creating a default mock (no -ParameterFilter specified) will take precedence over this.

.EXAMPLE
Mock Get-ChildItem { return @{FullName = "A_File.TXT"} }

Expand Down Expand Up @@ -219,15 +223,22 @@ function Mock {
.LINK
https://pester.dev/docs/usage/mocking
#>
[CmdletBinding()]
[CmdletBinding(DefaultParameterSetName = 'Default')]
# TODO: Breaking change due to parameter sets. Previously had positions:
# CommandName, MockWith, ParameterFilter, ModuleName, RemoveParameterType, RemoveParameterValidation
param(
[Parameter(Position = 0)]
[string]$CommandName,
[Parameter(Position = 1)]
[ScriptBlock]$MockWith = {},
[switch]$Verifiable,
[Parameter(ParameterSetName = 'WithParameterFilter')]
[ScriptBlock]$ParameterFilter,
[string]$ModuleName,
[string[]]$RemoveParameterType,
[string[]]$RemoveParameterValidation
[string[]]$RemoveParameterValidation,
[Parameter(ParameterSetName = 'WithParameterFilter')]
[switch]$AllowFallback
Comment on lines +226 to +241
Copy link
Collaborator Author

@fflaten fflaten Jul 14, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we want to support all original positional parameters or is CommandName + MockWith enough? Wouldn't expect the rest to be commonly used.

)
if (Is-Discovery) {
# this is to allow mocks in between Describe and It which is discouraged but common
Expand Down Expand Up @@ -280,7 +291,7 @@ function Mock {
$mockData.Behaviors[$contextInfo.Command.Name] = $behaviors
}

$behavior = New-MockBehavior -ContextInfo $contextInfo -MockWith $MockWith -Verifiable:$Verifiable -ParameterFilter $ParameterFilter -Hook $hook
$behavior = New-MockBehavior -ContextInfo $contextInfo -MockWith $MockWith -Verifiable:$Verifiable -ParameterFilter $ParameterFilter -Hook $hook -AllowFallback:$AllowFallback
if ($PesterPreference.Debug.WriteDebugMessages.Value) {
Write-PesterDebugMessage -Scope Mock -Message "Adding a new $(if ($behavior.IsDefault) {"default"} else {"parametrized"}) behavior to $(if ($behavior.ModuleName) { "$($behavior.ModuleName) - "})$($behavior.CommandName)."
}
Expand Down Expand Up @@ -1045,7 +1056,7 @@ function Invoke-Mock {
param ($b)
" Target module: $(if ($b.ModuleName) { $b.ModuleName } else { '$null' })`n"
" Body: { $($b.ScriptBlock.ToString().Trim()) }`n"
" Filter: $(if (-not $b.IsDefault) { "{ $($b.Filter.ToString().Trim()) }" } else { '$null' })`n"
" Filter$(if ($b.AllowFallback) { ' (with fallback)' }): $(if (-not $b.IsDefault) { "{ $($b.Filter.ToString().Trim()) }" } else { '$null' })`n"
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should probably update text and/or use separate line.

" Default: $(if ($b.IsDefault) { '$true' } else { '$false' })`n"
" Verifiable: $(if ($b.Verifiable) { '$true' } else { '$false' })"
}
Expand Down
111 changes: 92 additions & 19 deletions tst/functions/Mock.Tests.ps1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -374,7 +374,7 @@ Describe 'When calling Mock, StrictMode is enabled, and variables are used in th

Describe "When calling Mock on existing function without matching bound params" {
It "Should redirect to real function" {
Mock FunctionUnderTest { return "fake results" } -parameterFilter { $param1 -eq "test" }
Mock FunctionUnderTest { return "fake results" } -parameterFilter { $param1 -eq "test" } -AllowFallback
$result = FunctionUnderTest "badTest"
$result | Should -Be "I am a real world test"
}
Expand All @@ -390,7 +390,7 @@ Describe "When calling Mock on existing function with matching bound params" {

Describe "When calling Mock on existing function without matching unbound arguments" {
It "Should redirect to real function" {
Mock FunctionUnderTestWithoutParams { return "fake results" } -parameterFilter { $param1 -eq "test" -and $args[0] -eq 'notArg0' }
Mock FunctionUnderTestWithoutParams { return "fake results" } -parameterFilter { $param1 -eq "test" -and $args[0] -eq 'notArg0' } -AllowFallback
$result = FunctionUnderTestWithoutParams -param1 "test" "arg0"
$result | Should -Be "I am a real world test with no params"
}
Expand Down Expand Up @@ -518,15 +518,15 @@ Describe 'When calling Mock on a module-internal function.' {
BeforeAll {
Mock -ModuleName TestModule InternalFunction { 'I am the mock test' }
Mock -ModuleName TestModule Start-Sleep { }
Mock -ModuleName TestModule2 InternalFunction -ParameterFilter { $args[0] -eq 'Test' } {
Mock -ModuleName TestModule2 InternalFunction -ParameterFilter { $args[0] -eq 'Test' } -AllowFallback {
"I'm the mock who's been passed parameter Test"
}
# Mock -ModuleName TestModule2 InternalFunction2 {
# # this does not work in v5 because we are running the mock in the test scope
# # so this module internal function is not accessible to the mock body
# InternalFunction 'Test'
# }
Mock -ModuleName TestModule2 Get-CallerModuleName -ParameterFilter { $false }
Mock -ModuleName TestModule2 Get-CallerModuleName -ParameterFilter { $false } -AllowFallback
Mock -ModuleName TestModule2 Get-Content { }
}

Expand Down Expand Up @@ -632,7 +632,7 @@ Describe "When Applying multiple Mocks on a single command where one has no filt
Describe "When Creating Verifiable Mock that is not called" {
Context "In the test script's scope" {
It "Should throw" {
Mock FunctionUnderTest { return "I am a verifiable test" } -Verifiable -parameterFilter { $param1 -eq "one" }
Mock FunctionUnderTest { return "I am a verifiable test" } -Verifiable -parameterFilter { $param1 -eq "one" } -AllowFallback
FunctionUnderTest "three" | Out-Null
$result = $null
try {
Expand Down Expand Up @@ -1952,9 +1952,9 @@ Describe 'Mocking a function taking input from pipeline' {
$noMockResultByProperty = $psobj | PipelineInputFunction -PipeStr 'val'
$noMockArrayResultByProperty = $psArrayobj | PipelineInputFunction -PipeStr 'val'

Mock PipelineInputFunction { write-output 'mocked' } -ParameterFilter { $PipeStr -eq 'blah' }
Mock PipelineInputFunction { write-output 'mocked' } -ParameterFilter { $PipeStr -eq 'blah' } -AllowFallback
}
context 'when calling original function with an array' {
Context 'when calling original function with an array' {
BeforeAll {
$result = @(1, 2) | PipelineInputFunction
}
Expand All @@ -1967,7 +1967,7 @@ Describe 'Mocking a function taking input from pipeline' {
}
}

context 'when calling original function with an int' {
Context 'when calling original function with an int' {
BeforeAll {
$result = 1 | PipelineInputFunction
}
Expand All @@ -1978,7 +1978,7 @@ Describe 'Mocking a function taking input from pipeline' {
}
}

context 'when calling original function with a string' {
Context 'when calling original function with a string' {
BeforeAll {
$result = '1' | PipelineInputFunction
}
Expand All @@ -1989,7 +1989,7 @@ Describe 'Mocking a function taking input from pipeline' {
}
}

context 'when calling original function and pipeline is bound by property name' {
Context 'when calling original function and pipeline is bound by property name' {
BeforeAll {
$result = $psobj | PipelineInputFunction -PipeStr 'val'
}
Expand All @@ -2001,7 +2001,7 @@ Describe 'Mocking a function taking input from pipeline' {
}
}

context 'when calling original function and forcing a parameter binding exception' {
Context 'when calling original function and forcing a parameter binding exception' {
BeforeAll {
Mock PipelineInputFunction {
if ($MyInvocation.ExpectingInput) {
Expand All @@ -2017,7 +2017,7 @@ Describe 'Mocking a function taking input from pipeline' {
}
}

context 'when calling original function and pipeline is bound by property name with array values' {
Context 'when calling original function and pipeline is bound by property name with array values' {
BeforeAll {
$result = $psArrayobj | PipelineInputFunction -PipeStr 'val'
}
Expand All @@ -2029,7 +2029,7 @@ Describe 'Mocking a function taking input from pipeline' {
}
}

context 'when calling the mocked function' {
Context 'when calling the mocked function' {
BeforeAll {
$result = 'blah' | PipelineInputFunction
}
Expand Down Expand Up @@ -2289,7 +2289,7 @@ Describe 'Nested Mock calls' {
BeforeAll {
$testDate = New-Object DateTime(2012, 6, 13)

Mock Get-Date -ParameterFilter { $null -eq $Date } {
Mock Get-Date -ParameterFilter { $null -eq $Date } -AllowFallback {
Get-Date -Date $testDate -Format o
}
}
Expand Down Expand Up @@ -2452,7 +2452,7 @@ Describe "Mocking Set-Variable" {
# we mock the command but the mock will never be triggered because
# the filter will never pass, so this mock will always call through
# to the real Set-Variable
Mock Set-Variable -ParameterFilter { $false }
Mock Set-Variable -ParameterFilter { $false } -AllowFallback

Set-Variable -Name v2 -Value 10

Expand All @@ -2467,7 +2467,7 @@ Describe "Mocking Set-Variable" {
Set-Variable -Name v1 -Value 1
$v1 | Should -Be 1 -Because "we defined it without mocking Set-Variable"

Mock Set-Variable -ParameterFilter { $false }
Mock Set-Variable -ParameterFilter { $false } -AllowFallback

Set-Variable -Name v2 -Value 11 -Scope 0
$v2 | Should -Be 11
Expand All @@ -2478,7 +2478,7 @@ Describe "Mocking Set-Variable" {
Set-Variable -Name v1 -Value 1
$v1 | Should -Be 1 -Because "we defined it without mocking Set-Variable"

Mock Set-Variable -ParameterFilter { $false }
Mock Set-Variable -ParameterFilter { $false } -AllowFallback

Set-Variable -Name v2 -Value 12 -Scope Local

Expand Down Expand Up @@ -2509,7 +2509,7 @@ Describe "Mocking Set-Variable" {
& {
# scope 1
& {
Mock Set-Variable -ParameterFilter { $false }
Mock Set-Variable -ParameterFilter { $false } -AllowFallback
Set-Variable -Name v2 -Value 11 -Scope 3
}
}
Expand All @@ -2536,7 +2536,7 @@ Describe "Mocking functions with conflicting parameters" {
$ParamToAvoid
}

Mock Get-ExampleTest { "World" } -ParameterFilter { $_ParamToAvoid -eq "Hello" }
Mock Get-ExampleTest { "World" } -ParameterFilter { $_ParamToAvoid -eq "Hello" } -AllowFallback
}

It 'executes the mock' {
Expand Down Expand Up @@ -2798,6 +2798,79 @@ Describe 'Mocking using ParameterFilter' {
}
}

Describe 'Calls not matching ParameterFilter' {
BeforeAll {
function demo ($name) {
"hello $name"
}
}

Context 'When default mock does not exist in parent scope' {
It 'Throws by default' {
Mock demo -ParameterFilter { $name -eq 'world' } -MockWith { 'mocked' }
{ demo -name 'you' } | Should -Throw
}

It 'Calls locally defined default mock' {
Mock demo -ParameterFilter { $name -eq 'world' } -MockWith { 'mocked' }
Mock demo { 'default mock' }
demo -name 'you' | Should -Be 'default mock'
}

It 'Calls real function when -AllowFallback is used' {
Mock demo -ParameterFilter { $name -eq 'world' } -MockWith { 'mocked' } -AllowFallback
demo -name 'you' | Should -Be 'hello you'
}

It 'Calls real function when at least one parameterized mock has -AllowFallback' {
# TODO: Is this expected? Or should all parameterized mocks allow fallback?
Mock demo -ParameterFilter { $name -eq 'world' } -MockWith { 'mocked' } -AllowFallback
Mock demo -ParameterFilter { $name -eq 'Wisconsin' } -MockWith { 'mocked2' }
demo -name 'you' | Should -Be 'hello you'
}
Comment on lines +2825 to +2830
Copy link
Collaborator Author

@fflaten fflaten Jul 14, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How should this work? This is where -AllowFallback/AllowOriginal/AllowPassthrough gets ugly compared to an explicit Mock demo -Throw shortcut to implement a guard mock.

}

Context 'When default mock exists in parent scope' {
BeforeAll {
Mock demo -MockWith { 'default mock' }
}
It 'Calls default mock without -AllowFallback' {
# TODO: Is this expected? Should -AllowFallback only have effect when there's no default mock at any level?
Mock demo -ParameterFilter { $name -eq 'world' } -MockWith { 'mocked' }
demo -name 'you' | Should -Be 'default mock'
}
Comment on lines +2837 to +2841
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is likely a common use case where a command returns a default result while some parameterized mocks returns special output. Good as is or any exceptions to this behavior?

}

Context 'When mock with ParameterFilter exists in parent scope' {
BeforeAll {
Mock demo -ParameterFilter { $name -eq 'world' } -MockWith { 'mocked' }
}
It 'Throws by default' {
{ demo -name 'you' } | Should -Throw
}

It 'Calls locally defined default mock' {
Mock demo { 'default mock' }
demo -name 'you' | Should -Be 'default mock'
}
}

Context 'When mock with ParameterFilter and -AllowFallback exists in parent scope' {
BeforeAll {
Mock demo -ParameterFilter { $name -eq 'world' } -MockWith { 'mocked' } -AllowFallback
}
It 'Calls real function' {
demo -name 'you' | Should -Be 'hello you'
}

It 'Throws when a more local parameterized mock does not allow fallback' -Skip {
# TODO:Do we expect this? If so, we need to expose mock scope depth from Get-AllMockBehaviors
Mock demo -ParameterFilter { $name -eq 'world' } -MockWith { 'mocked' }
{ demo -name 'you' } | Should -Throw
Comment on lines +2866 to +2869
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How should this scenario work? Should a local parameterized mock be able to block fallback to original command when a parent mock allows it?

}
}
}


if ($PSVersionTable.PSVersion.Major -ge 3) {
Describe "-RemoveParameterType" {
Expand Down
Loading