From 369c61e6cb722998f86a8a23aba8a801e046a0bf Mon Sep 17 00:00:00 2001 From: Simon Rozsival Date: Fri, 26 Jun 2026 16:52:06 +0200 Subject: [PATCH] [tests] Verbose apkdiff output for BuildReleaseArm64 regressions When the `BuildReleaseArm64` APK size regression check fails, the test only reported: apkdiff regression test failed with exit code: 3. See test attachments. Expected: True But was: False ...but the referenced `apkdiff.log` was written to the build directory and never actually added as a test attachment, so diagnosing a regression meant digging through CI build artifacts. This makes the failure self-explanatory and copy-paste-ready: * `RunApkDiffCommand` now adds `apkdiff.log` (the full apkdiff report) as a real test attachment. * On failure, `BuildReleaseArm64` now fails with a detailed message containing the full apkdiff output (stdout + stderr) and a per-entry `.apkdesc` diff (reference -> current) with `"Size"` +/- lines and byte deltas, so the regression is obvious at a glance. * The freshly-generated `.apkdesc` is attached too, so the updated reference can be grabbed straight from the test results. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../Xamarin.Android.Build.Tests/BuildTest2.cs | 73 ++++++++++++++++++- .../Utilities/BaseTest.cs | 1 + 2 files changed, 73 insertions(+), 1 deletion(-) diff --git a/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/BuildTest2.cs b/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/BuildTest2.cs index 6d92d889def..47e284b602e 100644 --- a/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/BuildTest2.cs +++ b/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/BuildTest2.cs @@ -6,6 +6,7 @@ using System.Reflection; using System.Runtime.InteropServices; using System.Text; +using System.Text.Json; using System.Threading.Tasks; using System.Xml; using System.Xml.Linq; @@ -313,8 +314,78 @@ public void BuildReleaseArm64 ([Values] bool forms, [Values (AndroidRuntime.Core var apkDescPath = Path.Combine (Root, apkDescFilename); var apkDescReferencePath = Path.Combine (Root, b.ProjectDirectory, apkDescReference); var (code, stdOut, stdErr) = RunApkDiffCommand ($"-s --save-description-2={apkDescPath} --descrease-is-regression {regressionCheckArgs} {apkDescReferencePath} {apkFile}", Path.Combine (Root, b.ProjectDirectory, "apkdiff.log")); - Assert.IsTrue (code == 0, $"apkdiff regression test failed with exit code: {code}. See test attachments."); + if (code != 0) { + if (File.Exists (apkDescPath)) { + TestContext.AddTestAttachment (apkDescPath, apkDescFilename); + } + + var message = new StringBuilder (); + message.AppendLine ($"apkdiff regression test failed with exit code: {code}."); + message.AppendLine (); + message.AppendLine ("== apkdiff output =="); + if (!stdOut.IsNullOrEmpty ()) { + message.AppendLine (stdOut); + } + if (!stdErr.IsNullOrEmpty ()) { + message.AppendLine (stdErr); + } + message.AppendLine (); + message.AppendLine ("== .apkdesc diff (reference -> current) =="); + message.AppendLine (GetApkDescDiff (apkDescReferencePath, apkDescPath)); + message.AppendLine (); + message.AppendLine ($"If this change is intended, update the reference '{apkDescFilename}' with the current '.apkdesc' attached to this test (or run build-tools/scripts/UpdateApkSizeReference.sh)."); + Assert.Fail (message.ToString ()); + } + } + } + + static string GetApkDescDiff (string referencePath, string currentPath) + { + if (!File.Exists (referencePath)) { + return $"(reference apkdesc not found: {referencePath})"; + } + if (!File.Exists (currentPath)) { + return $"(current apkdesc not found: {currentPath})"; + } + + var reference = ReadApkDescEntries (referencePath); + var current = ReadApkDescEntries (currentPath); + + var sb = new StringBuilder (); + foreach (var key in reference.Keys.Union (current.Keys).OrderBy (k => k, StringComparer.Ordinal)) { + bool inReference = reference.TryGetValue (key, out long oldSize); + bool inCurrent = current.TryGetValue (key, out long newSize); + if (inReference && inCurrent) { + if (oldSize != newSize) { + sb.AppendLine ($" {key}"); + sb.AppendLine ($"- \"Size\": {oldSize}"); + sb.AppendLine ($"+ \"Size\": {newSize} ({(newSize - oldSize):+#,0;-#,0;0} bytes)"); + } + } else if (inReference) { + sb.AppendLine ($"- {key} (\"Size\": {oldSize}) [removed]"); + } else { + sb.AppendLine ($"+ {key} (\"Size\": {newSize}) [added]"); + } + } + + if (sb.Length == 0) { + return "(no per-entry size differences)"; + } + return sb.ToString (); + } + + static Dictionary ReadApkDescEntries (string path) + { + var result = new Dictionary (StringComparer.Ordinal); + using var doc = JsonDocument.Parse (File.ReadAllText (path)); + if (doc.RootElement.TryGetProperty ("Entries", out var entries)) { + foreach (var entry in entries.EnumerateObject ()) { + if (entry.Value.TryGetProperty ("Size", out var size)) { + result [entry.Name] = size.GetInt64 (); + } + } } + return result; } static IEnumerable Get_BuildHasNoWarningsData () diff --git a/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Utilities/BaseTest.cs b/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Utilities/BaseTest.cs index 77770bd014e..5f3757da1a7 100644 --- a/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Utilities/BaseTest.cs +++ b/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Utilities/BaseTest.cs @@ -124,6 +124,7 @@ protected static (int code, string stdOutput, string stdError) RunApkDiffCommand $"\ncontext: https://github.com/xamarin/xamarin-android/blob/main/Documentation/project-docs/ApkSizeRegressionChecks.md" + $"\nstdOut:\n{result.stdOutput}\nstdErr:\n{result.stdError}"; File.WriteAllText (logFilePath, logContent); + TestContext.AddTestAttachment (logFilePath, Path.GetFileName (logFilePath)); return result; }