Refactor AvdManagerRunner: resolved path constructor, pattern matching, ThrowIfFailed overload

Apply review patterns from PR #283 (AdbRunner):
- Constructor takes resolved 'string avdManagerPath' instead of Func<string?> delegates
- Add IDictionary<string, string>? environmentVariables for ANDROID_HOME/JAVA_HOME
- Remove AvdManagerPath property, IsAvailable, RequireAvdManagerPath(), ConfigureEnvironment()
- Use 'is { Length: > 0 }' pattern matching for null/empty checks
- Add ThrowIfFailed(StringWriter) overload to ProcessUtils
- Change ThrowIfFailed/ValidateNotNullOrEmpty/FindCmdlineTool to internal visibility
- Update tests: FindCmdlineTool tests replace AvdManagerPath tests, add constructor validation

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Author: rmarinho

src/Xamarin.Android.Tools.AndroidSdk/ProcessUtils.cs

@@ -207,27 +207,32 @@ static string JoinArguments (string[] args)
 		/// Throws <see cref="InvalidOperationException"/> when <paramref name="exitCode"/> is non-zero.
 		/// Includes stderr/stdout context in the message when available.
 		/// </summary>
-		public static void ThrowIfFailed (int exitCode, string command, string? stderr = null, string? stdout = null)
+		internal static void ThrowIfFailed (int exitCode, string command, string? stderr = null, string? stdout = null)
 		{
 			if (exitCode == 0)
 				return;
 
 			var message = $"'{command}' failed with exit code {exitCode}.";
 
-			if (!string.IsNullOrEmpty (stderr))
-				message += $" stderr:{Environment.NewLine}{stderr!.Trim ()}";
-			if (!string.IsNullOrEmpty (stdout))
-				message += $" stdout:{Environment.NewLine}{stdout!.Trim ()}";
+			if (stderr is { Length: > 0 })
+				message += $" stderr:{Environment.NewLine}{stderr.Trim ()}";
+			if (stdout is { Length: > 0 })
+				message += $" stdout:{Environment.NewLine}{stdout.Trim ()}";
 
 			throw new InvalidOperationException (message);
 		}
 
+		internal static void ThrowIfFailed (int exitCode, string command, StringWriter? stderr, StringWriter? stdout = null)
+		{
+			ThrowIfFailed (exitCode, command, stderr?.ToString (), stdout?.ToString ());
+		}
+
 		/// <summary>
 		/// Validates that <paramref name="value"/> is not null or empty.
 		/// Throws <see cref="ArgumentNullException"/> for null values and
 		/// <see cref="ArgumentException"/> for empty strings.
 		/// </summary>
