Custom Error Message On Should-Fail

[dargmuesli]

Question

What is the suggested way to display additional error information about a test failure if available?

Think of a check for every element in a list. When there is an error, how do you know on which element it occurred?

Description

My Pester test currently looks like this:

It "has a property for each var" {
  ForEach ($Var In $VarList.GetEnumerator()) {
    $Test = Test-PropertyExists -Object $Var.Value -PropertyName "pn"

    If (-Not $Test) {
      Write-Host $Var.Key
    }

    $Test | Should Be $True
  }
}

Is there a way to make it do something along the (fewer!) lines of this?

It "has a property for each var" {
  ForEach ($Var In $VarList.GetEnumerator()) {
    Test-PropertyExists -Object $Var.Value -PropertyName "pn" | Should Be $True ElseError $Var.Key
  }
}

Or am I doing something entirely wrong? Like I could be not supposed to put a foreach in an ìt` block... I'm fairly new to Pester, so that's possible I guess ;)

[johlju]

Try changing to having the It-block inside the foreach-block. So for every value the it-block and the Should is run. And then you can change the It-block description dynamically so it says something like 'Should contain the correct value for variable X'.

[nohwnd]

I don't think creating dynamic list of tests for this is the best choice, because: 1) foreach does nothing when there are no items in the list, and test that does nothing passes. 2) you will end up with potentially huge list of tests simply because you gave it a long list. 3) generating tests dynamically makes it difficult to easily compare what fails between two test runs. Ideally you want to start with say 10 tests, add one test and have 10 passing tests and 1 failing test. Then when you break your code you want to have 11 tests in total and say 5 of them failing. But not start with 10 tests, then have 34 tests because you forgot to filter a collection, then have 18 tests but 7 of them failing becuase they don't have the correct property and so on. 4) there should be no untested conditions or loops in your unit tests

Problem 1) is the most serious though, you should always look out for places where doing nothing will let the test pass. If you do decide to put foreach in your tests do it this way, unless there is no risk in falsely passing the test/test suite run:

# test this function separately, it builds the vocabulary
# of your tests
function Assert-HasAtLeastOneItem ($collection) {
    if ($collection.Count -lt 1) {
        throw "Expected collection to have at least one item"
    }
}

Describe "foreach" {
    It "passes when you give it nothing" {
        foreach ($item in $items) 
        {
            # wrong
        }
    }
    It "fails when you give it nothing" {
        foreach ($item in (Assert-HasAtLeastOneItem $items)) 
        {
            # okay if you really need to do this
        }
    }
}

It still is a pattern I would use only when validating environments (in other words: in a situation when I use Pester as a task runner to run set of tests against unknown data), when I write unit tests I would not use this approach because I am controlling the input to my functions so I know exactly what the function should return.

---

Now back to your question 😄 All assertions work based on exceptions, so throwing an exception is the way to tell the test runner that the test failed. So you need a way to generate an exception that will summarize what happened.

The code you have in your test is not incorrect, but it is in the wrong place. Your test should only have one way to run, there should be no untested conditions or loops in your tests, in fact ideally there would be no parameters to your assertions except for one to pass the actual value in. So assuming we are testing Get-VarList function, our test would look like this:

$actual = Get-VarList 
Assert-PropertyPnExistsOnEveryObjectInCollection $actual

This assertion is extremely specialized, and that allows us to write comprehensive suite of tests for it. This in turn reassures us that once we use it in a test, it will fail when any object is missing the pn property. In other words we are extremely sure our test is correct. Which is very useful for testing environments, where we don't know what the real data will be.

Writing a specialized assertion for every test case is quite impractical, so for unit testing we normally use a more generic set of assertions that we can parametrize to suit the need of the test:

$actual = Get-VarList 
$actual | Assert-TrueForAll -ScriptBlock { Test-PropertyExists -Object $_.Value -PropertyName "pn" }

This is not without downsides though, every parameter is a potential bug in our test. If we write pm in the test instead of pn, we are now testing for the wrong thing. To mitigate the risk we follow a strict set of steps dictated by TDD. We first validate that our test fails in assertion, and then we validate that it passes.

This is only possible if we control the source of data for Get-VarList of course, such as Get-Content, so in fact our test would look more like this:

# imagine this is how the real function looks like
function Get-VarList () {
    Get-Content products.csv | ConvertFrom-Csv -Delimiter ',' 
}

#our test stabilizes the input to be able to validate the result
Mock Get-Content { "pn,price`nMilk,10`nCoffee,20" }

$actual = Get-VarList 

$actual | Assert-TrueForAll -ScriptBlock { Test-PropertyExists -Object $_.Value -PropertyName "pn" }

So all you need to do now is to write Assert-TrueForAll 😄

Luckily, I had the same problem, so I decided to write my own set of assertions that will allow me do stuff like that, so my test would probably look like this:

# functions like this go in your test helper library and must be tested! 
function Test-Property ($InputObject, $PropertyName) {
    $InputObject.PsObject.Properties.Match($PropertyName)
}

# imagine this is how the real function looks like
function Get-VarList () {
    Get-Content products.csv | ConvertFrom-Csv -Delimiter ',' 
}

Import-Module Assert

Describe "Get-VarList" { 
    It "All objects have property '<property>'" -TestCases @(
        @{Property = "pn"}
        @{Property = "id"}
    ) {
        param ($Property)

        # -- Arrange
        Mock Get-Content { "pn,price`nMilk,10`nCoffee,20" }

        # -- Act
        $actual = Get-VarList 

        # -- Assert
        $actual | Assert-All -FilterScript { Test-Property -InputObject $_ -PropertyName $Property }
    }
}

I am still working on those assertions, and it would be great if you used them in your project and reported back about stuff you like, don't like, or find unexpected or confusing.

[it-praktyk]

1. foreach does nothing when there are no items in the list, and test that does nothing passes.

To handle it Set-TestInconclusive can be used.

[dargmuesli]

Thank you so much for your detailed answers!

Later, I will try to understand and implement everything in detail and report back to you about it. Until then you are free to go on with suggestions ;)

[dargmuesli]

Ok, I tried your suggestion now @nohwnd. The problem is, that I have varying input content that is to be checked. That means a varying $actual in your example. I cannot mock it, because I need to check every element of that list.

I will definitly use your assert library for all other cases, but for this one it seems that I must follow @johlju's advice. It's the same as putting the list as testcases by the way if I get that right.

I'm trying to do something similar to Using Pester to test your Comment Based Help, which is pretty much out of scope for unit test, I guess. But I think many others will find your assertion library useful :)

[nohwnd]

@Dargmuesli I understand, and I would like to know what is your scenario, because the code example that you gave was obviously too simplistic. 🙂Are you validating environment or doing unit tests? Who provides the varList? Could you maybe come up with an example that is closer to the real thing, but still simple enough for me to understand. Because then I can comment on it a bit more, and post it for others to read :)

[dargmuesli]

Just similar to the article linked above, I want to validate the correct alphabetical order of functions in my modules and a correct .LINK (comment based help) attribute. You can see the changes I've chosen to apply in my latest commit @ Dargmuesli/powershell-lib. Or have a look at this code snippet for the current version:

Set-StrictMode -Version Latest

Import-Module -Name "${PSScriptRoot}\..\..\PowerShell-Lib\PowerShell-Lib.psd1" -Force

Describe "PowerShell-Lib" {
    $NestedModules = (Get-Module PowerShell-Lib).NestedModules

    ForEach ($NestedModule In $NestedModules) {
        Context $NestedModule {
            $ModuleFileContent = $Null
            $ModuleFunctionNames = $Null
            $ModuleFunctionNamesSorted = $Null

            Import-Module -Name $NestedModule.Path -Force

            $ModuleFileContent = Get-Content -Path $NestedModule.Path -Raw
            $ModuleFunctionNames = [Object[]] (Read-FunctionNames -InputString $ModuleFileContent)
            $ModuleFunctionNamesSorted = [Object[]] ($ModuleFunctionNames | Sort-Object)

            It "contains functions in alphabetical order" {
                $ModuleFunctionNames = $ModuleFunctionNames
                $ModuleFunctionNamesSorted = $ModuleFunctionNamesSorted

                For ($I = 0; $I -Lt $ModuleFunctionNames.Length; $I++) {
                    $ModuleFunctionNamesSorted.Get_Item($I) | Should Be $ModuleFunctionNames.Get_Item($I)
                }
            }

            ForEach ($ModuleFunctionName In $ModuleFunctionNamesSorted) {
                $ModuleFunctionsHelp = Get-Help -Name $ModuleFunctionName -Full

                It "has a correct `".LINK`" for function $($ModuleFunctionsHelp.Name)" {
                    Test-PropertyExists -Object $ModuleFunctionsHelp -PropertyName "relatedLinks.navigationLink.uri" |
                        Should Be $True

                    $ModuleFunctionsHelp.relatedLinks.navigationLink.uri |
                        Should Match "^https:\/\/github\.com\/Dargmuesli\/powershell-lib\/blob\/master\/Docs\/$($ModuleFunctionsHelp.Name)\.md$"
                }
            }
        }
    }
}

