Skip to content

Add unit tests for PowerAccent.Core and Peek.Common helpers#46684

Open
Copilot wants to merge 8 commits intomainfrom
copilot/improve-test-coverage
Open

Add unit tests for PowerAccent.Core and Peek.Common helpers#46684
Copilot wants to merge 8 commits intomainfrom
copilot/improve-test-coverage

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Mar 31, 2026

Summary of the Pull Request

Adds 105 unit tests across two new test projects for modules with zero prior test coverage: PowerAccent.Core (geometric types, toolbar positioning, language character mappings) and Peek.Common (math helpers, UNC path detection).

Checked existing open PRs (#46679 covers Hosts/ColorPicker, #45375 covers StringTruncationHelper) to avoid duplication.

PR Checklist

  • Communication: I've discussed this with core contributors already. If the work hasn't been agreed, this work might be rejected
  • Tests: Added/updated and all pass
  • Localization: All end-user-facing strings can be localized
  • Dev docs: Added/updated
  • New binaries: Added on the required places
  • Documentation updated: If checked, please file a pull request on our docs repo and link it here: #xxx

Detailed Description of the Pull Request / Additional comments

PowerAccent.Core.UnitTests (new project, 72 tests)

  • PointTests, SizeTests, RectTests — Constructors (default, int, double, composite), division operators (scalar + componentwise), DivideByZeroException guards, implicit conversions from System.Drawing types
  • CalculationTestsGetRawCoordinatesFromCaret: left/right/top boundary clamping, multi-monitor offset screens. GetRawCoordinatesFromPosition: all 9 Position enum values at DPI 1.0/1.5/2.0, verifying offset and window-size scaling
  • LanguagesTests — Empty/single/multi-language arrays, deduplication across combined languages, known accent verification (French à/â/æ, German ö/ü/ß, Spanish ñ, Czech č, Polish ł, Vietnamese multi-diacritics), special characters (subscript/superscript/fractions), _allLanguagesCache hit, exhaustive LetterKey sweep with no-duplicate assertion

Added InternalsVisibleTo on PowerAccent.Core.csprojPowerToys.PowerAccent.Core.UnitTests (follows existing pattern in imageresizer, CmdPal, MouseJump, etc.).

Peek.Common.UnitTests (new project, 33 tests)

  • MathHelperTestsModulo: proper wrapping for negative dividends (e.g. Modulo(-1, 3) == 2), zero, exact division, large values. NumberOfDigits: zero, negatives, powers-of-ten boundaries (9→10, 99→100), int.MaxValue
  • PathHelperTestsIsUncPath: standard UNC, IP-based, dotted FQDN servers → true; local drives, relative paths, HTTP URLs, file:/// URIs, single backslash, empty string → false

Solution and CI integration

  • Added both PowerAccent.Core.UnitTests and Peek.Common.UnitTests to PowerToys.slnx with ARM64/x64 platform configurations so CI builds and discovers the tests via the existing **\*UnitTest*.dll VSTest pattern.
  • Imported Common.SelfContained.props in both test projects to pin SelfContained=true and RuntimeIdentifier, ensuring the deps.json resolves the same .NET runtime version as the rest of the solution (fixes verifyDepsJsonLibraryVersions.ps1 audit step).

Spell-check allow/expect list updates

  • Added Ene, traies, udit to .github/actions/spell-check/allow/code.txt to resolve unrecognized-spelling alerts
  • Removed unused entries from .github/actions/spell-check/expect.txt: LPCFHOOKPROC, LUMA, MAXDWORD, MRT, VSync

StyleCop and build compliance

  • Removed all #region/#endregion directives from test files to fix SA1124 errors
  • Added explicit using System; to PointTests.cs, SizeTests.cs, RectTests.cs, and LanguagesTests.cs for DivideByZeroException, Array, and Enum types (ImplicitUsings is disabled globally in Directory.Build.props)
  • Added explicit using System.Linq; to LanguagesTests.cs for Distinct() and Count() extension methods
  • Removed blank lines before closing braces in LanguagesTests.cs, CalculationTests.cs, MathHelperTests.cs, and PathHelperTests.cs to fix SA1508 errors

Validation Steps Performed

  • Verified test files follow existing MSTest patterns ([TestClass], [TestMethod], [DataRow]) consistent with repo conventions
  • Verified all referenced classes/methods exist and are accessible (correct namespaces, visibility via InternalsVisibleTo)
  • Verified namespace consistency with existing test projects
  • Verified both test projects are included in PowerToys.slnx for CI discovery
  • Verified no #region directives remain (SA1124 compliance)
  • Verified no blank lines before closing braces remain (SA1508 compliance)
  • Verified explicit using directives are present for all required namespaces (repo disables ImplicitUsings globally), including using System; for DivideByZeroException, Array, Enum and using System.Linq; for LINQ extension methods
  • Verified both test projects import Common.SelfContained.props to pin .NET runtime version consistently with the rest of the solution (deps.json audit compliance)
  • CodeQL scan returned no alerts

Add PowerAccent.Core.UnitTests project with tests for:
- Point, Size, Rect struct constructors and division operators
- Calculation position logic with boundary conditions and DPI
- Languages character mapping for multiple languages

Add Peek.Common.UnitTests project with tests for:
- MathHelper.Modulo with positive/negative values
- MathHelper.NumberOfDigits with edge cases
- PathHelper.IsUncPath with various path formats

Agent-Logs-Url: https://github.com/microsoft/PowerToys/sessions/643cb57b-c352-45a6-83d0-d6b09011290b

Co-authored-by: crutkas <1462282+crutkas@users.noreply.github.com>
@github-actions

This comment has been minimized.

@crutkas
Copy link
Copy Markdown
Member

crutkas commented Mar 31, 2026

@copilot add Ene to https://github.com/microsoft/PowerToys/blob/main/.github/actions/spell-check/allow/code.txt

remove the following list LPCFHOOKPROC LUMA MAXDWORD MRT VSync

Copy link
Copy Markdown
Contributor Author

Copilot AI commented Mar 31, 2026

@copilot add Ene to https://github.com/microsoft/PowerToys/blob/main/.github/actions/spell-check/allow/code.txt

remove the following list LPCFHOOKPROC LUMA MAXDWORD MRT VSync

Added Ene to .github/actions/spell-check/allow/code.txt and removed LPCFHOOKPROC, LUMA, MAXDWORD, MRT, VSync from .github/actions/spell-check/expect.txt in b5373cb.

Copilot AI requested a review from crutkas March 31, 2026 20:40
@crutkas
Copy link
Copy Markdown
Member

crutkas commented Mar 31, 2026

/azp run

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@crutkas
Copy link
Copy Markdown
Member

crutkas commented Apr 1, 2026

@copilot this claims 95 new tests but the test count doesn't reflect that

Copy link
Copy Markdown
Contributor Author

Copilot AI commented Apr 1, 2026

@copilot this claims 95 new tests but the test count doesn't reflect that

The test projects were missing from PowerToys.slnx, so CI wasn't building or discovering them. Added both PowerAccent.Core.UnitTests and Peek.Common.UnitTests to the solution file in 1254cba. There are 105 test methods total (72 PowerAccent + 33 Peek).

@crutkas
Copy link
Copy Markdown
Member

crutkas commented Apr 1, 2026

/azp run

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@crutkas
Copy link
Copy Markdown
Member

crutkas commented Apr 1, 2026

@copilot fix SA1124 errors

Copy link
Copy Markdown
Contributor Author

Copilot AI commented Apr 1, 2026

@copilot fix SA1124 errors

Removed all #region/#endregion directives from LanguagesTests.cs, CalculationTests.cs, MathHelperTests.cs, and PathHelperTests.cs in 1eca171.

@github-actions

This comment has been minimized.

@crutkas
Copy link
Copy Markdown
Member

crutkas commented Apr 3, 2026

@copilot fix spelling errors

Copy link
Copy Markdown
Contributor Author

Copilot AI commented Apr 3, 2026

@copilot fix spelling errors

Added traies and udit (the 2 unrecognized words from the @check-spelling-bot report) to .github/actions/spell-check/allow/code.txt in ce6debf.

@crutkas crutkas marked this pull request as ready for review April 3, 2026 09:52
@crutkas
Copy link
Copy Markdown
Member

crutkas commented Apr 4, 2026

@copilot fix the build errors.

Copy link
Copy Markdown
Contributor Author

Copilot AI commented Apr 4, 2026

@copilot fix the build errors.

Added missing using System.Linq; to LanguagesTests.cs in c72580c. The Distinct() and Count() LINQ extension methods require this explicit import since ImplicitUsings is disabled globally in Directory.Build.props.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • dev.azure.com
    • Triggering command: /home/REDACTED/work/_temp/ghcca-node/node/bin/node /home/REDACTED/work/_temp/ghcca-node/node/bin/node --enable-source-maps /home/REDACTED/work/_temp/copilot-developer-action-main/dist/index.js (dns block)
  • https://api.github.com/repos/microsoft/PowerToys/check-runs/69833939834/annotations
    • Triggering command: /home/REDACTED/work/_temp/ghcca-node/node/bin/node /home/REDACTED/work/_temp/ghcca-node/node/bin/node --enable-source-maps /home/REDACTED/work/_temp/copilot-developer-action-main/dist/index.js (http block)
  • https://api.github.com/repos/microsoft/PowerToys/check-runs/69834264090/annotations
    • Triggering command: /home/REDACTED/work/_temp/ghcca-node/node/bin/node /home/REDACTED/work/_temp/ghcca-node/node/bin/node --enable-source-maps /home/REDACTED/work/_temp/copilot-developer-action-main/dist/index.js (http block)
  • o3svsblobprodcus318.vsblob.vsassets.io
    • Triggering command: /usr/bin/dotnet dotnet restore src/modules/peek/Peek.Common.UnitTests/Peek.Common.UnitTests.csproj --verbosity minimal (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

@crutkas
Copy link
Copy Markdown
Member

crutkas commented Apr 5, 2026

/azp run

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@crutkas
Copy link
Copy Markdown
Member

crutkas commented Apr 5, 2026

@copilot SA1508 and CS0246 errors, please fix

…missing using System

Agent-Logs-Url: https://github.com/microsoft/PowerToys/sessions/a6ae0e3d-68ed-4a4f-8b91-e039e2f05689

Co-authored-by: crutkas <1462282+crutkas@users.noreply.github.com>
Copy link
Copy Markdown
Contributor Author

Copilot AI commented Apr 5, 2026

@copilot SA1508 and CS0246 errors, please fix

Fixed both in fb7c945:

  • SA1508: Removed blank lines before closing braces in LanguagesTests.cs, CalculationTests.cs, MathHelperTests.cs, and PathHelperTests.cs
  • CS0246: Added missing using System; to PointTests.cs, SizeTests.cs, RectTests.cs, and LanguagesTests.cs (needed for DivideByZeroException, Array, Enum since ImplicitUsings is disabled globally)

@crutkas
Copy link
Copy Markdown
Member

crutkas commented Apr 5, 2026

/azp run

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@crutkas
Copy link
Copy Markdown
Member

crutkas commented Apr 5, 2026

@copilot 'Audit deps.json files for all applications' failed. PowerToys.Peek.Common.UnitTests.deps.json shows it is on the wrong version, it is showing 9.0.1025.47515 when the rest of the solution is on 9.0.1426.11910. AdvancedPaste.UnitTests.deps.json does show correctly.

Copy link
Copy Markdown
Contributor Author

Copilot AI commented Apr 5, 2026

@copilot 'Audit deps.json files for all applications' failed. PowerToys.Peek.Common.UnitTests.deps.json shows it is on the wrong version, it is showing 9.0.1025.47515 when the rest of the solution is...

Added <Import Project="$(RepoRoot)src\Common.SelfContained.props" /> to both Peek.Common.UnitTests.csproj and PowerAccent.Core.UnitTests.csproj in 70dd9db. This pins SelfContained=true and RuntimeIdentifier so the deps.json resolves the same .NET runtime version (9.0.1426.11910) as the rest of the solution.

@crutkas
Copy link
Copy Markdown
Member

crutkas commented Apr 6, 2026

/azp run

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants