Skip to content

馃 [Fix]: Forced reinstalls and CI checks are repaired - #70

Open
Marius Storhaug (MariusStorhaug) wants to merge 8 commits into
mainfrom
review-upstream-fonts
Open

馃 [Fix]: Forced reinstalls and CI checks are repaired#70
Marius Storhaug (MariusStorhaug) wants to merge 8 commits into
mainfrom
review-upstream-fonts

Conversation

@MariusStorhaug

@MariusStorhaug Marius Storhaug (MariusStorhaug) commented Aug 28, 2026

Copy link
Copy Markdown
Member

Forced font installation now replaces existing files, the Process-PSModule v8 documentation site uses Zensical, and the existing coverage gate is satisfied with meaningful behavioral tests.

Fixed: Forced font reinstalls

  • Install-Font -Force uses the overwrite-capable FileInfo.CopyTo overload.
  • Calls without -Force continue to preserve installed files.
  • The copy retry guard now checks $fileCopied instead of the unrelated $fileRemoved variable.
  • A 681,900-byte benchmark showed unchanged fresh-install performance (1.755 ms before, 1.756 ms after median) and reduced forced replacement from repeated failure after about 9 seconds to 1.435 ms median success.

Fixed: Documentation site build

  • Replaces .github/mkdocs.yml with the Process-PSModule v8-compatible .github/zensical.toml.
  • Preserves automatic generated-command discovery and the current Process-PSModule theme, extensions, and shared assets.
  • Repairs the README's broken contribution link.

Fixed: Coverage gate and edge cases

  • Expands isolated Pester coverage across font discovery, recursion, unsupported files, missing paths, non-force preservation, administrator denial, uninstall retries, and missing installed files.
  • Prevents Get-Font from returning unrelated state when a configured scope folder is absent.
  • Corrects the administrator error path used by Uninstall-Font.
  • Local result: 19 tests pass with 88.42% source coverage against the existing 80% target.

Relevant issues

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates the Install-Font behavior so -Force can re-install fonts by overwriting existing destination files, and adds regression coverage to ensure forced installs restore the correct file contents.

Changes:

  • Enable overwrite behavior during installation by using FileInfo.CopyTo(dest, overwrite) with overwrite tied to -Force.
  • Fix the retry-loop guard to check $fileCopied (copy success) instead of an unrelated/undefined variable.
  • Add a Pester regression test that corrupts an installed font file and verifies Install-Font -Force restores the original hash.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
src/functions/public/Install-Font.ps1 Implements overwrite-on-force via CopyTo(..., $Force.IsPresent) and corrects retry failure guard logic.
tests/Fonts.Tests.ps1 Adds a regression test validating forced reinstalls overwrite an existing (corrupted) font file.

馃挕 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@MariusStorhaug
Marius Storhaug (MariusStorhaug) marked this pull request as ready for review August 28, 2026 23:57
@MariusStorhaug Marius Storhaug (MariusStorhaug) changed the title 馃 [Fix]: Existing fonts can be forcibly reinstalled 馃 [Fix]: Forced reinstalls and CI checks are repaired Aug 29, 2026

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.

Comment thread tests/Fonts.Tests.ps1
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Prerelease release:patch Publish a patch release.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

馃[Bug]: -Force option not working

2 participants