This is the version that generates an "it" for every function. That results in a fairly long test list just for that, but that's the most convenient way to solve this problem, I think.

I could try to make a list out of $ModuleFunctionsHelp and use your assert-approach (maybe together with testcases?), but any error message prints the whole source list containing all help information about all functions of a module. That's not really user-friendly ;) What I can try to do is to filter the help information to only get relatedLinks.navigationLink.uri, but the list is still going to be long.

So, again, my suggestion would be to have something like:

It "has a correct `".LINK`" for every function" {
    ForEach ($ModuleFunctionName In $ModuleFunctionNamesSorted) {
        $ModuleFunctionsHelp = Get-Help -Name $ModuleFunctionName -Full

        Test-PropertyExists -Object $ModuleFunctionsHelp -PropertyName "relatedLinks.navigationLink.uri" |
            Should Be $True ElseError "Property `"relatedLinks.navigationLink.uri`" does not exist for function `"$($ModuleFunctionsHelp.Name)`""

        $ModuleFunctionsHelp.relatedLinks.navigationLink.uri |
            Should Match "^https:\/\/github\.com\/Dargmuesli\/powershell-lib\/blob\/master\/Docs\/$($ModuleFunctionsHelp.Name)\.md$" #nextline
            ElseError "Function `"$($ModuleFunctionsHelp.Name)`" does not have a valid .NOTE"
    }
}

But, as I said before, I guess that is out of scope for unit tests.

[nohwnd]

You cannot provide custom error messages to Should, but my assertions allow you to do that. I just don't have Match assertion yet, but it should be simple to add :)

If you want to do foreach then put it around It, otherwise it will fail on first error. In the future you might be able to use combined assertions.

My purist side does not like generating tests, but in this case I agree this gives you little to no maintenance, and has very little risk associated with it. I would protect the foreach though, you can assume your module publishes at least one function, why have it otherwise.

I would also test the match separately and use it as constant, if not as custom assertion (for that I will also add Assert-ThrowAssertionException assertion, so you can easily wrap my asssertions in your customized assertions and test them, the axioms in my library do the same thing, follow this issue if you are interested).

Anyways I guess your problem is sorted out now, thanks for sharing the info. Re-open if you need more assistance, or have more related stuff to discuss.