Repository navigation
test(ui): raise UI-command logic coverage to >=95% per file - #645
Conversation
Add fakes-driven tests across the UI-command surface (commands, session resolution, value setting, key parsing, helpers, models) to bring every hand-written file in scope to >=95% Debug line coverage without any real desktop/UIA dependency. Test-only additions dominate; two behavior-preserving OS-boundary seams were introduced so runnable resolver/discovery logic becomes testable with fakes (production wires the real implementations, behavior unchanged): - IOwnedWindowFinder / RealOwnedWindowFinder: extracts UiScreenshotCommand's owned-window desktop enumeration behind an interface. - ISystemUiQuery / SystemUiQuery + UiProcessInfo: abstracts UiSessionService's process enumeration and Win32 window queries; resolver decision logic (PID/name/partial-name selection, auto-select, HWND resolution) is now fully unit-testable. Also removed dead code in UiSessionService (unused GetProcessNameSafe and an unreachable RefreshWindowTitle fallback) and made ClassifyWindow internal static so its branches are directly testable. Genuine native/OS-only guards are left uncovered with explaining source comments (honest ceiling) rather than excluded; the two native seam impls (SystemUiQuery, RealOwnedWindowFinder) are 0% by design, matching the existing RealForegroundGuard category. No [ExcludeFromCodeCoverage] added to hand-written logic. No product behavior changes. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
…us/ceiling gaps
Extend ISystemUiQuery with the window-metadata reads (GetWindowClassName /
GetWindowSize / GetWindowOwner) so UI-command selection logic is fully testable
with fakes, and close the four review findings:
M1: route UiSessionService.PickLargestWindow through the injected seam and give
the two auto-select tests distinct window areas so they assert the explicitly
largest handle wins (previously vacuous "0x111 or 0x222"). UiSessionService.cs
27% -> 100%.
M2: inject ISystemUiQuery into UiScreenshotCommand and read the direct-HWND
PID/title through the seam (GetProcessIdForWindow/GetWindowText), replacing the
inline PInvokes; add fake valid-HWND + owned-dialog tests. 36% -> 100%.
M3: read the target class name through the seam and classify via the pure
FrameworkHint.IsXamlClassName; add XAML/non-XAML warning tests (ambient console
captured under [DoNotParallelize]). Deletes now-dead FrameworkHint.IsLikelyXaml.
91.7% -> 100%.
M4: introduce IPollDelay/RealPollDelay and inject it into UiWaitForCommand so the
"condition not met yet - keep polling" continuations (appear-after-transient,
present-then-gone, value-changes-across-polls) are deterministically testable.
50% -> 100%.
Seams (behavior-preserving, internal): ISystemUiQuery window-metadata extension
(a UiSessionService.s_sharedQuery bridge keeps the static shims that UiAutomation-
Service relies on pointed at the real impl) and IPollDelay. Production wires the
real path for both.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Raises deterministic winapp ui command coverage toward the ≥95% per-file target from issue #630.
Changes:
- Adds injectable process, Win32 window, owned-window, and polling seams.
- Routes screenshot, send-keys, wait-for, and session resolution through those seams.
- Expands command, resolver, gesture, helper, and error-path tests.
Reviewed changes
Copilot reviewed 26 out of 26 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
Services/UiSessionService.cs |
Uses injectable OS queries. |
Services/SystemUiQuery.cs |
Implements real process/window queries. |
Services/ISystemUiQuery.cs |
Defines the OS-query seam. |
Helpers/RealPollDelay.cs |
Implements production polling delay. |
Helpers/RealOwnedWindowFinder.cs |
Extracts owned-window enumeration. |
Helpers/IPollDelay.cs |
Defines polling abstraction. |
Helpers/IOwnedWindowFinder.cs |
Defines owned-window abstraction. |
Helpers/HostBuilderExtensions.cs |
Registers new services. |
Helpers/FrameworkHint.cs |
Makes XAML detection pure. |
Commands/UiWaitForCommand.cs |
Injects polling delay. |
Commands/UiSendKeysCommand.cs |
Injects window-class lookup. |
Commands/UiScreenshotCommand.cs |
Injects window metadata/discovery. |
Tests/UiSessionServiceTests.cs |
Expands resolver coverage. |
Tests/UiHelpersTests.cs |
Covers UI helper behavior. |
Tests/UiCommandTests.WaitFor.cs |
Covers wait and retry paths. |
Tests/UiCommandTests.SimpleVerbs.cs |
Covers simple UI commands. |
Tests/UiCommandTests.SendKeys.cs |
Covers errors and XAML warnings. |
Tests/UiCommandTests.Search.cs |
Covers search rendering/errors. |
Tests/UiCommandTests.Screenshot.cs |
Covers screenshot workflows. |
Tests/UiCommandTests.ListWindows.cs |
Covers window listing branches. |
Tests/UiCommandTests.Inspect.cs |
Covers inspect rendering/filtering. |
Tests/UiCommandTests.Gestures.cs |
Covers gesture guards and errors. |
Tests/UiCommandTests.cs |
Registers new test fakes. |
Tests/FrameworkHintTests.cs |
Updates classifier tests. |
Tests/FakeUiServices.cs |
Adds configurable UI test seams. |
Tests/BaseCommandTests.cs |
Supports custom cancellation tokens. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| try | ||
| { | ||
| var handle = new Windows.Win32.Foundation.HWND((nint)hwnd); | ||
| var buffer = new char[256]; |
There was a problem hiding this comment.
Fixed in f83031c. Bumped the GetWindowText buffer back to char[512] to match the prior inline direct-HWND path this seam replaced (origin/main used 512 in two places). char[256] silently truncated window titles of 256-511 chars, so the seam is now genuinely behavior-preserving as its doc-comment claims.
| Assert.AreEqual(0, exitCode); | ||
| Assert.IsTrue(File.Exists(path)); |
There was a problem hiding this comment.
Fixed in f83031c. Verified real behavior first: --window 0 with no --app clears the missing-app guard, DiscoverAllWindows returns null, then ResolveSessionAsync throws "Specify --app..." (proven by UiSessionServiceTests.ResolveSession_WhitespaceApp_ZeroHwnd_Throws) -> exit 1, no file written. Rewrote the test to model that real rejection and assert exit 1 + no screenshot, instead of the vacuous exit-0 the fake previously allowed. Added a genuine positive single-window test so the discovery-null single-capture success path stays covered.
| Assert.AreEqual(0, exitCode); | ||
| Assert.IsTrue(File.Exists(path)); |
There was a problem hiding this comment.
Fixed in f83031c. --window 999999 -> GetProcessIdForWindow yields pid 0 -> DiscoverAllWindows bails -> ResolveByHwnd throws "Window HWND 999999 not found or not accessible" (proven by UiSessionServiceTests.ResolveByHwnd_WindowNotFound_Throws) -> exit 1, no file. Rewrote the test to assert the real exit-1 + no-screenshot outcome rather than the fake accepting any handle.
Build Metrics ReportBinary Sizes
Test Results✅ 2220 passed, 1 skipped out of 2221 tests in 700.3s (+147 tests, -3.8s vs. baseline) Test CoverageCLI Startup Time46ms median (x64, Updated 2026-07-15 21:33:46 UTC · commit |
… tests Addresses three auto-review findings on the UI-command coverage PR: - SystemUiQuery.GetWindowText: restore the char[512] buffer the seam replaced (the prior inline direct-HWND path used 512). char[256] silently truncated window titles 256-511 chars, making the seam's "behavior unchanged" claim inaccurate. - UiCommandTests.Screenshot: the --window 0 and invalid-HWND tests asserted success (exit 0 + file written), which the fake allowed but production rejects -- ResolveSessionAsync throws for a blank --app with a non-positive HWND, and ResolveByHwnd throws for a dead handle (both proven directly by UiSessionServiceTests). Rewrote both to model the real rejection and assert exit 1 with no screenshot written, and added a genuine positive single-window test (-a) so the discovery-null -> single-capture path stays covered. All four affected files remain at 100% Debug line coverage. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Reconcile PR7 (UI-command logic coverage) with two changes that landed on main while it was in review: - #629 (NuGet migration): auto-merged. HostBuilderExtensions.cs keeps both #629's NugetSourceProvider/NugetPackageDownloader registrations and my IOwnedWindowFinder/IPollDelay/ISystemUiQuery seam registrations. BaseCommandTests.cs: #629 and this branch independently added the same 3-arg ParseAndInvokeWithCaptureAsync overload; collapsed to one (kept #629's 2-arg delegator + my doc-comment) to avoid a duplicate method. - #603 (--allow-system-keys on ui send-keys): reconciled both product and tests so the feature AND my IPollDelay/ISystemUiQuery seam both survive. UiSendKeysCommand.cs: kept #603's AllowSystemKeysOption, warnings list, never-bypassable/soft-combo guard rewrite and JSON Warnings, plus my ISystemUiQuery systemQuery ctor param and IsXamlClassName seam read. UiCommandTests.SendKeys.cs: manual conflict resolution keeps the exact union of both sides' test methods (41 total, no duplicates). Debug 0W/0E; Release both projects 0W/0E. UiCommandTests + UiSessionServiceTests + SystemKeyGuard: 310 passed / 0 failed / 0 skipped. All 32 UI-scope files remain >=95% (UiSendKeysCommand 232/232=100%). Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
b0e02a2
into
main
…629)" This reverts the NuGet-client migration (#629) to unblock ADO main builds, which fail on the prerelease NuGet.* 7.9.0-rc packages sourced from the dnceng dotnet-tools feed (not nuget.org). See #653. Conflict resolution for coverage PRs that landed on top of #629: - FakeNugetService.cs restored to its pre-#629 form. - BaseCommandTests.cs: reverted #629 changes but kept the generic ParseAndInvokeWithCaptureAsync(..., CancellationToken) overload, which is unrelated test infra used by UI-command cancellation tests (#645). - Deleted PackageInstallationServiceTests.cs and UpdateCommandTests.cs (new in #629; later expanded by #640/#641 for the migrated code). - Deleted PackageInstallationServiceCacheMarkerTests.cs (added by #641; exercised the migrated NugetService via now-removed NugetFeedTestHelpers and INugetService.IsPackageInstalled). Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: d55408ba-4e7d-41aa-95a0-f7c801632eaf
…#654) Reverts the NuGet-client migration (#629) and its stacked coverage PR (#647). Tracks #653. ## Why After migrating `NugetService` to the official NuGet client libraries, `main` builds fail on ADO due to a security issue: #629 pins the `NuGet.*` packages to the prerelease **`7.9.0-rc.36120`** sourced from the dnceng **dotnet-tools** public feed (not nuget.org). The security gate flags consuming a prerelease from a secondary public feed. This was always intended as temporary — the `NuGet.UseSystemTextJsonDeserialization` switch needed to keep the Native AOT publish clean isn't in a nuget.org stable yet. ## What this reverts - **#647** first (reverse-merge order, since it's stacked on #629): removes the added coverage tests and the `NugetPackageDownloader.cs` test seams. - **#629** next: restores the hand-rolled `NugetService`, and removes the `NuGet.*` RC pins, the `dotnet-tools` source + `packageSourceMapping` from `nuget.config`, the `.pipelines/release-nuget.config` upstream, and the `Directory.Packages.props` pins. ## Conflict resolution notes Several coverage PRs merged after #629 built on top of its surface, so a plain revert left conflicts. Resolved as follows: - `FakeNugetService.cs` — restored to its pre-#629 form. - `BaseCommandTests.cs` — reverted #629's NuGet-specific changes but **kept** the generic `ParseAndInvokeWithCaptureAsync(..., CancellationToken)` overload, which is unrelated test infrastructure used by UI-command cancellation tests from #645. - Deleted `PackageInstallationServiceTests.cs` and `UpdateCommandTests.cs` (both new in #629; later expanded by #640/#641 for the migrated code, which no longer exists). - Deleted `PackageInstallationServiceCacheMarkerTests.cs` (added by #641; exercised the migrated `NugetService` against a local folder feed via `NugetFeedTestHelpers` + `INugetService.IsPackageInstalled`, all of which are removed here). ## Validation - `dotnet build winapp.sln -c Debug` — **0 warnings / 0 errors** - Test project builds clean; UI-command and NuGet-service test suites pass (447 passed / 0 failed / 1 skipped). - No remaining references to `dotnet-tools`, `7.9.0`, or `packageSourceMapping` in `nuget.config` / `Directory.Packages.props` / `.pipelines/release-nuget.config`. ## Re-landing This migration should be reapplied **only once stable `NuGet.Protocol 7.9.0` and all related `NuGet.*` packages are published on nuget.org** (not on dnceng dotnet-tools or any other secondary public feed). At that point: repoint the pins to the nuget.org stable, and drop the `dotnet-tools` source, `packageSourceMapping`, and the release-nuget.config upstream. See #653. --------- Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
PR 7 — Test coverage: UI-command logic
Raises test coverage on the
winapp uicommand surface (the UIA-driven inspect/click/type/screenshot/wait-for commands and the session resolver) toward the ≥95% per-file bar, part of the coverage-to-95% effort (issue #630). This slice covers command logic and the session resolver via injectable seams — the real in-process UIA COM/GPU/input layer (UiAutomationServiceand theReal*adapters) is deliberately left for a dedicated follow-up PR that drives a live window.Real-workflow tests first (resolver disambiguation, auto-select, JSON envelopes, error paths), then targeted tests for the retry/warning branches — all deterministic, no live desktop required.
Per-file Debug line coverage (authoritative
coverage-report.ps1 -Configuration Debug, full run)All 32 UI logic files are ≥95% (min 97.6%). Highlights:
(The remaining commands/helpers/models were already ≥95% or are now 100%.)
Product changes — test seams only, all behavior-preserving
No runtime-behavior change, no coverage exclusions, no visibility widened beyond
internal. Production wires the real path in every case:ISystemUiQuerywindow-metadata extension —GetWindowClassName/GetWindowSize/GetWindowOwneradded to the seam (native P/Invoke bodies moved verbatim intoSystemUiQuery).UiSessionService.PickLargestWindowis now an instance method that reads window sizes through the injected seam, so the "largest window" auto-select is unit-testable with fabricated sizes. BecauseUiListWindowsCommand/UiScreenshotCommand/UiAutomationServicestill callUiSessionService.GetWindowClassName/GetWindowInfostatically, a statelesss_sharedQuerybridges those static shims to the same real implementation (both point at the identical OS boundary; DI registers the realSystemUiQuery).ISystemUiQueryintoUiScreenshotCommand— the direct-HWND--windowPID/title reads route through the seam (real handles resolve identically).FrameworkHint.IsXamlClassName(string?)— the oldIsLikelyXaml(long)(which read a class name off a live HWND) is replaced by this pure classifier; the command reads the class name through the seam and passes it in. Now unit-testable without a live XAML window.IPollDelay/RealPollDelay— thewinapp ui wait-forinter-poll delay is behind an interface (RealPollDelayis a straightTask.Delaypassthrough), so the retry-loop "condition not met — keep polling" continuations are testable deterministically without a wall-clock wait.Seam precedent:
IOwnedWindowFinder/RealOwnedWindowFinderand the otherReal*adapters in this same command surface.Dead code deleted (per policy, not excluded):
FrameworkHint.IsLikelyXaml(long)became unused once the command reads the class name through the seam (only a test + a doc cref referenced it; both updated).Meaningful (non-vacuous) tests
WinUIDesktopWin32WindowClass) fires the advisory warning (keys still posted), a Win32 class (Notepad) does not. Warnings route through the static ambientAnsiConsole(TextWriterLogger), so these two tests swap it to a capturing console under[DoNotParallelize]with a try/finally restore — the established pattern in this repo.IPollDelayseam; each asserts the real outcome (exit code / JSON"found": true) and that a keep-polling iteration actually ran (FakePollDelay.CallCount). No wall-clock timing.Honest ceiling
Services/SystemUiQuery.cs(native Win32/Processadapter) sits at ~27% andRealPollDelay/RealOwnedWindowFinderat ~0% — these are thin OS-boundary wrappers with no branching logic, the same honest-ceiling category as the existingRealForegroundGuard/RealMouseInput/RealKeyboardInputadapters, documented in their XML comments. They are not hand-written logic and carry no[ExcludeFromCodeCoverage].Verification
WinApp.Cli.csproj+WinApp.Cli.Tests.csproj,-p:UseSharedCompilation=false -nodeReuse:false): 0 warnings / 0 errors (warnings-as-errors, incl. CA1859).EndToEndTests.E2E_Node*tests needing the un-built Nodesrc/winapp-npm/dist/cli.js; none from this PR.Review
This branch was reviewed with the repo's
pr-reviewskill (7 specialists + a multi-model cross-check across the GPT and Gemini families) before opening. It surfaced four medium findings — a vacuous "largest window" assertion and three declared coverage "ceilings" that were actually reachable once a single seam gap was closed (GetWindowClassName/GetWindowSize/GetWindowOwnerwere still hardcoded P/Invokes bypassingISystemUiQuery). All four were fixed as above (the three "ceilings" are now genuinely 100%), and the vacuous test now asserts the correct largest-area window.Co-authored-by: Copilot App 223556219+Copilot@users.noreply.github.com