-		public static void ValidateNotNullOrEmpty (string? value, string paramName)
+		internal static void ValidateNotNullOrEmpty (string? value, string paramName)
 		{
 			if (value is null)
 				throw new ArgumentNullException (paramName);
@@ -239,7 +244,7 @@ public static void ValidateNotNullOrEmpty (string? value, string paramName)
 		/// Searches versioned cmdline-tools directories (descending) and "latest" for a specific tool binary.
 		/// Falls back to the legacy tools/bin path. Returns null if not found.
 		/// </summary>
-		public static string? FindCmdlineTool (string sdkPath, string toolName, string extension)
+		internal static string? FindCmdlineTool (string sdkPath, string toolName, string extension)
 		{
 			var cmdlineToolsDir = Path.Combine (sdkPath, "cmdline-tools");
 

src/Xamarin.Android.Tools.AndroidSdk/Runners/AvdManagerRunner.cs

@@ -3,7 +3,6 @@
 
 using System;
 using System.Collections.Generic;
-using System.Diagnostics;
 using System.IO;
 using System.Linq;
 using System.Threading;
@@ -16,53 +15,30 @@ namespace Xamarin.Android.Tools;
 /// </summary>
 public class AvdManagerRunner
 {
-	readonly Func<string?> getSdkPath;
-	readonly Func<string?>? getJdkPath;
+	readonly string avdManagerPath;
+	readonly IDictionary<string, string>? environmentVariables;
 
-	public AvdManagerRunner (Func<string?> getSdkPath)
-		: this (getSdkPath, null)
-	{
-	}
-
-	public AvdManagerRunner (Func<string?> getSdkPath, Func<string?>? getJdkPath)
-	{
-		this.getSdkPath = getSdkPath ?? throw new ArgumentNullException (nameof (getSdkPath));
-		this.getJdkPath = getJdkPath;
-	}
-
-	public string? AvdManagerPath {
-		get {
-			var sdkPath = getSdkPath ();
-			if (string.IsNullOrEmpty (sdkPath))
-				return null;
-
-			return ProcessUtils.FindCmdlineTool (sdkPath, "avdmanager", OS.IsWindows ? ".bat" : "");
-		}
-	}
-
-	public bool IsAvailable => !string.IsNullOrEmpty (AvdManagerPath);
-
-	string RequireAvdManagerPath ()
-	{
-		return AvdManagerPath ?? throw new InvalidOperationException ("AVD Manager not found.");
-	}
-
-	void ConfigureEnvironment (ProcessStartInfo psi)
+	/// <summary>
+	/// Creates a new AvdManagerRunner with the full path to the avdmanager executable.
+	/// </summary>
+	/// <param name="avdManagerPath">Full path to avdmanager (e.g., "/path/to/sdk/cmdline-tools/latest/bin/avdmanager").</param>
+	/// <param name="environmentVariables">Optional environment variables to pass to avdmanager processes.</param>
+	public AvdManagerRunner (string avdManagerPath, IDictionary<string, string>? environmentVariables = null)
 	{
-		AndroidEnvironmentHelper.ConfigureEnvironment (psi, getSdkPath (), getJdkPath?.Invoke ());
+		if (string.IsNullOrWhiteSpace (avdManagerPath))
+			throw new ArgumentException ("Path to avdmanager must not be empty.", nameof (avdManagerPath));
+		this.avdManagerPath = avdManagerPath;
+		this.environmentVariables = environmentVariables;
 	}
 
 	public async Task<IReadOnlyList<AvdInfo>> ListAvdsAsync (CancellationToken cancellationToken = default)
 	{
-		var avdManagerPath = RequireAvdManagerPath ();
-
 		using var stdout = new StringWriter ();
 		using var stderr = new StringWriter ();
 		var psi = ProcessUtils.CreateProcessStartInfo (avdManagerPath, "list", "avd");
-		ConfigureEnvironment (psi);
-		var exitCode = await ProcessUtils.StartProcess (psi, stdout, stderr, cancellationToken).ConfigureAwait (false);
+		var exitCode = await ProcessUtils.StartProcess (psi, stdout, stderr, cancellationToken, environmentVariables).ConfigureAwait (false);
 
-		ProcessUtils.ThrowIfFailed (exitCode, "avdmanager list avd", stderr.ToString ());
+		ProcessUtils.ThrowIfFailed (exitCode, "avdmanager list avd", stderr);
 
 		return ParseAvdListOutput (stdout.ToString ());
 	}
@@ -73,8 +49,6 @@ public async Task<AvdInfo> CreateAvdAsync (string name, string systemImage, stri
 		ProcessUtils.ValidateNotNullOrEmpty (name, nameof (name));
 		ProcessUtils.ValidateNotNullOrEmpty (systemImage, nameof (systemImage));
 
-		var avdManagerPath = RequireAvdManagerPath ();
-
 		// Check if AVD already exists — return it instead of failing
 		if (!force) {
 			var existing = (await ListAvdsAsync (cancellationToken).ConfigureAwait (false))
@@ -89,7 +63,7 @@ public async Task<AvdInfo> CreateAvdAsync (string name, string systemImage, stri
 			force = true;
 
 		var args = new List<string> { "create", "avd", "-n", name, "-k", systemImage };
-		if (!string.IsNullOrEmpty (deviceProfile))
+		if (deviceProfile is { Length: > 0 })
 			args.AddRange (new [] { "-d", deviceProfile });
 		if (force)
 			args.Add ("--force");
@@ -98,10 +72,9 @@ public async Task<AvdInfo> CreateAvdAsync (string name, string systemImage, stri
 		using var stderr = new StringWriter ();
 		var psi = ProcessUtils.CreateProcessStartInfo (avdManagerPath, args.ToArray ());
 		psi.RedirectStandardInput = true;
-		ConfigureEnvironment (psi);
 
 		// avdmanager prompts "Do you wish to create a custom hardware profile?" — answer "no"
-		var exitCode = await ProcessUtils.StartProcess (psi, stdout, stderr, cancellationToken,
+		var exitCode = await ProcessUtils.StartProcess (psi, stdout, stderr, cancellationToken, environmentVariables,
 			onStarted: p => {
 				try {
 					p.StandardInput.WriteLine ("no");
@@ -111,7 +84,7 @@ public async Task<AvdInfo> CreateAvdAsync (string name, string systemImage, stri
 				}
 			}).ConfigureAwait (false);
 
-		ProcessUtils.ThrowIfFailed (exitCode, $"avdmanager create avd -n {name}", stderr.ToString (), stdout.ToString ());
+		ProcessUtils.ThrowIfFailed (exitCode, $"avdmanager create avd -n {name}", stderr, stdout);
 
 		// Re-list to get the actual path from avdmanager (respects ANDROID_USER_HOME/ANDROID_AVD_HOME)
 		var avds = await ListAvdsAsync (cancellationToken).ConfigureAwait (false);
@@ -131,14 +104,11 @@ public async Task DeleteAvdAsync (string name, CancellationToken cancellationTok
 	{
 		ProcessUtils.ValidateNotNullOrEmpty (name, nameof (name));
 
-		var avdManagerPath = RequireAvdManagerPath ();
-
 		using var stderr = new StringWriter ();
 		var psi = ProcessUtils.CreateProcessStartInfo (avdManagerPath, "delete", "avd", "--name", name);
-		ConfigureEnvironment (psi);
-		var exitCode = await ProcessUtils.StartProcess (psi, null, stderr, cancellationToken).ConfigureAwait (false);
+		var exitCode = await ProcessUtils.StartProcess (psi, null, stderr, cancellationToken, environmentVariables).ConfigureAwait (false);
 
-		ProcessUtils.ThrowIfFailed (exitCode, $"avdmanager delete avd --name {name}", stderr.ToString ());
+		ProcessUtils.ThrowIfFailed (exitCode, $"avdmanager delete avd --name {name}", stderr);
 	}
 
 	internal static List<AvdInfo> ParseAvdListOutput (string output)
@@ -173,12 +143,12 @@ static string GetAvdRootDirectory ()
 	{
 		// ANDROID_AVD_HOME takes highest priority
 		var avdHome = Environment.GetEnvironmentVariable (EnvironmentVariableNames.AndroidAvdHome);
-		if (!string.IsNullOrEmpty (avdHome))
+		if (avdHome is { Length: > 0 })
 			return avdHome;
 
 		// ANDROID_USER_HOME/avd is the next option
 		var userHome = Environment.GetEnvironmentVariable (EnvironmentVariableNames.AndroidUserHome);
-		if (!string.IsNullOrEmpty (userHome))
+		if (userHome is { Length: > 0 })
 			return Path.Combine (userHome, "avd");
 
 		// Default: ~/.android/avd

tests/Xamarin.Android.Tools.AndroidSdk-Tests/AvdManagerRunnerTests.cs

@@ -89,7 +89,7 @@ public void ParseAvdListOutput_SingleAvdNoDevice ()
 	}
 
 	[Test]
-	public void AvdManagerPath_FindsVersionedDir ()
+	public void FindCmdlineTool_FindsVersionedDir ()
 	{
 		var tempDir = Path.Combine (Path.GetTempPath (), $"avd-test-{Path.GetRandomFileName ()}");
 		var binDir = Path.Combine (tempDir, "cmdline-tools", "12.0", "bin");
@@ -99,16 +99,16 @@ public void AvdManagerPath_FindsVersionedDir ()
 			var avdMgrName = OS.IsWindows ? "avdmanager.bat" : "avdmanager";
 			File.WriteAllText (Path.Combine (binDir, avdMgrName), "");
 
-			var runner = new AvdManagerRunner (() => tempDir, null);
-			Assert.IsNotNull (runner.AvdManagerPath);
-			Assert.IsTrue (runner.AvdManagerPath!.Contains ("12.0"));
+			var path = ProcessUtils.FindCmdlineTool (tempDir, "avdmanager", OS.IsWindows ? ".bat" : "");
+			Assert.IsNotNull (path);
+			Assert.IsTrue (path!.Contains ("12.0"));
 		} finally {
 			Directory.Delete (tempDir, true);
 		}
 	}
 
 	[Test]
-	public void AvdManagerPath_PrefersHigherVersion ()
+	public void FindCmdlineTool_PrefersHigherVersion ()
 	{
 		var tempDir = Path.Combine (Path.GetTempPath (), $"avd-test-{Path.GetRandomFileName ()}");
 		var avdMgrName = OS.IsWindows ? "avdmanager.bat" : "avdmanager";
@@ -121,16 +121,16 @@ public void AvdManagerPath_PrefersHigherVersion ()
 		File.WriteAllText (Path.Combine (binDir12, avdMgrName), "");
 
 		try {
-			var runner = new AvdManagerRunner (() => tempDir, null);
-			Assert.IsNotNull (runner.AvdManagerPath);
-			Assert.IsTrue (runner.AvdManagerPath!.Contains ("12.0"));
+			var path = ProcessUtils.FindCmdlineTool (tempDir, "avdmanager", OS.IsWindows ? ".bat" : "");
+			Assert.IsNotNull (path);
+			Assert.IsTrue (path!.Contains ("12.0"));
 		} finally {
 			Directory.Delete (tempDir, true);
 		}
 	}
 
 	[Test]
-	public void AvdManagerPath_FallsBackToLatest ()
+	public void FindCmdlineTool_FallsBackToLatest ()
 	{
 		var tempDir = Path.Combine (Path.GetTempPath (), $"avd-test-{Path.GetRandomFileName ()}");
 		var binDir = Path.Combine (tempDir, "cmdline-tools", "latest", "bin");
@@ -140,60 +140,65 @@ public void AvdManagerPath_FallsBackToLatest ()
 			var avdMgrName = OS.IsWindows ? "avdmanager.bat" : "avdmanager";
 			File.WriteAllText (Path.Combine (binDir, avdMgrName), "");
 
-			var runner = new AvdManagerRunner (() => tempDir, null);
-			Assert.IsNotNull (runner.AvdManagerPath);
-			Assert.IsTrue (runner.AvdManagerPath!.Contains ("latest"));
+			var path = ProcessUtils.FindCmdlineTool (tempDir, "avdmanager", OS.IsWindows ? ".bat" : "");
+			Assert.IsNotNull (path);
+			Assert.IsTrue (path!.Contains ("latest"));
 		} finally {
 			Directory.Delete (tempDir, true);
 		}
 	}
 
 	[Test]
-	public void AvdManagerPath_NullSdk_ReturnsNull ()
+	public void FindCmdlineTool_MissingSdk_ReturnsNull ()
 	{
-		var runner = new AvdManagerRunner (() => null, null);
-		Assert.IsNull (runner.AvdManagerPath);
+		var path = ProcessUtils.FindCmdlineTool ("/nonexistent/path", "avdmanager", OS.IsWindows ? ".bat" : "");
+		Assert.IsNull (path);
 	}
 
 	[Test]
-	public void AvdManagerPath_MissingSdk_ReturnsNull ()
+	public void Constructor_NullPath_ThrowsArgumentException ()
 	{
-		var runner = new AvdManagerRunner (() => "/nonexistent/path", null);
-		Assert.IsNull (runner.AvdManagerPath);
+		Assert.Throws<ArgumentException> (() => new AvdManagerRunner (null!));
+	}
+
+	[Test]
+	public void Constructor_EmptyPath_ThrowsArgumentException ()
+	{
+		Assert.Throws<ArgumentException> (() => new AvdManagerRunner (""));
 	}
 
 	[Test]
 	public void CreateAvdAsync_NullName_ThrowsArgumentNullException ()
 	{
-		var runner = new AvdManagerRunner (() => "/fake/sdk", null);
+		var runner = new AvdManagerRunner ("/fake/avdmanager");
 		Assert.ThrowsAsync<ArgumentNullException> (() => runner.CreateAvdAsync (null!, "system-image"));
 	}
 
 	[Test]
 	public void CreateAvdAsync_EmptyName_ThrowsArgumentException ()
 	{
-		var runner = new AvdManagerRunner (() => "/fake/sdk", null);
+		var runner = new AvdManagerRunner ("/fake/avdmanager");
 		Assert.ThrowsAsync<ArgumentException> (() => runner.CreateAvdAsync ("", "system-image"));
 	}
 
 	[Test]
 	public void CreateAvdAsync_EmptySystemImage_ThrowsArgumentException ()
 	{
-		var runner = new AvdManagerRunner (() => "/fake/sdk", null);
+		var runner = new AvdManagerRunner ("/fake/avdmanager");
 		Assert.ThrowsAsync<ArgumentException> (() => runner.CreateAvdAsync ("test-avd", ""));
 	}
 
 	[Test]
 	public void DeleteAvdAsync_NullName_ThrowsArgumentNullException ()
 	{
-		var runner = new AvdManagerRunner (() => "/fake/sdk", null);
+		var runner = new AvdManagerRunner ("/fake/avdmanager");
 		Assert.ThrowsAsync<ArgumentNullException> (() => runner.DeleteAvdAsync (null!));
 	}
 
 	[Test]
 	public void DeleteAvdAsync_EmptyName_ThrowsArgumentException ()
 	{
-		var runner = new AvdManagerRunner (() => "/fake/sdk", null);
+		var runner = new AvdManagerRunner ("/fake/avdmanager");
 		Assert.ThrowsAsync<ArgumentException> (() => runner.DeleteAvdAsync (""));
 	}
 }