From 40c20d0a3344a7b6f5235af71e570581f68f8129 Mon Sep 17 00:00:00 2001 From: Rui Marinho Date: Thu, 30 Apr 2026 14:55:40 +0100 Subject: [PATCH 1/3] [AdbRunner] Add ADB forward port management (follow-up to #305) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds the symmetric forward-port pair to the reverse-port methods that landed in #305. Same `AdbPortSpec` / `AdbPortRule` / `AdbProtocol` types — just four new methods. | Method | adb command | |-------------------------------------------------|--------------------------------------------| | ForwardPortAsync(serial, local, remote) | adb -s forward | | RemoveForwardPortAsync(serial, local) | adb -s forward --remove | | RemoveAllForwardPortsAsync(serial) | adb -s forward --remove-all | | ListForwardPortsAsync(serial) | adb forward --list (filtered by serial) | `adb forward` and `adb reverse` are not interchangeable — they connect opposite directions. `forward` is host->device (the IDE/harness reaches a service running on the device) and is the path used for JDWP debugger attach (`forward tcp:N jdwp:`), perf-tracing endpoints exposed by the runtime, and host-side DevFlow agent connect when the agent listens on a device port. Output-format note for `--list`: `adb forward --list` emits one line per rule across all devices in the form ` ` (different from `(reverse) `). `ListForwardPortsAsync` uses the unscoped `adb forward --list` and filters to the requested serial in `ParseForwardListOutput`. Serial match is case-sensitive (matches adb). - 12 new parser tests in `ParseForwardListOutput_*` mirroring the reverse parser tests (single rule, multiple rules, serial filtering, empty output, malformed lines, non-tcp specs, Windows line endings, tab separation, case sensitivity). - 7 new parameter-validation tests covering empty serial / null spec for the four new public methods. - VS Code MAUI extension ServiceHub->CLI migration (`forwardPort` in `MauiAndroidPlatform.ts` — debugger configurations, perf tooling). - MAUI DevTools CLI (dotnet/maui-labs#197) — `maui android port forward` group, sibling of the existing `reverse` surface. - Visual Studio `ClientTools.Platform` — same paths that drive reverse today. Discussed in https://github.com/dotnet/android-tools/pull/305#issuecomment-4352578817 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../PublicAPI/net10.0/PublicAPI.Unshipped.txt | 4 + .../netstandard2.0/PublicAPI.Unshipped.txt | 4 + .../Runners/AdbRunner.cs | 120 ++++++++++ .../AdbRunnerTests.cs | 226 ++++++++++++++++++ 4 files changed, 354 insertions(+) diff --git a/src/Xamarin.Android.Tools.AndroidSdk/PublicAPI/net10.0/PublicAPI.Unshipped.txt b/src/Xamarin.Android.Tools.AndroidSdk/PublicAPI/net10.0/PublicAPI.Unshipped.txt index dd3c96f1..b75def5f 100644 --- a/src/Xamarin.Android.Tools.AndroidSdk/PublicAPI/net10.0/PublicAPI.Unshipped.txt +++ b/src/Xamarin.Android.Tools.AndroidSdk/PublicAPI/net10.0/PublicAPI.Unshipped.txt @@ -199,6 +199,10 @@ virtual Xamarin.Android.Tools.AdbRunner.ListReversePortsAsync(string! serial, Sy virtual Xamarin.Android.Tools.AdbRunner.RemoveAllReversePortsAsync(string! serial, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) -> System.Threading.Tasks.Task! virtual Xamarin.Android.Tools.AdbRunner.RemoveReversePortAsync(string! serial, Xamarin.Android.Tools.AdbPortSpec! remote, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) -> System.Threading.Tasks.Task! virtual Xamarin.Android.Tools.AdbRunner.ReversePortAsync(string! serial, Xamarin.Android.Tools.AdbPortSpec! remote, Xamarin.Android.Tools.AdbPortSpec! local, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) -> System.Threading.Tasks.Task! +virtual Xamarin.Android.Tools.AdbRunner.ForwardPortAsync(string! serial, Xamarin.Android.Tools.AdbPortSpec! local, Xamarin.Android.Tools.AdbPortSpec! remote, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) -> System.Threading.Tasks.Task! +virtual Xamarin.Android.Tools.AdbRunner.ListForwardPortsAsync(string! serial, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) -> System.Threading.Tasks.Task!>! +virtual Xamarin.Android.Tools.AdbRunner.RemoveAllForwardPortsAsync(string! serial, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) -> System.Threading.Tasks.Task! +virtual Xamarin.Android.Tools.AdbRunner.RemoveForwardPortAsync(string! serial, Xamarin.Android.Tools.AdbPortSpec! local, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) -> System.Threading.Tasks.Task! Xamarin.Android.Tools.AvdManagerRunner.ListDeviceProfilesAsync(System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) -> System.Threading.Tasks.Task!>! Xamarin.Android.Tools.AvdDeviceProfile Xamarin.Android.Tools.AvdDeviceProfile.AvdDeviceProfile(string! Id) -> void diff --git a/src/Xamarin.Android.Tools.AndroidSdk/PublicAPI/netstandard2.0/PublicAPI.Unshipped.txt b/src/Xamarin.Android.Tools.AndroidSdk/PublicAPI/netstandard2.0/PublicAPI.Unshipped.txt index dd3c96f1..b75def5f 100644 --- a/src/Xamarin.Android.Tools.AndroidSdk/PublicAPI/netstandard2.0/PublicAPI.Unshipped.txt +++ b/src/Xamarin.Android.Tools.AndroidSdk/PublicAPI/netstandard2.0/PublicAPI.Unshipped.txt @@ -199,6 +199,10 @@ virtual Xamarin.Android.Tools.AdbRunner.ListReversePortsAsync(string! serial, Sy virtual Xamarin.Android.Tools.AdbRunner.RemoveAllReversePortsAsync(string! serial, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) -> System.Threading.Tasks.Task! virtual Xamarin.Android.Tools.AdbRunner.RemoveReversePortAsync(string! serial, Xamarin.Android.Tools.AdbPortSpec! remote, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) -> System.Threading.Tasks.Task! virtual Xamarin.Android.Tools.AdbRunner.ReversePortAsync(string! serial, Xamarin.Android.Tools.AdbPortSpec! remote, Xamarin.Android.Tools.AdbPortSpec! local, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) -> System.Threading.Tasks.Task! +virtual Xamarin.Android.Tools.AdbRunner.ForwardPortAsync(string! serial, Xamarin.Android.Tools.AdbPortSpec! local, Xamarin.Android.Tools.AdbPortSpec! remote, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) -> System.Threading.Tasks.Task! +virtual Xamarin.Android.Tools.AdbRunner.ListForwardPortsAsync(string! serial, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) -> System.Threading.Tasks.Task!>! +virtual Xamarin.Android.Tools.AdbRunner.RemoveAllForwardPortsAsync(string! serial, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) -> System.Threading.Tasks.Task! +virtual Xamarin.Android.Tools.AdbRunner.RemoveForwardPortAsync(string! serial, Xamarin.Android.Tools.AdbPortSpec! local, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) -> System.Threading.Tasks.Task! Xamarin.Android.Tools.AvdManagerRunner.ListDeviceProfilesAsync(System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) -> System.Threading.Tasks.Task!>! Xamarin.Android.Tools.AvdDeviceProfile Xamarin.Android.Tools.AvdDeviceProfile.AvdDeviceProfile(string! Id) -> void diff --git a/src/Xamarin.Android.Tools.AndroidSdk/Runners/AdbRunner.cs b/src/Xamarin.Android.Tools.AndroidSdk/Runners/AdbRunner.cs index 0eadc70e..ad5e8c9b 100644 --- a/src/Xamarin.Android.Tools.AndroidSdk/Runners/AdbRunner.cs +++ b/src/Xamarin.Android.Tools.AndroidSdk/Runners/AdbRunner.cs @@ -355,6 +355,126 @@ internal static IReadOnlyList ParseReverseListOutput (IEnumerable + /// Sets up forward port forwarding via 'adb -s <serial> forward <local> <remote>'. + /// The host-side <local> socket is forwarded to the device-side <remote> socket, + /// the symmetric pair to . + /// + /// Device serial number. + /// Local (host-side) port spec. + /// Remote (device-side) port spec. + /// Cancellation token. + public virtual async Task ForwardPortAsync (string serial, AdbPortSpec local, AdbPortSpec remote, CancellationToken cancellationToken = default) + { + if (string.IsNullOrWhiteSpace (serial)) + throw new ArgumentException ("Serial must not be empty.", nameof (serial)); + if (local is null) + throw new ArgumentNullException (nameof (local)); + if (remote is null) + throw new ArgumentNullException (nameof (remote)); + if (local.Port <= 0 || local.Port > 65535) + throw new ArgumentOutOfRangeException (nameof (local), local.Port, "Port must be between 1 and 65535."); + if (remote.Port <= 0 || remote.Port > 65535) + throw new ArgumentOutOfRangeException (nameof (remote), remote.Port, "Port must be between 1 and 65535."); + + var psi = ProcessUtils.CreateProcessStartInfo (adbPath, "-s", serial, "forward", local.ToSocketSpec (), remote.ToSocketSpec ()); + using var stderr = new StringWriter (); + var exitCode = await ProcessUtils.StartProcess (psi, null, stderr, cancellationToken, environmentVariables).ConfigureAwait (false); + ProcessUtils.ThrowIfFailed (exitCode, $"adb -s {serial} forward {local} {remote}", stderr); + } + + /// + /// Removes a specific forward port forwarding rule via + /// 'adb -s <serial> forward --remove <local>'. + /// + /// Device serial number. + /// Local (host-side) port spec to remove. + /// Cancellation token. + public virtual async Task RemoveForwardPortAsync (string serial, AdbPortSpec local, CancellationToken cancellationToken = default) + { + if (string.IsNullOrWhiteSpace (serial)) + throw new ArgumentException ("Serial must not be empty.", nameof (serial)); + if (local is null) + throw new ArgumentNullException (nameof (local)); + if (local.Port <= 0 || local.Port > 65535) + throw new ArgumentOutOfRangeException (nameof (local), local.Port, "Port must be between 1 and 65535."); + + var psi = ProcessUtils.CreateProcessStartInfo (adbPath, "-s", serial, "forward", "--remove", local.ToSocketSpec ()); + using var stderr = new StringWriter (); + var exitCode = await ProcessUtils.StartProcess (psi, null, stderr, cancellationToken, environmentVariables).ConfigureAwait (false); + ProcessUtils.ThrowIfFailed (exitCode, $"adb -s {serial} forward --remove {local}", stderr); + } + + /// + /// Removes all forward port forwarding rules for the device via + /// 'adb -s <serial> forward --remove-all'. + /// Note that the underlying adb command operates globally, but we scope it via -s. + /// + public virtual async Task RemoveAllForwardPortsAsync (string serial, CancellationToken cancellationToken = default) + { + if (string.IsNullOrWhiteSpace (serial)) + throw new ArgumentException ("Serial must not be empty.", nameof (serial)); + + var psi = ProcessUtils.CreateProcessStartInfo (adbPath, "-s", serial, "forward", "--remove-all"); + using var stderr = new StringWriter (); + var exitCode = await ProcessUtils.StartProcess (psi, null, stderr, cancellationToken, environmentVariables).ConfigureAwait (false); + ProcessUtils.ThrowIfFailed (exitCode, $"adb -s {serial} forward --remove-all", stderr); + } + + /// + /// Lists active forward port forwarding rules for the specified device via + /// 'adb forward --list'. + /// The underlying command always lists rules across all devices, so the + /// result is filtered to entries matching . + /// + public virtual async Task> ListForwardPortsAsync (string serial, CancellationToken cancellationToken = default) + { + if (string.IsNullOrWhiteSpace (serial)) + throw new ArgumentException ("Serial must not be empty.", nameof (serial)); + + using var stdout = new StringWriter (); + using var stderr = new StringWriter (); + var psi = ProcessUtils.CreateProcessStartInfo (adbPath, "forward", "--list"); + var exitCode = await ProcessUtils.StartProcess (psi, stdout, stderr, cancellationToken, environmentVariables).ConfigureAwait (false); + ProcessUtils.ThrowIfFailed (exitCode, $"adb forward --list", stderr, stdout); + + return ParseForwardListOutput (stdout.ToString ().Split ('\n'), serial); + } + + /// + /// Parses the output of 'adb forward --list'. + /// Each line is "<serial> <local> <remote>", e.g. "emulator-5554 tcp:5000 tcp:6000". + /// Only rules matching are returned. Lines with + /// unparseable socket specs are skipped. + /// + internal static IReadOnlyList ParseForwardListOutput (IEnumerable lines, string serial) + { + var rules = new List (); + if (string.IsNullOrEmpty (serial)) + return rules; + + foreach (var line in lines) { + var trimmed = line.Trim (); + if (string.IsNullOrEmpty (trimmed)) + continue; + + // Expected format: " " + var parts = trimmed.Split ((char[]?) null, StringSplitOptions.RemoveEmptyEntries); + if (parts.Length < 3) + continue; + + if (!string.Equals (parts [0], serial, StringComparison.Ordinal)) + continue; + + var local = AdbPortSpec.TryParse (parts [1]); + var remote = AdbPortSpec.TryParse (parts [2]); + if (local is { } l && remote is { } r) + rules.Add (new AdbPortRule (r, l)); + } + + return rules; + } + /// /// Parses the output lines from 'adb devices -l'. /// Accepts an to avoid allocating a joined string. diff --git a/tests/Xamarin.Android.Tools.AndroidSdk-Tests/AdbRunnerTests.cs b/tests/Xamarin.Android.Tools.AndroidSdk-Tests/AdbRunnerTests.cs index 00b5ccc6..9fe488dc 100644 --- a/tests/Xamarin.Android.Tools.AndroidSdk-Tests/AdbRunnerTests.cs +++ b/tests/Xamarin.Android.Tools.AndroidSdk-Tests/AdbRunnerTests.cs @@ -863,6 +863,168 @@ public void ParseReverseListOutput_TabSeparated () Assert.AreEqual (8081, rules [1].Remote.Port); } + // --- ParseForwardListOutput tests --- + // Consumer: MAUI DevTools (via ListForwardPortsAsync — host→device tunnel for JDWP debugger + // attach, perf endpoints, host-side DevFlow agent connect), vscode-maui ServiceHub replacement. + // Output format differs from reverse: "(reverse) " → " ". + + [Test] + public void ParseForwardListOutput_SingleRule () + { + var output = new [] { + "emulator-5554 tcp:5000 tcp:6000", + }; + + var rules = AdbRunner.ParseForwardListOutput (output, "emulator-5554"); + + Assert.AreEqual (1, rules.Count); + Assert.AreEqual (AdbProtocol.Tcp, rules [0].Local.Protocol); + Assert.AreEqual (5000, rules [0].Local.Port); + Assert.AreEqual (AdbProtocol.Tcp, rules [0].Remote.Protocol); + Assert.AreEqual (6000, rules [0].Remote.Port); + } + + [Test] + public void ParseForwardListOutput_MultipleRulesSameDevice () + { + var output = new [] { + "emulator-5554 tcp:5000 tcp:6000", + "emulator-5554 tcp:8081 tcp:8081", + "emulator-5554 tcp:9222 tcp:9223", + }; + + var rules = AdbRunner.ParseForwardListOutput (output, "emulator-5554"); + + Assert.AreEqual (3, rules.Count); + Assert.AreEqual (5000, rules [0].Local.Port); + Assert.AreEqual (6000, rules [0].Remote.Port); + Assert.AreEqual (8081, rules [1].Local.Port); + Assert.AreEqual (8081, rules [1].Remote.Port); + Assert.AreEqual (9222, rules [2].Local.Port); + Assert.AreEqual (9223, rules [2].Remote.Port); + } + + [Test] + public void ParseForwardListOutput_FiltersByDeviceSerial () + { + // adb forward --list returns rules across ALL devices; we must filter. + var output = new [] { + "emulator-5554 tcp:5000 tcp:5000", + "emulator-5556 tcp:5001 tcp:5001", + "emulator-5554 tcp:8081 tcp:8081", + "abcd1234device tcp:9000 tcp:9000", + }; + + var rules = AdbRunner.ParseForwardListOutput (output, "emulator-5554"); + + Assert.AreEqual (2, rules.Count); + Assert.AreEqual (5000, rules [0].Local.Port); + Assert.AreEqual (8081, rules [1].Local.Port); + } + + [Test] + public void ParseForwardListOutput_EmptyOutput () + { + var output = new [] { "", " " }; + var rules = AdbRunner.ParseForwardListOutput (output, "emulator-5554"); + Assert.AreEqual (0, rules.Count); + } + + [Test] + public void ParseForwardListOutput_NoLines () + { + var rules = AdbRunner.ParseForwardListOutput (Array.Empty (), "emulator-5554"); + Assert.AreEqual (0, rules.Count); + } + + [Test] + public void ParseForwardListOutput_EmptySerial_ReturnsEmpty () + { + var output = new [] { "emulator-5554 tcp:5000 tcp:5000" }; + var rules = AdbRunner.ParseForwardListOutput (output, ""); + Assert.AreEqual (0, rules.Count); + } + + [Test] + public void ParseForwardListOutput_NoMatchingDevice () + { + var output = new [] { + "emulator-5554 tcp:5000 tcp:5000", + "emulator-5556 tcp:5001 tcp:5001", + }; + var rules = AdbRunner.ParseForwardListOutput (output, "missing-device"); + Assert.AreEqual (0, rules.Count); + } + + [Test] + public void ParseForwardListOutput_MalformedLine_InsufficientParts () + { + var output = new [] { + "emulator-5554 tcp:5000", // missing remote spec + }; + + var rules = AdbRunner.ParseForwardListOutput (output, "emulator-5554"); + Assert.AreEqual (0, rules.Count); + } + + [Test] + public void ParseForwardListOutput_NonTcpSpecs_SkipsUnparseable () + { + var output = new [] { + "emulator-5554 tcp:9222 localabstract:chrome_devtools_remote", + "emulator-5554 tcp:5000 tcp:5000", + }; + + var rules = AdbRunner.ParseForwardListOutput (output, "emulator-5554"); + + // localabstract:chrome_devtools_remote has a non-numeric port, so it is skipped + Assert.AreEqual (1, rules.Count); + Assert.AreEqual (5000, rules [0].Local.Port); + Assert.AreEqual (5000, rules [0].Remote.Port); + } + + [Test] + public void ParseForwardListOutput_WindowsLineEndings () + { + var output = new [] { + "emulator-5554 tcp:5000 tcp:6000\r", + "emulator-5554 tcp:8081 tcp:8081\r", + }; + + var rules = AdbRunner.ParseForwardListOutput (output, "emulator-5554"); + + Assert.AreEqual (2, rules.Count); + Assert.AreEqual (5000, rules [0].Local.Port); + Assert.AreEqual (6000, rules [0].Remote.Port); + Assert.AreEqual (8081, rules [1].Local.Port); + } + + [Test] + public void ParseForwardListOutput_TabSeparated () + { + var output = new [] { + "emulator-5554\ttcp:5000\ttcp:6000", + }; + + var rules = AdbRunner.ParseForwardListOutput (output, "emulator-5554"); + + Assert.AreEqual (1, rules.Count); + Assert.AreEqual (5000, rules [0].Local.Port); + Assert.AreEqual (6000, rules [0].Remote.Port); + } + + [Test] + public void ParseForwardListOutput_SerialMatch_IsCaseSensitive () + { + // adb device serials are case-sensitive. + var output = new [] { + "emulator-5554 tcp:5000 tcp:5000", + }; + + var rules = AdbRunner.ParseForwardListOutput (output, "EMULATOR-5554"); + Assert.AreEqual (0, rules.Count); + } + // --- AdbPortSpec tests --- [Test] @@ -1069,6 +1231,70 @@ public void ListReversePortsAsync_EmptySerial_ThrowsArgumentException () async () => await runner.ListReversePortsAsync ("")); } + // --- ForwardPortAsync parameter validation tests --- + + [Test] + public void ForwardPortAsync_EmptySerial_ThrowsArgumentException () + { + var runner = new AdbRunner ("/fake/sdk/platform-tools/adb"); + Assert.ThrowsAsync ( + async () => await runner.ForwardPortAsync ("", new AdbPortSpec (AdbProtocol.Tcp, 5000), new AdbPortSpec (AdbProtocol.Tcp, 5000))); + } + + [Test] + public void ForwardPortAsync_NullLocal_ThrowsArgumentNull () + { + var runner = new AdbRunner ("/fake/sdk/platform-tools/adb"); + Assert.ThrowsAsync ( + async () => await runner.ForwardPortAsync ("emulator-5554", (AdbPortSpec) null!, new AdbPortSpec (AdbProtocol.Tcp, 5000))); + } + + [Test] + public void ForwardPortAsync_NullRemote_ThrowsArgumentNull () + { + var runner = new AdbRunner ("/fake/sdk/platform-tools/adb"); + Assert.ThrowsAsync ( + async () => await runner.ForwardPortAsync ("emulator-5554", new AdbPortSpec (AdbProtocol.Tcp, 5000), (AdbPortSpec) null!)); + } + + // --- RemoveForwardPortAsync parameter validation tests --- + + [Test] + public void RemoveForwardPortAsync_EmptySerial_ThrowsArgumentException () + { + var runner = new AdbRunner ("/fake/sdk/platform-tools/adb"); + Assert.ThrowsAsync ( + async () => await runner.RemoveForwardPortAsync ("", new AdbPortSpec (AdbProtocol.Tcp, 5000))); + } + + [Test] + public void RemoveForwardPortAsync_NullLocal_ThrowsArgumentNull () + { + var runner = new AdbRunner ("/fake/sdk/platform-tools/adb"); + Assert.ThrowsAsync ( + async () => await runner.RemoveForwardPortAsync ("emulator-5554", (AdbPortSpec) null!)); + } + + // --- RemoveAllForwardPortsAsync parameter validation tests --- + + [Test] + public void RemoveAllForwardPortsAsync_EmptySerial_ThrowsArgumentException () + { + var runner = new AdbRunner ("/fake/sdk/platform-tools/adb"); + Assert.ThrowsAsync ( + async () => await runner.RemoveAllForwardPortsAsync ("")); + } + + // --- ListForwardPortsAsync parameter validation tests --- + + [Test] + public void ListForwardPortsAsync_EmptySerial_ThrowsArgumentException () + { + var runner = new AdbRunner ("/fake/sdk/platform-tools/adb"); + Assert.ThrowsAsync ( + async () => await runner.ListForwardPortsAsync ("")); + } + // --- GetEmulatorAvdNameAsync + ListDevicesAsync tests --- // These tests use a fake 'adb' script to control process output, // verifying AVD detection order and offline emulator handling. From 07b13d428b00fdd06f43eeaa02d1ff6dced6d3f2 Mon Sep 17 00:00:00 2001 From: Rui Marinho Date: Mon, 1 Jun 2026 15:06:10 +0100 Subject: [PATCH 2/3] Address review feedback: capture stdout in ThrowIfFailed, parser asymmetry comment, drop null!, remove test region dividers - ForwardPortAsync/RemoveForwardPortAsync/RemoveAllForwardPortsAsync now capture stdout and pass it to ProcessUtils.ThrowIfFailed (matches repo convention; adb sometimes writes errors to stdout). - Added block on ParseForwardListOutput calling out the field-order asymmetry vs ParseReverseListOutput (forward: serial local remote; reverse: (reverse) remote local). - Replaced '(AdbPortSpec) null!' with '(AdbPortSpec) null' in 3 forward-port test sites to match reverse-test convention and repo no-null-forgiving rule. - Removed all '// --- ... ---' region-like divider comments in AdbRunnerTests.cs (per jonathanpeppers feedback in PR #351). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../Runners/AdbRunner.cs | 26 +++++++++---- .../AdbRunnerTests.cs | 38 ++----------------- 2 files changed, 22 insertions(+), 42 deletions(-) diff --git a/src/Xamarin.Android.Tools.AndroidSdk/Runners/AdbRunner.cs b/src/Xamarin.Android.Tools.AndroidSdk/Runners/AdbRunner.cs index ad5e8c9b..af296197 100644 --- a/src/Xamarin.Android.Tools.AndroidSdk/Runners/AdbRunner.cs +++ b/src/Xamarin.Android.Tools.AndroidSdk/Runners/AdbRunner.cs @@ -378,9 +378,10 @@ public virtual async Task ForwardPortAsync (string serial, AdbPortSpec local, Ad throw new ArgumentOutOfRangeException (nameof (remote), remote.Port, "Port must be between 1 and 65535."); var psi = ProcessUtils.CreateProcessStartInfo (adbPath, "-s", serial, "forward", local.ToSocketSpec (), remote.ToSocketSpec ()); + using var stdout = new StringWriter (); using var stderr = new StringWriter (); - var exitCode = await ProcessUtils.StartProcess (psi, null, stderr, cancellationToken, environmentVariables).ConfigureAwait (false); - ProcessUtils.ThrowIfFailed (exitCode, $"adb -s {serial} forward {local} {remote}", stderr); + var exitCode = await ProcessUtils.StartProcess (psi, stdout, stderr, cancellationToken, environmentVariables).ConfigureAwait (false); + ProcessUtils.ThrowIfFailed (exitCode, $"adb -s {serial} forward {local} {remote}", stderr, stdout); } /// @@ -400,9 +401,10 @@ public virtual async Task RemoveForwardPortAsync (string serial, AdbPortSpec loc throw new ArgumentOutOfRangeException (nameof (local), local.Port, "Port must be between 1 and 65535."); var psi = ProcessUtils.CreateProcessStartInfo (adbPath, "-s", serial, "forward", "--remove", local.ToSocketSpec ()); + using var stdout = new StringWriter (); using var stderr = new StringWriter (); - var exitCode = await ProcessUtils.StartProcess (psi, null, stderr, cancellationToken, environmentVariables).ConfigureAwait (false); - ProcessUtils.ThrowIfFailed (exitCode, $"adb -s {serial} forward --remove {local}", stderr); + var exitCode = await ProcessUtils.StartProcess (psi, stdout, stderr, cancellationToken, environmentVariables).ConfigureAwait (false); + ProcessUtils.ThrowIfFailed (exitCode, $"adb -s {serial} forward --remove {local}", stderr, stdout); } /// @@ -416,9 +418,10 @@ public virtual async Task RemoveAllForwardPortsAsync (string serial, Cancellatio throw new ArgumentException ("Serial must not be empty.", nameof (serial)); var psi = ProcessUtils.CreateProcessStartInfo (adbPath, "-s", serial, "forward", "--remove-all"); + using var stdout = new StringWriter (); using var stderr = new StringWriter (); - var exitCode = await ProcessUtils.StartProcess (psi, null, stderr, cancellationToken, environmentVariables).ConfigureAwait (false); - ProcessUtils.ThrowIfFailed (exitCode, $"adb -s {serial} forward --remove-all", stderr); + var exitCode = await ProcessUtils.StartProcess (psi, stdout, stderr, cancellationToken, environmentVariables).ConfigureAwait (false); + ProcessUtils.ThrowIfFailed (exitCode, $"adb -s {serial} forward --remove-all", stderr, stdout); } /// @@ -447,6 +450,14 @@ public virtual async Task> ListForwardPortsAsync (str /// Only rules matching are returned. Lines with /// unparseable socket specs are skipped. /// + /// + /// Note the field-order asymmetry vs : + /// forward --list: <serial> <local> <remote> + /// reverse --list: (reverse) <remote> <local> + /// Both parsers construct an whose constructor takes + /// (Remote, Local), so the order in which we pass the parsed parts differs between + /// the two parsers — keep that in mind when modifying either of them. + /// internal static IReadOnlyList ParseForwardListOutput (IEnumerable lines, string serial) { var rules = new List (); @@ -458,7 +469,8 @@ internal static IReadOnlyList ParseForwardListOutput (IEnumerable " + // Expected format: " " — see above for + // the field-order asymmetry with reverse --list. var parts = trimmed.Split ((char[]?) null, StringSplitOptions.RemoveEmptyEntries); if (parts.Length < 3) continue; diff --git a/tests/Xamarin.Android.Tools.AndroidSdk-Tests/AdbRunnerTests.cs b/tests/Xamarin.Android.Tools.AndroidSdk-Tests/AdbRunnerTests.cs index 9fe488dc..8919ed53 100644 --- a/tests/Xamarin.Android.Tools.AndroidSdk-Tests/AdbRunnerTests.cs +++ b/tests/Xamarin.Android.Tools.AndroidSdk-Tests/AdbRunnerTests.cs @@ -28,7 +28,6 @@ namespace Xamarin.Android.Tools.Tests; [TestFixture] public class AdbRunnerTests { - // --- ParseAdbDevicesOutput tests --- // Consumer: dotnet/android GetAvailableAndroidDevices.cs, MAUI DevTools (via ListDevicesAsync) [Test] @@ -263,7 +262,6 @@ public void ParseAdbDevicesOutput_AdbDaemonStarting () Assert.AreEqual (2, devices.Count, "Should parse devices even with daemon startup messages"); } - // --- BuildDeviceDescription tests --- // Consumer: dotnet/android GetAvailableAndroidDevices.cs [Test] @@ -301,7 +299,6 @@ public void BuildDeviceDescription_EmulatorWithUppercaseAvdName () Assert.AreEqual ("Pixel 9 Pro XL", description, "Offline emulator should still get AVD name with uppercase preserved"); } - // --- FormatDisplayName tests --- // Consumer: dotnet/android GetAvailableAndroidDevicesTests, BuildDeviceDescription (via AVD name formatting) [Test] @@ -373,7 +370,6 @@ public void FormatDisplayName_DoesNotReplaceApiInsideWords () Assert.AreEqual ("Erapidevice", AdbRunner.FormatDisplayName ("erapidevice")); } - // --- MapAdbStateToStatus tests --- // Consumer: ParseAdbDevicesOutput (internal mapping), public for custom consumers [Test] @@ -386,7 +382,6 @@ public void MapAdbStateToStatus_AllStates () Assert.AreEqual (AdbDeviceStatus.Unknown, AdbRunner.MapAdbStateToStatus ("something-else")); } - // --- MergeDevicesAndEmulators tests --- // Consumer: dotnet/android GetAvailableAndroidDevices.cs [Test] @@ -510,7 +505,6 @@ public void MergeDevicesAndEmulators_EmptyAdbDevices_ReturnsAllAvailable () Assert.AreEqual ("Pixel 9 API 36 (Not Running)", result [1].Description); } - // --- AdbPath tests --- // Consumer: MAUI DevTools Adb provider (AdbPath, IsAvailable properties) [Test] @@ -638,7 +632,6 @@ public void MapAdbStateToStatus_Sideload_ReturnsUnknown () Assert.AreEqual (AdbDeviceStatus.Unknown, AdbRunner.MapAdbStateToStatus ("sideload")); } - // --- WaitForDeviceAsync tests --- // Consumer: MAUI DevTools Adb provider (WaitForDeviceAsync) [Test] @@ -657,8 +650,6 @@ public void WaitForDeviceAsync_ZeroTimeout_ThrowsArgumentOutOfRange () async () => await runner.WaitForDeviceAsync (timeout: System.TimeSpan.Zero)); } - // --- FirstNonEmptyLine tests --- - [Test] public void FirstNonEmptyLine_ReturnsFirstLine () { @@ -716,7 +707,6 @@ public void FirstNonEmptyLine_PmPathOutput () Assert.AreEqual ("package:/system/framework/framework-res.apk", AdbRunner.FirstNonEmptyLine (output)); } - // --- ParseReverseListOutput tests --- // Consumer: MAUI DevTools (via ListReversePortsAsync), vscode-maui ServiceHub replacement [Test] @@ -863,7 +853,6 @@ public void ParseReverseListOutput_TabSeparated () Assert.AreEqual (8081, rules [1].Remote.Port); } - // --- ParseForwardListOutput tests --- // Consumer: MAUI DevTools (via ListForwardPortsAsync — host→device tunnel for JDWP debugger // attach, perf endpoints, host-side DevFlow agent connect), vscode-maui ServiceHub replacement. // Output format differs from reverse: "(reverse) " → " ". @@ -1025,8 +1014,6 @@ public void ParseForwardListOutput_SerialMatch_IsCaseSensitive () Assert.AreEqual (0, rules.Count); } - // --- AdbPortSpec tests --- - [Test] public void AdbPortSpec_TryParse_ValidTcp () { @@ -1130,8 +1117,6 @@ public void AdbPortSpec_TryParse_Roundtrip () Assert.AreEqual (original, parsed); } - // --- AdbPortRule tests --- - [Test] public void AdbPortRule_ValueEquality () { @@ -1167,8 +1152,6 @@ public void AdbPortRule_ToString () Assert.That (str, Does.Contain ("tcp:3000")); } - // --- ReversePortAsync parameter validation tests --- - [Test] public void ReversePortAsync_EmptySerial_ThrowsArgumentException () { @@ -1193,8 +1176,6 @@ public void ReversePortAsync_NullLocal_ThrowsArgumentNull () async () => await runner.ReversePortAsync ("emulator-5554", new AdbPortSpec (AdbProtocol.Tcp, 5000), (AdbPortSpec) null)); } - // --- RemoveReversePortAsync parameter validation tests --- - [Test] public void RemoveReversePortAsync_EmptySerial_ThrowsArgumentException () { @@ -1211,8 +1192,6 @@ public void RemoveReversePortAsync_NullRemote_ThrowsArgumentNull () async () => await runner.RemoveReversePortAsync ("emulator-5554", (AdbPortSpec) null)); } - // --- RemoveAllReversePortsAsync parameter validation tests --- - [Test] public void RemoveAllReversePortsAsync_EmptySerial_ThrowsArgumentException () { @@ -1221,8 +1200,6 @@ public void RemoveAllReversePortsAsync_EmptySerial_ThrowsArgumentException () async () => await runner.RemoveAllReversePortsAsync ("")); } - // --- ListReversePortsAsync parameter validation tests --- - [Test] public void ListReversePortsAsync_EmptySerial_ThrowsArgumentException () { @@ -1231,8 +1208,6 @@ public void ListReversePortsAsync_EmptySerial_ThrowsArgumentException () async () => await runner.ListReversePortsAsync ("")); } - // --- ForwardPortAsync parameter validation tests --- - [Test] public void ForwardPortAsync_EmptySerial_ThrowsArgumentException () { @@ -1246,7 +1221,7 @@ public void ForwardPortAsync_NullLocal_ThrowsArgumentNull () { var runner = new AdbRunner ("/fake/sdk/platform-tools/adb"); Assert.ThrowsAsync ( - async () => await runner.ForwardPortAsync ("emulator-5554", (AdbPortSpec) null!, new AdbPortSpec (AdbProtocol.Tcp, 5000))); + async () => await runner.ForwardPortAsync ("emulator-5554", (AdbPortSpec) null, new AdbPortSpec (AdbProtocol.Tcp, 5000))); } [Test] @@ -1254,11 +1229,9 @@ public void ForwardPortAsync_NullRemote_ThrowsArgumentNull () { var runner = new AdbRunner ("/fake/sdk/platform-tools/adb"); Assert.ThrowsAsync ( - async () => await runner.ForwardPortAsync ("emulator-5554", new AdbPortSpec (AdbProtocol.Tcp, 5000), (AdbPortSpec) null!)); + async () => await runner.ForwardPortAsync ("emulator-5554", new AdbPortSpec (AdbProtocol.Tcp, 5000), (AdbPortSpec) null)); } - // --- RemoveForwardPortAsync parameter validation tests --- - [Test] public void RemoveForwardPortAsync_EmptySerial_ThrowsArgumentException () { @@ -1272,11 +1245,9 @@ public void RemoveForwardPortAsync_NullLocal_ThrowsArgumentNull () { var runner = new AdbRunner ("/fake/sdk/platform-tools/adb"); Assert.ThrowsAsync ( - async () => await runner.RemoveForwardPortAsync ("emulator-5554", (AdbPortSpec) null!)); + async () => await runner.RemoveForwardPortAsync ("emulator-5554", (AdbPortSpec) null)); } - // --- RemoveAllForwardPortsAsync parameter validation tests --- - [Test] public void RemoveAllForwardPortsAsync_EmptySerial_ThrowsArgumentException () { @@ -1285,8 +1256,6 @@ public void RemoveAllForwardPortsAsync_EmptySerial_ThrowsArgumentException () async () => await runner.RemoveAllForwardPortsAsync ("")); } - // --- ListForwardPortsAsync parameter validation tests --- - [Test] public void ListForwardPortsAsync_EmptySerial_ThrowsArgumentException () { @@ -1295,7 +1264,6 @@ public void ListForwardPortsAsync_EmptySerial_ThrowsArgumentException () async () => await runner.ListForwardPortsAsync ("")); } - // --- GetEmulatorAvdNameAsync + ListDevicesAsync tests --- // These tests use a fake 'adb' script to control process output, // verifying AVD detection order and offline emulator handling. From 9bbbf8821118097475d6535932c0bd40d10d6cad Mon Sep 17 00:00:00 2001 From: Rui Marinho Date: Mon, 1 Jun 2026 15:50:27 +0100 Subject: [PATCH 3/3] Fix RemoveAllForwardPortsAsync to honour per-serial scope The underlying 'adb forward --remove-all' (and the wire-protocol equivalent 'host-serial::killforward-all') is daemon-global -- the '-s ' flag does not scope it. The previous implementation would silently remove forwards for every connected device despite the method's per-device API contract. Reimplement by listing forwards for the given serial via ListForwardPortsAsync and removing them individually via RemoveForwardPortAsync. Update the XML docs to describe the actual behaviour. Add two new tests using a recording subclass of AdbRunner that overrides ListForwardPortsAsync and RemoveForwardPortAsync to verify (1) only ports for the requested serial are removed, and (2) an empty listing is a no-op. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../Runners/AdbRunner.cs | 23 ++++--- .../AdbRunnerTests.cs | 64 +++++++++++++++++++ 2 files changed, 79 insertions(+), 8 deletions(-) diff --git a/src/Xamarin.Android.Tools.AndroidSdk/Runners/AdbRunner.cs b/src/Xamarin.Android.Tools.AndroidSdk/Runners/AdbRunner.cs index af296197..924f4674 100644 --- a/src/Xamarin.Android.Tools.AndroidSdk/Runners/AdbRunner.cs +++ b/src/Xamarin.Android.Tools.AndroidSdk/Runners/AdbRunner.cs @@ -408,20 +408,27 @@ public virtual async Task RemoveForwardPortAsync (string serial, AdbPortSpec loc } /// - /// Removes all forward port forwarding rules for the device via - /// 'adb -s <serial> forward --remove-all'. - /// Note that the underlying adb command operates globally, but we scope it via -s. + /// Removes all forward port forwarding rules for the specified device. /// + /// + /// The underlying adb forward --remove-all command (and its wire-protocol + /// equivalent host-serial:<serial>:killforward-all) operates globally on the + /// adb daemon — the -s <serial> flag does not scope it, and calling it + /// would remove forwards for every connected device. To honour the per-device + /// contract of this method, we list the forwards for + /// via and remove them individually via + /// . + /// public virtual async Task RemoveAllForwardPortsAsync (string serial, CancellationToken cancellationToken = default) { if (string.IsNullOrWhiteSpace (serial)) throw new ArgumentException ("Serial must not be empty.", nameof (serial)); - var psi = ProcessUtils.CreateProcessStartInfo (adbPath, "-s", serial, "forward", "--remove-all"); - using var stdout = new StringWriter (); - using var stderr = new StringWriter (); - var exitCode = await ProcessUtils.StartProcess (psi, stdout, stderr, cancellationToken, environmentVariables).ConfigureAwait (false); - ProcessUtils.ThrowIfFailed (exitCode, $"adb -s {serial} forward --remove-all", stderr, stdout); + var rules = await ListForwardPortsAsync (serial, cancellationToken).ConfigureAwait (false); + foreach (var rule in rules) { + cancellationToken.ThrowIfCancellationRequested (); + await RemoveForwardPortAsync (serial, rule.Local, cancellationToken).ConfigureAwait (false); + } } /// diff --git a/tests/Xamarin.Android.Tools.AndroidSdk-Tests/AdbRunnerTests.cs b/tests/Xamarin.Android.Tools.AndroidSdk-Tests/AdbRunnerTests.cs index 8919ed53..acd29f1d 100644 --- a/tests/Xamarin.Android.Tools.AndroidSdk-Tests/AdbRunnerTests.cs +++ b/tests/Xamarin.Android.Tools.AndroidSdk-Tests/AdbRunnerTests.cs @@ -1256,6 +1256,70 @@ public void RemoveAllForwardPortsAsync_EmptySerial_ThrowsArgumentException () async () => await runner.RemoveAllForwardPortsAsync ("")); } + [Test] + public async Task RemoveAllForwardPortsAsync_RemovesOnlyPortsForGivenSerial () + { + var listedSerials = new List (); + var removed = new List<(string Serial, AdbPortSpec Local)> (); + var runner = new RecordingAdbRunner ( + listForwards: (serial, _) => { + listedSerials.Add (serial); + return Task.FromResult> (new [] { + new AdbPortRule (new AdbPortSpec (AdbProtocol.Tcp, 6000), new AdbPortSpec (AdbProtocol.Tcp, 5000)), + new AdbPortRule (new AdbPortSpec (AdbProtocol.Tcp, 6001), new AdbPortSpec (AdbProtocol.Tcp, 5001)), + }); + }, + removeForward: (serial, local, _) => { + removed.Add ((serial, local)); + return Task.CompletedTask; + }); + + await runner.RemoveAllForwardPortsAsync ("emulator-5554"); + + Assert.AreEqual (new [] { "emulator-5554" }, listedSerials, "ListForwardPortsAsync should be called exactly once with the target serial."); + Assert.AreEqual (2, removed.Count, "Both listed rules should be removed."); + Assert.That (removed.Select (r => r.Serial), Is.All.EqualTo ("emulator-5554"), "Removes must target only the requested serial."); + Assert.AreEqual (5000, removed [0].Local.Port); + Assert.AreEqual (5001, removed [1].Local.Port); + } + + [Test] + public async Task RemoveAllForwardPortsAsync_EmptyList_IsNoOp () + { + var removed = new List<(string Serial, AdbPortSpec Local)> (); + var runner = new RecordingAdbRunner ( + listForwards: (_, __) => Task.FromResult> (Array.Empty ()), + removeForward: (serial, local, _) => { + removed.Add ((serial, local)); + return Task.CompletedTask; + }); + + await runner.RemoveAllForwardPortsAsync ("emulator-5554"); + + Assert.IsEmpty (removed, "No removes should be issued when the listing is empty."); + } + + sealed class RecordingAdbRunner : AdbRunner + { + readonly Func>> listForwards; + readonly Func removeForward; + + public RecordingAdbRunner ( + Func>> listForwards, + Func removeForward) + : base ("/fake/sdk/platform-tools/adb") + { + this.listForwards = listForwards; + this.removeForward = removeForward; + } + + public override Task> ListForwardPortsAsync (string serial, System.Threading.CancellationToken cancellationToken = default) + => listForwards (serial, cancellationToken); + + public override Task RemoveForwardPortAsync (string serial, AdbPortSpec local, System.Threading.CancellationToken cancellationToken = default) + => removeForward (serial, local, cancellationToken); + } + [Test] public void ListForwardPortsAsync_EmptySerial_ThrowsArgumentException () {