Pester showed me a bug in our existing build process. Can you find it?

Words: 729

Time to read: ~ 4 minutes

Continuous Improvement

Working on the goal of continuous improvement of our processes, I got given access to the PowerShell scripts for our Build Process.

Credit where credit is due, these PowerShell scripts were created by people unfamiliar with the language.

They have done a great job with their research to build scripts that do what they want so I’m not going to nit-pick or critique.

I’m just going to improve and show/teach my colleagues why it’s an improvement.

Original State

The current state of the script is monolithic.

We have a single script that defines functions and then calls them later on. All mixed in with different foreach () and Write-Hosts.

Here’s a rough outline of the script.

$param01 = $args[0]
$param02 = $args[1]
$param03 = $args[2] -replace 'randomstring'

... Generic PowerShell commands ...

function 01 {
    function 01 definition
}

function 02 {
    function 02 definition
}

function GetPreviousTag {
    function GetPreviousTag definition
}

... More generic PowerShell commands ...
... that call our GetPreviousTag function ...
... etc ...

That was it.

1 giant script.
0 tests.

Extracting Functions for Tests

Now scripts are notoriously hard to test, I’ve written about how I’ve done that before but, honestly, if you really want to know then you need to check out Jakub Jares ( blog | twitter ).

Knowing how difficult testing scripts are, the first thing I decided to do was take the functions in the script and split them out. This way they can be abstracted away and tested safely.

I also didn’t want to take on too much at one time so I choose a random function, GetPreviousTag, and only actioned that one first.

Taking a look at GetPreviousTag

The simplest test that I can think of is a pass/fail test.

What do we expect to happen when it passes and what do we expect to happen when it fails.

To figure that out we’ll need to see the GetPreviousTag function. So I copied and pasted the code over to its own file GetPreviousTag.ps1. (sanitised, of course)

function GetPreviousTag {
    # Run the "git describe" command to return the latest tag
    $lastTag = git describe
    # If no tag is present then return false
    if ([string]::IsNullOrEmpty($lastTag)) {
        return $false
    }
    else {
        # If a tag is returned then we need to ensure that its in our expected format:
        # If a commit has taken place but the tag hasn't been bumped then the git describe command will return 
        # refs/tags/1.1.0.a.1-33-gcfsxxxxx, we only want the 1.1.0.a.1 part of the tag so we split off everything after
        # the "-" and trim the "refs/tags/" text.   
        $lastTagTrimmed = $lastTag.Split("-") | Select-Object -First 1
        $lastTagTrimmed = $lastTagTrimmed -replace 'refs/tags/',''
        # Verify that last tag is now in the expected format
        if ([regex]::Match($lastTagTrimmed,'\d+\.\d+\.\d+\.\c\.\d+')) {
            return $lastTagTrimmed
        }
        else {
            return $false
        }
    }
}

It’s nicely commented and glancing through it, we can see what it does.

• Gets the output of git describe
  • If there’s no output:
    • return $false
  • If there is output:
    • Split on a dash, and get the first split
    • Remove the string ‘refs/tags/’
      • If the remainder matches the regex:
        • Return the remainder
      • If the remainder does not match the regex:
        • return $false

So we have our pass outcome, the remainder, and fail outcome, $false.

More importantly, can you see the bug?

The Pester Test

Here is the Pester test I created for the above function.

It’s relatively simple but even this simple test highlighted the bug that had gone hidden for so long.

$here = Split-Path -Parent $MyInvocation.MyCommand.Path
$sut = (Split-Path -Leaf $MyInvocation.MyCommand.Path) -replace '\.Tests\.', '.'
. "$here\$sut"

Describe "GetPreviousTag" {
    Context -Name 'Pass' -Fixture {
        Mock -CommandName git -MockWith {
            'refs/tags/1.1.0.a.1-33-gcfsxxxxx'
        }

        It -Name 'returns expected previous tag' -Test {
            GetPreviousTag | Should -BeExactly '1.1.0.a.1'
        }
    }

    Context -Name 'Fail : empty git describe' -Fixture {
        Mock -CommandName git -MockWith {}

        It -Name 'returns false' -Test {
            GetPreviousTag | Should -BeFalse
        }
    }

    Context -Name 'Fail : regex does not match' -Fixture {
        Mock -CommandName git -MockWith {
            'refs/tags/NothingToSeeHere-33-gcfsxxxxx'
        }

        It -Name 'returns false' -Test {
            GetPreviousTag | Should -BeFalse
        }
    }
}

Thanks to the above Pester test, I was able to find the bug, fix it, and also be in a position to improve the function in the future.

If you can’t find the bug, run the above test and it should show you.

Finally

If there’s one thing to take away from this post, it is to test your scripts

I’ve found Pester so useful that I decided to put my money where my mouth is…literally.

@PSPester is such an essential part of my workload! Happy to support
I’ve just donated $25 to Pester. Consider donating too, every little helps! https://t.co/jyvI5reCiP via @opencollect— Shane O’Neill (@SOZDBA) July 2, 2019

It’s more than deserved. Now back to continuous improvement…