Skip to content

Invoke-PSBuildModuleSigning has no test that calls it #217

Description

@tablackburn

tests/Invoke-PSBuildModuleSigning.tests.ps1 has eleven passing tests and none of them call Invoke-PSBuildModuleSigning. The function's body has never executed.

What happens

Two tests are named as though they exercise the function's file discovery:

It 'Searches for files matching Include patterns' {
    ...
    Mock Set-AuthenticodeSignature { [PSCustomObject]@{ Status = 'Valid'; Path = $InputObject } }

    # We need to skip this test if we can't create a real cert, or just verify file discovery
    # Instead of mocking cert, just count the files that would be signed
    $files = Get-ChildItem -Path $testDir -Recurse -Include '*.psd1', '*.psm1', '*.ps1'
    $files.Count | Should -Be 3  # Should not include .txt file
}

The Get-ChildItem call is in the test body. It asserts that Get-ChildItem filters by -Include, which is a fact about PowerShell, not about this module. Uses custom Include patterns when specified does the same thing with a one-element pattern list.

The Mock Set-AuthenticodeSignature in the first of the two is declared and then never used. It is also unscoped — no -ModuleName 'PowerShellBuild' — so it could not have reached the call inside the function even if the function had been called. That is the same defect as the six mocks in the sibling certificate test file.

The remaining nine tests inspect Get-Command metadata: that the command is exported, that its help has a synopsis and an example, that Path and Certificate are mandatory, that HashAlgorithm has the expected ValidateSet. Useful, but none of them run the function.

Measured

Code coverage over Invoke-PSBuildModuleSigning.ps1 with the whole file running:

11 tests passed, 0 failed
5 of 17 commands covered

All five covered commands are the ValidateScript on the Path parameter, which runs during parameter binding — reached by Validates that Path must be a directory, the one test that does bind against the command:

Line  Command
  60  if (-not (Test-Path -Path $_ -PathType Container)) { throw ... }
  60  Test-Path -Path $_ -PathType Container
  61  throw ($LocalizedData.PathArgumentMustBeAFolder)

Everything past the param() block is unexecuted:

$files = Get-ChildItem -Path $Path -Recurse -Include $Include
Write-Verbose ($LocalizedData.SigningModuleFiles -f $files.Count, ($Include -join ', '), $Path)

$sigParams = @{
    Certificate     = $Certificate
    TimestampServer = $TimestampServer
    HashAlgorithm   = $HashAlgorithm
}

$files | Set-AuthenticodeSignature @sigParams

Why it matters

This is the function that signs a consumer's shipped module. Nothing currently checks that:

  • the Include patterns the consumer configured are the ones passed to Get-ChildItem, or that -Recurse is passed at all;
  • TimestampServer and HashAlgorithm reach Set-AuthenticodeSignature rather than being accepted and dropped — the tests assert only that the parameters exist;
  • $Certificate is the certificate used;
  • every discovered file is signed, rather than only the first.

Any of those could be broken by a refactor with the suite staying green. $PSBPreference.Sign.HashAlgorithm could silently stop being honoured the same way $PSBPreference.Sign.SkipCertificateValidation silently stopped being honoured in #193, and there is no test here that would notice.

The gap is easy to close, because Set-AuthenticodeSignature is the only thing in the body that touches the outside world. Mocked with -ModuleName 'PowerShellBuild', the function can be driven end to end against files in $TestDrive with no certificate and no signing.

Options

  1. Call the function with a mocked Set-AuthenticodeSignature. A PSCustomObject stands in for the certificate, since the parameter is typed X509Certificate2 — which needs either a real certificate or a small [X509Certificate2]::new() from generated bytes to satisfy the binder. Then Should -Invoke -ModuleName 'PowerShellBuild' Set-AuthenticodeSignature -Times 3 -Exactly with a -ParameterFilter on TimestampServer and HashAlgorithm covers the whole body and every forwarding question above. This is the shape the retry-loop tests in Test-PSBuildScriptAnalysis.tests.ps1 already use, and they are the strongest tests in the suite.
  2. Rewrite the two misnamed tests rather than adding to them. Searches for files matching Include patterns and Uses custom Include patterns when specified should either call the function or be deleted; leaving them as they are means the file reports coverage of behaviour it does not have.
  3. Remove the unused Mock Set-AuthenticodeSignature, or scope it and use it. As written it reads like protection against accidentally signing something during a test run, and it is not.

(1) subsumes (2) and (3). The Certificate parameter's type is the only real obstacle, and a self-signed certificate generated into $TestDrive or a PSCustomObject cast through a looser parameter type both solve it.

Related: #103 (Publish-PSBuildModule had the same zero-coverage shape and the same consequence — a real defect, #203, sat undetected behind passing tests). Shares its mock-scoping defect and its remediation with #216, which has the same unscoped Mock problem in tests/Get-PSBuildCertificate.tests.ps1.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Projects

    No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions