[Xamarin.Android.Build.Tasks] merge targetSdkVersion into existing <uses-sdk>

src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/ManifestDocumentTest.cs

@@ -0,0 +1,112 @@
+using System;
+using System.Collections.Generic;
+using System.IO;
+using System.Linq;
+using System.Xml.Linq;
+using Java.Interop.Tools.Cecil;
+using Microsoft.Build.Utilities;
+using Mono.Cecil;
+using NUnit.Framework;
+using Xamarin.Android.Tasks;
+
+namespace Xamarin.Android.Build.Tests;
+
+/// <summary>
+/// In-process tests for how <see cref="ManifestDocument"/> merges the computed
+/// minSdkVersion/targetSdkVersion into a user-authored &lt;uses-sdk/&gt; element.
+/// </summary>
+[TestFixture]
+[Parallelizable (ParallelScope.Self)]
+public class ManifestDocumentTest : BaseTest
+{
+	const string DefaultMinSdkVersion = "21";
+	const string DefaultTargetSdkVersion = "37";
+
+	static readonly XNamespace AndroidNs = "http://schemas.android.com/apk/res/android";
+
+	static string CreateManifest (string usesSdk) => $@"<?xml version=""1.0"" encoding=""utf-8""?>
+<manifest xmlns:android=""http://schemas.android.com/apk/res/android"" package=""com.xamarin.usessdk"">
+	{usesSdk}
+	<application android:label=""UsesSdk"">
+	</application>
+</manifest>";
+
+	static XElement MergeAndGetUsesSdk (string usesSdk, out XDocument document)
+	{
+		var templateFile = Path.GetTempFileName ();
+		try {
+			File.WriteAllText (templateFile, CreateManifest (usesSdk));
+
+			var manifest = new ManifestDocument (templateFile) {
+				PackageName = "com.xamarin.usessdk",
+				TargetSdkVersion = DefaultTargetSdkVersion,
+				MinSdkVersion = DefaultMinSdkVersion,
+				VersionResolver = new MockVersionResolver {
+					GetApiLevelFromIdFunc = id => int.TryParse (id, out var apiLevel) ? (int?) apiLevel : null,
+					GetIdFromApiLevelFunc = apiLevel => apiLevel,
+				},
+			};
+
+			var engine = new MockBuildEngine (TestContext.Out);
+			var log = new TaskLoggingHelper (engine, nameof (ManifestDocumentTest));
+			manifest.Merge (log, new TypeDefinitionCache (), new List<TypeDefinition> (), applicationClass: null, embed: false, bundledWearApplicationName: null, mergedManifestDocuments: null);
+
+			var writer = new StringWriter ();
+			manifest.Save ((code, message) => { }, writer);
+
+			document = XDocument.Parse (writer.ToString ());
+			var element = document.Root.Element ("uses-sdk");
+			Assert.IsNotNull (element, "uses-sdk element should exist");
+			return element;
+		} finally {
+			File.Delete (templateFile);
+		}
+	}
+
+	static void AssertUsesSdk (string usesSdk, string expectedMinSdkVersion, string expectedTargetSdkVersion)
+	{
+		var element = MergeAndGetUsesSdk (usesSdk, out var document);
+
+		Assert.AreEqual (expectedMinSdkVersion, element.Attribute (AndroidNs + "minSdkVersion")?.Value, "unexpected android:minSdkVersion");
+		Assert.AreEqual (expectedTargetSdkVersion, element.Attribute (AndroidNs + "targetSdkVersion")?.Value, "unexpected android:targetSdkVersion");
+
+		foreach (var comment in document.DescendantNodes ().OfType<XComment> ()) {
+			StringAssert.DoesNotContain ("UsesMinSdkAttributes", comment.Value, "the lint suppression comment should no longer be emitted");
+		}
+	}
+
+	[Test]
+	public void NoUsesSdkElement ()
+	{
+		AssertUsesSdk ("", DefaultMinSdkVersion, DefaultTargetSdkVersion);
+	}
+
+	[Test]
+	public void MinSdkVersionOnly ()
+	{
+		// Regression test: targetSdkVersion was never written, so `aapt2` defaulted it to minSdkVersion.
+		AssertUsesSdk (@"<uses-sdk android:minSdkVersion=""24"" />", "24", DefaultTargetSdkVersion);
+	}
+
+	[Test]
+	public void TargetSdkVersionOnly ()
+	{
+		AssertUsesSdk (@"<uses-sdk android:targetSdkVersion=""30"" />", DefaultMinSdkVersion, "30");
+	}
+
+	[Test]
+	public void MinAndTargetSdkVersion ()
+	{
+		AssertUsesSdk (@"<uses-sdk android:minSdkVersion=""24"" android:targetSdkVersion=""30"" />", "24", "30");
+	}
+}
+
+class MockVersionResolver : IVersionResolver
+{
+	public Func<string, int?> GetApiLevelFromIdFunc { get; set; } = _ => 99;
+	public Func<string, string> GetIdFromApiLevelFunc { get; set; } = _ => "API-99";
+
+	public int? GetApiLevelFromId (string id) => GetApiLevelFromIdFunc (id);
+
+	public string GetIdFromApiLevel (string apiLevel) => GetIdFromApiLevelFunc (apiLevel);
+}

src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/ManifestTest.cs

@@ -1728,16 +1728,6 @@ public void TargetSdkVersion_361In_36Out ()
 				File.Delete (templateFile);
 			}
 		}
-
-		class MockVersionResolver : IVersionResolver
-		{
-			public Func<string, int?> GetApiLevelFromIdFunc { get; set; } = _ => 99;
-			public Func<string, string> GetIdFromApiLevelFunc { get; set; } = _ => "API-99";
-
-			public int? GetApiLevelFromId (string id) => GetApiLevelFromIdFunc (id);
-
-			public string GetIdFromApiLevel (string apiLevel) => GetIdFromApiLevelFunc (apiLevel);
-		}
 	}
 }
 

src/Xamarin.Android.Build.Tasks/Utilities/ManifestDocument.cs

@@ -324,13 +324,14 @@ public IList<string> Merge (TaskLoggingHelper log, TypeDefinitionCache cache, Li
 				uses.SetAttributeValue (androidNs + "minSdkVersion", minSdkVersionString);
 			}
 
+			// If no targetSdkVersion is specified, set it. Otherwise `aapt2` defaults it to minSdkVersion.
 			string targetSdkVersion;
 			var tsv = uses.Attribute (androidNs + "targetSdkVersion");
 			if (tsv != null)
 				targetSdkVersion = tsv.Value;
 			else {
 				targetSdkVersion = TargetSdkVersionName;
-				uses.AddBeforeSelf (new XComment ("suppress UsesMinSdkAttributes"));
+				uses.SetAttributeValue (androidNs + "targetSdkVersion", targetSdkVersion);
 			}
 
 			int? tryTargetSdkVersion  = VersionResolver.GetApiLevelFromId (targetSdkVersion);