[tests] Verbose apkdiff output for BuildReleaseArm64 regressions by simonrozsival · Pull Request #11758 · dotnet/android

simonrozsival · 2026-06-26T14:52:50Z

Summary

When the BuildReleaseArm64 APK size regression check fails, the only feedback was:

apkdiff regression test failed with exit code: 3. See test attachments.
Expected: True
But was:  False

The referenced apkdiff.log (which holds the actual apkdiff report) was written to the build directory but never added as a test attachment — so "See test attachments" was misleading, and diagnosing a regression meant digging through the CI build artifacts archive.

This PR makes the failure self-explanatory and copy-paste-ready, so the regression details (and the updated reference) can go straight into a PR without hunting through CI artifacts.

Changes

BaseTest.cs — RunApkDiffCommand: now adds apkdiff.log (the full apkdiff stdout/stderr report) as a real TestContext.AddTestAttachment.
BuildTest2.cs — BuildReleaseArm64: on a regression (exit code != 0), replaces the terse Assert.IsTrue with an Assert.Fail that includes:
- the full apkdiff output (stdout + stderr);
- a per-entry .apkdesc diff (reference → current) with "Size" -/+ lines and (±N bytes) deltas, plus [added]/[removed] entries, so the regression is obvious at a glance;
- the raw, full contents of the current .apkdesc, so it can be copy-pasted directly into the reference file and committed without downloading any attachment.
- The freshly-generated .apkdesc is also added as an attachment.
- The detailed message is built only on failure (no overhead on the passing path).
Added small GetApkDescDiff / ReadApkDescEntries helpers using System.Text.Json (already used elsewhere in this test project).

Example output on failure

apkdiff regression test failed with exit code: 3.

== apkdiff output ==
Size difference in bytes ([*1] apk1 only, [*2] apk2 only):
  +      47,648 classes.dex
Error: apkdiff: File 'classes.dex' has changed by 47,648 bytes (10.59 %). This exceeds the threshold of 5.00 %.
  -      61,344 lib/arm64-v8a/libassembly-store.so
Summary:
  +      47,648 Dalvik executables 11.84% (of 402,352)
  -      61,344 Shared libraries -0.44% (of 14,089,448)
Error: apkdiff: Size regression occured, 1 check(s) failed.

== .apkdesc diff (reference -> current) ==
  classes.dex
-     "Size": 402352
+     "Size": 450000   (+47,648 bytes)
  lib/arm64-v8a/libassembly-store.so
-     "Size": 3461344
+     "Size": 3400000   (-61,344 bytes)

== current 'BuildReleaseArm64SimpleDotNet.CoreCLR.apkdesc' (copy/paste to update the reference) ==
{
  "Comment": null,
  "Entries": {
    "AndroidManifest.xml": {
      "Size": 3036
    },
    ...
  }
}

If this change is intended, update the reference '...apkdesc' with the current '.apkdesc' above (or attached to this test), or run build-tools/scripts/UpdateApkSizeReference.sh.

Notes

Test-infrastructure only — no product code, dependency, or threshold changes; the passing path is unchanged.
See Documentation/project-docs/ApkSizeRegressionChecks.md for background on these checks.

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>

Copilot

Pull request overview

Improves diagnosability of BuildReleaseArm64 APK size regression failures by ensuring the apkdiff report is surfaced as a real test attachment and by emitting a copy/paste-friendly failure message that includes both apkdiff output and a focused per-entry .apkdesc size diff.

Changes:

Attach apkdiff.log produced by RunApkDiffCommand via TestContext.AddTestAttachment.
Replace the terse assertion in BuildReleaseArm64 with an on-failure Assert.Fail message that includes full apkdiff output plus a computed .apkdesc per-entry size diff.
Attach the newly generated .apkdesc on failure to make reference updates easier.

Show a summary per file

File	Description
src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Utilities/BaseTest.cs	Adds `apkdiff.log` as a test attachment (with a suggested tweak to only attach on failure).
src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/BuildTest2.cs	Enhances `BuildReleaseArm64` regression failures with verbose output, `.apkdesc` diff helpers, and attaches the generated `.apkdesc` on failure.

Copilot's findings

Files reviewed: 2/2 changed files
Comments generated: 1

Print the full contents of the freshly-generated `.apkdesc` in the failure message so it can be copy-pasted directly into the reference `.apkdesc` file and committed, without downloading the test attachment. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

jonathanpeppers · 2026-06-26T16:40:18Z

/review

github-actions · 2026-06-26T16:41:16Z

✅ Android PR Reviewer completed successfully!

github-actions

⚠️ Needs Changes

