psake / psake/PowerShellBuild

Invoke-PSBuildModuleSigning has no test that calls it

Đang mở
#217 0 bình luận 0 reaction 0 người được giao Xem trên GitHub

Chưa có ai nhận issue này.

bug
Ngôn ngữ chính
PowerShell
Star
145
Fork
27
Merge trung bình
10 giờ 16 phút
Pull request đã merge (30 ngày)
34

Mô tả

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.

Hướng dẫn đóng góp

Mở hướng dẫn đóng góp

Bắt đầu từ đâu

  1. Đọc hết issue, rồi đọc hướng dẫn đóng góp của dự án.
  2. Bình luận trên issue rằng bạn sẽ nhận — tránh hai người làm cùng một việc.
  3. Fork repository và làm thay đổi trên một nhánh.
  4. Mở pull request có tham chiếu số hiệu của issue.

Hướng nghiên cứu

Bắt đầu với tests/Invoke-PSBuildModuleSigning.tests.ps1 và phần thân hàm Invoke-PSBuildModuleSigning.ps1. Thay thế hoặc sửa lại hai bài kiểm thử phát hiện tệp để chúng gọi hàm, sử dụng mẫu mock có phạm vi của Set-AuthenticodeSignature được mô tả trong issue. Chạy tệp kiểm thử và xác minh rằng độ bao phủ bao gồm việc phát hiện, chuyển tiếp tham số và ký mọi tệp được phát hiện.

Do mô hình lập chỉ mục viết ra từ nội dung của issue.

Đánh giá

Công nghệ
powershell
Lĩnh vực
testing-qa
Loại issue
Tái cấu trúc
Độ khó
3/5
Thời gian dự kiến
1-2 ngày
Mức độ hoạt động
Sôi nổi
Độ rõ ràng
Đặc tả rõ ràng
Mức phù hợp với người mới
76/100

Nhận issue mới trong hộp thư của bạn

Bản tóm tắt ngắn những issue GitHub phù hợp với người mới.