Fix line-based filter for tests with multiline testcases

[fflaten]

1. General summary of the pull request

"Run Test" in VS Code doesn't work for running a single test which contains multiple lines to define testcases. The issue is caused by differences in how VS Code would identify the line number for a test compared to Pester's logic. Pester uses the start of scriptblock as the line number, while VS Code uses the line of It. VS Code's approach is preferred going forward as it's more consistent with other scenarios like describe, context and tests without testcases as well as being easier for an end user.

It "Test with multiline testcases" -TestCases @(
    @{Value = 1}
    @{Value = 2}
    ) {
        1 | Should -Be 1
    }
}

# Pester's expectation of linefilter for the test above
# Before fix: filepath:4 (start of scriptblock)
# After fix: filepath:1 (line of "It")

This PR changes the linefilter-check to use a new "StartLine" property in Pester-objects of type Block (Describe/Context) and Test (It) which stores the location of the function name. This should solve the issue mentioned above.

Fix #1659
Original issue: PowerShell/vscode-powershell#2880

• Extend Block/Test.cs
• Update line filter
• Add test for multiline testcases
• Fix failing tests
• Update affected output functions: Get-WriteScreenPlugin

[fflaten]

Would this be considered a breaking fix considering someone might have scripts using line filter based on the current behaviour?

[nohwnd]

@fflaten I don't think that it is breaking change, I doubt anyone was using it apart from VSCode and our own tests.

[fflaten]

Instead of specifying StartLine as a parameter to New-Test/New-Block like -StartLine $MyInvocation.ScriptLineNumber, we could use Get-PSCallStack inside those functions to dynamically get the first external method-call. This would probably work with future methods as well as doesn't require a change to It/Context/Describe.

# DEMO
function New-Test {
...

#$script:PSCommandPath = Pester.psm1 location
$caller = (& $SafeCommands["Get-PSCallStack"] | & $SafeCommands["Where-Object"] -FilterScript { $_.InvocationInfo.PSCommandPath -ne $script:PSCommandPath })[0].InvocationInfo
#Write-Host "New-Test called from command '$($caller.MyCommand)' in $($caller.ScriptName):$($caller.ScriptLineNumber). Line '$($caller.Line)'."

$test = [Pester.Test]::New()
$test.StartLine = $caller.ScriptLineNumber
...

Implemented it here to test. Thoughts?
Personally like it better. It requires a lot of new calls to read the callstack, but that doesn't seem to slow down the execution time for the projects test-suite that much (<0.5 sec on my computer). However, it won't work for any alternatives to "It/Describe/Context" provided by an external module as the callstack filter stops at first invocation outside Pester.psm1. Is that a scenario we need to cover? If so, I'd have to reverse 134f088 anyways to provide a fallback.

[nohwnd]

Get-PSCallstack is very slow, please don't use it.

[nohwnd]

Converted it to a Draft PR, I always forget to remove the WIP: prefix. Please let me know when it's ready for review & merge.

[fflaten]

Cool. What about the last part in my previous comment. Do we need to revert 134f088 to stay backward compatible with third-party implementations of Block/Test or is it probably not needed?

That's the only thing outstanding AFAIK. Not sure why the PR fails some checks. Feels like the PS4 host is failing random PTests.

[nohwnd]

You don't have to revert it, I don't think there are any such implementations.

[nohwnd]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[nohwnd]

Just two suggestions, to make the parameter mandatory, to prevent us from errors, and to let possible external implementations (but there are probably none), that that this is needed.

[fflaten]

Just two suggestions, to make the parameter mandatory, to prevent us from errors, and to let possible external implementations (but there are probably none), that that this is needed.

I actually avoided that due to all the tests using New-Block/Test directly 🙂

[nohwnd]

Duh!

[nohwnd]

Tried it with this code on VSCode and it works perfect:

Describe "d1" `
{
    Context "c1" `
    {
        It "User with guard" `
        {
            1 | Should -Be 2
        }
    }
}

One more tiny suggestion added in comments.