Honour ANDROID_ADB_SERVER_PORT and test the daemon failure paths against a fake adb

README.md

@@ -165,7 +165,10 @@ adb server
 
 Each package is tested on its own. None of the tests need a phone, adb, or
 scrcpy: they use an in-process fake adb server, fake repositories, and
-OS-assigned loopback ports.
+OS-assigned loopback ports. The daemon's process-level tests start the real
+`bin/daemon.dart` against the fake adb server by setting
+`ANDROID_ADB_SERVER_PORT` (the variable adb itself honours) and an isolated
+`APPDATA`, so they never touch a real adb server or your Voyager config.
 
 ```powershell
 cd packages\core

packages/core/lib/src/adb/adb_client.dart

@@ -20,10 +20,27 @@ import 'byte_reader.dart';
 class AdbClient {
   AdbClient({
     this.host = '127.0.0.1',
-    this.port = 5037,
+    int? port,
     this.adbPath = 'adb',
     this.connectTimeout = const Duration(seconds: 3),
-  });
+  }) : port = port ?? defaultPort;
+
+  /// The port adb servers listen on unless told otherwise.
+  static const int standardPort = 5037;
+
+  /// The port used when none is given: `ANDROID_ADB_SERVER_PORT` if it is set
+  /// to a valid port (the variable adb itself honours), otherwise
+  /// [standardPort]. This is what lets tests point a client or the daemon at a
+  /// fake adb server without touching the real one on 5037.
+  static int get defaultPort =>
+      parsePort(Platform.environment['ANDROID_ADB_SERVER_PORT']);
+
+  /// [value] as a port number, or [standardPort] if it is missing or not a
+  /// valid port (1 to 65535).
+  static int parsePort(String? value) {
+    final port = int.tryParse(value ?? '');
+    return port != null && port > 0 && port <= 65535 ? port : standardPort;
+  }
 
   final String host;
   final int port;
@@ -45,7 +62,10 @@ class AdbClient {
       await socket.close();
       socket.destroy();
     } on SocketException {
-      final result = await Process.run(adbPath, ['start-server']);
+      // -P: start the server on the port this client uses, so a client
+      // pointed at a non-standard port never starts one on 5037.
+      final result =
+          await Process.run(adbPath, ['-P', '$port', 'start-server']);
       if (result.exitCode != 0) {
         throw AdbServerException(
           'could not start adb server: ${result.stderr}',

packages/core/test/adb_port_test.dart

@@ -0,0 +1,89 @@
+import 'dart:io';
+
+import 'package:scrcpy_core/scrcpy_core.dart';
+import 'package:test/test.dart';
+
+void main() {
+  group('AdbClient.parsePort', () {
+    test('uses a valid port', () {
+      expect(AdbClient.parsePort('5038'), 5038);
+      expect(AdbClient.parsePort('1'), 1);
+      // Stray whitespace around the number is tolerated.
+      expect(AdbClient.parsePort(' 5038 '), 5038);
+      expect(AdbClient.parsePort('65535'), 65535);
+    });
+
+    test('falls back to 5037 when missing or not a valid port', () {
+      expect(AdbClient.parsePort(null), 5037);
+      expect(AdbClient.parsePort(''), 5037);
+      expect(AdbClient.parsePort('abc'), 5037);
+      expect(AdbClient.parsePort('0'), 5037);
+      expect(AdbClient.parsePort('-5'), 5037);
+      expect(AdbClient.parsePort('65536'), 5037);
+      expect(AdbClient.parsePort('50.38'), 5037);
+    });
+  });
+
+  group('AdbClient port', () {
+    test('an explicit port wins over everything else', () {
+      expect(AdbClient(port: 12345).port, 12345);
+    });
+
+    test('defaults to the environment-derived port', () {
+      // Whatever ANDROID_ADB_SERVER_PORT is on this machine, the default must
+      // be what defaultPort says, and a valid port.
+      expect(AdbClient().port, AdbClient.defaultPort);
+      expect(AdbClient.defaultPort, inInclusiveRange(1, 65535));
+    });
+  });
+
+  group('AdbClient.ensureServerRunning', () {
+    late Directory dir;
+
+    setUp(() => dir = Directory.systemTemp.createTempSync('adb_port_test'));
+    tearDown(() => dir.deleteSync(recursive: true));
+
+    /// A stand-in `adb` that records its arguments and exits 0.
+    Future<File> recordingAdb(File argsFile) async {
+      if (Platform.isWindows) {
+        final script = File('${dir.path}\\fakeadb.bat');
+        script.writeAsStringSync('@echo %* > "${argsFile.path}"\r\n');
+        return script;
+      }
+      final script = File('${dir.path}/fakeadb.sh');
+      script.writeAsStringSync('#!/bin/sh\necho "\$@" > "${argsFile.path}"\n');
+      await Process.run('chmod', ['+x', script.path]);
+      return script;
+    }
+
+    Future<int> unusedPort() async {
+      final socket = await ServerSocket.bind(InternetAddress.loopbackIPv4, 0);
+      final port = socket.port;
+      await socket.close();
+      return port;
+    }
+
+    test('starts the server on the port the client uses, not on 5037',
+        () async {
+      final argsFile = File('${dir.path}/args.txt');
+      final adb = await recordingAdb(argsFile);
+      final port = await unusedPort();
+
+      await AdbClient(adbPath: adb.path, port: port).ensureServerRunning();
+
+      expect(argsFile.readAsStringSync().trim(), '-P $port start-server');
+    });
+
+    test('does not start anything when a server already answers', () async {
+      final server = await FakeAdbServer.start();
+      addTearDown(server.close);
+      final argsFile = File('${dir.path}/args.txt');
+      final adb = await recordingAdb(argsFile);
+
+      await AdbClient(adbPath: adb.path, port: server.port)
+          .ensureServerRunning();
+
+      expect(argsFile.existsSync(), isFalse);
+    });
+  });
+}

packages/daemon/test/daemon_process_test.dart

@@ -0,0 +1,120 @@
+@Timeout(Duration(seconds: 120))
+library;
+
+import 'dart:io';
+
+import 'package:scrcpy_core/scrcpy_core.dart';
+import 'package:test/test.dart';
+
+import 'support/daemon_harness.dart';
+
+/// Runs the real daemon as a child process against the repository's fake adb
+/// server, to check the failure paths from #14 end to end: whatever goes wrong
+/// in a fire-and-forget path, the daemon must stay up and keep answering IPC.
+///
+/// Nothing here touches a real adb server, scrcpy or a phone (see
+/// DaemonHarness).
+void main() {
+  late FakeAdbServer adb;
+  DaemonHarness? daemon;
+
+  setUp(() async {
+    adb = await FakeAdbServer.start();
+  });
+
+  tearDown(() async {
+    await daemon?.stop();
+    daemon = null;
+    await adb.close();
+  });
+
+  Future<DaemonHarness> startDaemon({
+    bool autoLaunch = false,
+    String? adbPath,
+    int? adbPort,
+  }) async {
+    final d = await DaemonHarness.start(
+      adbPort: adbPort ?? adb.port,
+      autoLaunch: autoLaunch,
+      adbPath: adbPath,
+    );
+    daemon = d;
+    return d;
+  }
+
+  Future<void> expectStillServing(DaemonHarness d,
+      {int devices = 1}) async {
+    expect(d.isAlive, isTrue, reason: 'daemon exited:\n${d.log.join('\n')}');
+    expect(await d.listDevices(), hasLength(devices));
+    expect(d.isAlive, isTrue);
+  }
+
+  test('an invalid scrcpyPath during auto-launch is logged and the daemon '
+      'keeps serving', () async {
+    final d = await startDaemon(autoLaunch: true);
+
+    // The fake adb reports one ready phone, so auto-launch fires and fails.
+    await d.waitForLog('auto-launch for Pixel 7 failed');
+
+    await expectStillServing(d);
+  });
+
+  test('the p key with an invalid scrcpyPath is logged and the daemon keeps '
+      'serving', () async {
+    final d = await startDaemon();
+    await d.waitForLog('[+] Pixel 7');
+
+    d.type('p');
+    await d.waitForLog('command "p" failed');
+
+    await expectStillServing(d);
+  });
+
+  test('the u key failing is logged and the daemon keeps serving', () async {
+    // The fake adb answers FAIL to the usb: request, like a phone that
+    // dropped between the check and the call.
+    final d = await startDaemon();
+    await d.waitForLog('[+] Pixel 7');
+
+    d.type('u');
+    await d.waitForLog('command "u" failed');
+
+    await expectStillServing(d);
+  });
+
+  test('adb that cannot be restarted is logged and the daemon keeps serving',
+      () async {
+    final missingAdb =
+        '${Directory.systemTemp.path}${Platform.pathSeparator}no-such-adb-binary';
+    final d = await startDaemon(adbPath: missingAdb);
+    await d.waitForLog('[+] Pixel 7');
+
+    // The adb server goes away. The daemon's retry loop tries to start it
+    // again, which fails because the adb binary does not exist.
+    await adb.close();
+    // Twice: the retry loop is still running after the first failure, not
+    // just surviving it (an error that escaped the loop would log once at
+    // most and end device tracking).
+    await d.waitForLog('could not start the adb server', count: 2);
+
+    expect(d.isAlive, isTrue);
+    // The device list is the last known one; the point is that IPC answers.
+    expect(await d.listDevices(), isA<List<Map<String, dynamic>>>());
+    expect(d.isAlive, isTrue);
+  });
+
+  test('no adb server at startup is logged and the daemon stays up',
+      () async {
+    final missingAdb =
+        '${Directory.systemTemp.path}${Platform.pathSeparator}no-such-adb-binary';
+    final unusedPort = await DaemonHarness.freePort();
+    final d = await startDaemon(adbPath: missingAdb, adbPort: unusedPort);
+
+    await d.waitForLog('adb server not available yet');
+    // The tracking loop is retrying, not stopped.
+    await d.waitForLog('could not start the adb server', count: 2);
+
+    expect(d.isAlive, isTrue);
+    expect(await d.listDevices(), isEmpty);
+  });
+}

packages/daemon/test/support/daemon_harness.dart

@@ -0,0 +1,132 @@
+import 'dart:async';
+import 'dart:convert';
+import 'dart:io';
+
+import 'package:scrcpy_ipc/scrcpy_ipc.dart';
+import 'package:test/test.dart';
+
+/// Runs the real daemon (`bin/daemon.dart`) as a child process, isolated from
+/// the machine it runs on:
+/// - `ANDROID_ADB_SERVER_PORT` points it at a fake adb server, so the real adb
+///   server on 5037 is never contacted, stopped or restarted;
+/// - `APPDATA` and `HOME` point at a temporary folder, so the developer's
+///   real `config.json` and known-devices file are never read or written;
+/// - the IPC port is a free port chosen for this run.
+class DaemonHarness {
+  DaemonHarness._(this._process, this._dir, this.ipcPort);
+
+  final Process _process;
+  final Directory _dir;
+  final int ipcPort;
+
+  final List<String> log = [];
+  bool _exited = false;
+  int? exitCode;
+
+  /// True while the daemon process is still alive.
+  bool get isAlive => !_exited;
+
+  static Future<int> freePort() async {
+    final socket = await ServerSocket.bind(InternetAddress.loopbackIPv4, 0);
+    final port = socket.port;
+    await socket.close();
+    return port;
+  }
+
+  /// Starts the daemon against the adb server on [adbPort].
+  ///
+  /// [scrcpyPath] defaults to a path that does not exist. [adbPath], if given,
+  /// is passed as the daemon's first argument (the adb binary it would run to
+  /// start a server).
+  static Future<DaemonHarness> start({
+    required int adbPort,
+    bool autoLaunch = false,
+    String? scrcpyPath,
+    String? adbPath,
+  }) async {
+    final dir = Directory.systemTemp.createTempSync('daemon_harness');
+    final ipcPort = await freePort();
+    final configDir = Directory('${dir.path}${Platform.pathSeparator}scrcpy_gui')
+      ..createSync(recursive: true);
+    File('${configDir.path}${Platform.pathSeparator}config.json')
+        .writeAsStringSync(jsonEncode({
+      'adbPath': 'adb',
+      'scrcpyPath': scrcpyPath ??
+          '${dir.path}${Platform.pathSeparator}no-such-dir${Platform.pathSeparator}scrcpy',
+      'autoLaunch': autoLaunch,
+      'scrcpyArgs': <String>[],
+      'ipcPort': ipcPort,
+    }));
+
+    final process = await Process.start(
+      Platform.resolvedExecutable,
+      ['bin/daemon.dart', if (adbPath != null) adbPath],
+      workingDirectory: Directory.current.path,
+      environment: {
+        'ANDROID_ADB_SERVER_PORT': '$adbPort',
+        'APPDATA': dir.path,
+        'HOME': dir.path,
+      },
+    );
+    final harness = DaemonHarness._(process, dir, ipcPort);
+    for (final stream in [process.stdout, process.stderr]) {
+      stream
+          .transform(utf8.decoder)
+          .transform(const LineSplitter())
+          .listen(harness.log.add);
+    }
+    unawaited(process.exitCode.then((code) {
+      harness._exited = true;
+      harness.exitCode = code;
+    }));
+    return harness;
+  }
+
+  /// Waits until [count] log lines contain [text] (once by default); fails
+  /// the test after [timeout]. Waiting for 2 shows that something repeats.
+  Future<String> waitForLog(String text,
+      {int count = 1, Duration timeout = const Duration(seconds: 30)}) async {
+    final deadline = DateTime.now().add(timeout);
+    while (true) {
+      final matches = log.where((line) => line.contains(text)).toList();
+      if (matches.length >= count) return matches.last;
+      if (_exited) {
+        fail('daemon exited (code $exitCode) before logging "$text". '
+            'Log:\n${log.join('\n')}');
+      }
+      if (DateTime.now().isAfter(deadline)) {
+        fail('"$text" not logged $count time(s) within $timeout. Log:\n${log.join('\n')}');
+      }
+      await Future<void>.delayed(const Duration(milliseconds: 25));
+    }
+  }
+
+  /// Sends a line to the daemon's stdin, like typing a key in its terminal.
+  void type(String line) => _process.stdin.writeln(line);
+
+  /// Asks the daemon for its device list over IPC. Completing at all proves it
+  /// is alive and serving requests.
+  Future<List<Map<String, dynamic>>> listDevices() async {
+    final client = IpcClient();
+    await client.connect(port: ipcPort);
+    try {
+      final result = await client.send(IpcCommands.listDevices);
+      return (result.data['devices'] as List).cast<Map<String, dynamic>>();
+    } finally {
+      await client.close();
+    }
+  }
+
+  Future<void> stop() async {
+    _process.kill();
+    await _process.exitCode.timeout(const Duration(seconds: 10), onTimeout: () {
+      _process.kill(ProcessSignal.sigkill);
+      return -1;
+    });
+    try {
+      _dir.deleteSync(recursive: true);
+    } catch (_) {
+      // Windows may still hold the folder for a moment; it is a temp folder.
+    }
+  }
+}