Pester icon indicating copy to clipboard operation
Pester copied to clipboard

Allow [ExcludeFromCodeCoverage()] on the param() block to exclude a function from code coverage

Open alexwnovak opened this issue 2 years ago • 6 comments

Checklist

Summary of the feature request

I don't know whether this is possible, but I hope so.

I know Pester has configurable options to exclude files, but I would love to exclude a function in the familiar way we'd do this in C#. PowerShell already allows us to use [SuppressMessageAttribute()] on the param block for PSScriptAnalyzer violations.

Bringing this configuration into the code makes it more powerful, since the author can specify the expectations--some functions shouldn't be counted in the overall coverage. Otherwise, Pester must be (separately) invoked correctly, leaving room for discrepancy.

How should it work?

Here's an example:

function Invoke-MyFunction {
    # Perform logic here that should be under test

    $value = Invoke-Dependency  # This dependency would be mocked out, but shouldn't count toward code coverage

    # Perform more logic with $value
}

function Invoke-Dependency {
    # Perform some operation. This may be a simple pass-through that's tedious/non-valuable/difficult to test.
}

alexwnovak avatar Nov 22 '22 21:11 alexwnovak

Thanks for the suggestion!

Attributes like [ExcludeFromCodeCoverage()] would require either authors to strip it during build-process or for all users to have Pester imported during runtime to be able to resolve the custom attribute. Both is a bad user experience IMO.

PSScriptAnalyzer reuse a diagnostics-attribute included in .NET itself which is always available. Technically we could reuse it or something similar, but attributes are attached to certain symbols like function definitions, properties etc. which is also a limitation with PSSA atm.

An alternative like comment-based regions (#PesterStartExcludeCC and #PesterEndExcludeCC) could be possible, but it adds additional processing to analyze every file. We could also support more granular CC inclusion/exclusion configuration in general and let third-party modules create that using their preferred method.

Waiting on feedback from the CC-wizard @nohwnd 🧙‍♂️

fflaten avatar Nov 23 '22 17:11 fflaten

The ExcludeFromCodeCoverageAttribute is part of .net standard 2.0 which was shiped with .net framework 4.6.2 (which is now the lowest supported version of the .net framework) https://learn.microsoft.com/en-us/dotnet/api/system.diagnostics.codeanalysis.excludefromcodecoverageattribute?view=netstandard-2.0

I think it's safe to use for this purpose.

FilipDeVos avatar Oct 15 '23 19:10 FilipDeVos

@FilipDeVos thanks for correcting me 🙂 Not sure how I missed that.

Waiting on @nohwnd's view on this as we don't use attributes atm. Normally we'd provide a CodeCoverage.ExcludeFunction option

The user experience should be considered with #2145 and possibly replacement for the removed hashtable-configuration for a consistent API (attribute vs comment vs configuration).

fflaten avatar Oct 16 '23 05:10 fflaten

I like it, I think it would be pretty easy to implement.

function a  {
    [System.Diagnostics.CodeAnalysis.ExcludeFromCodeCoverageAttribute()]
    param()
 }


(get-command a).ScriptBlock.Attributes
(Get-Command a).ScriptBlock.Ast.Extent.File
(get-command a).ScriptBlock.Ast.Extent.StartLineNumber
(get-command a).ScriptBlock.Ast.Extent.EndColumnNumber

If someone wants to try to implement this into either of the coverages then I can offer some advice. Lot of the code would also be found in pester 4, where similar (even more advanced functionality) exists.

nohwnd avatar Oct 31 '23 08:10 nohwnd

The high level view of code coverage is that we take note of every single place that we want to hit. I the breakpoint based coverage we put a breakpoint (🔴) to every place that we want to hit. In profiler based code coverage we make a table of locations. All these locations are our 100% coverage. Like this:

function a () { 
🔴	b
}

function b () {
🔴  "I call c:"
🔴  c
}

function c () {
🔴   "hello"
🔴  "this"
🔴  "is"
🔴  "dog"
}

This is called instrumentation. The code that does the instrumentation (finds all executable nodes, and places breakpoints), is already using AST, so all you need to do is inspect the code for exclusions, and don't instrument it. This will remove the excluded code from the 100% code coverage.

nohwnd avatar Oct 31 '23 08:10 nohwnd

Been looking a bit more on how to implement this, this way I can resolve the function from parser, and make exclude rules. The whole old infra for excluding is still in place, but is not publicly exposed, but we should avoid parsing the file twice, because that is expensive, but pieces of that existing code can be re-used I think.

The way it works now, is that we get config from user, we translate it from the new format to the other, ignoring some of the features that used to be there. The old code that has all those capabilities then runs, and figures out a list of interesting places in code (we call them breakpoints, but they are not real breakpoints). We then take this list and translate it to either real breakpoint locations, and set those breakpoints, or to ast locations, and use profiler based code coverage.

So good news is that implementing the exclusions can be done once for both types of coverage.

# in file a.ps1
function a {
    [Diagnostics.CodeAnalysis.ExcludeFromCodeCoverageAttribute()]
    param ()

    "hello"
}

# parsing to get the function from the attribute (I don't check the attribute type here yet
$pre = { $args[0] -is [System.Management.Automation.Language.CommandBaseAst] }
$errs = @()
$p = [System.Management.Automation.Language.Parser]::ParseFile("S:\t\pwsh1\a.ps1", [ref] $null, [ref] $errs)
$errs

$pre = {$args[0] -is [System.Management.Automation.Language.AttributeAst] -and $args[0].Parent -and $args[0].Parent -is [System.Management.Automation.Language.ParamBlockAst] -and $args[0].Parent.Parent -is [System.Management.Automation.Language.ScriptBlockAst] -and $args[0].Parent.Parent.Parent -is [System.Management.Automation.Language.FunctionDefinitionAst]  }
$p.FindAll($pre, $true).Parent.Parent.Parent.Parent.Statements.Name

If you as why not simply expose the additional features? I think that this attribute based approach is much better, we can already exclude whole files, and I don't think anyone wants to fiddle with specifying the exact location of their function by lines, when that can change with any code change.

nohwnd avatar Nov 06 '23 08:11 nohwnd