Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Improve error logging for failed resizetizering #22064

Merged
merged 2 commits into from
Apr 28, 2024
Merged

Improve error logging for failed resizetizering #22064

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
910fe43
Improve error logging for failed resizetizering
mattleibow Apr 25, 2024
ecf7f41
Fix tests
mattleibow Apr 25, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 1 addition & 3 deletions src/SingleProject/Resizetizer/src/ResizetizeImages.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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}");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to check I'm understanding: Because this is now logged as an error, that will cause the task from an MSBuild perspective to fail (and thus stop the build, or whatever), right?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is correct, yes.

}
});

Expand Down
5 changes: 4 additions & 1 deletion src/SingleProject/Resizetizer/src/SkiaSharpSvgTools.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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() =>
Expand Down
42 changes: 36 additions & 6 deletions src/SingleProject/Resizetizer/test/UnitTests/MSBuildTaskFixture.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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<TTask> : BaseTest, IBuildEngine
where TTask : Microsoft.Build.Framework.ITask
{
protected readonly TestLogger Logger;
protected readonly TestLogger? Logger;

protected List<BuildErrorEventArgs> LogErrorEvents = new List<BuildErrorEventArgs>();
protected List<BuildMessageEventArgs> LogMessageEvents = new List<BuildMessageEventArgs>();
protected List<CustomBuildEventArgs> LogCustomEvents = new List<CustomBuildEventArgs>();
protected List<BuildWarningEventArgs> LogWarningEvents = new List<BuildWarningEventArgs>();

protected MSBuildTaskTestFixture()
: this(null)
{
}

protected MSBuildTaskTestFixture(ITestOutputHelper? output)
{
Output = output;
}

public ITestOutputHelper? Output { get; }

// IBuildEngine

bool IBuildEngine.ContinueOnError => false;
Expand All @@ -27,12 +41,28 @@ public abstract class MSBuildTaskTestFixture<TTask> : 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}");
}
}
}
88 changes: 87 additions & 1 deletion src/SingleProject/Resizetizer/test/UnitTests/ResizetizeImagesTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
using Microsoft.Build.Utilities;
using SkiaSharp;
using Xunit;
using Xunit.Abstractions;

namespace Microsoft.Maui.Resizetizer.Tests
{
Expand All @@ -16,6 +17,11 @@ public abstract class ExecuteForApp : MSBuildTaskTestFixture<ResizetizeImages>
{
protected static readonly Dictionary<string, string> ResizeMetadata = new() { ["Resize"] = "true" };

protected ExecuteForApp(ITestOutputHelper output)
: base(output)
{
}

protected ResizetizeImages GetNewTask(string type, params ITaskItem[] items) =>
new ResizetizeImages
{
Expand All @@ -33,13 +39,66 @@ 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);

protected virtual string GetPlatformCopyOutputFileName(string file) =>
GetPlatformOutputFileName(file);

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);

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]
[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()
{
Expand Down Expand Up @@ -179,11 +238,19 @@ public void FailsOnAlmostExactMatchingMultipleFiles()

public class ExecuteForAndroid : ExecuteForPlatformApp
{
public ExecuteForAndroid(ITestOutputHelper output)
: base(output)
{
}

protected override string Platform => "android";

protected override string GetPlatformOutputFileName(string file) =>
$"drawable-mdpi/{file}";

protected override string GetPlatformCopyOutputFileName(string file) =>
$"drawable/{file}";

[Fact]
public void NoItemsSucceed()
{
Expand Down Expand Up @@ -930,9 +997,18 @@ 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}";
protected override string GetPlatformOutputFileName(string file) =>
$"{file}";

protected override string GetPlatformCopyOutputFileName(string file) =>
$"Resources/{file}";

[Fact]
public void NoItemsSucceed()
Expand Down Expand Up @@ -1243,6 +1319,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) =>
Expand Down Expand Up @@ -1602,6 +1683,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)]
Expand Down
4 changes: 4 additions & 0 deletions src/SingleProject/Resizetizer/test/UnitTests/images/link_out.svg
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading