Should-Throw: How to negate the evaluation?

[johlju]

Checklist

• Issue has a meaningful title
• I have searched the existing issues. See all issues
• I have tested using the latest version of Pester. See Installation and update guide.

What is the issue?

In Pester 5 syntax we could say Should -Not -Throw. I can't find a way to negate the Should-Throw command as it does not have a Not parameter or a command Should-NotThrow.

Expected Behavior

Possibility to evaluate a command to not throw.

Steps To Reproduce

not applicable

Describe your environment

Pester version     : 6.0.0-alpha3 /Users/johlju/source/PesterConverter/output/RequiredModules/Pester/6.0.0/Pester.psm1  
PowerShell version : 7.4.3
OS version         : Unix 14.5.0

Possible Solution?

Either add a Not parameter to the command Should-Throw or add a new command Should-NotThrow?

[fflaten]

-Not -Throw is left out intentionally atm. We kinda discussed this on discord in May when you requested more verbose error output. 🙂

From #testing:

How about running the code without Should -Not -Throw? The exception itself will fail the test and show full stack. Not throw mostly provides a simple error message which you found too simple.

Did you find a scenario where Should -Not -Throw still makes sense?

[johlju]

We kinda discussed this on discord in May

Right... remember that. 🙂 Didn't thought it would apply here. 😉

Did you find a scenario where Should -Not -Throw still makes sense?

Only when automating conversion of test scripts. E.g. only in SqlServerDsc there are 788 lines using the syntax
{ ... } | Should -Not -Throw. Changing them to not pass a scriptblock through the pipeline and not use -Not manually would take a long time. I have now code ready that can easily convert Pester 5 syntax to either Should-NotThrow or Should-Throw with -Not. If we are not considering adding either the command Should-NotThrow or have -Not added to Should-Throw then I need to look into code that can convert all { ... } | Should -Not -Throw tests so they just run the code inside the scriptblock and without passing it to Should-*. 🤔

[fflaten]

AFAIK it's not set in stone, but by starting without it we get the discussion. 🙂

If it can be covered by a simple migration step trough docs, made even easier by your automated module, that'd be cool.

The easiest conversion is probably to replace Should -Not -Throw with ForEach-Object { & $_ } | Out-Null as it would also support the undocumented support for multiple scriptblocks through pipeline. E.g.

# Defining outside to be available for test in the end
$scriptBlock = { throw 'one' }

# Intentionally failing to confirm they actually executed without syntax errors
$source = {
    $scriptBlock | Should -Not -Throw

    { throw 'two' } | Should -Not -Throw -Because 'why not'

    { throw 'three' } |
    Should -Not -Throw -Because 'why not' -ExpectedMessage 'ignore'

    Should -Not -Throw -Because 'why not' -ExpectedMessage 'ignore' -ActualValue { throw 'four' }

    # I'm sure there's one out there :D
    "throw 'five'" | ForEach-Object { [scriptblock]::Create($_) } | should -Throw -Not

    # Fails on second block. Hope nobody use this
    { 'works' }, { throw 'six' } | Should -Throw -Not
}

$Ast = $source.Ast
$todo = $Ast.FindAll({ param($a) $a -is [System.Management.Automation.Language.CommandAst] -and $a.GetCommandName() -eq 'Should' }, $true)
$todo | ForEach-Object {
    $pipelineElements = $_.Parent.PipelineElements
    $actualValue = $null
    if ($pipelineElements.Count -eq 1) {
        # Should -Not -Throw -ActualValue { something }
        # AFAIK positional doesn't work so being naive. 99% probably use pipeline input like docs
        for ($i = 0; $i -lt $_.CommandElements.Count; $i++) {
            $p = $_.CommandElements[$i]
            if ($p -is [System.Management.Automation.Language.CommandParameterAst] -and $p.ParameterName -like 'A*') {
                $actualValue =$_.CommandElements[$i + 1].Extent.Text
                break
            }
        }
    } elseif ($pipelineElements.Count -gt 1 -and $pipelineElements[-1] -eq $_) {
        # something | maybe something else | Should -Not -Throw
        $replaceStart = $_.Extent.StartOffset - $_.Parent.Extent.StartOffset
        $actualValue = $_.Parent.Extent.Text.Remove($replaceStart) -replace '\s*\|\s*$'
    } else {
        throw 'What sorcery is this?'
    }

    # New parent extent
    $newParentExtentText = "$($actualValue) | ForEach-Object { & `$_ } | Out-Null # TODO: Cleanup. Automatic conversion from Should -Not -Throw"
    # Test
    [System.Management.Automation.Language.Parser]::ParseInput($newParentExtentText, [ref]$null, [ref]$null).GetScriptBlock().Invoke()
}

[johlju]

Thanks for the code example, and the suggestions for use of ForEach-Object, did not think of that. It is the safest (only) option if scriptblocks are passed as variables to Should. Maybe I will try to have both options 1) using ForEach-Object when there are a variable and 2) if a scriptblock, parse out the code in the scriptblock so it can run directly in the It-block as it is currently meant to do in Pester 6 alpha. 🤔 I will see how it works out. 🙂

Btw. using ForEach-Object outputs a bit more line numbers (first one) then having the code directly in the scriptblock (second one). So to avoid using ForEach-Object would be cleaner I think. 🤔

It 'Should not throw, passing to ForEach-Object' {
    {
        Write-Error -Message 'MockErrorMessage' -ErrorId 'MockErrorId' -Category 'InvalidOperation' -TargetObject 'MockTargetObject' -ErrorAction 'Stop'
    } | ForEach-Object -Process { & $_ } | Out-Null
}

It 'Should not throw, without any scriptblock' {
    Write-Error -Message 'MockErrorMessage' -ErrorId 'MockErrorId' -Category 'InvalidOperation' -TargetObject 'MockTargetObject' -ErrorAction 'Stop'
}

Outputs:

[-] Should not throw, passing to ForEach-Object 19ms (14ms|5ms)
 WriteErrorException: MockErrorMessage
 at <ScriptBlock>, /Users/johlju/source/PesterConverter/tests/Unit/Pester6.tests.ps1:42
 at <ScriptBlock>, /Users/johlju/source/PesterConverter/tests/Unit/Pester6.tests.ps1:43
 at <ScriptBlock>, /Users/johlju/source/PesterConverter/tests/Unit/Pester6.tests.ps1:39
[-] Should not throw, without any scriptblock 10ms (6ms|4ms)
 WriteErrorException: MockErrorMessage
 at <ScriptBlock>, /Users/johlju/source/PesterConverter/tests/Unit/Pester6.tests.ps1:57

[fflaten]

You can invoke a scriptblock-variable with call operator &/.. E.g. $null = & (<actualvalue>) should work for variables, script blocks and expressions (note parentheses)

Only reason I didn't use it was the edge case of multiple script blocks, which you can probably ignore.

Running the code directly will send data to StandardOutput in the Test-object. Not a big deal unless there's a lot of output, but might as well assign it to null like -Throw.

[johlju]

Then I go with $null = & (<actualvalue>), that will simplify and generalize it and avoid running code directly in the It-block to not mess up the Test-object (there could probably be a lot of output). 🙂