Skip to content

Test suite overstates coverage: 73/195 functions never invoked, and the coverage ratchet is regex-based #152

Description

@sanderdewit

Measured by AST-parsing FunctionsToExport against CommandAst invocations in the test tree.

Metric Value
Exported functions 195
Never invoked by any test 73 (37.4%)
— of those, HIGH-RISK (mutate/sign/download/mount) 33
Tests asserting on module source text rather than behaviour 50
Tests whose only assertion is Should -Not -Throw 17
Coverage measurement in CI none

The coverage ratchet certifies functions on the strength of Get-Command and It titles

MSIX.Tests/MSIX.Contract.CoverageMap.Tests.ps1:59 defines "invoked" as a text regex:

$rx = [regex]::Escape($Name) + '\s+[-$@]'

Get-Command Add-MsixFoo -Module MSIX matches. So do comments and Context/It titles. Eight mutators are falsely certified as covered (regex says yes, AST says never invoked): Set-MsixInstalledLocationVirtualization, Add-MsixLoaderSearchPathOverride, Remove-MsixUpdaterArtifact, Update-MsixMgr, Update-MsixSdkTool, Add-MsixFirewallRule, Add-MsixStartupTask, Update-MsixPackageVersion. The $KnownUncovered ratchet reads empty ("burned down in #102") while 12 mutators are genuinely uninvoked.

Critical paths with no real assertion

  • Signing-failure rollback — all 12 UnsignedOutputPath references are Parameters.ContainsKey or source regex. No test ever fails a signing operation and asserts the original bytes survive.
  • -WhatIf side-effect freedom — no test runs a mutator with -WhatIf and asserts the file is byte-identical. (This is why the broken -WhatIf went unnoticed.)
  • Error-path workspace cleanup — New-MsixWorkspace is only ever mocked; nothing asserts cleanup after failure.
  • Authenticode rejection — _MsixVerifyAuthenticode is invoked twice but both mocks hard-code Status = 'Valid', so the throw branch at MSIX.PsfBinaries.ps1:151 never executes.
  • Idempotency — real double-run tests exist for only 6 of ~60 mutators, all behind the MakeAppx gate.

MSIX.WhatIf.Tests.ps1:85-162 verifies the atomic pack-sign-move invariant by grepping module source for a Move-Item line — it passes if the line exists in a dead branch or after an early return.

Other

  • MSIX.Tests/MSIX.OfflineRegistry.Tests.ps1:173 has no assertion at all in the body — it passes unconditionally.
  • MSIX.AuthenticodeFilter.Tests.ps1:47-56 reimplements the production filter expression in the test body and asserts the copy — it passes even if the real function is deleted.
  • MSIX.MutatorIntegration.Tests.ps1:45 depends on execution order across It blocks.

Suggested fixes (prioritised)

  1. Replace the CoverageMap regex with AST CommandAst detection; re-baseline $KnownUncovered to the true list.
  2. Add a signing-failure rollback test (assert original SHA-256 unchanged).
  3. Add a -WhatIf byte-identity test across all SupportsShouldProcess mutators.
  4. Add Authenticode rejection tests (NotSigned/HashMismatch/UnknownError → throws).
  5. Invoke the 21 uncovered read-only scanners against a manifest missing <Properties>/<Extensions>/<Applications>.

Activity

  1. sanderdewit commented on Aug 3, 2026

    @sanderdewit
    OwnerAuthor

    Partially fixed in 0.74.0 (#154) — leaving open for the remaining coverage gap.

    Fixed — the ratchet no longer lies:

    • Test-MsixInvoked now uses AST CommandAst detection instead of a regex over test source text, so Get-Command Add-MsixFoo -Module MSIX, comments, and Context/It titles no longer count as coverage.
    • That immediately exposed the 8 falsely-certified mutators named in this issue. Six gained real behavioural tests (MSIX.UncoveredMutators.Tests.ps1); Update-MsixMgr and Update-MsixSdkTool moved to PermanentlyExcluded as the same network-updater class already excluded.
    • Writing those six tests found three real bugs in cmdlets that could never produce a packable package — see Correctness: discarded return values, missing idempotency guards, and @($null) null-deref cluster #153.

    Still open:

    • 73 of 195 exported functions are still never invoked by a test (33 of them high-risk: mutators, downloaders, signing, disk-image mounting). The six fixed here were the falsely-certified subset only.
    • No code-coverage measurement in CI.
    • The critical-path gaps listed in this issue remain unaddressed except where 0.74.0 touched them: signing-failure rollback and -WhatIf byte-identity now have real tests (added with CRITICAL: three paths silently destroy or corrupt the operator's package #145), but Authenticode rejection is still only exercised with mocks hard-coded to Status = 'Valid'.

    Worth noting the hit rate as justification for continuing: six never-tested mutators yielded three real bugs.

  2. added a commit that references this issue on Aug 4, 2026
  3. sanderdewit commented on Aug 4, 2026

    @sanderdewit
    OwnerAuthor

    Completed in #155.

    • Never-invoked exported functions: 73 → 26. All 26 that remain are genuinely environment-bound — network installers, Hyper-V/VHDX, the ProcMon kernel driver, Windows Sandbox, or a live installed package — and each is listed with its reason rather than left silent.
    • The ratchet now guards the whole exported surface, not just Add/Remove/Set/Update mutators. That narrow scope is exactly how 73 functions came to be uncovered with nothing reporting it. Three orchestrators (Invoke-MsixAccelerator, Invoke-MsixAutoFixLoop, Invoke-MsixRemediationPlan) are recorded as real debt with a ceiling so the list can only shrink.
    • Authenticode rejection is tested for the first time. Both existing tests mocked Get-AuthenticodeSignature to return Valid, so the throw branch — the control that stops a planted toolchain binary from being executed — had never executed. Every rejection status is now driven, plus a valid-signature-but-untrusted-publisher case and a positive control so the suite cannot pass by simply throwing at everything.
    • Read-only scanner matrix: all 15 scanners against a well-formed package, against the degenerate manifest shape the module generates itself (no <Applications>/<Extensions>/<Capabilities> — the shape behind the null-deref class), and against a missing package to prove they fail loudly instead of returning an empty result that reads as "clean". Explicit named invocations back the matrix, because & $Name dispatch is invisible to AST detection and teaching the ratchet to trust strings again is the exact weakness this issue was about.
    • Code coverage is now measured in CI and reported in the job summary. There was no measurement at all before.
    • Test isolation fixed: the update-check tests wrote to the real user cache at %LOCALAPPDATA%\MSIX\update-check.json, poisoning it with a mocked version.

    Suite: 765 → 848 passing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions