test: raise Manifest & asset service coverage to >=95% (Debug)#638
Conversation
There was a problem hiding this comment.
Pull request overview
Expands Debug coverage for manifest and image-asset services through test-only changes.
Changes:
- Adds manifest generation, alias, template, and file-reference tests.
- Adds raster/SVG asset generation and error-path tests.
- Introduces a shared raster PNG fixture helper.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
PngHelper.cs |
Adds raster PNG fixture generation. |
ManifestTemplateServiceTests.cs |
Tests template expansion and ID sanitization. |
ManifestServiceTests.cs |
Tests manifest, asset, and alias workflows. |
ManifestFileReferenceHelperTests.cs |
Tests manifest file-reference detection. |
ImageAssetServiceTests.cs |
Tests image and ICO generation. |
AppxManifestDocumentTests.cs |
Extends manifest document coverage. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| [DataRow("My App", "My.App", DisplayName = "space becomes a dot separator")] | ||
| [DataRow("com.my-app", "com.my.app", DisplayName = "dash becomes a dot separator")] | ||
| [DataRow("123abc", "A123abc", DisplayName = "segment starting with digit gets A prefix")] | ||
| [DataRow("Contoso.App_2024!", "Contoso.App.A2024", DisplayName = "invalid chars split, digit segment prefixed")] |
There was a problem hiding this comment.
Fixed in 83a9342. FixAsciiWindowsId now falls back to "Default" when an all-separator input reassembles to an empty id, and I added the "..." -> "Default" DataRow that exposed it.
| <Applications> | ||
| <Application Id="App"> | ||
| <uap:VisualElements Square150x150Logo="Assets\MedTile.png"> | ||
| <uap:DefaultTile Wide310x150Logo="Assets\WideTile.png" /> | ||
| </uap:VisualElements> |
There was a problem hiding this comment.
Fixed in 83a9342. The test now lists an earlier reference in a different directory (Images\) and gives Assets\ the majority (2 refs vs 1), then asserts app.ico lands in the majority directory and NOT in the first-listed one — so it actually exercises majority selection.
| Assert.IsTrue(File.Exists(Path.Combine(_tempDir.FullName, "Package.appxmanifest")), | ||
| "the manifest is generated even when relying on executable icon extraction"); |
There was a problem hiding this comment.
Fixed in 83a9342. Added a behavior-preserving ExecutableIconExtractor seam (default ShellIcon.GetJumboIcon); the test injects a deterministic icon and now asserts the extracted Assets\app.ico, covering the extracted-icon path without depending on the headless shell image list.
| var content = await File.ReadAllTextAsync(manifest.FullName); | ||
| StringAssert.Contains(content, "uap5:ExecutionAlias"); | ||
| StringAssert.Contains(content, "Alias=\"myapp.exe\""); | ||
| StringAssert.Contains(content, "uap5"); // ignorable namespaces updated |
There was a problem hiding this comment.
Fixed in 83a9342. The test now parses the manifest and asserts the actual IgnorableNamespaces attribute value (contains uap5, preserves uap10) instead of the tautological Contains("uap5"). Also added a test for the uap5-namespace-declaration branch.
Add MSTest coverage for the Manifest & assets service surface, driven by real workflows (manifest generation/edit, identity/capabilities/extensions, template expansion, namespace-aware XDocument reads/writes, image/asset generation, file-reference resolution) plus error and edge paths. New: ManifestServiceTests, ImageAssetServiceTests, ManifestTemplateServiceTests, ManifestFileReferenceHelperTests. Extended AppxManifestDocumentTests with 6 branch-gap tests (null-attribute removal, Resources ordering after Dependencies, package-level extensions on root). Serialize the two GDI+-heavy test classes with [DoNotParallelize]: System.Drawing's encoder lookup is not thread-safe and races under MSTest method-level parallelism. Production renders assets sequentially, so no product change is needed. No product code changed. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
32eff72 to
e39600e
Compare
Build Metrics ReportBinary Sizes
Test Results✅ 1749 passed, 1 skipped out of 1750 tests in 385.7s (+112 tests, -11.4s vs. baseline) Test Coverage❌ 48.4% line coverage, 54.3% branch coverage · ✅ +0.4% vs. baseline CLI Startup Time50ms median (x64, Updated 2026-07-15 11:28:18 UTC · commit |
- FixAsciiWindowsId: fall back to "Default" when an all-separator input
(e.g. "...") reassembles to an empty id; add the "..." -> "Default"
DataRow that exposed it.
- UsesMostCommonAssetDirectory: add an earlier reference in a different
directory so Assets wins by majority (2 vs 1), and assert app.ico lands
in the majority dir and not the first-listed one.
- GenerateManifest_WithExecutable: assert the extracted app.ico via a new
behavior-preserving ExecutableIconExtractor seam (default GetJumboIcon),
so the extracted-icon path is covered deterministically instead of
depending on the headless shell image list. Adds a uap5-namespace-
declaration test.
- AddAlias: assert the actual IgnorableNamespaces attribute value instead
of a tautological Contains("uap5").
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Summary
Raises hand-written line coverage on the Manifest & image-asset service surface to ≥95% per file (measured in Debug), with use-case-driven tests. Test-only — no product code changes.
Per-file coverage (authoritative full Debug run:
pwsh scripts/coverage-report.ps1 -Configuration Debug)Tests
112 net-new executed cases (78
[TestMethod]s + 39[DataRow]s) across 5 classes. Use-case-driven: manifest generate/edit, identity/capabilities/extensions, all execution-alias status branches, template expansion + ASCII/id sanitization, namespace-awareXDocumentreads/writes, raster + SVG rendering, asset/ICO generation, file-reference resolution — plus malformed/edge/error paths. All passing; Release build clean (warnings-as-errors).Notes
-Configuration Debug(the switch to Debug-by-default is Collect coverage on a Debug build for accurate line coverage (#630) #637).[DoNotParallelize]on the two GDI+-heavy test classes is a test attribute only — it avoids aSystem.Drawingencoder-lookup race under MSTest's 32-worker method-level parallelism. Production renders assets sequentially, so this is a test-only artifact..;XDocument.Parsethrows before it could return a nullRoot) — verified empirically and left in place. No[ExcludeFromCodeCoverage], no product changes.PngHelper.CreateRasterPngrenders test PNG fixtures in one shared place (dedup from review).Review
Reviewed with the repo
pr-reviewskill (7 specialist dimensions + independent multi-model cross-check): 0 critical, 0 high. The single medium finding (duplicated raster-PNG helper) is resolved in this branch.