From 910fe4386f63a45d8231f8f0108cd99887d953d0 Mon Sep 17 00:00:00 2001 From: Matthew Leibowitz Date: Fri, 26 Apr 2024 03:13:10 +0800 Subject: [PATCH 1/2] Improve error logging for failed resizetizering --- .../Resizetizer/src/ResizetizeImages.cs | 4 +- .../Resizetizer/src/SkiaSharpSvgTools.cs | 5 +- .../test/UnitTests/MSBuildTaskFixture.cs | 42 +++++++++-- .../test/UnitTests/ResizetizeImagesTests.cs | 73 +++++++++++++++++++ .../test/UnitTests/images/link_out.svg | 4 + 5 files changed, 118 insertions(+), 10 deletions(-) create mode 100644 src/SingleProject/Resizetizer/test/UnitTests/images/link_out.svg diff --git a/src/SingleProject/Resizetizer/src/ResizetizeImages.cs b/src/SingleProject/Resizetizer/src/ResizetizeImages.cs index efd069b67e24..a604d65e927a 100644 --- a/src/SingleProject/Resizetizer/src/ResizetizeImages.cs +++ b/src/SingleProject/Resizetizer/src/ResizetizeImages.cs @@ -88,9 +88,7 @@ public override System.Threading.Tasks.Task ExecuteAsync() } catch (Exception ex) { - LogWarning("MAUI0000", ex.ToString()); - - throw; + LogCodedError("MAUI0000", $"There was an exception processing the image '{img.Filename}': {ex}"); } }); diff --git a/src/SingleProject/Resizetizer/src/SkiaSharpSvgTools.cs b/src/SingleProject/Resizetizer/src/SkiaSharpSvgTools.cs index 430fcc5d6af7..e164128f4e61 100644 --- a/src/SingleProject/Resizetizer/src/SkiaSharpSvgTools.cs +++ b/src/SingleProject/Resizetizer/src/SkiaSharpSvgTools.cs @@ -21,10 +21,13 @@ public SkiaSharpSvgTools(string filename, SKSize? baseSize, SKColor? backgroundC sw.Start(); svg = new SKSvg(); - svg.Load(filename); + var pic = svg.Load(filename); sw.Stop(); Logger?.Log($"Open SVG took {sw.ElapsedMilliseconds}ms ({filename})"); + + if (pic.CullRect.Size.IsEmpty) + Logger?.Log($"SVG picture did not have a size and will fail to generate. ({Filename})"); } public override SKSize GetOriginalSize() => diff --git a/src/SingleProject/Resizetizer/test/UnitTests/MSBuildTaskFixture.cs b/src/SingleProject/Resizetizer/test/UnitTests/MSBuildTaskFixture.cs index 72ec25e7c25d..35060cafa5bb 100644 --- a/src/SingleProject/Resizetizer/test/UnitTests/MSBuildTaskFixture.cs +++ b/src/SingleProject/Resizetizer/test/UnitTests/MSBuildTaskFixture.cs @@ -1,20 +1,34 @@ -using System; +#nullable enable +using System; using System.Collections; using System.Collections.Generic; using Microsoft.Build.Framework; +using Xunit.Abstractions; namespace Microsoft.Maui.Resizetizer.Tests { public abstract class MSBuildTaskTestFixture : BaseTest, IBuildEngine where TTask : Microsoft.Build.Framework.ITask { - protected readonly TestLogger Logger; + protected readonly TestLogger? Logger; protected List LogErrorEvents = new List(); protected List LogMessageEvents = new List(); protected List LogCustomEvents = new List(); protected List LogWarningEvents = new List(); + protected MSBuildTaskTestFixture() + : this(null) + { + } + + protected MSBuildTaskTestFixture(ITestOutputHelper? output) + { + Output = output; + } + + public ITestOutputHelper? Output { get; } + // IBuildEngine bool IBuildEngine.ContinueOnError => false; @@ -27,12 +41,28 @@ public abstract class MSBuildTaskTestFixture : BaseTest, IBuildEngine bool IBuildEngine.BuildProjectFile(string projectFileName, string[] targetNames, IDictionary globalProperties, IDictionary targetOutputs) => throw new NotImplementedException(); - void IBuildEngine.LogCustomEvent(CustomBuildEventArgs e) => LogCustomEvents.Add(e); + void IBuildEngine.LogCustomEvent(CustomBuildEventArgs e) + { + LogCustomEvents.Add(e); + Output?.WriteLine($"CUSTOM : {e.Message}"); + } - void IBuildEngine.LogErrorEvent(BuildErrorEventArgs e) => LogErrorEvents.Add(e); + void IBuildEngine.LogErrorEvent(BuildErrorEventArgs e) + { + LogErrorEvents.Add(e); + Output?.WriteLine($"ERROR : {e.Message}"); + } - void IBuildEngine.LogMessageEvent(BuildMessageEventArgs e) => LogMessageEvents.Add(e); + void IBuildEngine.LogMessageEvent(BuildMessageEventArgs e) + { + LogMessageEvents.Add(e); + Output?.WriteLine($"MESSAGE: {e.Message}"); + } - void IBuildEngine.LogWarningEvent(BuildWarningEventArgs e) => LogWarningEvents.Add(e); + void IBuildEngine.LogWarningEvent(BuildWarningEventArgs e) + { + LogWarningEvents.Add(e); + Output?.WriteLine($"WARNING: {e.Message}"); + } } } \ No newline at end of file diff --git a/src/SingleProject/Resizetizer/test/UnitTests/ResizetizeImagesTests.cs b/src/SingleProject/Resizetizer/test/UnitTests/ResizetizeImagesTests.cs index 9e7ff2156295..1962b92e281a 100644 --- a/src/SingleProject/Resizetizer/test/UnitTests/ResizetizeImagesTests.cs +++ b/src/SingleProject/Resizetizer/test/UnitTests/ResizetizeImagesTests.cs @@ -7,6 +7,7 @@ using Microsoft.Build.Utilities; using SkiaSharp; using Xunit; +using Xunit.Abstractions; namespace Microsoft.Maui.Resizetizer.Tests { @@ -16,6 +17,11 @@ public abstract class ExecuteForApp : MSBuildTaskTestFixture { protected static readonly Dictionary ResizeMetadata = new() { ["Resize"] = "true" }; + protected ExecuteForApp(ITestOutputHelper output) + : base(output) + { + } + protected ResizetizeImages GetNewTask(string type, params ITaskItem[] items) => new ResizetizeImages { @@ -33,6 +39,11 @@ protected ITaskItem GetCopiedResource(ResizetizeImages task, string path) => public abstract class ExecuteForPlatformApp : ExecuteForApp { + protected ExecuteForPlatformApp(ITestOutputHelper output) + : base(output) + { + } + protected abstract string Platform { get; } protected abstract string GetPlatformOutputFileName(string file); @@ -40,6 +51,48 @@ public abstract class ExecuteForPlatformApp : ExecuteForApp protected ResizetizeImages GetNewTask(params ITaskItem[] items) => GetNewTask(Platform, items); + [Theory] + [InlineData("appicon.svg")] + [InlineData("bicycle.svg")] + [InlineData("camera.svg")] + [InlineData("camera.png")] + [InlineData("dotnet_bot.svg")] + [InlineData("dotnet_logo.svg")] + [InlineData("find_icon.svg")] + [InlineData("not_working.svg")] + [InlineData("prismicon.svg")] + [InlineData("warning.svg")] + [InlineData("yes_working.svg")] + public void BasicImageProcessingWorks(string image) + { + var items = new[] + { + new TaskItem($"images/{image}"), + }; + + var task = GetNewTask(items); + var success = task.Execute(); + Assert.True(success, LogErrorEvents.FirstOrDefault()?.Message); + + AssertFileExists(GetPlatformOutputFileName(Path.ChangeExtension(image, ".png"))); + } + + [Theory] + [InlineData("link_out.svg")] + public void BadImagesReportImageWithError(string image) + { + var items = new[] + { + new TaskItem($"images/{image}"), + }; + + var task = GetNewTask(items); + var success = task.Execute(); + Assert.False(success); + + Assert.Contains(image, LogErrorEvents.FirstOrDefault()?.Message, StringComparison.OrdinalIgnoreCase); + } + [Fact] public void GenerationSkippedOnIncrementalBuild() { @@ -179,6 +232,11 @@ public void FailsOnAlmostExactMatchingMultipleFiles() public class ExecuteForAndroid : ExecuteForPlatformApp { + public ExecuteForAndroid(ITestOutputHelper output) + : base(output) + { + } + protected override string Platform => "android"; protected override string GetPlatformOutputFileName(string file) => @@ -930,6 +988,11 @@ public void NonExistantFilesAreDeleted() public class ExecuteForiOS : ExecuteForPlatformApp { + public ExecuteForiOS(ITestOutputHelper output) + : base(output) + { + } + protected override string Platform => "ios"; protected override string GetPlatformOutputFileName(string file) => $"{file}"; @@ -1243,6 +1306,11 @@ public void NonExistantFilesAreDeleted() public class ExecuteForWindows : ExecuteForPlatformApp { + public ExecuteForWindows(ITestOutputHelper output) + : base(output) + { + } + protected override string Platform => "uwp"; protected override string GetPlatformOutputFileName(string file) => @@ -1602,6 +1670,11 @@ public void NonExistantFilesAreDeleted() public class ExecuteForAny : ExecuteForApp { + public ExecuteForAny(ITestOutputHelper output) + : base(output) + { + } + [Theory] [InlineData("image.svg", "100,100", true)] [InlineData("image.png", "100,100", true)] diff --git a/src/SingleProject/Resizetizer/test/UnitTests/images/link_out.svg b/src/SingleProject/Resizetizer/test/UnitTests/images/link_out.svg new file mode 100644 index 000000000000..348077858097 --- /dev/null +++ b/src/SingleProject/Resizetizer/test/UnitTests/images/link_out.svg @@ -0,0 +1,4 @@ + + + + From ecf7f419027486ed032a00c64230accc67fc8f86 Mon Sep 17 00:00:00 2001 From: Matthew Leibowitz Date: Fri, 26 Apr 2024 03:48:25 +0800 Subject: [PATCH 2/2] Fix tests --- .../test/UnitTests/ResizetizeImagesTests.cs | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/src/SingleProject/Resizetizer/test/UnitTests/ResizetizeImagesTests.cs b/src/SingleProject/Resizetizer/test/UnitTests/ResizetizeImagesTests.cs index 1962b92e281a..c84289ccb489 100644 --- a/src/SingleProject/Resizetizer/test/UnitTests/ResizetizeImagesTests.cs +++ b/src/SingleProject/Resizetizer/test/UnitTests/ResizetizeImagesTests.cs @@ -48,6 +48,9 @@ protected ExecuteForPlatformApp(ITestOutputHelper output) protected abstract string GetPlatformOutputFileName(string file); + protected virtual string GetPlatformCopyOutputFileName(string file) => + GetPlatformOutputFileName(file); + protected ResizetizeImages GetNewTask(params ITaskItem[] items) => GetNewTask(Platform, items); @@ -74,7 +77,10 @@ public void BasicImageProcessingWorks(string image) var success = task.Execute(); Assert.True(success, LogErrorEvents.FirstOrDefault()?.Message); - AssertFileExists(GetPlatformOutputFileName(Path.ChangeExtension(image, ".png"))); + if (Path.GetExtension(image) == ".png" || Path.GetExtension(image) == ".jpeg" || Path.GetExtension(image) == ".gif") + AssertFileExists(GetPlatformCopyOutputFileName(Path.ChangeExtension(image, ".png"))); + else + AssertFileExists(GetPlatformOutputFileName(Path.ChangeExtension(image, ".png"))); } [Theory] @@ -242,6 +248,9 @@ public ExecuteForAndroid(ITestOutputHelper output) protected override string GetPlatformOutputFileName(string file) => $"drawable-mdpi/{file}"; + protected override string GetPlatformCopyOutputFileName(string file) => + $"drawable/{file}"; + [Fact] public void NoItemsSucceed() { @@ -995,7 +1004,11 @@ public ExecuteForiOS(ITestOutputHelper output) protected override string Platform => "ios"; - protected override string GetPlatformOutputFileName(string file) => $"{file}"; + protected override string GetPlatformOutputFileName(string file) => + $"{file}"; + + protected override string GetPlatformCopyOutputFileName(string file) => + $"Resources/{file}"; [Fact] public void NoItemsSucceed()