psake / psake/PowerShellBuild

A skipped Test-PSBuildPester test can never run, and its comment says it does

Đang mở
#220 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/Test-PSBuildPester.tests.ps1, the honors the Pester version that is already loaded test at line 305, is skipped by a condition that can never be false. Its comment says otherwise, and that is the part most worth fixing.

What happens

It 'honors the Pester version that is already loaded' -Skip:($script:innerPesterVersions.Count -lt 2) {

$innerPesterVersions is built in BeforeDiscovery as the newest installed Pester of each supported major:

$script:innerPesterVersions = @(
    foreach ($majorVersion in 6) {
        $newestOfMajor = Get-Module -Name 'Pester' -ListAvailable |
            Where-Object { $_.Version.Major -eq $majorVersion } |
            Sort-Object -Property 'Version' -Descending |
            Select-Object -First 1
        ...

The loop runs over a single major, and Select-Object -First 1 takes one version from it. The collection therefore holds at most one element, so Count -lt 2 is always true and the test is always skipped. Pester 6 has been the only supported major since #172, which removed the 5.x matrix.

The comment is wrong in a specific, misleading way

# This skips in CI as of #172. It needs two installed Pester versions to tell
# "used the loaded one" apart from "imported the newest", and dropping the 5.x
# matrix left exactly one. It still runs on a developer machine with more than one
# Pester installed. Restoring it in CI would mean installing a second version that
# is never imported -- the machinery #172 deleted -- so it is left skipped
# deliberately rather than by oversight.

"It still runs on a developer machine with more than one Pester installed" is false. The collection is not "installed Pester versions"; it is "the newest installed version of each supported major". A machine can have any number of Pester versions installed and still produce a one-element collection.

Measured on a workstation with five Pester versions installed — 3.4.0, 5.7.1, 6.0.0, 6.0.1, and 6.1.0, comfortably "more than one Pester installed" — $innerPesterVersions resolves to a single entry, 6.1.0, and the test skips. There is no machine configuration, developer or otherwise, on which it runs.

Why it matters

The comment is the reason this went unnoticed and is the thing that will keep it unnoticed. It is written in the repository's usual careful register, it names the pull request that caused the situation, and it explicitly claims the skip is deliberate rather than oversight — so a reader auditing skipped tests has every reason to move on. A test that is permanently dead is a cost; a permanently dead test with a comment asserting it is alive is a trap.

The coverage that is actually lost is real but narrow. The regression it was written for was an unconditional Import-Module Pester -MinimumVersion 5.0.0 that loaded the newest installed Pester on top of an already-loaded older one, crashing on a Pester.dll version conflict. Test-PSBuildPester now checks the loaded version rather than forcing an import, and the assertion this test makes:

$result.LoadedModuleVersion['Pester'] | Should -Be @($script:oldestInnerVersion)

can only distinguish "used the loaded one" from "imported the newest" when the two differ — which needs two versions the harness is willing to pin. Everything else in that context uses $newestInnerVersion and passes.

Note also that the test's body would still be self-defeating if the skip were removed today, because $oldestInnerVersion and $newestInnerVersion resolve to the same single element, so the assertion would pass without distinguishing anything. That makes this the second defect in this list where two problems mask each other.

Options

  1. Delete the test and its comment. Honest, and it removes the trap. It costs the only guard against the import regression coming back, which would show up as a hard crash rather than a silent wrong answer — so the cost is smaller than it looks.
  2. Restore the coverage without a second Pester major. The inner job pins a version by importing it; a second minor version of Pester 6 would serve just as well as a second major for telling "used the loaded one" apart from "imported the newest", since the assertion compares exact version strings. Widening $innerPesterVersions to collect, say, the newest and the oldest installed 6.x would make the test run wherever two are present and skip cleanly where only one is — with a comment that says exactly that. It would run on the workstation measured above, and on CI only if a second 6.x were installed.
  3. Keep it skipped but tell the truth. Change the condition to a plain -Skip with a comment saying it can never run as written and why, so the next reader is not misled. Cheapest, and it leaves a dead test in the file.
  4. Restore it in CI. Install a second Pester version on the runner purely so this test has something to pin against. #172 deliberately removed that machinery; re-adding it for one test is probably not worth it, and the comment is right about that much.

(2) is the option that keeps the coverage and matches what the comment currently promises. (1) is the right answer if the coverage is judged not worth the machinery — in which case the comment should go with it rather than being left to describe a test that is not there.

Related: #172 (removed the 5.x matrix and created this situation).

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 trong tests/Test-PSBuildPester.tests.ps1, quanh phần thiết lập BeforeDiscovery và bài kiểm thử "honors the Pester version that is already loaded" ở dòng 305. Theo dõi cách $innerPesterVersions, $oldestInnerVersion và $newestInnerVersion được điền, sau đó chạy các bài kiểm thử Test-PSBuildPester mục tiêu. Hoàn tất khi bài kiểm thử và chú thích của nó phản ánh chính xác thời điểm có thể đạt được độ bao phủ có ý nghĩa.

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
Lỗi
Độ 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
Khá rõ ràng
Mức phù hợp với người mới
55/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.