Update Magick.NET-Q8-AnyCPU to 14.13.1 - #35880
Conversation
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.sh | bash -s -- 35880Or
iex "& { $(irm https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.ps1) } 35880" |
|
/review -b feature/enhanced-reviewer -p android |
MauiBot
left a comment
There was a problem hiding this comment.
AI Review Summary
@Redth — new AI review results are available based on this last commit:
965037b. To request a fresh review after new comments or commits, comment/review rerun.
Review Sessions — click to expand
Gate — Test Before & After Fix
Gate Result: ⚠️ SKIPPED
No tests were detected in this PR.
Recommendation: Add tests to verify the fix using the write-tests-agent.
Pre-Flight — Context & Validation
Issue: N/A - No linked issue detected
PR: #35880 - Update Magick.NET-Q8-AnyCPU to 14.13.1
Platforms Affected: Android UI-test host infrastructure (also shared UI test utilities for iOS, MacCatalyst, Windows)
Files Changed: 1 implementation, 0 test
Key Findings
- PR updates
src/TestUtils/src/VisualTestUtils.MagickNet/VisualTestUtils.MagickNet.csprojfromMagick.NET-Q8-AnyCPU14.12.0to14.13.1. - The package is test infrastructure only; Android impact is host-side visual comparison/native loading, not Android app runtime behavior.
- Gate was already skipped because no tests were detected; no linked issue was found.
Code Review Summary
Verdict: NEEDS_DISCUSSION
Confidence: low
Errors: 0 | Warnings: 0 | Suggestions: 1
Key code review findings:
- 💡 Consider Android host visual-test smoke coverage because restore/build does not prove ImageMagick native-load or screenshot comparison behavior in the Android UI-test host.
Fix Candidates
| # | Source | Approach | Test Result | Files Changed | Notes |
|---|---|---|---|---|---|
| PR | PR #35880 | Directly update Magick.NET-Q8-AnyCPU from 14.12.0 to 14.13.1 |
src/TestUtils/src/VisualTestUtils.MagickNet/VisualTestUtils.MagickNet.csproj |
Original PR |
Code Review — Deep Analysis
Code Review — PR #35880
Independent Assessment
What this changes: Updates src/TestUtils/src/VisualTestUtils.MagickNet/VisualTestUtils.MagickNet.csproj:9 from Magick.NET-Q8-AnyCPU 14.12.0 to 14.13.1.
Inferred motivation: Security/dependency hygiene. The package is used by visual-test utilities and the newer package version is intended to replace the older dependency.
Reconciliation with PR Narrative
Author claims: Updates Magick.NET-Q8-AnyCPU from 14.12.0 to 14.13.1 in VisualTestUtils.MagickNet, with restore/build validation.
Agreement/disagreement: The PR diff matches the description exactly. No product/runtime MAUI code is modified.
Prior Review Reconciliation
No prior ❌ Error findings found. Public PR comments contain the dogfood bot comment and a /review -b feature/enhanced-reviewer -p android request; public inline review comments are empty.
Blast Radius Assessment
- Runs for all instances: No product instances. It affects all UI test projects that reference
VisualTestUtils.MagickNet. - Startup impact: No app startup impact; possible host-side UI-test startup/native-load impact when ImageMagick is first used.
- Static/shared state: No new static/shared state.
CI Status
- Required-check result: unavailable through
gh pr checks --requiredbecause GitHub CLI is unauthenticated in this environment. - Classification: undetermined for this local review; user-provided gate result says gate was skipped because no tests were detected.
- Action taken: capped confidence; did not rerun gate verification.
Findings
No ❌ Error findings.
No
💡 Suggestion — Add Android host visual-test smoke coverage
The package is consumed by Android UI test infrastructure through src/Controls/tests/TestCases.Android.Tests/Controls.TestCases.Android.Tests.csproj and shared UI test code that uses ImageMagick. Restore/build of the utility project proves compile compatibility, but it does not prove native Magick load or screenshot compare/diff behavior in the Android UI-test host.
Failure-Mode Probing
- What happens if this package changes native asset probing on the Android UI-test host? The MAUI app would not crash, but host-side visual comparison could fail when the first
MagickImageis created. - What happens if visual comparison algorithms change subtly? Screenshot comparison thresholds could become more/less sensitive across Android UI tests even though MAUI rendering is unchanged.
- What happens if the large AnyCPU package has restore/feed issues? UI test project restore/build can fail before tests run.
Verdict: NEEDS_DISCUSSION
Confidence: low. The code change is localized and appears sound, but CI/gate evidence is unavailable in this environment and the detected gate was skipped because no tests were present.
Summary: No blocking code issue was found. The main residual risk is Android UI-test host restore/native-load/visual-comparison behavior, which is not covered by the skipped gate.
Fix — Analysis & Comparison
Fix Candidates
| # | Source | Approach | Test Result | Files Changed | Notes |
|---|---|---|---|---|---|
| 1 | try-fix | Keep Magick.NET-Q8-AnyCPU 14.13.1, but move the version to a local $(MagickNetPackageVersion) MSBuild property. |
✅ PASS | 1 file | Improves maintainability/auditability, but only marginally better than the direct PR bump. |
| 2 | try-fix | Replace Magick.NET-Q8-AnyCPU with narrower Magick.NET-Q8-x64 14.13.1 for host-side Android UI-test use. |
❌ FAIL | 1 file | Failed with NU1101; package ID is not available from configured feeds. |
| 3 | try-fix | Keep Magick.NET-Q8-AnyCPU 14.13.1 and add direct Magick.NET.Core 14.13.1 pin. |
✅ PASS | 1 file | More explicit transitive dependency pin, but likely redundant because NuGet already resolves the wrapper's core dependency. |
| PR | PR #35880 | Directly update Magick.NET-Q8-AnyCPU from 14.12.0 to 14.13.1. |
1 file | Original PR; simplest package-servicing fix. |
Cross-Pollination
| Model | Round | New Ideas? | Details |
|---|---|---|---|
| maui-expert-reviewer | 1 | Yes | Suggested central/property versioning, host-RID-specific package, validated internal package, dependency-validation path, or suppressing only if the vulnerable path is impossible. |
| maui-expert-reviewer | 2 | No | After attempts 1-3, expert reviewer concluded the direct AnyCPU bump is the cleanest appropriate fix; attempt 1 is indirection, attempt 3 is redundant, and attempt 2 is invalid/unavailable and less cross-host safe. |
Exhausted: Yes
Selected Fix: PR's fix — It is the simplest viable dependency servicing change. Candidate #1 and #3 passed the available checks but are not demonstrably better: #1 adds indirection without stronger validation, and #3 adds a redundant direct dependency. Candidate #2 failed restore.
Detailed Attempt Narratives
try-fix-1
Approach
Keep Magick.NET-Q8-AnyCPU at 14.13.1, but move the version into a local MSBuild property ($(MagickNetPackageVersion)) and reference that property from the PackageReference.
This is different from PR #35880's direct PackageReference version bump because it makes the dependency version easier to audit/service in the project file while preserving the same resolved package.
Diff
diff --git a/src/TestUtils/src/VisualTestUtils.MagickNet/VisualTestUtils.MagickNet.csproj b/src/TestUtils/src/VisualTestUtils.MagickNet/VisualTestUtils.MagickNet.csproj
index 13c824627a..c94e0de741 100644
--- a/src/TestUtils/src/VisualTestUtils.MagickNet/VisualTestUtils.MagickNet.csproj
+++ b/src/TestUtils/src/VisualTestUtils.MagickNet/VisualTestUtils.MagickNet.csproj
@@ -3,10 +3,11 @@
<PropertyGroup>
<TargetFramework>netstandard2.0</TargetFramework>
<PackageReadmeFile>package-readme.md</PackageReadmeFile>
+ <MagickNetPackageVersion>14.13.1</MagickNetPackageVersion>
</PropertyGroup>
<ItemGroup>
- <PackageReference Include="Magick.NET-Q8-AnyCPU" Version="14.13.1" />
+ <PackageReference Include="Magick.NET-Q8-AnyCPU" Version="$(MagickNetPackageVersion)" />
</ItemGroup>
<ItemGroup>Test Results
✅ PASS
Commands:
dotnet restore src/TestUtils/src/VisualTestUtils.MagickNet/VisualTestUtils.MagickNet.csproj -v:minimaldotnet build src/TestUtils/src/VisualTestUtils.MagickNet/VisualTestUtils.MagickNet.csproj -c Release --no-restore -v:minimaldotnet restore src/Controls/tests/TestCases.Android.Tests/Controls.TestCases.Android.Tests.csproj -v:minimal
Failure Analysis
No failure. This candidate validates restore/build and Android UI-test project restore, but it is only marginally better than the PR's fix: it improves maintainability without improving runtime confidence or adding coverage.
try-fix-2
Approach
Replace Magick.NET-Q8-AnyCPU with a narrower host-specific Magick.NET-Q8-x64 package at 14.13.1 to reduce native asset footprint for Android UI tests, which run ImageMagick host-side rather than on-device.
This is different from PR #35880's direct AnyCPU package bump because it attempts to reduce restore/native-load blast radius instead of keeping the broad AnyCPU package.
Diff
diff --git a/src/TestUtils/src/VisualTestUtils.MagickNet/VisualTestUtils.MagickNet.csproj b/src/TestUtils/src/VisualTestUtils.MagickNet/VisualTestUtils.MagickNet.csproj
index 13c824627a..cfab8f7824 100644
--- a/src/TestUtils/src/VisualTestUtils.MagickNet/VisualTestUtils.MagickNet.csproj
+++ b/src/TestUtils/src/VisualTestUtils.MagickNet/VisualTestUtils.MagickNet.csproj
@@ -6,7 +6,7 @@
</PropertyGroup>
<ItemGroup>
- <PackageReference Include="Magick.NET-Q8-AnyCPU" Version="14.13.1" />
+ <PackageReference Include="Magick.NET-Q8-x64" Version="14.13.1" />
</ItemGroup>
<ItemGroup>Test Results
❌ FAIL
Commands:
dotnet restore src/TestUtils/src/VisualTestUtils.MagickNet/VisualTestUtils.MagickNet.csproj -v:minimaldotnet build src/TestUtils/src/VisualTestUtils.MagickNet/VisualTestUtils.MagickNet.csproj -c Release --no-restore -v:minimaldotnet restore src/Controls/tests/TestCases.Android.Tests/Controls.TestCases.Android.Tests.csproj -v:minimal
Failure Analysis
Restore failed with NU1101: Unable to find package Magick.NET-Q8-x64 from the configured feeds. The narrower-package idea is not viable in this repository/feed configuration unless a real supported package ID is identified and ingested.
try-fix-3
Approach
Keep Magick.NET-Q8-AnyCPU at 14.13.1, and add an explicit direct Magick.NET.Core 14.13.1 reference so the core dependency is pinned at the same serviced version as the native wrapper package.
This is different from PR #35880's direct wrapper-only bump because it makes the transitive core dependency explicit and reduces ambiguity if other dependency graph changes later influence Magick.NET.Core resolution.
Diff
diff --git a/src/TestUtils/src/VisualTestUtils.MagickNet/VisualTestUtils.MagickNet.csproj b/src/TestUtils/src/VisualTestUtils.MagickNet/VisualTestUtils.MagickNet.csproj
index 13c824627a..fb70b6b5ef 100644
--- a/src/TestUtils/src/VisualTestUtils.MagickNet/VisualTestUtils.MagickNet.csproj
+++ b/src/TestUtils/src/VisualTestUtils.MagickNet/VisualTestUtils.MagickNet.csproj
@@ -7,6 +7,7 @@
<ItemGroup>
<PackageReference Include="Magick.NET-Q8-AnyCPU" Version="14.13.1" />
+ <PackageReference Include="Magick.NET.Core" Version="14.13.1" />
</ItemGroup>
<ItemGroup>Test Results
✅ PASS
Commands:
dotnet restore src/TestUtils/src/VisualTestUtils.MagickNet/VisualTestUtils.MagickNet.csproj -v:minimaldotnet build src/TestUtils/src/VisualTestUtils.MagickNet/VisualTestUtils.MagickNet.csproj -c Release --no-restore -v:minimaldotnet restore src/Controls/tests/TestCases.Android.Tests/Controls.TestCases.Android.Tests.csproj -v:minimal
Failure Analysis
No failure. This candidate passes the available checks. It is more explicit than the PR's fix but adds a direct dependency that NuGet already resolves transitively at the same version under normal lowest-applicable-version rules, so the practical benefit is limited.
Report — Final Recommendation
Comparative Report — PR #35880
Candidates compared
| Candidate | Approach | Regression result | Assessment |
|---|---|---|---|
pr |
Directly update Magick.NET-Q8-AnyCPU from 14.12.0 to 14.13.1. |
Simplest viable servicing fix. No expert-review blocking findings. | |
pr-plus-reviewer |
PR fix plus expert reviewer feedback. | Identical to pr because the expert reviewer found no actionable inline findings to apply. |
|
try-fix-1 |
Keep Magick.NET-Q8-AnyCPU 14.13.1, but move the version to $(MagickNetPackageVersion). |
✅ PASS | Functionally equivalent to the PR fix, with a small maintainability indirection. It does not improve runtime/native-load confidence. |
try-fix-2 |
Replace Magick.NET-Q8-AnyCPU with Magick.NET-Q8-x64 14.13.1. |
❌ FAIL | Ranked below passing candidates because restore failed with NU1101; the package ID is not available from configured feeds. |
try-fix-3 |
Keep Magick.NET-Q8-AnyCPU 14.13.1 and add explicit Magick.NET.Core 14.13.1. |
✅ PASS | More explicit, but redundant because the wrapper package already resolves the aligned core dependency. |
Expert PR evaluation
The expert reviewer found no actionable file:line findings against the PR diff. The reviewer also reported that Magick.NET-Q8-AnyCPU 14.13.1 preserves the relevant framework support and native runtime asset shape and aligns Magick.NET.Core to the same version.
Because there was no actionable feedback, pr-plus-reviewer does not differ from pr.
Ranking
pr— Best candidate. It is the smallest correct dependency-servicing fix and avoids redundant project-file indirection or direct transitive pins.pr-plus-reviewer— Equivalent code topr; ranked just below only because it adds no distinct improvement.try-fix-1— Passed available checks, but the MSBuild property adds indirection without centralizing the dependency version repository-wide or reducing risk.try-fix-3— Passed available checks, but the explicitMagick.NET.Corepin is redundant with the wrapper package dependency and increases dependency graph surface.try-fix-2— Failed restore and therefore must rank below all candidates that passed or remained viable.
Winner
Winner: pr
The raw PR fix is the best candidate because it directly addresses the dependency update with the least project-file churn, no failed regression signal, and no actionable expert-review findings. Passing try-fix candidates are not materially safer or more correct, and the failed try-fix candidate is not viable.
Future Action — review latest findings
No alternative fix was selected for this run. Review the session findings and CI results before merging.
|
Closing in favour of #35505 |
Note
Are you waiting for the changes in this PR to be merged?
It would be very helpful if you could test the resulting artifacts from this PR and let us know in a comment if this change resolves your issue. Thank you!
Description
Updates
Magick.NET-Q8-AnyCPUfrom14.12.0to14.13.1inVisualTestUtils.MagickNet.Testing
dotnet restore src/TestUtils/src/VisualTestUtils.MagickNet/VisualTestUtils.MagickNet.csprojdotnet build src/TestUtils/src/VisualTestUtils.MagickNet/VisualTestUtils.MagickNet.csproj -c Release --no-restore