Is there a better way than global variables to share minimal state between mocks?

[alx9r]

I've been writing idempotent functions which would become the building-blocks of DSC resources. The following code demonstrates a prototypical module and test fixture.

Module.psm1

function Test-Widget {}
function New-Widget  {}
function Set-Widget
{
    [CmdletBinding()]
    param ()
    process
    {
        if ( -not (Test-Widget) )
        {
            New-Widget

            if ( -not (Test-Widget))
            {
                return $false
            }
        }

        return $true
    }
}

Modules.Tests.ps1

Import-Module module

InModuleScope module {
    Describe 'simple state shared between mocks.' {
        BeforeEach {         $Global:widgetExists       = $false
                             $Global:widgetIncorrigible = $false }
        AfterAll   { Remove-Variable widgetExists -Scope Global  
                     Remove-Variable widgetIncorrigible -Scope Global }
        Mock -Verifiable New-Widget {
            if ( -not ($Global:widgetIncorrigible) )
            {
                $Global:widgetExists = $true
            }
        }
        Mock -Verifiable Test-Widget {
            return $widgetExists
        }

        Context 'widget exists.' {
            It 'returns true.' {
                $Global:widgetExists = $true

                Set-Widget | Should be $true
            }
            It 'calls Test-Widget once.' {
                Assert-MockCalled Test-Widget -Exactly -Times 1
            }
            It 'does not call New-Widget.' {
                Assert-MockCalled New-Widget  -Exactly -Times 0
            }
        }
        Context 'widget doesn''t exist.' {
            It 'returns true.' {
                Set-Widget | Should be $true
            }
            It 'calls Test-Widget twice.' {
                Assert-MockCalled Test-Widget -Exactly -Times 2
            }
            It 'calls New-Widget once.' {
                Assert-MockCalled New-Widget  -Exactly -Times 1
            }
        }
        Context 'widget can''t be created.' {
            It 'returns false.' {
                $Global:widgetIncorrigible = $true
                Set-Widget | Should be $false
            }
            It 'calls Test-Widget twice.' {
                Assert-MockCalled Test-Widget -Exactly -Times 2
            }
            It 'calls New-Widget once.' {
                Assert-MockCalled New-Widget  -Exactly -Times 1
            }
        }
    }
}

Mock State Variables

To get complete code coverage, Test-Widget gets called twice with different return values depending on the setup. The result of Test-Widget might also change depending on whether New-Widget has been called yet. The most obvious way to achieve this is using variables that can be accessed from the New- and Test- mocks and the It block.

The least bad way I have found to do this is using global variables. Is there a better way to do this? I generally don't like to use global variables because you risk defeating isolation between tests whenever you use them.

[dlwyatt]

