pester / Pester

Pester is the ubiquitous test and mock framework for PowerShell.
https://pester.dev/
Other
3.08k stars 470 forks source link

Checking a proper errors #673

Closed iSazonov closed 7 years ago

iSazonov commented 7 years ago

In Powerhell Core project, we often use the following template for checking a proper errors:

it "Get-Item on a nonexisting file should have error PathNotFound" {
    try
    {
        get-item "ThisFileCannotPossiblyExist" -ErrorAction Stop
        throw "No Exception!"
    }
    catch
    {
        $_.FullyQualifiedErrorId | should be "PathNotFound,Microsoft.PowerShell.Commands.GetItemCommand"
    }
}

It would be useful to implement something like:

{  get-item "ThisFileCannotPossiblyExist" -ErrorAction Stop } | Should Throw "PathNotFound,Microsoft.PowerShell.Commands.GetItemCommand"

or

{  get-item "ThisFileCannotPossiblyExist" -ErrorAction Stop } | Should FullyQualifiedErrorId "PathNotFound,Microsoft.PowerShell.Commands.GetItemCommand"

or

{  get-item "ThisFileCannotPossiblyExist" -ErrorAction Stop } | Should Throw { $_.FullyQualifiedErrorId -eq "PathNotFound,Microsoft.PowerShell.Commands.GetItemCommand" }
nohwnd commented 7 years ago

More extensibility for built in assertions is coming in pester 4, which I am currently piloting with sqldbatools, so hopefully it will be released soon.

BUT there is nothing preventing you from writing your own assertions.

So I would recomend that you do not use that template, it is introducing untested logic into your test scripts. I can see right away that you have potential bug in there. A much better option is to write your own assertion. And write tests for it!

Here is a quick prototype to get you started:

function ShouldThrowException
{
   param (
   [Parameter(Mandatory=$true, ValueFromPipeline=$true)]
   [scriptblock] $ScriptBlock, [type] $ExpectedExceptionType = [exception], $ExpectedExceptionMessage, $ExpectedFullyQualifiedErrorId) 

   $exceptionThrown = $false
   try {
        $null = &$ScriptBlock
   }
   catch {
        $exceptionThrown = $true
        # you might need to work here a bit to extract the error info correctly, there are some edge cases, feel free to look in pester internals and reuse that code
        $ex = $_.Exception
        if ($ex -isnot $ExpectedExceptionType)
        { throw "Expected an exception of type $($ExpectedExceptionType.Name) to be thrown, but exception of type '$($ex.GetType().Name) was thrown instead" }
        #... etc, rest of your criteria in the same manner
   }

   if (-not $exceptionThrown)
   {
       throw "Expected an exception to be thrown, but no exception was thrown"
   }
}

describe "my assert" {
    it "fails because nothing was thrown" {
        {"10"} | ShouldThrowException 
    }
    it "passes because an exception was thrown" {
        {throw "this code fails"} | ShouldThrowException 
    }

    it "fails because exception of a wrong type was thrown" {
        {throw "this code fails"} | ShouldThrowException -ExpectedExceptionType InvalidOperationException
    }

}

When writing tests for this, it's best to keep the throwing and the internal logic separeted, write a function that returns a result object { passed: $false; Errors: "Expected an exception to be thrown, but no exception was thrown" } and then another one that will just take the result and throw the apropriate exception if it fails. That way you will have much easier time testing.

iSazonov commented 7 years ago

@nohwnd Thanks for great comments! Please clarify:

  1. "will be released soon" - Is it a month? Year?
  2. Can we copy-past this new Pester 4 assertion as temporary function to later easier to migrate to Pester 4? Or can you publish "API" for new "should throw" assertion so we can now write the temporary function and easy migrate to Pester 4?
nohwnd commented 7 years ago
  1. The version 4 has been idling in our development branch for some time. At the moment I am piloting it on two projects to make sure it's not broken and I will be asking the community to jump on and test it as well. So if everything is fine and no major bugs are found it will be released in few weeks. In the meantime you can have a look at the DevelopmentV4 branch and try to run it against your test suite, chances are everything will just work, Dave did a great job at keeping stuff backwards compatible. (The module version on the v4 branch is currently 3.4.4 so don't get alarmed by that. I'd love to mark it as 4.0.0-rc1 but module manifests don't support that.)

  2. I am not sure how feasible it is to take v4 assertion and move it to v3, but I doubt it's easier than rolling your own assertion, as shown in the code snippet I posted in the first response.

The syntax in v4 did not change much, the positional parameters became dynamic parameters on the should command, so all you need to do is add dashes in front of your throws and nots. The old syntax is still possible though so this can be gradual.

{scriptblock} | Should -Throw [nottyped] -Not [string]-ExpectedMessage "what I expect the message to be" [string]-Message "my own message for the assertion" [string]-FullyQualifiedErrorId "fqeid" [Type]-ExceptionType ArgumentException`

The last two switches are not implemented yet, but the exception type will match exceptions of the given type and any derived types (ArgumentException will match ArgumentException as well as ArgumentNullException). You can see the new syntax in use in Should tests.

One more thing. Prompted by this discussions, other discussions and my own frustration with Pester assertions I started writing a set of assertions that aim to be more consistent, easily discoverable and offer richer functionality than the built-in Pester assertions. Those will be Pester v3 and v4 compatible, and can offer you the extended functionality that Pester v4 assertions can have. I will release those as soon as I feel I have something worth using, to see what the community thinks (so in few days).

That would then allow you to import my assertion module and write tests like this:

It "Get-Item on a nonexisting file should have error PathNotFound" {
    { Get-Item "ThisFileCannotPossiblyExist" } | 
        Should-Throw -FullyQualifiedErrorId "PathNotFound,Microsoft.PowerShell.Commands.GetItemCommand"
}

Pretty much the same way you would in Pester v4, but in all Pester versions. Then if people see my assertion module as a valuable tool, and help me stabilize it, I can make it part of Pester v5.

nohwnd commented 7 years ago

@iSazonov #683 v4 RC is out, please run it on your testbase to see if it works.

iSazonov commented 7 years ago

@nohwnd Thanks for great news! Please clarify. Currently Powershell Core repo use old Pester fork because of porting to Unix. I do not know whether these local changes was moved here. Whether the current Pester version a Unix (Linux, Ubuntu) compatible?

nohwnd commented 7 years ago

@iSazonov Aha okay, then disregard this, Pester v3 nor v4 is compatible with that fork. That is the next step.

it-praktyk commented 7 years ago

@iSazonov, we are close not to release Pester v.4. Can you test the newest version?

iSazonov commented 7 years ago

@vors is already opened https://github.com/PowerShell/PowerShell/pull/4618 and we see some problems.

it-praktyk commented 7 years ago

Due to lack of additional information, the issue is closed now. Please feel free to open when you feel that your question or request was not handled correctly.