Skip to content

Install-GoogleFont: follow-ups from PR #210 review (-Force vs cache, -WhatIf, retries, cache path, tests) #211

@MariusStorhaug

Description

Install-GoogleFont: follow-ups from PR #210 review

These are valid concerns raised by reviewers on #210 that were out of scope for the perf iteration cycle. The perf work in #210 delivered measured wins (Single −67 % / −87 %, Subset −83 % / −90 %, AlreadyInstalled −97 %); this issue captures the correctness/robustness polish layered on top.

Tasks

  • -Force bypasses cache. FromCache is computed from Test-Path only and cache hits are always copied. When -Force is set, treat the cache as cold so a fresh download is performed.
  • -WhatIf semantics. Downloads and cache writes happen unconditionally; with -WhatIf the function still opens HTTP connections and writes to disk. Gate the download/copy/cache-write work behind ShouldProcess (e.g. build an approved list up front).
  • Download retries. Switching from Invoke-WebRequest -RetryIntervalSec 5 -MaximumRetryCount 5 to raw HttpClient.GetByteArrayAsync removed the existing transient-failure retries. Add a bounded retry loop around each request.
  • Cache root platform conventions. macOS currently uses $HOME/.cache/PSModule/GoogleFonts instead of $HOME/Library/Caches/PSModule/GoogleFonts, and Linux ignores $Env:XDG_CACHE_HOME. Branch on $IsMacOS and $Env:XDG_CACHE_HOME when constructing the cache root.
  • Scope $ProgressPreference override. It is currently set for the entire function in begin and restored in clean. Narrow it to a try/finally around the download call so unrelated progress output is not suppressed, and so a clean-block exception cannot prevent restoration.
  • clean block robustness. Remove the temp directory with -Recurse, and restore $ProgressPreference before/independently of Remove-Item so a delete failure cannot leak the preference change into the caller's session.
  • Verbose for skipped fonts. Emit a Write-Verbose "[$fontName] already installed - skipping" per entry that the skip-installed filter drops, so users understand why fewer items were processed.
  • Tighten Pester coverage. Existing tests assert only that calls do not throw. Add assertions that:
    • second install without -Force does not call Invoke-WebRequest / GetByteArrayAsync (mock-based).
    • wildcard input (ABee*) installs at least one expected font.
    • URL-based dedup avoids duplicate downloads for overlapping -Name inputs.
  • Perf harness hardening.
    • Get-Module GoogleFonts ... .ToString() throws if the module is not loaded. Capture the module object first and tolerate $null.
    • Ensure the parent directory of -ResultsPath exists before Add-Content, so custom output locations work.

Origin

Raised across review comments on #210; deferred from that PR because the iteration cycle and prerelease measurements were already complete and these items belong to a separate review-able change.

Metadata

Metadata

Assignees

No one assigned

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions