Add EmulatorRunner for emulator CLI operations

[rmarinho]

Summary

Wraps emulator CLI operations for starting and managing Android emulators. Addresses #278.

Changes

EmulatorRunner — Emulator boot and management

New: BootAndWaitAsync — Ports the boot emulator logic from dotnet/android's BootAndroidEmulator MSBuild task into the shared library.

Three-phase boot:

1. Check if device is already online in adb devices → pass through
2. Check if AVD is already running (but still booting) → wait for full boot
3. Launch emulator + poll until it appears + wait for full boot

Full boot = sys.boot_completed == "1" AND pm path android returns package: prefix.

New: EmulatorBootResult — Result type with Success, Serial, ErrorMessage
New: EmulatorBootOptions — Options type with BootTimeout, AdditionalArgs, ColdBoot, PollInterval

AdbRunner — Shell command helpers

New: GetShellPropertyAsync — Runs adb -s serial shell getprop property
New: RunShellCommandAsync — Runs adb -s serial shell command

These support the boot wait logic (checking sys.boot_completed and pm path android).

API

public class EmulatorRunner
{
    public EmulatorRunner (Func<string?> getSdkPath);
    public EmulatorRunner (Func<string?> getSdkPath, Func<string?>? getJdkPath);

    public string? EmulatorPath { get; }
    public bool IsAvailable { get; }

    Process StartAvd (string avdName, bool coldBoot = false, string? additionalArgs = null);
    Task<IReadOnlyList<string>> ListAvdNamesAsync (CancellationToken ct = default);

    // NEW: Boot and wait for emulator to be fully ready
    Task<EmulatorBootResult> BootAndWaitAsync (
        string deviceOrAvdName,
        AdbRunner adbRunner,
        EmulatorBootOptions? options = null,
        Action<TraceLevel, string>? logger = null,
        CancellationToken cancellationToken = default);
}

public class EmulatorBootResult
{
    public bool Success { get; set; }
    public string? Serial { get; set; }
    public string? ErrorMessage { get; set; }
}

public class EmulatorBootOptions
{
    public TimeSpan BootTimeout { get; set; } = TimeSpan.FromSeconds(300);
    public string? AdditionalArgs { get; set; }
    public bool ColdBoot { get; set; }
    public TimeSpan PollInterval { get; set; } = TimeSpan.FromMilliseconds(500);
}

Downstream Consumer

dotnet/android BootAndroidEmulator.cs will delegate to EmulatorRunner.BootAndWaitAsync() via the submodule — similar to how GetAvailableAndroidDevices delegates to AdbRunner.ParseAdbDevicesOutput().

Draft dotnet/android PR to follow.

Tests

13 unit tests (EmulatorRunnerTests.cs):

• ParseListAvdsOutput: multiple AVDs, empty, Windows newlines, blank lines
• EmulatorPath: SDK discovery, missing SDK, null SDK
• AlreadyOnlineDevice_PassesThrough: device already in adb devices
• AvdAlreadyRunning_WaitsForFullBoot: AVD running, wait for boot completion
• BootEmulator_AppearsAfterPolling: AVD not running, appears after polling
• LaunchFailure_ReturnsError: emulator path missing
• BootTimeout_BootCompletedNeverReaches1: boot timeout
• MultipleEmulators_FindsCorrectAvd: finds correct AVD among multiple

Test scenarios ported from dotnet/android BootAndroidEmulatorTests.cs.

Dependencies

Requires #283 (AdbRunner) — uses ListDevicesAsync, GetEmulatorAvdNameAsync, GetShellPropertyAsync, RunShellCommandAsync.

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

[Copilot]

Pull request overview

This PR adds a new EmulatorRunner to Xamarin.Android.Tools.AndroidSdk intended to wrap Android emulator CLI operations, alongside new shared infrastructure for running Android SDK command-line tools with environment setup and result modeling.

Changes:

• Added EmulatorRunner to start an AVD, stop an emulator, and list available AVD names.
• Added AndroidToolRunner utility to run SDK tools sync/async (with timeouts) and to start long-running background processes.
• Added AndroidEnvironmentHelper and ToolRunnerResult / ToolRunnerResult<T> to standardize tool environment and execution results.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 8 comments.

File	Description
src/Xamarin.Android.Tools.AndroidSdk/Runners/EmulatorRunner.cs	Introduces emulator wrapper methods (start/stop/list AVDs) built on the tool runner infrastructure.
src/Xamarin.Android.Tools.AndroidSdk/Runners/AndroidToolRunner.cs	Adds process execution helpers (sync/async + background) with timeout/output capture.
src/Xamarin.Android.Tools.AndroidSdk/Runners/AndroidEnvironmentHelper.cs	Adds env var setup and mapping helpers (ABI/API/tag display names).
src/Xamarin.Android.Tools.AndroidSdk/Models/ToolRunnerResult.cs	Adds a shared result model for tool execution.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[jonathanpeppers]

I'd like to get the System.Diagnostics.Process code unified like mentioned here:

• Add base Android tool runner infrastructure #281 (comment)

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 4 comments.

[rmarinho]

Review feedback addressed — commit references

Feedback	Commit	Details
Port BootAndroidEmulator logic from dotnet/android	0088e39	BootAndWaitAsync with 3-phase boot, GetShellPropertyAsync, RunShellCommandAsync, 6 new tests

New files:

• Models/EmulatorBootResult.cs, Models/EmulatorBootOptions.cs
• Tests: 6 async boot scenarios ported from BootAndroidEmulatorTests.cs

Modified:

• Runners/EmulatorRunner.cs — BootAndWaitAsync, FindRunningAvdSerial, WaitForFullBootAsync
• Runners/AdbRunner.cs — GetShellPropertyAsync, RunShellCommandAsync (+ ListDevicesAsync made virtual for testability)

Draft dotnet/android consumer PR to follow.

[jonathanpeppers]

🤖 AI Review Summary

Found 7 issues: 1 correctness, 2 error handling, 1 API design, 1 code duplication, 1 code organization, 1 naming.

• Correctness: StartAvd redirects stdout/stderr but never drains the pipes — OS buffer fill will deadlock the emulator process (EmulatorRunner.cs:74)
• API design: AdditionalArgs is a single string — will be treated as one argument by ProcessUtils.ArgumentList, breaking multi-token args like -gpu swiftshader_indirect (EmulatorBootOptions.cs:14)
• Error handling: ListDevicesAsync ignores the exit code from ProcessUtils.StartProcess while sibling methods in AvdManagerRunner check it consistently (AdbRunner.cs:72)
• Code duplication: AvdManagerRunner.AvdManagerPath reimplements the cmdline-tools version scanning that ProcessUtils.FindCmdlineTool (added in this same PR) already provides (AvdManagerRunner.cs:33)
• Error handling: Bare catch { } swallows all exceptions without capturing them (AdbRunner.cs:107)

👍 Solid three-phase boot logic ported faithfully from dotnet/android. Good use of virtual on AdbRunner methods to enable clean test mocking. Thorough test coverage with 13+ unit tests covering parsing, edge cases, and the full boot flow. Nice extraction of AndroidEnvironmentHelper for shared env var setup.

---

This review was generated by the android-tools-reviewer skill based on review guidelines established by @jonathanpeppers.

[jonathanpeppers]

🤖 Pattern / Correctness — RedirectStandardOutput and RedirectStandardError are set to true but the output is never drained. When the emulator writes enough output to fill the OS pipe buffer (~4–64 KB depending on platform), the write will block and the emulator process will deadlock.

Since StartAvd is a fire-and-forget launch that returns the Process to the caller, the simplest fix is to call BeginOutputReadLine() and BeginErrorReadLine() immediately after Start() to asynchronously discard the output:

process.Start ();
process.BeginOutputReadLine ();
process.BeginErrorReadLine ();

Alternatively, if you don't need to redirect at all (just prevent pipe inheritance), you could set Redirect* = false and rely on UseShellExecute = false (already set by ProcessUtils.CreateProcessStartInfo) which doesn't inherit the caller's console.

