psake / psake/PowerShellBuild

Invoke-PSBuildModuleSigning has no test that calls it

Ouverte
#217 0 commentaires 0 réactions 0 personnes assignées Voir sur GitHub

Personne n'a encore pris cette issue.

bug
Langage dominant
PowerShell
Étoiles
145
Forks
27
Merge moyen
10 h 16 min
PR mergées (30 j)
34

Description

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.

Guide de contribution

Ouvrir le guide de contribution

Par où commencer

  1. Lisez l'issue en entier, puis le guide de contribution du projet.
  2. Signalez en commentaire que vous la prenez — cela évite que deux personnes fassent le même travail.
  3. Forkez le dépôt et travaillez sur une branche.
  4. Ouvrez une pull request qui référence le numéro de l'issue.

Piste de recherche

Commencez par tests/Invoke-PSBuildModuleSigning.tests.ps1 et le corps de la fonction Invoke-PSBuildModuleSigning.ps1. Remplacez ou révisez les deux tests de découverte de fichiers afin qu’ils appellent la fonction, en utilisant le modèle de mock scoped de Set-AuthenticodeSignature décrit dans l’issue. Exécutez le fichier de test et vérifiez que la couverture inclut la découverte, la transmission des paramètres et la signature de chaque fichier découvert.

Rédigé par le modèle d'indexation à partir du texte de l'issue.

Évaluation

Stack technique
powershell
Domaine
testing-qa
Type d'issue
Refactorisation
Difficulté
3/5
Temps estimé
1-2 jours
Activité
Active
Clarté
Clairement spécifiée
Accessibilité débutants
76/100

Recevez les nouvelles issues par e-mail

Un résumé court des issues GitHub adaptées aux débutants.