Skip to content

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

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.

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions