Skip to content

[tests] Verbose apkdiff output for BuildReleaseArm64 regressions #11758

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

Merged
jonathanpeppers merged 2 commits into from
Jun 26, 2026
Merged

[tests] Verbose apkdiff output for BuildReleaseArm64 regressions #11758

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
369c61e
[tests] Verbose apkdiff output for BuildReleaseArm64 regressions
simonrozsival Jun 26, 2026
f73394e
[tests] Also dump raw current .apkdesc on regression
simonrozsival Jun 26, 2026
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
80 changes: 79 additions & 1 deletion src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/BuildTest2.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -313,8 +314,85 @@ 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 ($"== current '{apkDescFilename}' (copy/paste to update the reference) ==");
if (File.Exists (apkDescPath)) {
message.AppendLine (File.ReadAllText (apkDescPath));
} else {
message.AppendLine ($"(current apkdesc not found: {apkDescPath})");
}
message.AppendLine ();
message.AppendLine ($"If this change is intended, update the reference '{apkDescFilename}' with the current '.apkdesc' above (or 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<string, long> ReadApkDescEntries (string path)
{
var result = new Dictionary<string, long> (StringComparer.Ordinal);
using var doc = JsonDocument.Parse (File.ReadAllText (path));

@github-actions github-actions Bot Jun 26, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🤖 ⚠️ Error handling — ReadApkDescEntries calls JsonDocument.Parse (throws JsonException on malformed/truncated JSON) and size.GetInt64 () (throws if Size isn't a number or overflows long). GetApkDescDiff runs at line 334 — before the rest of the failure message is assembled — so any throw here propagates out of the test and replaces the carefully-built diagnostic (apkdiff output and the copy-paste .apkdesc) with an opaque stack trace, defeating the whole point of this change.

The current .apkdesc is freshly generated and could be empty/truncated if apkdiff bailed early. Consider wrapping the parse in a try/catch inside GetApkDescDiff and returning a fallback string, so the per-entry diff is best-effort and never clobbers the real failure message.

(Rule: Diagnostic/error paths must degrade gracefully and never mask the original failure)

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<object[]> Get_BuildHasNoWarningsData ()
Expand Down
1 change: 1 addition & 0 deletions src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Utilities/BaseTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Comment thread
jonathanpeppers marked this conversation as resolved.
return result;
}

Expand Down
Loading