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

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 Resolved

This branch has not been deployed

No deployments
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