From e39600ed2aa6b6d0fca70c56b53b0c1cd54af75d Mon Sep 17 00:00:00 2001 From: Alexandre Zollinger Chohfi Date: Wed, 15 Jul 2026 00:01:24 -0700 Subject: [PATCH 1/2] test: raise Manifest & asset service coverage with use-case-driven tests 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> --- .../AppxManifestDocumentTests.cs | 102 ++++ .../ImageAssetServiceTests.cs | 352 +++++++++++ .../ManifestFileReferenceHelperTests.cs | 209 +++++++ .../WinApp.Cli.Tests/ManifestServiceTests.cs | 575 ++++++++++++++++++ .../ManifestTemplateServiceTests.cs | 249 ++++++++ src/winapp-CLI/WinApp.Cli.Tests/PngHelper.cs | 18 + 6 files changed, 1505 insertions(+) create mode 100644 src/winapp-CLI/WinApp.Cli.Tests/ImageAssetServiceTests.cs create mode 100644 src/winapp-CLI/WinApp.Cli.Tests/ManifestFileReferenceHelperTests.cs create mode 100644 src/winapp-CLI/WinApp.Cli.Tests/ManifestServiceTests.cs create mode 100644 src/winapp-CLI/WinApp.Cli.Tests/ManifestTemplateServiceTests.cs diff --git a/src/winapp-CLI/WinApp.Cli.Tests/AppxManifestDocumentTests.cs b/src/winapp-CLI/WinApp.Cli.Tests/AppxManifestDocumentTests.cs index 07068827f..d98f22efd 100644 --- a/src/winapp-CLI/WinApp.Cli.Tests/AppxManifestDocumentTests.cs +++ b/src/winapp-CLI/WinApp.Cli.Tests/AppxManifestDocumentTests.cs @@ -231,6 +231,18 @@ public void ApplicationProperties_SetOnMissing_IsNoOp() Assert.IsNull(doc.ApplicationId); // still null because there's no Application element } + [TestMethod] + public void ApplicationExecutable_SetNull_RemovesAttribute() + { + var doc = AppxManifestDocument.Parse(MinimalManifest); + Assert.AreEqual("TestApp.exe", doc.ApplicationExecutable); + + doc.ApplicationExecutable = null; + + Assert.IsNull(doc.ApplicationExecutable); + Assert.DoesNotContain("Executable=", doc.ToXml()); + } + #endregion #region VisualElements @@ -252,6 +264,27 @@ public void VisualElementsDisplayName_ReturnsNull_WhenMissing() Assert.IsNull(doc.VisualElementsDisplayName); } + [TestMethod] + public void VisualElementsDisplayName_SetNull_RemovesAttribute() + { + var doc = AppxManifestDocument.Parse(MinimalManifest); + Assert.AreEqual("Test App", doc.VisualElementsDisplayName); + + doc.VisualElementsDisplayName = null; + + Assert.IsNull(doc.VisualElementsDisplayName); + } + + [TestMethod] + public void VisualElementsDisplayName_SetOnMissing_IsNoOp() + { + var doc = AppxManifestDocument.Parse(BareMinimalManifest); + + doc.VisualElementsDisplayName = "ShouldNotThrow"; + + Assert.IsNull(doc.VisualElementsDisplayName); // still null because there is no VisualElements element + } + #endregion #region Resource Languages @@ -327,6 +360,60 @@ public void SetResourceLanguages_CreatesResourcesElement_WhenMissing() Assert.AreEqual("en-US", languages[0]); } + [TestMethod] + public void SetResourceLanguages_AddsToRoot_WhenNoIdentityOrDependencies() + { + // No Identity and no Dependencies, so the new Resources element falls through + // to being appended directly to the Package root. + var xml = """ + + + + + + """; + var doc = AppxManifestDocument.Parse(xml); + + doc.SetResourceLanguages(["en-US", "fr-FR"]); + var languages = doc.GetResourceLanguages(); + + Assert.AreEqual(2, languages.Count); + Assert.AreEqual("en-US", languages[0]); + Assert.AreEqual("fr-FR", languages[1]); + } + + [TestMethod] + public void SetResourceLanguages_InsertsAfterDependencies_WhenResourcesMissing() + { + // Identity + Dependencies but no Resources: the new Resources element must be + // inserted immediately after Dependencies to satisfy the manifest schema ordering. + var xml = """ + + + + + + + + + + """; + var doc = AppxManifestDocument.Parse(xml); + + doc.SetResourceLanguages(["en-US"]); + + var result = doc.ToXml(); + var languages = doc.GetResourceLanguages(); + Assert.AreEqual(1, languages.Count); + Assert.AreEqual("en-US", languages[0]); + Assert.IsTrue( + result.IndexOf("", StringComparison.Ordinal) < result.IndexOf(" +/// Tests for and its helper. +/// Drives the real asset-generation workflow (scale + targetsize variants, light-theme +/// variants, ICO writing) from raster / SVG / ICO sources, plus the decode and +/// not-found error paths that surface actionable exceptions. +/// +// These tests render real bitmaps through GDI+ (System.Drawing). GDI+ encoder lookup +// (Image.Save with an ImageFormat) is not thread-safe and races under MSTest's +// method-level parallelism, intermittently throwing "Value cannot be null. +// (Parameter 'encoder')". Production renders assets sequentially, so serialize this +// class rather than altering product code. +[DoNotParallelize] +[TestClass] +public class ImageAssetServiceTests +{ + private DirectoryInfo _tempDir = null!; + + [TestInitialize] + public void Setup() + { + _tempDir = new DirectoryInfo(Path.Combine(Path.GetTempPath(), $"ImageAssetTest_{Guid.NewGuid():N}")); + _tempDir.Create(); + } + + [TestCleanup] + public void Cleanup() + { + if (_tempDir.Exists) + { + _tempDir.Delete(recursive: true); + } + } + + private static TaskContext CreateTaskContext() + { + var task = new GroupableTask("test", null); + var console = new Spectre.Console.Testing.TestConsole(); + return new TaskContext(task, null, console, NullLogger.Instance, new Lock()); + } + + private string Path_(string name) => Path.Combine(_tempDir.FullName, name); + + private string CreateSvg(string name, int width, int height) + { + var path = Path_(name); + File.WriteAllText(path, + $""""""); + return path; + } + + #region ImageSource + + [TestMethod] + public void ImageSource_FromRaster_ReportsBitmapMetadata() + { + var path = PngHelper.CreateRasterPng(Path_("wide.png"), 40, 20); + + using var source = ImageSource.FromFile(new FileInfo(path)); + + Assert.IsFalse(source.IsSvg, "a PNG source is not an SVG"); + Assert.AreEqual(2f, source.AspectRatio, 0.01, "40x20 has a 2:1 aspect ratio"); + Assert.AreEqual("40x20", source.DimensionsLabel); + } + + [TestMethod] + public void ImageSource_FromSvg_ReportsSvgMetadata() + { + var path = CreateSvg("wide.svg", 200, 100); + + using var source = ImageSource.FromFile(new FileInfo(path)); + + Assert.IsTrue(source.IsSvg, "an SVG source reports IsSvg"); + Assert.AreEqual(2f, source.AspectRatio, 0.01); + StringAssert.Contains(source.DimensionsLabel, "(SVG)"); + StringAssert.Contains(source.DimensionsLabel, "200x100"); + } + + [TestMethod] + public void ImageSource_RenderToPng_ProducesRequestedSize() + { + using var source = ImageSource.FromFile(new FileInfo(PngHelper.CreateRasterPng(Path_("src.png"), 40, 20))); + + var png = source.RenderToPng(64, 64); + + using var ms = new MemoryStream(png); + using var decoded = new Bitmap(ms); + Assert.AreEqual(64, decoded.Width); + Assert.AreEqual(64, decoded.Height); + } + + [TestMethod] + public void ImageSource_FromInvalidSvg_Throws() + { + var path = Path_("broken.svg"); + File.WriteAllText(path, "this is not svg"); + + var threw = false; + try + { + using var _ = ImageSource.FromFile(new FileInfo(path)); + } + catch + { + threw = true; + } + + Assert.IsTrue(threw, "an unparseable SVG must surface an error rather than a null picture"); + } + + [TestMethod] + public void ImageSource_FromZeroDimensionSvg_Throws() + { + var path = Path_("empty.svg"); + File.WriteAllText(path, + """"""); + + Assert.ThrowsExactly(() => + { + using var _ = ImageSource.FromFile(new FileInfo(path)); + }); + } + + #endregion + + #region GenerateAssetsFromManifestAsync - workflows + + [TestMethod] + public async Task GenerateAssets_RasterSource_WritesScaleAndTargetSizeVariants() + { + // A 44x44 asset reference triggers both scale variants and the targetsize plated/unplated set. + var source = new FileInfo(PngHelper.CreateRasterPng(Path_("app.png"), 40, 20)); + var refs = new[] { new ManifestAssetReference(@"Assets\AppList.png", 44, 44) }; + + await new ImageAssetService().GenerateAssetsFromManifestAsync( + source, _tempDir, refs, CreateTaskContext()); + + var assetsDir = Path.Combine(_tempDir.FullName, "Assets"); + Assert.IsTrue(File.Exists(Path.Combine(assetsDir, "AppList.png")), "base scale asset written"); + Assert.IsTrue(File.Exists(Path.Combine(assetsDir, "AppList.scale-200.png")), "scale-200 variant written"); + Assert.IsTrue(File.Exists(Path.Combine(assetsDir, "AppList.targetsize-16.png")), "targetsize variant written"); + Assert.IsTrue(File.Exists(Path.Combine(assetsDir, "AppList.targetsize-256_altform-unplated.png")), "unplated targetsize variant written"); + } + + [TestMethod] + public async Task GenerateAssets_NonIconReference_SkipsTargetSizeVariants() + { + var source = new FileInfo(PngHelper.CreateRasterPng(Path_("app.png"), 40, 20)); + var refs = new[] { new ManifestAssetReference(@"Assets\MedTile.png", 150, 150) }; + + await new ImageAssetService().GenerateAssetsFromManifestAsync( + source, _tempDir, refs, CreateTaskContext()); + + var assetsDir = Path.Combine(_tempDir.FullName, "Assets"); + Assert.IsTrue(File.Exists(Path.Combine(assetsDir, "MedTile.png"))); + Assert.IsFalse(File.Exists(Path.Combine(assetsDir, "MedTile.targetsize-16.png")), + "non 44x44 assets do not get targetsize variants"); + } + + [TestMethod] + public async Task GenerateAssets_SvgSource_RendersVariants() + { + var source = new FileInfo(CreateSvg("app.svg", 200, 100)); + var refs = new[] + { + new ManifestAssetReference(@"Assets\AppList.png", 44, 44), + new ManifestAssetReference(@"Assets\WideTile.png", 310, 150), + }; + + await new ImageAssetService().GenerateAssetsFromManifestAsync( + source, _tempDir, refs, CreateTaskContext()); + + var assetsDir = Path.Combine(_tempDir.FullName, "Assets"); + Assert.IsTrue(File.Exists(Path.Combine(assetsDir, "AppList.png"))); + Assert.IsTrue(File.Exists(Path.Combine(assetsDir, "WideTile.png"))); + } + + [TestMethod] + public async Task GenerateAssets_WithLightImage_WritesThemedVariants() + { + var source = new FileInfo(PngHelper.CreateRasterPng(Path_("dark.png"), 44, 44)); + var light = new FileInfo(PngHelper.CreateRasterPng(Path_("light.png"), 44, 44)); + var refs = new[] { new ManifestAssetReference(@"Assets\AppList.png", 44, 44) }; + + await new ImageAssetService().GenerateAssetsFromManifestAsync( + source, _tempDir, refs, CreateTaskContext(), light); + + var assetsDir = Path.Combine(_tempDir.FullName, "Assets"); + Assert.IsTrue( + File.Exists(Path.Combine(assetsDir, "AppList.scale-100_altform-colorful_theme-light.png")), + "light theme scale variant written"); + Assert.IsTrue( + File.Exists(Path.Combine(assetsDir, "AppList.targetsize-16_altform-lightunplated.png")), + "light theme targetsize variant written"); + } + + [TestMethod] + public async Task GenerateAssetsAsync_ConvenienceOverload_UsesDefaultReferenceSet() + { + var source = new FileInfo(PngHelper.CreateRasterPng(Path_("app.png"), 40, 20)); + + await new ImageAssetService().GenerateAssetsAsync(source, _tempDir, CreateTaskContext()); + + // Default reference set includes StoreLogo (50x50) and the 44x44 app icon. + Assert.IsTrue(File.Exists(Path.Combine(_tempDir.FullName, "StoreLogo.png"))); + Assert.IsTrue(File.Exists(Path.Combine(_tempDir.FullName, "AppList.png"))); + Assert.IsTrue(File.Exists(Path.Combine(_tempDir.FullName, "AppList.targetsize-32.png"))); + } + + #endregion + + #region GenerateAssetsFromManifestAsync - guard rails + + [TestMethod] + public async Task GenerateAssets_MissingSource_ThrowsFileNotFound() + { + var missing = new FileInfo(Path_("does-not-exist.png")); + var refs = new[] { new ManifestAssetReference("StoreLogo.png", 50, 50) }; + + await Assert.ThrowsExactlyAsync(() => + new ImageAssetService().GenerateAssetsFromManifestAsync(missing, _tempDir, refs, CreateTaskContext())); + } + + [TestMethod] + public async Task GenerateAssets_MissingLightImage_ThrowsFileNotFound() + { + var source = new FileInfo(PngHelper.CreateRasterPng(Path_("app.png"), 44, 44)); + var missingLight = new FileInfo(Path_("no-light.png")); + var refs = new[] { new ManifestAssetReference("StoreLogo.png", 50, 50) }; + + await Assert.ThrowsExactlyAsync(() => + new ImageAssetService().GenerateAssetsFromManifestAsync(source, _tempDir, refs, CreateTaskContext(), missingLight)); + } + + [TestMethod] + public async Task GenerateAssets_EmptyReferenceList_WarnsAndReturns() + { + var source = new FileInfo(PngHelper.CreateRasterPng(Path_("app.png"), 44, 44)); + + await new ImageAssetService().GenerateAssetsFromManifestAsync( + source, _tempDir, Array.Empty(), CreateTaskContext()); + + // No assets should be produced, and no exception thrown — only the source file remains. + Assert.AreEqual(1, _tempDir.GetFiles("*.png", SearchOption.AllDirectories).Length, + "an empty reference list generates nothing; only the source png exists"); + } + + [TestMethod] + public async Task GenerateAssets_UndecodableSource_ThrowsInvalidOperation() + { + var bad = new FileInfo(Path_("garbage.png")); + File.WriteAllText(bad.FullName, "not really a png"); + var refs = new[] { new ManifestAssetReference("StoreLogo.png", 50, 50) }; + + await Assert.ThrowsExactlyAsync(() => + new ImageAssetService().GenerateAssetsFromManifestAsync(bad, _tempDir, refs, CreateTaskContext())); + } + + [TestMethod] + public async Task GenerateAssets_UndecodableLightImage_ThrowsInvalidOperation() + { + var source = new FileInfo(PngHelper.CreateRasterPng(Path_("app.png"), 44, 44)); + var badLight = new FileInfo(Path_("garbage-light.png")); + File.WriteAllText(badLight.FullName, "not really a png"); + var refs = new[] { new ManifestAssetReference("StoreLogo.png", 50, 50) }; + + await Assert.ThrowsExactlyAsync(() => + new ImageAssetService().GenerateAssetsFromManifestAsync(source, _tempDir, refs, CreateTaskContext(), badLight)); + } + + [TestMethod] + public async Task GenerateAssets_Cancelled_ThrowsOperationCanceled() + { + var source = new FileInfo(PngHelper.CreateRasterPng(Path_("app.png"), 44, 44)); + var refs = new[] { new ManifestAssetReference(@"Assets\AppList.png", 44, 44) }; + using var cts = new CancellationTokenSource(); + cts.Cancel(); + + try + { + await new ImageAssetService().GenerateAssetsFromManifestAsync( + source, _tempDir, refs, CreateTaskContext(), null, cts.Token); + Assert.Fail("expected cancellation to propagate"); + } + catch (OperationCanceledException) + { + // Expected. + } + } + + #endregion + + #region GenerateIcoAsync + + [TestMethod] + public async Task GenerateIco_RasterSource_WritesMultiSizeIcon() + { + var source = new FileInfo(PngHelper.CreateRasterPng(Path_("app.png"), 40, 20)); + var icoPath = Path.Combine(_tempDir.FullName, "icons", "app.ico"); + + await new ImageAssetService().GenerateIcoAsync(source, icoPath, CreateTaskContext()); + + Assert.IsTrue(File.Exists(icoPath), "nested output directory should be created and the ICO written"); + + var bytes = File.ReadAllBytes(icoPath); + Assert.AreEqual(0, BitConverter.ToUInt16(bytes, 0), "reserved field"); + Assert.AreEqual(1, BitConverter.ToUInt16(bytes, 2), "type = icon"); + Assert.AreEqual(5, BitConverter.ToUInt16(bytes, 4), "one directory entry per ICO size"); + } + + [TestMethod] + public async Task GenerateIco_SvgSource_CanBeReloadedAsIcoImageSource() + { + // Generate an ICO from an SVG, then feed that ICO back in as a source to cover the .ico branch. + var svg = new FileInfo(CreateSvg("app.svg", 200, 100)); + var icoPath = Path.Combine(_tempDir.FullName, "app.ico"); + await new ImageAssetService().GenerateIcoAsync(svg, icoPath, CreateTaskContext()); + + using var fromIco = ImageSource.FromFile(new FileInfo(icoPath)); + Assert.IsFalse(fromIco.IsSvg, "an .ico is loaded as a raster bitmap source"); + } + + [TestMethod] + public async Task GenerateIco_MissingSource_ThrowsFileNotFound() + { + var missing = new FileInfo(Path_("missing.png")); + + await Assert.ThrowsExactlyAsync(() => + new ImageAssetService().GenerateIcoAsync(missing, Path.Combine(_tempDir.FullName, "x.ico"), CreateTaskContext())); + } + + [TestMethod] + public async Task GenerateIco_UndecodableSource_ThrowsInvalidOperation() + { + var bad = new FileInfo(Path_("bad.svg")); + File.WriteAllText(bad.FullName, "definitely not svg"); + + await Assert.ThrowsExactlyAsync(() => + new ImageAssetService().GenerateIcoAsync(bad, Path.Combine(_tempDir.FullName, "x.ico"), CreateTaskContext())); + } + + #endregion +} diff --git a/src/winapp-CLI/WinApp.Cli.Tests/ManifestFileReferenceHelperTests.cs b/src/winapp-CLI/WinApp.Cli.Tests/ManifestFileReferenceHelperTests.cs new file mode 100644 index 000000000..c19ac43ec --- /dev/null +++ b/src/winapp-CLI/WinApp.Cli.Tests/ManifestFileReferenceHelperTests.cs @@ -0,0 +1,209 @@ +// Copyright (c) Microsoft Corporation and Contributors. All rights reserved. +// Licensed under the MIT License. + +using WinApp.Cli.Helpers; + +namespace WinApp.Cli.Tests; + +/// +/// Tests for , the pure static helper that +/// walks an AppxManifest and extracts relative file-path references (used by the bundle +/// path to gather payload files). Covers the real extraction workflow plus the +/// IsLikelyFilePath accept/reject heuristics that keep versions, GUIDs, URIs and +/// dotted class names out of the payload set. +/// +[TestClass] +public class ManifestFileReferenceHelperTests +{ + #region ExtractAllFileReferencesFromManifest + + [TestMethod] + public void ExtractAllFileReferences_RealManifest_FindsAttributeAndElementPaths() + { + // A representative manifest mixing file-path attributes, element-text paths, + // and plenty of non-path values that must NOT be treated as references. + var manifest = """ + + + + + + + + + + + + Runtime.dll + + + + + """; + + var refs = ManifestFileReferenceHelper.ExtractAllFileReferencesFromManifest(manifest); + + // File-path values are collected and normalized to backslashes. + Assert.IsTrue(refs.Contains(@"app\my-app.exe"), "Executable path should be extracted"); + Assert.IsTrue(refs.Contains(@"Assets\Logo.png"), "Forward-slash asset path should be normalized and extracted"); + Assert.IsTrue(refs.Contains(@"Assets\Small.png"), "Backslash asset path should be extracted"); + Assert.IsTrue(refs.Contains("Runtime.dll"), "Element-text DLL path should be extracted"); + Assert.AreEqual(4, refs.Count, "Only the four real file paths should be extracted"); + + // Non-path values must be rejected. + Assert.IsFalse(refs.Contains("1.0.0.0"), "Version strings are not file paths"); + Assert.IsFalse(refs.Contains("Windows.FullTrustApplication"), "Dotted class names are not file paths"); + } + + [TestMethod] + public void ExtractAllFileReferences_DeduplicatesCaseInsensitively() + { + var manifest = """ + + + + + + + + """; + + var refs = ManifestFileReferenceHelper.ExtractAllFileReferencesFromManifest(manifest); + + Assert.AreEqual(1, refs.Count, "The same path in different case/separators should dedupe to one entry"); + } + + [TestMethod] + public void ExtractAllFileReferences_IgnoresUrisVersionsAndNamespaces() + { + var manifest = """ + + + + + ms-resource:AppName + https://example.com/index.html + + + """; + + var refs = ManifestFileReferenceHelper.ExtractAllFileReferencesFromManifest(manifest); + + Assert.AreEqual(0, refs.Count, "URIs, ms-resource references and version strings should not be extracted"); + } + + [TestMethod] + public void ExtractAllFileReferences_MalformedXml_ReturnsEmpty() + { + var refs = ManifestFileReferenceHelper.ExtractAllFileReferencesFromManifest(""); + + Assert.AreEqual(0, refs.Count, "Unparseable manifest content should yield an empty set, not throw"); + } + + [TestMethod] + public void ExtractAllFileReferences_SkipsTextOfNonLeafElements() + { + // The text content check only applies to leaf elements; a parent element's + // concatenated inner text must not be misread as a file path. + var manifest = """ + + + + readme.md + + + """; + + var refs = ManifestFileReferenceHelper.ExtractAllFileReferencesFromManifest(manifest); + + Assert.AreEqual(1, refs.Count); + Assert.IsTrue(refs.Contains("readme.md"), "Only the leaf element's text should be extracted"); + } + + #endregion + + #region IsLikelyFilePath - accepted + + [TestMethod] + [DataRow("app.exe", DisplayName = "bare executable")] + [DataRow("Assets\\Logo.png", DisplayName = "relative image path")] + [DataRow("sub/dir/module.dll", DisplayName = "forward-slash nested dll")] + [DataRow("styles.css", DisplayName = "web asset")] + [DataRow("data.json", DisplayName = "config data")] + [DataRow("Resources.pri", DisplayName = "pri resource index")] + [DataRow("font.ttf", DisplayName = "font file")] + public void IsLikelyFilePath_AcceptsRelativePathsWithKnownExtensions(string value) + { + Assert.IsTrue(ManifestFileReferenceHelper.IsLikelyFilePath(value), $"'{value}' should be treated as a file path"); + } + + #endregion + + #region IsLikelyFilePath - rejected + + [TestMethod] + [DataRow("", DisplayName = "empty")] + [DataRow(" ", DisplayName = "whitespace")] + [DataRow("NoExtensionHere", DisplayName = "no dot / no extension")] + [DataRow("http://example.com/logo.png", DisplayName = "http uri")] + [DataRow("https://example.com/logo.png", DisplayName = "https uri")] + [DataRow("ms-appx:///Assets/Logo.png", DisplayName = "ms-appx uri")] + [DataRow("ms-resource:AppName", DisplayName = "ms-resource uri")] + [DataRow("..\\parent\\evil.exe", DisplayName = "path traversal")] + [DataRow("1.0.0.0", DisplayName = "version string")] + [DataRow("10.0.19041.0", DisplayName = "os build version")] + [DataRow("Windows.FullTrustApplication", DisplayName = "dotted class name / unknown extension")] + [DataRow("MyCompany.MyApp", DisplayName = "namespace-like identifier")] + [DataRow("archive.tar", DisplayName = "extension not in allow list")] + [DataRow("assets.bundle/icon", DisplayName = "dotted directory but final segment has no extension")] + [DataRow("my.folder\\readme", DisplayName = "dotted directory, extensionless leaf")] + public void IsLikelyFilePath_RejectsNonPaths(string value) + { + Assert.IsFalse(ManifestFileReferenceHelper.IsLikelyFilePath(value), $"'{value}' should NOT be treated as a file path"); + } + + [TestMethod] + public void IsLikelyFilePath_RejectsRootedPath() + { + Assert.IsFalse(ManifestFileReferenceHelper.IsLikelyFilePath(@"C:\Windows\app.exe"), "Absolute paths should be rejected"); + } + + [TestMethod] + public void IsLikelyFilePath_RejectsUncPath() + { + Assert.IsFalse(ManifestFileReferenceHelper.IsLikelyFilePath(@"\\server\share\app.exe"), "UNC paths should be rejected"); + } + + [TestMethod] + public void IsLikelyFilePath_RejectsGuid() + { + Assert.IsFalse( + ManifestFileReferenceHelper.IsLikelyFilePath("12345678-1234-1234-1234-123456789abc"), + "GUID values should be rejected"); + } + + #endregion + + #region NormalizePathSeparators + + [TestMethod] + public void NormalizePathSeparators_ConvertsForwardSlashes() + { + Assert.AreEqual(@"Assets\Sub\Logo.png", ManifestFileReferenceHelper.NormalizePathSeparators("Assets/Sub/Logo.png")); + } + + [TestMethod] + public void NormalizePathSeparators_LeavesBackslashPathUnchanged() + { + Assert.AreEqual(@"Assets\Logo.png", ManifestFileReferenceHelper.NormalizePathSeparators(@"Assets\Logo.png")); + } + + [TestMethod] + public void NormalizePathSeparators_NoSeparators_ReturnsSameValue() + { + Assert.AreEqual("Logo.png", ManifestFileReferenceHelper.NormalizePathSeparators("Logo.png")); + } + + #endregion +} diff --git a/src/winapp-CLI/WinApp.Cli.Tests/ManifestServiceTests.cs b/src/winapp-CLI/WinApp.Cli.Tests/ManifestServiceTests.cs new file mode 100644 index 000000000..ef25ce668 --- /dev/null +++ b/src/winapp-CLI/WinApp.Cli.Tests/ManifestServiceTests.cs @@ -0,0 +1,575 @@ +// Copyright (c) Microsoft Corporation and Contributors. All rights reserved. +// Licensed under the MIT License. + +using System.Text.RegularExpressions; +using System.Xml.Linq; +using Microsoft.Extensions.Logging.Abstractions; +using Spectre.Console; +using Spectre.Console.Testing; +using WinApp.Cli.ConsoleTasks; +using WinApp.Cli.Models; +using WinApp.Cli.Services; + +namespace WinApp.Cli.Tests; + +/// +/// Tests for , the orchestrator that turns CLI inputs into a +/// generated AppxManifest and its assets. Exercises the real prompt/derive workflow, manifest +/// generation with logo and executable-icon extraction, asset-reference extraction (including +/// splash / lock-screen / custom-dimension parsing), attribute formatting, and the full set of +/// execution-alias result branches. +/// +// UpdateManifestAssets renders bitmaps through GDI+ (System.Drawing), whose encoder +// lookup is not thread-safe and races under MSTest's method-level parallelism. Production +// renders assets sequentially, so serialize this class rather than altering product code. +[DoNotParallelize] +[TestClass] +public class ManifestServiceTests +{ + private DirectoryInfo _tempDir = null!; + + [TestInitialize] + public void Setup() + { + _tempDir = new DirectoryInfo(Path.Combine(Path.GetTempPath(), $"ManifestSvcTest_{Guid.NewGuid():N}")); + _tempDir.Create(); + } + + [TestCleanup] + public void Cleanup() + { + if (_tempDir.Exists) + { + try { _tempDir.Delete(recursive: true); } catch (IOException) { /* best effort */ } + } + } + + private static ManifestService NewService(IAnsiConsole? console = null) + => new(new ManifestTemplateService(), new ImageAssetService(), console ?? new TestConsole()); + + private static TaskContext CreateTaskContext() + { + var task = new GroupableTask("test", null); + return new TaskContext(task, null, new TestConsole(), NullLogger.Instance, new Lock()); + } + + private string WriteManifest(string name, string content) + { + var path = Path.Combine(_tempDir.FullName, name); + File.WriteAllText(path, content); + return path; + } + + #region PromptForManifestInfoAsync + + [TestMethod] + public async Task PromptForManifestInfo_UseDefaults_CleansNameAndFillsDefaults() + { + var info = await NewService().PromptForManifestInfoAsync( + _tempDir, packageName: "My App!!", publisherName: null, version: "1.0.0.0", + description: null, executable: null, useDefaults: true); + + Assert.AreEqual("MyApp", info.PackageName, "invalid characters are stripped from the package name"); + Assert.AreEqual("1.0.0.0", info.Version); + Assert.IsFalse(string.IsNullOrWhiteSpace(info.PublisherName), "a default publisher is filled in"); + Assert.IsFalse(string.IsNullOrWhiteSpace(info.Description), "a default description is filled in"); + } + + [TestMethod] + public async Task PromptForManifestInfo_WithExecutable_DerivesMetadataFromVersionInfo() + { + var exe = Environment.ProcessPath; + if (exe is null || !File.Exists(exe)) + { + Assert.Inconclusive("No process executable available to read version info from."); + return; + } + + var info = await NewService().PromptForManifestInfoAsync( + _tempDir, packageName: null, publisherName: null, version: "1.0.0.0", + description: null, executable: exe, useDefaults: true); + + Assert.IsFalse(string.IsNullOrWhiteSpace(info.PackageName), "package name is derived from the executable metadata"); + Assert.IsTrue(Regex.IsMatch(info.PackageName, "^[-.A-Za-z0-9]+$"), "derived package name is schema-clean"); + Assert.IsFalse(string.IsNullOrWhiteSpace(info.PublisherName), "publisher is derived from the executable company name"); + } + + [TestMethod] + public async Task PromptForManifestInfo_Interactive_UsesPromptedValues() + { + var console = new TestConsole(); + console.Profile.Capabilities.Interactive = true; + console.Input.PushTextWithEnter("MyPkg"); + console.Input.PushTextWithEnter("CN=Me"); + console.Input.PushTextWithEnter("9.9.9.9"); + console.Input.PushTextWithEnter("My description"); + + var info = await NewService(console).PromptForManifestInfoAsync( + _tempDir, packageName: "seed", publisherName: "seedPub", version: "1.0.0.0", + description: "seedDesc", executable: null, useDefaults: false); + + Assert.AreEqual("MyPkg", info.PackageName); + Assert.AreEqual("CN=Me", info.PublisherName); + Assert.AreEqual("9.9.9.9", info.Version); + Assert.AreEqual("My description", info.Description); + } + + #endregion + + #region CleanPackageName + + [TestMethod] + [DataRow("My App!!", "MyApp", DisplayName = "strips spaces and punctuation")] + [DataRow("Contoso.Sample-App", "Contoso.Sample-App", DisplayName = "keeps dots and hyphens")] + [DataRow("!!!", "DefaultPackage", DisplayName = "all-invalid falls back to DefaultPackage")] + [DataRow(" ", "DefaultPackage", DisplayName = "whitespace falls back to DefaultPackage")] + [DataRow("ab", "ab1", DisplayName = "pads to minimum length of 3")] + public void CleanPackageName_SanitizesToIdentitySchema(string input, string expected) + { + Assert.AreEqual(expected, ManifestService.CleanPackageName(input)); + } + + [TestMethod] + public void CleanPackageName_TruncatesToFiftyCharacters() + { + var result = ManifestService.CleanPackageName(new string('a', 80)); + Assert.AreEqual(50, result.Length); + } + + #endregion + + #region GenerateManifestAsync + + [TestMethod] + public async Task GenerateManifest_NoLogoNoExecutable_WritesManifest() + { + var info = new ManifestGenerationInfo("MyApp", "CN=Test", "1.0.0.0", "A description"); + + await NewService().GenerateManifestAsync( + _tempDir, info, ManifestTemplates.Packaged, logoPath: null, executable: null, CreateTaskContext()); + + Assert.IsTrue(File.Exists(Path.Combine(_tempDir.FullName, "Package.appxmanifest"))); + } + + [TestMethod] + public async Task GenerateManifest_WithLogo_RegeneratesAssetsAndIco() + { + var info = new ManifestGenerationInfo("MyApp", "CN=Test", "1.0.0.0", "A description"); + var logo = new FileInfo(PngHelper.CreateRasterPng(Path.Combine(_tempDir.FullName, "logo.png"), 64, 64, System.Drawing.Color.SlateBlue)); + + await NewService().GenerateManifestAsync( + _tempDir, info, ManifestTemplates.Packaged, logo, executable: null, CreateTaskContext()); + + Assert.IsTrue(File.Exists(Path.Combine(_tempDir.FullName, "Package.appxmanifest"))); + // The generated manifest references Assets\* logos, so assets are regenerated from the + // provided logo and an app.ico is written into that assets directory. + Assert.IsTrue(File.Exists(Path.Combine(_tempDir.FullName, "Assets", "app.ico")), "an app.ico is produced next to the assets"); + } + + [TestMethod] + public async Task GenerateManifest_WithExecutable_ExtractsIconAndCreatesManifest() + { + // notepad.exe reliably ships an icon; fall back to the running host if unavailable. + var notepad = Path.Combine(Environment.SystemDirectory, "notepad.exe"); + var exe = File.Exists(notepad) ? notepad : Environment.ProcessPath; + if (exe is null || !File.Exists(exe)) + { + Assert.Inconclusive("No executable with an icon available."); + return; + } + + var info = new ManifestGenerationInfo("MyApp", "CN=Test", "1.0.0.0", "A description"); + + await NewService().GenerateManifestAsync( + _tempDir, info, ManifestTemplates.Packaged, logoPath: null, executable: exe, CreateTaskContext()); + + Assert.IsTrue(File.Exists(Path.Combine(_tempDir.FullName, "Package.appxmanifest")), + "the manifest is generated even when relying on executable icon extraction"); + } + + #endregion + + #region UpdateManifestAssetsAsync + + [TestMethod] + public async Task UpdateManifestAssets_ManifestWithoutReferences_GeneratesDefaultAssets() + { + var manifest = WriteManifest("Package.appxmanifest", """ + + + + + """); + var logo = new FileInfo(PngHelper.CreateRasterPng(Path.Combine(_tempDir.FullName, "logo.png"), 40, 20, System.Drawing.Color.SlateBlue)); + + await NewService().UpdateManifestAssetsAsync(new FileInfo(manifest), logo, CreateTaskContext()); + + var assetsDir = Path.Combine(_tempDir.FullName, "Assets"); + Assert.IsTrue(File.Exists(Path.Combine(assetsDir, "StoreLogo.png")), "default asset set is generated when the manifest has no references"); + Assert.IsTrue(File.Exists(Path.Combine(assetsDir, "app.ico")), "an app.ico is written into the default assets directory"); + } + + [TestMethod] + public async Task UpdateManifestAssets_ManifestWithReferences_RegeneratesReferencedAssets() + { + var manifest = WriteManifest("Package.appxmanifest", """ + + + + + + + + + """); + var logo = new FileInfo(PngHelper.CreateRasterPng(Path.Combine(_tempDir.FullName, "logo.png"), 40, 20, System.Drawing.Color.SlateBlue)); + + await NewService().UpdateManifestAssetsAsync(new FileInfo(manifest), logo, CreateTaskContext()); + + var assetsDir = Path.Combine(_tempDir.FullName, "Assets"); + Assert.IsTrue(File.Exists(Path.Combine(assetsDir, "AppList.png")), "referenced 44x44 asset is regenerated"); + Assert.IsTrue(File.Exists(Path.Combine(assetsDir, "MedTile.png")), "referenced 150x150 asset is regenerated"); + Assert.IsTrue(File.Exists(Path.Combine(assetsDir, "app.ico")), "app.ico lands next to the 44x44 app icon"); + } + + [TestMethod] + public async Task UpdateManifestAssets_ReferencesWithoutAppIcon_UsesMostCommonAssetDirectory() + { + var manifest = WriteManifest("Package.appxmanifest", """ + + + + + + + + + + + """); + var logo = new FileInfo(PngHelper.CreateRasterPng(Path.Combine(_tempDir.FullName, "logo.png"), 40, 20, System.Drawing.Color.SlateBlue)); + + await NewService().UpdateManifestAssetsAsync(new FileInfo(manifest), logo, CreateTaskContext()); + + var assetsDir = Path.Combine(_tempDir.FullName, "Assets"); + Assert.IsTrue(File.Exists(Path.Combine(assetsDir, "MedTile.png")), "referenced 150x150 asset is regenerated"); + Assert.IsTrue(File.Exists(Path.Combine(assetsDir, "WideTile.png")), "referenced wide asset is regenerated"); + Assert.IsTrue(File.Exists(Path.Combine(assetsDir, "app.ico")), + "with no 44x44 icon, app.ico falls back to the most common asset directory"); + } + + #endregion + + #region ExtractAssetReferencesFromManifest + + [TestMethod] + public void ExtractAssetReferences_FindsEveryKnownAssetType() + { + var manifest = WriteManifest("m.xml", """ + + + + Assets\StoreLogo.png + + + + + + + + + + + + """); + + var refs = ManifestService.ExtractAssetReferencesFromManifest(new FileInfo(manifest), CreateTaskContext()); + + void AssertRef(string contains, int w, int h) => + Assert.IsTrue(refs.Any(r => r.RelativePath.Contains(contains) && r.BaseWidth == w && r.BaseHeight == h), + $"expected {contains} => {w}x{h}"); + + AssertRef("StoreLogo", 50, 50); + AssertRef("Med.png", 150, 150); + AssertRef("App.png", 44, 44); + AssertRef("Wide.png", 310, 150); + AssertRef("Splash.png", 620, 300); + AssertRef("Badge.png", 24, 24); + Assert.AreEqual(6, refs.Count); + } + + [TestMethod] + public void ExtractAssetReferences_CustomDimensionFilename_ParsedFromName() + { + var manifest = WriteManifest("m.xml", """ + + + + Assets\brand512x256.png + + + """); + + var refs = ManifestService.ExtractAssetReferencesFromManifest(new FileInfo(manifest), CreateTaskContext()); + + Assert.AreEqual(1, refs.Count); + Assert.AreEqual(512, refs[0].BaseWidth); + Assert.AreEqual(256, refs[0].BaseHeight); + } + + [TestMethod] + public void ExtractAssetReferences_UnknownFilename_DefaultsToStoreLogoSize() + { + var manifest = WriteManifest("m.xml", """ + + + + Assets\brand.png + + + """); + + var refs = ManifestService.ExtractAssetReferencesFromManifest(new FileInfo(manifest), CreateTaskContext()); + + Assert.AreEqual(1, refs.Count); + Assert.AreEqual(50, refs[0].BaseWidth); + Assert.AreEqual(50, refs[0].BaseHeight); + } + + [TestMethod] + public void ExtractAssetReferences_MalformedManifest_ReturnsEmpty() + { + var manifest = WriteManifest("bad.xml", ""); + + var refs = ManifestService.ExtractAssetReferencesFromManifest(new FileInfo(manifest), CreateTaskContext()); + + Assert.AreEqual(0, refs.Count, "an unparseable manifest yields no references rather than throwing"); + } + + #endregion + + #region FormatXmlAttributes + + [TestMethod] + public void FormatXmlAttributes_SplitsElementsWithMoreThanTwoAttributes() + { + var input = string.Join("\n", + "", + " ", + " ", + " plain text", + ""); + + var result = ManifestService.FormatXmlAttributes(input); + + // Element with 3 attributes is split so each attribute is on its own line. + StringAssert.Contains(result, ""); + // Element with 2 attributes stays on a single line. + StringAssert.Contains(result, ""); + // Non-tag lines pass through untouched. + StringAssert.Contains(result, "plain text"); + // The trailing newline added by the loop is trimmed. + Assert.IsTrue(result.EndsWith("", StringComparison.Ordinal), "output should not end with an extra newline"); + } + + #endregion + + #region AddExecutionAliasAsync + + private const string AliasManifestHeader = + ""; + + private FileInfo WriteAliasManifest(string applicationsXml) + { + var content = $""" + + {AliasManifestHeader} + + + {applicationsXml} + + + """; + return new FileInfo(WriteManifest("Package.appxmanifest", content)); + } + + [TestMethod] + public async Task AddAlias_InfersFromExecutable_AddsAliasAndDeclaresNamespace() + { + var manifest = WriteAliasManifest( + " "); + + var result = await NewService().AddExecutionAliasAsync(new AddExecutionAliasOptions(manifest, null, null)); + + Assert.AreEqual(AddExecutionAliasStatus.Added, result.Status); + Assert.AreEqual("myapp.exe", result.AliasName); + + 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 + } + + [TestMethod] + public async Task AddAlias_MalformedManifest_ReturnsParseError() + { + var manifest = new FileInfo(WriteManifest("Package.appxmanifest", "")); + + var result = await NewService().AddExecutionAliasAsync(new AddExecutionAliasOptions(manifest, null, null)); + + Assert.AreEqual(AddExecutionAliasStatus.ManifestParseError, result.Status); + Assert.IsFalse(string.IsNullOrEmpty(result.ErrorMessage)); + } + + [TestMethod] + public async Task AddAlias_NoApplicationElement_ReturnsNoApplication() + { + var manifest = new FileInfo(WriteManifest("Package.appxmanifest", $""" + + {AliasManifestHeader} + + + """)); + + var result = await NewService().AddExecutionAliasAsync(new AddExecutionAliasOptions(manifest, null, null)); + + Assert.AreEqual(AddExecutionAliasStatus.NoApplicationElement, result.Status); + } + + [TestMethod] + public async Task AddAlias_AppIdNotFound_ReturnsApplicationIdNotFound() + { + var manifest = WriteAliasManifest(" "); + + var result = await NewService().AddExecutionAliasAsync(new AddExecutionAliasOptions(manifest, null, "otherApp")); + + Assert.AreEqual(AddExecutionAliasStatus.ApplicationIdNotFound, result.Status); + } + + [TestMethod] + public async Task AddAlias_ExecutableWithTraversal_ReturnsInvalidAliasName() + { + var manifest = WriteAliasManifest(" "); + + var result = await NewService().AddExecutionAliasAsync(new AddExecutionAliasOptions(manifest, null, null)); + + Assert.AreEqual(AddExecutionAliasStatus.InvalidAliasName, result.Status); + } + + [TestMethod] + public async Task AddAlias_NoExecutableNoAlias_ReturnsCouldNotInfer() + { + var manifest = WriteAliasManifest(" "); + + var result = await NewService().AddExecutionAliasAsync(new AddExecutionAliasOptions(manifest, null, null)); + + Assert.AreEqual(AddExecutionAliasStatus.CouldNotInferAlias, result.Status); + } + + [TestMethod] + public async Task AddAlias_ExecutableEndsWithSeparator_ReturnsCouldNotInfer() + { + var manifest = WriteAliasManifest(" "); + + var result = await NewService().AddExecutionAliasAsync(new AddExecutionAliasOptions(manifest, null, null)); + + Assert.AreEqual(AddExecutionAliasStatus.CouldNotInferAlias, result.Status); + } + + [TestMethod] + public async Task AddAlias_UnsafeExplicitAlias_ReturnsInvalidAliasName() + { + var manifest = WriteAliasManifest(" "); + + var result = await NewService().AddExecutionAliasAsync(new AddExecutionAliasOptions(manifest, "bad/alias", null)); + + Assert.AreEqual(AddExecutionAliasStatus.InvalidAliasName, result.Status); + } + + [TestMethod] + public async Task AddAlias_SameAliasAlreadyPresent_ReturnsAlreadyExists() + { + var manifest = WriteAliasManifest(""" + + + + + + + + + + """); + + var result = await NewService().AddExecutionAliasAsync(new AddExecutionAliasOptions(manifest, "myapp", null)); + + Assert.AreEqual(AddExecutionAliasStatus.AlreadyExists, result.Status); + } + + [TestMethod] + public async Task AddAlias_DifferentAliasAlreadyPresent_ReturnsConflict() + { + var manifest = WriteAliasManifest(""" + + + + + + + + + + """); + + var result = await NewService().AddExecutionAliasAsync(new AddExecutionAliasOptions(manifest, "myapp", null)); + + Assert.AreEqual(AddExecutionAliasStatus.ConflictingAliasExists, result.Status); + Assert.AreEqual("other.exe", result.ExistingAlias); + } + + [TestMethod] + public async Task AddAlias_ExistingEmptyAppExecutionAliasBlock_AddsAlias() + { + var manifest = WriteAliasManifest(""" + + + + + + + + """); + + var result = await NewService().AddExecutionAliasAsync(new AddExecutionAliasOptions(manifest, "myapp", null)); + + Assert.AreEqual(AddExecutionAliasStatus.Added, result.Status); + StringAssert.Contains(await File.ReadAllTextAsync(manifest.FullName), "Alias=\"myapp.exe\""); + } + + [TestMethod] + public async Task AddAlias_ExistingExtensionWithoutAppExecutionAlias_CreatesBlock() + { + var manifest = WriteAliasManifest(""" + + + + + + """); + + var result = await NewService().AddExecutionAliasAsync(new AddExecutionAliasOptions(manifest, "myapp", null)); + + Assert.AreEqual(AddExecutionAliasStatus.Added, result.Status); + StringAssert.Contains(await File.ReadAllTextAsync(manifest.FullName), "uap5:AppExecutionAlias"); + } + + #endregion +} diff --git a/src/winapp-CLI/WinApp.Cli.Tests/ManifestTemplateServiceTests.cs b/src/winapp-CLI/WinApp.Cli.Tests/ManifestTemplateServiceTests.cs new file mode 100644 index 000000000..3e41d811a --- /dev/null +++ b/src/winapp-CLI/WinApp.Cli.Tests/ManifestTemplateServiceTests.cs @@ -0,0 +1,249 @@ +// Copyright (c) Microsoft Corporation and Contributors. All rights reserved. +// Licensed under the MIT License. + +using System.Xml.Linq; +using Microsoft.Extensions.Logging.Abstractions; +using WinApp.Cli.ConsoleTasks; +using WinApp.Cli.Models; +using WinApp.Cli.Services; + +namespace WinApp.Cli.Tests; + +/// +/// Tests for . Exercises the real +/// workflow (template +/// expansion, publisher normalization, ApplicationId derivation and default-asset extraction) +/// end-to-end, plus the ASCII Windows-id sanitizer and the DN helpers it exposes. +/// +[TestClass] +public class ManifestTemplateServiceTests +{ + private static readonly XNamespace Ns = "http://schemas.microsoft.com/appx/manifest/foundation/windows10"; + private static readonly XNamespace Uap = "http://schemas.microsoft.com/appx/manifest/uap/windows10"; + + private DirectoryInfo _tempDir = null!; + + [TestInitialize] + public void Setup() + { + _tempDir = new DirectoryInfo(Path.Combine(Path.GetTempPath(), $"ManifestTplTest_{Guid.NewGuid():N}")); + _tempDir.Create(); + } + + [TestCleanup] + public void Cleanup() + { + if (_tempDir.Exists) + { + _tempDir.Delete(recursive: true); + } + } + + private static TaskContext CreateTaskContext() + { + var task = new GroupableTask("test", null); + var console = new Spectre.Console.Testing.TestConsole(); + return new TaskContext(task, null, console, NullLogger.Instance, new Lock()); + } + + #region FixAsciiWindowsId + + [TestMethod] + [DataRow("", "Default", DisplayName = "empty -> Default fallback")] + [DataRow(" ", "Default", DisplayName = "whitespace -> Default fallback")] + [DataRow("MyApp", "MyApp", DisplayName = "already valid single segment")] + [DataRow("com.contoso.app", "com.contoso.app", DisplayName = "already valid dotted id")] + [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")] + public void FixAsciiWindowsId_SanitizesToValidIdentifier(string input, string expected) + { + Assert.AreEqual(expected, ManifestTemplateService.FixAsciiWindowsId(input)); + } + + [TestMethod] + public void FixAsciiWindowsId_TruncatesToMaxLength() + { + var input = new string('a', 300); + + var result = ManifestTemplateService.FixAsciiWindowsId(input); + + Assert.AreEqual(255, result.Length, "Windows ids are capped at 255 characters"); + Assert.IsTrue(result.All(c => c == 'a')); + } + + #endregion + + #region StripCnPrefix / IsDistinguishedName + + [TestMethod] + public void StripCnPrefix_RemovesCnFromSimpleDistinguishedName() + { + Assert.AreEqual("Contoso", ManifestTemplateService.StripCnPrefix("CN=Contoso")); + } + + [TestMethod] + public void StripCnPrefix_LeavesMultiComponentDistinguishedNameUnchanged() + { + const string dn = "CN=Contoso, O=Contoso Corp"; + Assert.AreEqual(dn, ManifestTemplateService.StripCnPrefix(dn)); + } + + [TestMethod] + public void StripCnPrefix_LeavesBareNameUnchanged() + { + Assert.AreEqual("Contoso", ManifestTemplateService.StripCnPrefix("Contoso")); + } + + [TestMethod] + [DataRow("CN=Contoso Ltd", true, DisplayName = "CN distinguished name")] + [DataRow("OU=Finance, DC=corp, DC=com", true, DisplayName = "multi-attribute distinguished name")] + [DataRow("Contoso Ltd", false, DisplayName = "bare name is not a DN")] + [DataRow("", false, DisplayName = "empty is not a DN")] + public void IsDistinguishedName_ClassifiesInput(string input, bool expected) + { + Assert.AreEqual(expected, ManifestTemplateService.IsDistinguishedName(input)); + } + + #endregion + + #region GenerateCompleteManifestAsync + + [TestMethod] + public async Task GenerateCompleteManifest_Packaged_ProducesManifestAndAssets() + { + var outDir = new DirectoryInfo(Path.Combine(_tempDir.FullName, "packaged")); + + await new ManifestTemplateService().GenerateCompleteManifestAsync( + outDir, + packageName: "MyTestApp", + publisherName: "Contoso Ltd", + version: "2.3.4.5", + manifestTemplate: ManifestTemplates.Packaged, + description: "A sample packaged app", + taskContext: CreateTaskContext()); + + var manifestPath = Path.Combine(outDir.FullName, "Package.appxmanifest"); + Assert.IsTrue(File.Exists(manifestPath), "Package.appxmanifest should be generated"); + + var doc = XDocument.Load(manifestPath); + var root = doc.Root!; + var identity = root.Element(Ns + "Identity")!; + + Assert.AreEqual("MyTestApp", identity.Attribute("Name")!.Value); + Assert.AreEqual("CN=Contoso Ltd", identity.Attribute("Publisher")!.Value, "bare publisher is wrapped as CN="); + Assert.AreEqual("2.3.4.5", identity.Attribute("Version")!.Value, "version placeholder should be replaced"); + + var props = root.Element(Ns + "Properties")!; + Assert.AreEqual("MyTestApp", props.Element(Ns + "DisplayName")!.Value); + Assert.AreEqual("Contoso Ltd", props.Element(Ns + "PublisherDisplayName")!.Value, "display name strips CN="); + + var app = root.Element(Ns + "Applications")!.Element(Ns + "Application")!; + Assert.AreEqual("myTestApp", app.Attribute("Id")!.Value, "ApplicationId is camelCased and ascii-sanitized"); + + var visual = app.Element(Uap + "VisualElements")!; + Assert.AreEqual("A sample packaged app", visual.Attribute("Description")!.Value); + Assert.AreEqual("MyTestApp", visual.Attribute("DisplayName")!.Value); + + // Default assets are extracted alongside the manifest. + var assetsDir = new DirectoryInfo(Path.Combine(outDir.FullName, "Assets")); + Assert.IsTrue(assetsDir.Exists, "Assets directory should be generated"); + Assert.IsTrue(assetsDir.GetFiles("*.png").Length > 0, "default PNG assets should be extracted"); + } + + [TestMethod] + public async Task GenerateCompleteManifest_Sparse_UsesSparseTemplate() + { + var outDir = new DirectoryInfo(Path.Combine(_tempDir.FullName, "sparse")); + + await new ManifestTemplateService().GenerateCompleteManifestAsync( + outDir, + packageName: "SparseApp", + publisherName: "Contoso", + version: "1.2.3.0", + manifestTemplate: ManifestTemplates.Sparse, + description: "Sparse sample", + taskContext: CreateTaskContext()); + + var manifestPath = Path.Combine(outDir.FullName, "Package.appxmanifest"); + var doc = XDocument.Load(manifestPath); + var root = doc.Root!; + + Assert.AreEqual("SparseApp", root.Element(Ns + "Identity")!.Attribute("Name")!.Value); + + // The sparse template carries AllowExternalContent, which the packaged one does not. + XNamespace uap10 = "http://schemas.microsoft.com/appx/manifest/uap/windows10/10"; + Assert.IsNotNull( + root.Element(Ns + "Properties")!.Element(uap10 + "AllowExternalContent"), + "sparse template should include uap10:AllowExternalContent"); + } + + [TestMethod] + public async Task GenerateCompleteManifest_PreservesExistingDistinguishedName() + { + var outDir = new DirectoryInfo(Path.Combine(_tempDir.FullName, "dn")); + + await new ManifestTemplateService().GenerateCompleteManifestAsync( + outDir, + packageName: "DnApp", + publisherName: "CN=Existing Corp, O=Existing", + version: "1.0.0.0", + manifestTemplate: ManifestTemplates.Packaged, + description: "DN publisher", + taskContext: CreateTaskContext()); + + var doc = XDocument.Load(Path.Combine(outDir.FullName, "Package.appxmanifest")); + var identity = doc.Root!.Element(Ns + "Identity")!; + + Assert.AreEqual("CN=Existing Corp, O=Existing", identity.Attribute("Publisher")!.Value, + "an already-valid DN must be preserved verbatim"); + Assert.AreEqual("CN=Existing Corp, O=Existing", + doc.Root!.Element(Ns + "Properties")!.Element(Ns + "PublisherDisplayName")!.Value, + "multi-component DN is shown in full as the publisher display name"); + } + + [TestMethod] + public async Task GenerateCompleteManifest_CamelCasesMultiWordPackageName() + { + var outDir = new DirectoryInfo(Path.Combine(_tempDir.FullName, "multiword")); + + await new ManifestTemplateService().GenerateCompleteManifestAsync( + outDir, + packageName: "My-Cool_App Name", + publisherName: "Contoso", + version: "1.0.0.0", + manifestTemplate: ManifestTemplates.Packaged, + description: "multi word", + taskContext: CreateTaskContext()); + + var doc = XDocument.Load(Path.Combine(outDir.FullName, "Package.appxmanifest")); + var app = doc.Root!.Element(Ns + "Applications")!.Element(Ns + "Application")!; + + Assert.AreEqual("myCoolAppName", app.Attribute("Id")!.Value, + "separators split words that are camelCased into a single identifier"); + } + + [TestMethod] + public async Task GenerateCompleteManifest_EmptyPackageName_FallsBackToDefaultApplicationId() + { + var outDir = new DirectoryInfo(Path.Combine(_tempDir.FullName, "empty")); + + await new ManifestTemplateService().GenerateCompleteManifestAsync( + outDir, + packageName: "", + publisherName: "Contoso", + version: "1.0.0.0", + manifestTemplate: ManifestTemplates.Packaged, + description: "empty name", + taskContext: CreateTaskContext()); + + var doc = XDocument.Load(Path.Combine(outDir.FullName, "Package.appxmanifest")); + var app = doc.Root!.Element(Ns + "Applications")!.Element(Ns + "Application")!; + + Assert.AreEqual("Default", app.Attribute("Id")!.Value, + "an empty package name yields the Default ApplicationId fallback"); + } + + #endregion +} diff --git a/src/winapp-CLI/WinApp.Cli.Tests/PngHelper.cs b/src/winapp-CLI/WinApp.Cli.Tests/PngHelper.cs index 8b2f87429..59bb11a6f 100644 --- a/src/winapp-CLI/WinApp.Cli.Tests/PngHelper.cs +++ b/src/winapp-CLI/WinApp.Cli.Tests/PngHelper.cs @@ -2,6 +2,7 @@ // Licensed under the MIT License. using System.Drawing; +using System.Drawing.Imaging; namespace WinApp.Cli.Tests; @@ -35,6 +36,23 @@ internal static void CreateTestSvgImage(string path) File.WriteAllText(path, svgContent); } + /// + /// Renders a real raster PNG of the given dimensions and fill color via GDI+. + /// Unlike (fixed 1x1 bytes), this produces an arbitrarily + /// sized, non-square image so aspect-ratio / scaling branches can be exercised. + /// GDI+ encoder lookup is not thread-safe, so callers must be serialized (see [DoNotParallelize]). + /// + internal static string CreateRasterPng(string path, int width, int height, Color? fill = null) + { + using var bmp = new Bitmap(width, height, PixelFormat.Format32bppArgb); + using (var g = Graphics.FromImage(bmp)) + { + g.Clear(fill ?? Color.CornflowerBlue); + } + bmp.Save(path, ImageFormat.Png); + return path; + } + /// /// Verifies that all pixels in the image are fully transparent (alpha = 0). /// From 83a9342f2169a357618a6d62a93e563645f82681 Mon Sep 17 00:00:00 2001 From: Alexandre Zollinger Chohfi Date: Wed, 15 Jul 2026 15:21:50 -0700 Subject: [PATCH 2/2] Address Copilot auto-review findings on PR #638 (manifest tests) - 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> --- .../WinApp.Cli.Tests/ManifestServiceTests.cs | 63 ++++++++++++++++--- .../ManifestTemplateServiceTests.cs | 1 + .../WinApp.Cli/Services/ManifestService.cs | 10 ++- .../Services/ManifestTemplateService.cs | 7 +++ 4 files changed, 73 insertions(+), 8 deletions(-) diff --git a/src/winapp-CLI/WinApp.Cli.Tests/ManifestServiceTests.cs b/src/winapp-CLI/WinApp.Cli.Tests/ManifestServiceTests.cs index ef25ce668..33aa67c9b 100644 --- a/src/winapp-CLI/WinApp.Cli.Tests/ManifestServiceTests.cs +++ b/src/winapp-CLI/WinApp.Cli.Tests/ManifestServiceTests.cs @@ -169,22 +169,30 @@ await NewService().GenerateManifestAsync( [TestMethod] public async Task GenerateManifest_WithExecutable_ExtractsIconAndCreatesManifest() { - // notepad.exe reliably ships an icon; fall back to the running host if unavailable. + // Any existing executable works: the icon extractor is stubbed below so the test does + // not depend on the real shell image list (which can be empty on a headless CI session). var notepad = Path.Combine(Environment.SystemDirectory, "notepad.exe"); var exe = File.Exists(notepad) ? notepad : Environment.ProcessPath; if (exe is null || !File.Exists(exe)) { - Assert.Inconclusive("No executable with an icon available."); + Assert.Inconclusive("No executable available."); return; } var info = new ManifestGenerationInfo("MyApp", "CN=Test", "1.0.0.0", "A description"); - await NewService().GenerateManifestAsync( + var service = NewService(); + // Deterministically exercise the extracted-icon -> app.ico path. The lambda returns a + // fresh icon each call; the production code owns and disposes it. + service.ExecutableIconExtractor = _ => new System.Drawing.Icon(System.Drawing.SystemIcons.Application, 256, 256); + + await service.GenerateManifestAsync( _tempDir, info, ManifestTemplates.Packaged, logoPath: null, executable: exe, CreateTaskContext()); Assert.IsTrue(File.Exists(Path.Combine(_tempDir.FullName, "Package.appxmanifest")), - "the manifest is generated even when relying on executable icon extraction"); + "the manifest is generated when relying on executable icon extraction"); + Assert.IsTrue(File.Exists(Path.Combine(_tempDir.FullName, "Assets", "app.ico")), + "the icon extracted from the executable is written out as app.ico"); } #endregion @@ -236,13 +244,16 @@ public async Task UpdateManifestAssets_ManifestWithReferences_RegeneratesReferen [TestMethod] public async Task UpdateManifestAssets_ReferencesWithoutAppIcon_UsesMostCommonAssetDirectory() { + // Deliberately list the lone Images\ reference FIRST and give Assets\ the majority + // (two references vs one). A "pick the first/only directory" implementation would put + // app.ico under Images\; correct majority selection must put it under Assets\. var manifest = WriteManifest("Package.appxmanifest", """ - + @@ -254,10 +265,14 @@ public async Task UpdateManifestAssets_ReferencesWithoutAppIcon_UsesMostCommonAs await NewService().UpdateManifestAssetsAsync(new FileInfo(manifest), logo, CreateTaskContext()); var assetsDir = Path.Combine(_tempDir.FullName, "Assets"); + var imagesDir = Path.Combine(_tempDir.FullName, "Images"); + Assert.IsTrue(File.Exists(Path.Combine(imagesDir, "SmallTile.png")), "the minority Images\\ reference is regenerated"); Assert.IsTrue(File.Exists(Path.Combine(assetsDir, "MedTile.png")), "referenced 150x150 asset is regenerated"); Assert.IsTrue(File.Exists(Path.Combine(assetsDir, "WideTile.png")), "referenced wide asset is regenerated"); Assert.IsTrue(File.Exists(Path.Combine(assetsDir, "app.ico")), - "with no 44x44 icon, app.ico falls back to the most common asset directory"); + "with no 44x44 icon, app.ico lands in the majority (Assets) directory, not the first-listed one"); + Assert.IsFalse(File.Exists(Path.Combine(imagesDir, "app.ico")), + "app.ico must not fall into the minority (Images) directory that was listed first"); } #endregion @@ -415,7 +430,41 @@ public async Task AddAlias_InfersFromExecutable_AddsAliasAndDeclaresNamespace() 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 + + // Parse the manifest and assert the actual IgnorableNamespaces attribute value. A plain + // Contains("uap5") is tautological once the uap5:ExecutionAlias element exists; this + // instead pins the attribute update at ManifestService lines ~619-627. + var ignorable = XDocument.Parse(content).Root! + .Attribute("IgnorableNamespaces")!.Value + .Split(' ', StringSplitOptions.RemoveEmptyEntries); + CollectionAssert.Contains(ignorable, "uap5", "uap5 is appended to IgnorableNamespaces"); + CollectionAssert.Contains(ignorable, "uap10", "the pre-existing IgnorableNamespaces entry is preserved"); + } + + [TestMethod] + public async Task AddAlias_ManifestWithoutUap5_DeclaresNamespaceOnPackage() + { + // Manifest declares neither uap5 nor IgnorableNamespaces, so adding an alias must add + // the uap5 namespace declaration to the Package element. + var manifest = new FileInfo(WriteManifest("Package.appxmanifest", """ + + + + + + + + """)); + + var result = await NewService().AddExecutionAliasAsync(new AddExecutionAliasOptions(manifest, null, null)); + + Assert.AreEqual(AddExecutionAliasStatus.Added, result.Status); + + var root = XDocument.Parse(await File.ReadAllTextAsync(manifest.FullName)).Root!; + Assert.AreEqual( + "http://schemas.microsoft.com/appx/manifest/uap/windows10/5", + root.GetNamespaceOfPrefix("uap5")?.NamespaceName, + "the uap5 namespace is declared when the manifest lacks it"); } [TestMethod] diff --git a/src/winapp-CLI/WinApp.Cli.Tests/ManifestTemplateServiceTests.cs b/src/winapp-CLI/WinApp.Cli.Tests/ManifestTemplateServiceTests.cs index 3e41d811a..4edec65b8 100644 --- a/src/winapp-CLI/WinApp.Cli.Tests/ManifestTemplateServiceTests.cs +++ b/src/winapp-CLI/WinApp.Cli.Tests/ManifestTemplateServiceTests.cs @@ -57,6 +57,7 @@ private static TaskContext CreateTaskContext() [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")] + [DataRow("...", "Default", DisplayName = "all-separator input -> Default fallback")] public void FixAsciiWindowsId_SanitizesToValidIdentifier(string input, string expected) { Assert.AreEqual(expected, ManifestTemplateService.FixAsciiWindowsId(input)); diff --git a/src/winapp-CLI/WinApp.Cli/Services/ManifestService.cs b/src/winapp-CLI/WinApp.Cli/Services/ManifestService.cs index b869b8728..6d4d0b41d 100644 --- a/src/winapp-CLI/WinApp.Cli/Services/ManifestService.cs +++ b/src/winapp-CLI/WinApp.Cli/Services/ManifestService.cs @@ -20,6 +20,14 @@ internal partial class ManifestService( IImageAssetService imageAssetService, IAnsiConsole ansiConsole) : IManifestService { + /// + /// Seam for extracting an icon from an executable. Defaults to the real shell-based + /// extractor. Tests override it so the extracted-icon -> app.ico path is exercised + /// deterministically, independent of whether the headless CI session has a populated + /// shell image list ( can return null there). + /// + internal Func ExecutableIconExtractor { get; set; } = ShellIcon.GetJumboIcon; + public async Task PromptForManifestInfoAsync( DirectoryInfo directory, string? packageName, @@ -123,7 +131,7 @@ await manifestTemplateService.GenerateCompleteManifestAsync( Icon? extractedIcon = null; try { - extractedIcon = ShellIcon.GetJumboIcon(executableAbsolute); + extractedIcon = ExecutableIconExtractor(executableAbsolute); // save temporary if (extractedIcon != null) { diff --git a/src/winapp-CLI/WinApp.Cli/Services/ManifestTemplateService.cs b/src/winapp-CLI/WinApp.Cli/Services/ManifestTemplateService.cs index 754b17a9b..75ccfa84a 100644 --- a/src/winapp-CLI/WinApp.Cli/Services/ManifestTemplateService.cs +++ b/src/winapp-CLI/WinApp.Cli/Services/ManifestTemplateService.cs @@ -172,6 +172,13 @@ public static string FixAsciiWindowsId(string input) // Reassemble var result = string.Join(".", fixedSegments); + // If the input was made up entirely of separators (e.g. "..."), every segment + // was empty and reassembly yields "", which is not a valid Windows id. Fall back. + if (string.IsNullOrEmpty(result)) + { + return "Default"; + } + // Enforce max length if (result.Length > 255) {