Tests: non-UI CLI commands coverage to =>95%#640
Merged
Conversation
Add meaningful, use-case-driven MSTest coverage for the non-UI winapp CLI commands, driving every in-scope command class to >=95% line coverage measured on a Debug build. Tests exercise real command invocations: option parsing/validation, help/error output, exit codes, and delegation to services mocked at the existing DI seam, covering both success and error/usage paths. Per-file Debug line coverage: Init 95.1, CreateDebugIdentity 95.5, Tool 97.5, WinAppRoot 98.4, ManifestAddAlias 98.5, Run 99.1, Unregister 99.1, Package 99.5, and Update / Restore / Complete / Sign / MSStore / CertGenerate / CertInfo / CertInstall / GetWinappPath / CreateExternalCatalog / ManifestGenerate / ManifestUpdateAssets all at 100. Two behavior-preserving product test seams, both defaulting to the production implementation so runtime behavior is unchanged (matching the existing Services/UpdateNotificationService internal Func seam precedent): - RunCommand.Handler.ResolveAliasProxy / ProcessStarter - MSStoreCommand.Handler.ProcessStarter Remove a provably-unreachable dead null guard in ManifestUpdateAssetsCommand: the image argument is required (arity ExactlyOne), so the parser rejects a missing value before the handler runs. The invariant is documented via a null-forgiving access instead of dead defensive code. Reusable fakes/helpers added for the new tests (FakeMSStoreCLIService, FakeWorkspaceSetupService, FakeCertificateService, FakePackageInstallationService, and a FakeDebugOutputService cancellation hook). Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR is focused on substantially increasing unit/integration-style test coverage for the non-UI winapp CLI command surface, with only a few minimal product-code adjustments to make OS-boundary branches deterministically testable.
Changes:
- Adds broad new test coverage across multiple command handlers (run, update, tool, store, restore, init, manifest, cert/sign, etc.), including error/edge branches and structured JSON error outputs.
- Introduces small test seams (
ProcessStarter,ResolveAliasProxy) inRunCommandandMSStoreCommandto cover hard-to-reproduce OS/process-start branches. - Removes unreachable/null-guard logic in
ManifestUpdateAssetsCommandand updates tests accordingly.
Reviewed changes
Copilot reviewed 29 out of 29 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/winapp-CLI/WinApp.Cli/Commands/RunCommand.cs | Adds internal test seams for execution-alias proxy resolution and process start. |
| src/winapp-CLI/WinApp.Cli/Commands/MSStoreCommand.cs | Adds internal ProcessStarter seam to cover null-process defensive branch. |
| src/winapp-CLI/WinApp.Cli/Commands/ManifestUpdateAssetsCommand.cs | Removes dead null-check and uses a null-forgiving required argument access. |
| src/winapp-CLI/WinApp.Cli.Tests/UpdateCommandTests.cs | New end-to-end-ish handler tests using faked services for update flow branches. |
| src/winapp-CLI/WinApp.Cli.Tests/UnregisterCommandTests.cs | Adds JSON-mode structured-error assertion when manifest is missing. |
| src/winapp-CLI/WinApp.Cli.Tests/ToolCommandTests.cs | New tests for arg forwarding/quoting + stdout/stderr forwarding (console redirection). |
| src/winapp-CLI/WinApp.Cli.Tests/SignCommandFakeTests.cs | Adds fake-certificate-service tests for success and exception branches. |
| src/winapp-CLI/WinApp.Cli.Tests/RunCommandTests.cs | Expands coverage for default launch, alias launch, debug-output, cancellation, unregister-on-exit, JSON errors. |
| src/winapp-CLI/WinApp.Cli.Tests/RestoreCommandTests.cs | New tests for restore option parsing + delegation to workspace-setup service. |
| src/winapp-CLI/WinApp.Cli.Tests/PackageCommandFakeMsixTests.cs | Adds fake-MSIX-service tests for signed/unsigned + bundle/package + failure branches. |
| src/winapp-CLI/WinApp.Cli.Tests/MSStoreCommandTests.cs | New tests for store command delegation, exit-code propagation, and seam-covered null-process branch. |
| src/winapp-CLI/WinApp.Cli.Tests/ManifestUpdateAssetsCommandTests.cs | Adds missing-manifest and invalid-image error-branch tests. |
| src/winapp-CLI/WinApp.Cli.Tests/ManifestGenerateCommandTests.cs | Adds tests for manifest generation templates and if-exists behaviors + failure injection. |
| src/winapp-CLI/WinApp.Cli.Tests/ManifestAddAliasCommandMappingTests.cs | Adds tests for status-to-message mapping via fake manifest service. |
| src/winapp-CLI/WinApp.Cli.Tests/InitCommandTests.cs | Adds interactive prompt/selection behavior tests for multi-project and no-project cases. |
| src/winapp-CLI/WinApp.Cli.Tests/GetWinappPathCommandErrorTests.cs | Adds tests that inject throwing directory service to cover defensive error handling. |
| src/winapp-CLI/WinApp.Cli.Tests/FakeWorkspaceSetupService.cs | Introduces reusable fake workspace-setup service for restore/update tests. |
| src/winapp-CLI/WinApp.Cli.Tests/FakePackageInstallationService.cs | Introduces reusable fake package-installation service for update/install tests. |
| src/winapp-CLI/WinApp.Cli.Tests/FakeMSStoreCLIService.cs | Adds fake msstore CLI service for deterministic MSStoreCommand tests. |
| src/winapp-CLI/WinApp.Cli.Tests/FakeMsixService.cs | Extends fake MSIX service to support additional command test scenarios and error branches. |
| src/winapp-CLI/WinApp.Cli.Tests/FakeDebugOutputService.cs | Extends fake debug loop to support cancellation-during-loop coverage. |
| src/winapp-CLI/WinApp.Cli.Tests/FakeCertificateService.cs | Adds reusable fake certificate service for sign/cert command tests. |
| src/winapp-CLI/WinApp.Cli.Tests/CustomHelpTests.cs | Expands help/short-description coverage to include nested subcommands. |
| src/winapp-CLI/WinApp.Cli.Tests/CreateExternalCatalogCommandOutputPathTests.cs | Adds tests covering output-path resolution and folder-splitting logic. |
| src/winapp-CLI/WinApp.Cli.Tests/CreateDebugIdentityCommandTests.cs | Adds tests for manifest resolution, option pass-through, and failure handling for debug identity creation. |
| src/winapp-CLI/WinApp.Cli.Tests/CompleteCommandTests.cs | Adds coverage for enum option-value completion branch (no-description candidates). |
| src/winapp-CLI/WinApp.Cli.Tests/CertInstallCommandTests.cs | Adds fake-certificate-service tests for install success/already-installed/failure branches. |
| src/winapp-CLI/WinApp.Cli.Tests/CertInfoCommandTests.cs | Adds TOCTOU coverage where cert file is removed after parse but before invocation (JSON and non-JSON). |
| src/winapp-CLI/WinApp.Cli.Tests/CertGenerateCommandTests.cs | Adds non-JSON coverage for if-exists Error/Skip/Overwrite behaviors. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+52
to
+54
| // ImageArgument is a required argument (arity ExactlyOne), so the parser rejects a | ||
| // missing value before the handler runs — GetValue never returns null here. | ||
| var imagePath = parseResult.GetValue(ImageArgument)!; |
| } | ||
|
|
||
| [TestMethod] | ||
| public void AllCommands_IncludingNested_ShortDescriptionMatchesDescription() |
Contributor
Build Metrics ReportBinary Sizes
Test Results✅ 2189 passed, 2 skipped out of 2191 tests in 640.0s (+117 tests, -64.1s vs. baseline) Test CoverageCLI Startup Time46ms median (x64, Updated 2026-07-15 21:33:10 UTC · commit |
Resolves the add/add conflict on WinApp.Cli.Tests/UpdateCommandTests.cs introduced when PR #629 (migrate NugetService to the official NuGet client libraries) merged to main and added its own UpdateCommandTests. Resolution keeps BOTH test sets in one class: - All 9 of this branch's control-flow tests (config handling, build-tools gating, runtime-install step, preview mode, unexpected-exception path) are kept. - All 5 of #629's version-decision-gate tests (LatestIsHigher/NormalizedEqual/ Lower, LatestVersionLookupFails => exit non-zero, CancelledDuringLookup) are added, adapted onto this branch's configurable fakes. - Dropped only this branch's Update_NuGetFailsForPackage_KeepsCurrentVersion AndSkipsInstall: #629 changed UpdateCommand so a lookup failure now exits non-zero and preserves the pin, which directly contradicts that test. It is superseded by #629's Update_LatestVersionLookupFails_ExitsNonZeroAndPreservesPin. - Added a computed InstalledPackages convenience to FakePackageInstallation Service so #629's ported assertions read naturally. UpdateCommandTests: 14/14 green (Debug). No product-code changes in the merge. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
azchohfi
added a commit
that referenced
this pull request
Jul 16, 2026
…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
azchohfi
added a commit
that referenced
this pull request
Jul 16, 2026
…#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>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Test coverage: non-UI CLI commands
Part of the CLI test-coverage push (tracking #630). This PR raises meaningful
line coverage across the non-UI command surface (cert, sign, package, run,
manifest, init, restore, update, tool, msstore, complete, unregister, and the
root/help wiring), targeting ≥95% per hand-written command file with real,
use-case-driven tests plus unit tests for the branch/error paths.
Per-file Debug line coverage (all 21 in-scope non-UI command files ≥95%)
100%— UpdateCommand, RestoreCommand, CompleteCommand, SignCommand,MSStoreCommand, CertGenerateCommand, CertInfoCommand, CertInstallCommand,
GetWinappPathCommand, CreateExternalCatalogCommand, ManifestGenerateCommand,
ManifestUpdateAssetsCommand.
≥95%— InitCommand 95.1 · CreateDebugIdentityCommand 95.5 · ToolCommand 97.5 ·WinAppRootCommand 98.4 · ManifestAddAliasCommand 98.5 · RunCommand 99.1 ·
UnregisterCommand 99.1 · PackageCommand 99.5.
No
[ExcludeFromCodeCoverage], no command exclusions.Product code (3 minimal, behavior-preserving edits)
launch path:
internal Func<string, FileInfo?> ResolveAliasProxy(alias-proxyresolution under
%LOCALAPPDATA%\Microsoft\WindowsApps) andinternal Func<ProcessStartInfo, Process?> ProcessStarter. Both default to theproduction call, so runtime behavior is unchanged. This lets tests exercise all
the surrounding validation / debug / cancellation / error-handling logic without
registering a real alias proxy or spawning the resolved binary. Follows the
existing
UpdateNotificationServiceinternal-seam precedent.ProcessStarterseam for the MSStoreCLI process start(defaults to
Process.Start), so the null-process defensive branch is testable.if (imagePath == null)guard.
ImageArgumentis a required argument (arityExactlyOne, no default), soSystem.CommandLine rejects a missing value before the handler runs and
GetValue(ImageArgument)is never null insideInvokeAsync. The guard wasunreachable dead code; deleting it (rather than excluding it) is the honest
coverage fix.
Notable test work
paths): a fake debug loop cancels the token during the loop, then the test
asserts the package's processes are terminated and the loop's exit code is still
returned. Deterministic (no timing race), and fails if the cleanup branch is
removed.
real captured console output (a space-containing arg is forwarded as a single
quoted token), not just exit codes.
messages.
cmd.exevia its fixedSystem32path rather than the%ComSpec%environment variable, to avoid any PATH/env hijack in the test host.Review
Reviewed with the repo
pr-reviewskill before opening (7 specialists + amulti-model cross-check): 0 critical / 0 high; all medium/low findings
addressed (padding tests upgraded to real behavior assertions, the two
debug-cancel branches covered, the dead guard deleted, the ComSpec/PATH hijack
removed, signed/unsigned and search-limit-hint messages asserted).
Coverage is measured on a Debug build (see #637) because optimized Release
under-counts standalone brace lines and inlined static helpers.