Also: using new Process { StartInfo = psi } + process.Start() directly bypasses ProcessUtils. This is an intentional pattern for long-running background processes, but worth a brief comment explaining why ProcessUtils.StartProcess isn't used here (it awaits completion).

Rule: Don't redirect stdout/stderr without draining (Postmortem #48); Use ProcessUtils (Postmortem #15)

[jonathanpeppers]

🤖 API design — AdditionalArgs is a single string? that gets passed directly to args.Add(additionalArgs) in StartAvd. Since ProcessUtils.CreateProcessStartInfo uses ArgumentList on net5+ (no shell parsing), a string like ""-gpu swiftshader_indirect"" becomes one argument instead of two separate ones.

Consider changing this to IEnumerable<string>? so each argument is passed separately:

public IEnumerable<string>? AdditionalArgs { get; set; }

And in StartAvd:

if (options.AdditionalArgs != null)
    args.AddRange (options.AdditionalArgs);

This is safer and consistent with how ProcessUtils is designed to work.

Rule: Structured args, not string interpolation (Postmortem #50)

[jonathanpeppers]

🤖 Error handling — ListDevicesAsync ignores the exit code returned by ProcessUtils.StartProcess. Compare with AvdManagerRunner.ListAvdsAsync which captures and checks it:

var exitCode = await ProcessUtils.StartProcess (psi, stdout, stderr, cancellationToken)...;
if (exitCode != 0)
    throw new InvalidOperationException (...);

The same pattern should be applied here (and in GetShellPropertyAsync, RunShellCommandAsync, WaitForDeviceAsync, StopEmulatorAsync). Inconsistent exit-code checking creates a false sense of safety — if ListAvdsAsync checks, callers will assume ListDevicesAsync does too.

Rule: Check exit codes consistently (Postmortem #48)

[jonathanpeppers]

🤖 Code duplication — This property reimplements the versioned cmdline-tools scanning (list dirs → parse versions → sort descending → try each → fallback to ""latest"" → fallback to legacy tools/bin) that ProcessUtils.FindCmdlineTool already provides. Both were added in this PR. Consider delegating:

public string? AvdManagerPath {
    get {
        var sdkPath = getSdkPath ();
        if (string.IsNullOrEmpty (sdkPath))
            return null;
        var ext = OS.IsWindows ? "".bat"" : """";
        return ProcessUtils.FindCmdlineTool (sdkPath, ""avdmanager"", ext);
    }
}

This removes ~20 lines and ensures both paths stay consistent if the search logic changes.

Rule: Reinventing the wheel / code duplication (Postmortem #40)

[jonathanpeppers]

🤖 Code organization — This file contains three public types: AdbDeviceType (enum), AdbDeviceStatus (enum), and AdbDeviceInfo (class). Per the one-type-per-file convention, each should be in its own file: AdbDeviceType.cs, AdbDeviceStatus.cs, and AdbDeviceInfo.cs.

Rule: One type per file (Postmortem #9)

[jonathanpeppers]

🤖 Naming — Raw string ""ANDROID_AVD_HOME"" should use a constant from EnvironmentVariableNames (same as AndroidUserHome added in this PR). This prevents silent failures from typos and keeps all environment variable names centralized.

Consider adding public const string AndroidAvdHome = ""ANDROID_AVD_HOME""; to EnvironmentVariableNames and using it here.

Rule: Environment variable constants (Postmortem #19)

[jonathanpeppers]

🤖 Error handling — Bare catch { } swallows all exceptions without capturing the Exception. Even though this failure is intentional (emulator may not support the command), capturing the exception enables diagnostic logging when issues surface:

} catch (Exception) {
    // Expected: emulator may not support 'emu avd name' command
}

At minimum, use catch (Exception) instead of bare catch so it's clear the intent is to catch CLR exceptions (not SEH exceptions on older runtimes).

Rule: No empty/bare catch blocks (Postmortem #11)