From 60593d60372a37b6161779b052f32fa5675d8059 Mon Sep 17 00:00:00 2001 From: Babul Raj Singh <95072470+ba6ul@users.noreply.github.com> Date: Tue, 27 Oct 2026 02:48:34 +0530 Subject: [PATCH] Honour ANDROID_ADB_SERVER_PORT and test the daemon's failure paths against a fake adb AdbClient now defaults its port to ANDROID_ADB_SERVER_PORT when it is a valid port (adb's own variable), else 5037, and starts the server with -P on the port it uses. Process-level tests run the real daemon against the fake adb server with an isolated config and cover an invalid scrcpyPath during auto-launch, the p and u keys, and adb that cannot be restarted. --- README.md | 5 +- packages/core/lib/src/adb/adb_client.dart | 26 +++- packages/core/test/adb_port_test.dart | 89 ++++++++++++ packages/daemon/test/daemon_process_test.dart | 120 ++++++++++++++++ .../daemon/test/support/daemon_harness.dart | 132 ++++++++++++++++++ 5 files changed, 368 insertions(+), 4 deletions(-) create mode 100644 packages/core/test/adb_port_test.dart create mode 100644 packages/daemon/test/daemon_process_test.dart create mode 100644 packages/daemon/test/support/daemon_harness.dart diff --git a/README.md b/README.md index 6bfdb30..5b7d3e2 100644 --- a/README.md +++ b/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 diff --git a/packages/core/lib/src/adb/adb_client.dart b/packages/core/lib/src/adb/adb_client.dart index 3eb8592..1d46926 100644 --- a/packages/core/lib/src/adb/adb_client.dart +++ b/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}', diff --git a/packages/core/test/adb_port_test.dart b/packages/core/test/adb_port_test.dart new file mode 100644 index 0000000..1f2c79f --- /dev/null +++ b/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 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 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); + }); + }); +} diff --git a/packages/daemon/test/daemon_process_test.dart b/packages/daemon/test/daemon_process_test.dart new file mode 100644 index 0000000..e7a27e5 --- /dev/null +++ b/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 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 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>>()); + 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); + }); +} diff --git a/packages/daemon/test/support/daemon_harness.dart b/packages/daemon/test/support/daemon_harness.dart new file mode 100644 index 0000000..6461660 --- /dev/null +++ b/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 log = []; + bool _exited = false; + int? exitCode; + + /// True while the daemon process is still alive. + bool get isAlive => !_exited; + + static Future 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 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': [], + '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 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.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>> 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>(); + } finally { + await client.close(); + } + } + + Future 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. + } + } +}