Rather than using global variables, I prefer to set up a hashtable as the container for my values, and modify that instead. This is a subtle difference; instead of writing to a PowerShell variable (which is scoped to the script block where the write happens, unless you do something like $script: or $global:, you're only reading the $hash value, which will resolve in the parent scopes. Once that happens, you can fiddle with the key/value pairs in the hashtable however you like.

Example:

InModuleScope module {
    Describe 'simple state shared between mocks.' {
        BeforeEach {
            $hash = @{
                widgetExists       = $false
                widgetIncorrigible = $false
            }
        }

        Mock -Verifiable New-Widget {
            if ( -not ($hash.widgetIncorrigible) )
            {
                $hash.widgetExists = $true
            }
        }
        Mock -Verifiable Test-Widget {
            return $hash.widgetExists
        }

        Context 'widget exists.' {
            It 'returns true.' {
                $hash.widgetExists = $true

                Set-Widget | Should be $true
            }
            It 'calls Test-Widget once.' {
                Assert-MockCalled Test-Widget -Exactly -Times 1
            }
            It 'does not call New-Widget.' {
                Assert-MockCalled New-Widget  -Exactly -Times 0
            }
        }
        Context 'widget doesn''t exist.' {
            It 'returns true.' {
                Set-Widget | Should be $true
            }
            It 'calls Test-Widget twice.' {
                Assert-MockCalled Test-Widget -Exactly -Times 2
            }
            It 'calls New-Widget once.' {
                Assert-MockCalled New-Widget  -Exactly -Times 1
            }
        }
        Context 'widget can''t be created.' {
            It 'returns false.' {
                $hash.widgetIncorrigible = $true
                Set-Widget | Should be $false
            }
            It 'calls Test-Widget twice.' {
                Assert-MockCalled Test-Widget -Exactly -Times 2
            }
            It 'calls New-Widget once.' {
                Assert-MockCalled New-Widget  -Exactly -Times 1
            }
        }
    }
}

[dlwyatt]

Incidentally, I use this same technique when I want to have assertions for both Should Not Throw and Should Be on the same function call:

Describe 'Something' {
    It 'Works' {
        $hash = @{}
        $scriptBlock = { $hash.Result = Do-Something }

        $scriptBlock | Should Not Throw
        $hash.Result | Should Be 'Something'
    }
}

[alx9r]

This makes good sense. Thanks very much @dlwyatt.

[nohwnd]

You are mixing two concerns in your Set-Widget function, that's why it's hard to test, because it creates the Widget and also Tests if it exists. If you implemented it like this:

function Get-Widget { 
    #here you'd find another boundary like Get-Item, that you'd be able to mock
    #and you'd test Get-Widget by mocking or faking it
}

function Test-Widget {
    [bool](Get-Widget)
}
function New-Widget  {
    #here you'd find another boundary, like New-Item, that you'd be able to mock
    #here and you'd test Get-Widget by mocking or faking it

}
function Set-Widget
{
    process
    {
        if (-not (Test-Widget)) 
        {
           New-Widget
        }

        Get-Widget
    }
}

Describe 'Test-Widget' {
    It 'Returns true if widget exists' {
        Mock Get-Widget {"widget"}

        Test-Widget | Should Be $true
    }

    It 'Returns false if widget does not exiss' {
        Mock Get-Widget {}

        Test-Widget | Should Be $false
    }
}

Describe 'Set-Widget' {
    It 'Widget is created when it does not exist' {
        Mock Test-Widget { $false }
        Mock New-Widget {}

        Set-Widget

        Assert-MockCalled New-Widget -Exactly 1 -Scope It
    }

    It 'Returns the current widget if it already exists' {
        Mock New-Widget {} # prevents test side-effects if accidentally called

        Mock Test-Widget { $true }
        Mock Get-Widget { "widget" }

        Set-Widget | Should Be "widget"
    }   
}

Your test would not be so coupled to the code and you would not need to share state between mocks.

[alx9r]

@nohwnd My example was meant to demonstrate explicit call history tracking, and nothing else. Are you implying there should never be a need to make multiple calls to the same mock from a function-under-test?

[nohwnd]

@alx9r I am implying that rather than coming up with ways to share the state between mocks so you can reuse them for all your tests cases. You should make your code and tests simpler by using different, more testable, design. And multiple local Mocks.

But to answer your question: I am reluctant to say never, but it should be very rare. Especially in unit tests. But I can imagine an integration test where you try to simulate some interaction with an external system and you have to simulate a different states based on incoming requests. But for that I would use an object that would have logic to simulate the actual system. Like this:

function Test-Widget {}
function New-Widget  {}
function Set-Widget
{
    [CmdletBinding()]
    param ()
    process
    {
        if ( -not (Test-Widget) )
        {
            New-Widget

            if ( -not (Test-Widget))
            {
                return $false
            }
        }

        return $true
    }
}

Describe 'simple state shared between mocks.' {
    Context 'widget exists.' {
        Mock New-Widget {} #prevents side-effects

        Mock Test-Widget { $true }

        It 'returns true.' {
            Set-Widget | Should be $true
        }

        It 'calls Test-Widget once.' { 
            # this operation has no side-effects you don't have to care how many times it was called
            #I would remove this test
            Assert-MockCalled Test-Widget -Exactly -Times 1
        }

        It 'does not call New-Widget.' {
            Assert-MockCalled New-Widget  -Exactly -Times 0
        }
    }

    Context 'widget doesn''t exist and can be created.' {
        $widgetFake = New-Module -AsCustomObject -Name "StateObject" {
            $script:exists = $false
            function Create () {
                $Script:exists = $true
            }

            function Exists() {
                $script:exists
            }
        }

        Mock New-Widget { $widgetFake.Create() } 
        Mock Test-Widget { $widgetFake.Exists() }

        It 'returns true.' {
            Set-Widget | Should be $true
        }
        It 'calls Test-Widget twice.' {
            Assert-MockCalled Test-Widget -Exactly -Times 2
        }
        It 'calls New-Widget once.' {
            Assert-MockCalled New-Widget  -Exactly -Times 1
        }
    }

    Context 'widget doesn''t exist and can''t be created.' {        
        Mock New-Widget { } #cant be created 
        Mock Test-Widget { $false }

        It 'returns false.' {
            Set-Widget | Should be $false
        }
        It 'calls Test-Widget twice.' {
            Assert-MockCalled Test-Widget -Exactly -Times 2
        }
        It 'calls New-Widget once.' {
            Assert-MockCalled New-Widget  -Exactly -Times 1
        }
    }
}

Please notice I am testing your original logic, and I only need to share the state for a single test case.

I also commented on your other issue about parameter values in mock, showing that no explicit mock history is needed there: https://github.com/pester/Pester/issues/325#issuecomment-97201503 hope this will make the bigger picture clearer and will help you make your tests simpler.

[alx9r]

@nohwnd OK, I get your point and I agree that, ideally, mocks live close to the test cases and are as simple as they can be for that particular test case. It does seem, however, that explicitly defining new mocks for each Context results in more repetition as the number of test cases that use the same mock behavior increases.

[nohwnd]

@alx9r I think that's is fine. DRY (Don't Repeat Yourself) is not priority for test code. Readability is. If you introduce some mocks here and some mocks there, you have to think about scopes and your tests are becoming more complex.

Plus if you start introducing logic in your code you will soon arrive at a point where you need tests to test your test code. Correctly I should've written a few tests for the "widget state object":

function New-WidgetFake
{ 
    New-Module -AsCustomObject -Name "StateObject" {
        [bool]$script:exists = $false
        function Create() {
            $Script:exists = $true
        }

        function Exists() {
            $script:exists
        }
    }
}

Describe 'WidgetFake' {
    It 'Fresh Widgetfake indicated that widget does not exist' {
        $fake = New-WidgetFake

        $fake.Exists() | Should Be $false
    }

    It 'Creating widget sets its internal state to true' {
         $fake = New-WidgetFake

         $fake.Create()

         $fake.Exists() | Should Be $true
    }
}

[alx9r]

Some argue that you should " favor DRY in production code, favor DAMP in test code", which makes sense to me.

[nohwnd]

@alx9r Yup, so we agree? :)