Nice quality-of-life improvement — turning the opaque apkdiff regression test failed ... See test attachments into a self-contained, copy-paste-ready failure message is genuinely useful, and the per-entry .apkdesc diff (reference → current with ±N bytes deltas) is a great touch. A couple of items before merge:

Issues

⚠️ Error handling (1) — the JSON parsing in GetApkDescDiff / ReadApkDescEntries can throw (JsonException, GetInt64) and clobber the very diagnostic message it exists to produce. See inline on BuildTest2.cs.
💡 Testing (1) — apkdiff.log is now attached on the success path too; confirm that's intended vs. failure-only. See inline on BaseTest.cs.

Verified (no action needed)

System.Text.Json is already referenced by other tests in this project (TrimmableTypeMapBuildTests.cs, JavaDependencyVerificationTests.cs), and .IsNullOrEmpty () resolves via the existing using Xamarin.Android.Tasks; — both compile cleanly.
The {(newSize - oldSize):+#,0;-#,0;0} format is correct: the negative section formats the absolute value with a literal -, so deltas render as +47,648 / -61,344.
The added/removed/changed key logic in GetApkDescDiff is sound (ordinal Union + TryGetValue on both sides; JsonDocument is disposed via using var).

CI

The suite that exercises the modified test — MSBuild Tests macOS (1–10) — is fully green, so BuildReleaseArm64 builds and passes with these changes.
One unrelated-looking failure: Package Tests macOS → APKs 1 (a different suite, not the modified test). Please confirm it's a known flake / re-run it. Several other legs are still in progress, so CI isn't green yet.

Test-infra only — no product, dependency, or threshold changes. Thanks! 🙏

Generated by Android PR Reviewer for issue #11758 · 665.7 AIC · ⌖ 47.1 AIC · ⊞ 40K
_{Comment /review to run again}

github-actions · 2026-06-26T16:53:47Z

+		static Dictionary<string, long> ReadApkDescEntries (string path)
+		{
+			var result = new Dictionary<string, long> (StringComparer.Ordinal);
+			using var doc = JsonDocument.Parse (File.ReadAllText (path));


🤖 ⚠️ 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)

jonathanpeppers

Test failures unrelated

Superseded

Copilot AI review requested due to automatic review settings June 26, 2026 14:52

simonrozsival requested a review from jonathanpeppers as a code owner June 26, 2026 14:52

Copilot started reviewing on behalf of simonrozsival June 26, 2026 14:53 View session

Copilot AI reviewed Jun 26, 2026

View reviewed changes

Comment thread src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Utilities/BaseTest.cs

github-actions Bot previously requested changes Jun 26, 2026

View reviewed changes

jonathanpeppers approved these changes Jun 26, 2026

View reviewed changes

jonathanpeppers merged commit 9e9cf97 into dotnet:main Jun 26, 2026
37 of 40 checks passed

jonathanpeppers mentioned this pull request Jun 26, 2026

[ci] Android PR Reviewer: restrict to COMMENT events only #11762

Merged

simonrozsival mentioned this pull request Jun 29, 2026

Fast Deployment XA0137 (run-as 'couldn't stat') races install on the PRIMARY user (user 0) ~daily in CI #11808

Closed

Copilot AI mentioned this pull request Jun 29, 2026

[FastDeploy] Retry first run-as query to absorb post-install data-dir race (XA0137) #11809

Merged

Copilot AI mentioned this pull request Jul 13, 2026

Report missing Fast Deployment packages as XA0132 #12060

Merged

github-actions Bot locked and limited conversation to collaborators Jul 27, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[tests] Verbose apkdiff output for BuildReleaseArm64 regressions - #11758

[tests] Verbose apkdiff output for BuildReleaseArm64 regressions#11758
jonathanpeppers merged 2 commits into
dotnet:mainfrom
simonrozsival:apkdiff-verbose-regression-output

simonrozsival commented Jun 26, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

jonathanpeppers commented Jun 26, 2026

Uh oh!

github-actions Bot commented Jun 26, 2026 •

edited

Loading

Uh oh!

github-actions Bot left a comment

Uh oh!

github-actions Bot Jun 26, 2026

Uh oh!

Uh oh!

jonathanpeppers left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

simonrozsival commented Jun 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Changes

Example output on failure

Notes

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Copilot's findings

Uh oh!

Uh oh!

jonathanpeppers commented Jun 26, 2026

Uh oh!

github-actions Bot commented Jun 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

⚠️ Needs Changes

Uh oh!

github-actions Bot Jun 26, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jonathanpeppers left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

simonrozsival commented Jun 26, 2026 •

edited

Loading

github-actions Bot commented Jun 26, 2026 •

edited

Loading