Allow adb stdout to contain the port number without failing (#31491)

* Allow adb stdout to contain the port number without failing

* Add tests that port forwarder correctly responds to known ADB output

packages/flutter_tools/lib/src/android/android_device.dart

@@ -885,7 +885,20 @@ class _AndroidDevicePortForwarder extends DevicePortForwarder {
         process.throwException('adb did not report forwarded port');
       hostPort = int.tryParse(process.stdout) ?? (throw 'adb returned invalid port number:\n${process.stdout}');
     } else {
-      if (process.stdout.isNotEmpty)
+      // stdout may be empty or the port we asked it to forward, though it's
+      // not documented (or obvious) what triggers each case.
+      //
+      // Observations are:
+      //   - On MacOS it's always empty when Flutter spawns the process, but
+      //   - On MacOS it prints the port number when run from the terminal, unless
+      //     the port is already forwarded, when it also prints nothing.
+      //   - On ChromeOS, the port appears to be printed even when Flutter spawns
+      //     the process
+      //
+      // To cover all cases, we accept the output being either empty or exactly
+      // the port number, but treat any other output as probably being an error
+      // message.
+      if (process.stdout.isNotEmpty && process.stdout.trim() != '$hostPort')
         process.throwException('adb returned error:\n${process.stdout}');
     }
 

packages/flutter_tools/test/android/android_device_test.dart

@@ -10,6 +10,7 @@ import 'package:flutter_tools/src/android/android_sdk.dart';
 import 'package:flutter_tools/src/base/config.dart';
 import 'package:flutter_tools/src/base/file_system.dart';
 import 'package:flutter_tools/src/base/io.dart';
+import 'package:flutter_tools/src/device.dart';
 import 'package:flutter_tools/src/project.dart';
 import 'package:mockito/mockito.dart';
 import 'package:process/process.dart';
@@ -196,6 +197,48 @@ flutter:
   }, overrides: <Type, Generator>{
     FileSystem: () => MemoryFileSystem(),
   });
+
+  group('portForwarder', () {
+    final ProcessManager mockProcessManager = MockProcessManager();
+    final AndroidDevice device = AndroidDevice('1234');
+    final DevicePortForwarder forwarder = device.portForwarder;
+
+    testUsingContext('returns the generated host port from stdout', () async {
+      when(mockProcessManager.run(argThat(contains('forward'))))
+      .thenAnswer((_) async => ProcessResult(0, 0, '456', ''));
+
+      expect(await forwarder.forward(123), equals(456));
+    }, overrides: <Type, Generator>{
+      ProcessManager: () => mockProcessManager,
+    });
+
+    testUsingContext('returns the supplied host port when stdout is empty', () async {
+      when(mockProcessManager.run(argThat(contains('forward'))))
+      .thenAnswer((_) async => ProcessResult(0, 0, '', ''));
+
+      expect(await forwarder.forward(123, hostPort: 456), equals(456));
+    }, overrides: <Type, Generator>{
+      ProcessManager: () => mockProcessManager,
+    });
+
+    testUsingContext('returns the supplied host port when stdout is the host port', () async {
+      when(mockProcessManager.run(argThat(contains('forward'))))
+      .thenAnswer((_) async => ProcessResult(0, 0, '456', ''));
+
+      expect(await forwarder.forward(123, hostPort: 456), equals(456));
+    }, overrides: <Type, Generator>{
+      ProcessManager: () => mockProcessManager,
+    });
+
+    testUsingContext('throws an error when stdout is not blank nor the host port', () async {
+      when(mockProcessManager.run(argThat(contains('forward'))))
+      .thenAnswer((_) async => ProcessResult(0, 0, '123456', ''));
+
+      expect(forwarder.forward(123, hostPort: 456), throwsA(isInstanceOf<ProcessException>()));
+    }, overrides: <Type, Generator>{
+      ProcessManager: () => mockProcessManager,
+    });
+  });
 }
 
 class MockProcessManager extends Mock implements ProcessManager {}