diff --git a/python/packages/jumpstarter-driver-adb/README.md b/python/packages/jumpstarter-driver-adb/README.md index 3cb3c4c1d..d10da91ff 100644 --- a/python/packages/jumpstarter-driver-adb/README.md +++ b/python/packages/jumpstarter-driver-adb/README.md @@ -1,6 +1,34 @@ # ADB Driver -`jumpstarter-driver-adb` tunnels Android Debug Bridge (ADB) connections over Jumpstarter, enabling remote Android device access via standard ADB tools such as Android Studio. +`jumpstarter-driver-adb` carries the Android Debug Bridge protocol between a remote +Android device and your workstation, so your **own** `adb` — and Android Studio, +tradefed, gradle — can drive a device on someone else's bench. + +## How it works + +Devices are **declared** in the exporter config, one driver instance per device, and +plugged into the exporter over USB (or reachable at their own TCP address). Jumpstarter +moves the ADB protocol to your machine; ADB does everything else. + +```text +DUT ──USB──▶ EXPORTER ──Jumpstarter tunnel──▶ YOU + (owns the USB (your own adb, + connection) Studio, tradefed…) +``` + +Two things worth knowing up front, because they shape the whole design: + +**Jumpstarter does not wrap the `adb` CLI.** There is no `j adb shell`, no +`j adb install`. You already have `adb`; the driver's job is to hand you an address +and get out of the way. `attach` runs a single `adb connect` for convenience, and +`endpoint` just prints the address so you can drive adb yourself. + +**Devices are declared, not discovered.** Each device is named in the exporter config +by the **bench USB port** it is plugged into. That is what makes a DUT composable with +its power relay and its console, so leasing the DUT leases the right device — and it +survives the two things that break auto-discovery: a relay power-cycle that +re-enumerates USB and changes the device's serial, and swapping hardware between +benches. ## Installation @@ -16,279 +44,386 @@ pip3 install --extra-index-url https://pkg.jumpstarter.dev/simple/ "jumpstarter- ## Configuration -Example exporter configuration: +A two-DUT bench, each DUT with its own power relay: ```yaml export: - adb: - type: jumpstarter_driver_adb.driver.AdbServer + dut1: + type: jumpstarter_driver_composite.driver.Composite + children: + power: + type: jumpstarter_driver_yepkit.driver.Ykush + config: { serial: "YK112233", port: "1" } + adb: + type: jumpstarter_driver_adb.driver.AdbDevice + config: + usb_port: "1-4.2" # the bench port: swap the DUT, config unchanged + + dut2: + type: jumpstarter_driver_composite.driver.Composite + children: + power: + type: jumpstarter_driver_yepkit.driver.Ykush + config: { serial: "YK112233", port: "2" } + adb: + type: jumpstarter_driver_adb.driver.AdbDevice + config: + usb_port: "1-4.3" + + # A device whose adbd already listens on TCP: a networked or AAOS head unit, + # or a virtual device. + dut3: + type: jumpstarter_driver_adb.driver.AdbDevice config: - host: "127.0.0.1" - port: 15037 + transport: tcp + address: "10.0.0.5:5555" ``` -### Configuration Parameters +Note there is **no ADB server entry**. The server is implicit: the first device to +need one starts or adopts it, and every device on the same port shares it. Declare an +`AdbServer` only if you want the server-level CLI (`j adb devices`, `j adb tunnel`). -| Parameter | Description | Type | Required | Default | -| --------- | ---------------------------------------------- | ---- | -------- | -------------------------- | -| adb_path | Path to the ADB executable on the exporter | str | no | "adb" (resolved from PATH) | -| host | Host address of the ADB server on the exporter | str | no | "127.0.0.1" | -| port | Port of the ADB server on the exporter | int | no | 15037 | -| connect_timeout | Timeout (seconds) for `connect`/`disconnect` commands | float | no | 30.0 | +### Finding a device's `usb_port` -### Port Assignment +```console +$ adb devices -l +List of devices attached +HVA1234567 device usb:1-4.2 product:sdk model:Pixel device:generic +``` -The exporter runs its own ADB server on a non-standard port (default: 15037) -to avoid conflicting with the standard ADB server on port 5037 -(if Jumpstarter is running in local mode). This is important because tools like -Android Studio automatically start and maintain an ADB server on port 5037 and -will restart it if killed. +The `usb:` field is the value to put in `usb_port` — with or without the `usb:` +prefix, both are accepted. On Linux it is the kernel's bus-port path (`1-4.2`), which +is stable for a given physical port. On macOS it is an IOKit location ID in hex +instead, and its exact form depends on which USB backend adb uses (`ADB_LIBUSB`), so +exporters are expected to be Linux. + +### `AdbDevice` parameters + +| Parameter | Description | Type | Required | Default | +| --- | --- | --- | --- | --- | +| transport | `usb` for a USB-attached device, `tcp` for one whose adbd already listens on TCP | str | no | `usb` | +| usb_port | **usb:** the bench USB port, as reported by `adb devices -l` | str | one of usb_port/serial | — | +| serial | **usb:** an explicit ADB serial, for hardware with no usable USB devpath | str | one of usb_port/serial | — | +| address | **tcp:** the device's own adbd endpoint, `host` or `host:port` | str | yes for tcp | — | +| adbd_port | adbd's TCP port on the device (`persist.adb.tcp.port`); also the default port for `address` | int | no | 5555 | +| adb_path | Path to the ADB executable on the exporter | str | no | `adb` (resolved from PATH) | +| connect_timeout | Timeout (seconds) for adb commands | float | no | 30.0 | +| server_port | Which ADB server to use. Rarely set — the server is implicit and shared | int | no | 15037 | +| adopt_existing_server | Use an ADB server already listening on `server_port` rather than starting another | bool | no | true | + +Prefer `usb_port` over `serial`. A serial identifies *a device*; the bench port +identifies *a position*, which is what stays true when hardware is swapped or a power +cycle changes the serial. + +### `AdbServer` parameters + +Optional. Declare it to point your tooling at the exporter's ADB server, or to get the +server-level CLI. + +| Parameter | Description | Type | Required | Default | +| --- | --- | --- | --- | --- | +| adb_path | Path to the ADB executable on the exporter | str | no | `adb` (resolved from PATH) | +| host | Host address of the ADB server on the exporter | str | no | `127.0.0.1` | +| port | Port of the ADB server on the exporter | int | no | 15037 | +| connect_timeout | Timeout (seconds) for adb commands | float | no | 30.0 | +| adopt_existing_server | Use an ADB server already listening on `port` instead of starting another | bool | no | true | + +### Running the exporter in a container + +adb finds USB devices by walking `/dev/bus/usb` and **rejects any path component that +is not all digits**, so it only ever looks at real `/dev/bus/usb//` nodes. A +friendly `/dev` symlink is therefore useful for the `podman run` line — Podman +resolves a symlinked `--device` and stores only the major/minor — but adb itself will +never see that name. Pass the device through at its real path: -On the client side, the `tunnel` command binds to an auto-assigned port by -default. Use `-P` to specify a port (such as 5037) if needed. +```shell +podman run --device /dev/bus/usb/001/017 ... +``` -## Usage +Permissions matter, and udev is the right place for them: adb falls back to read-only +(and cannot talk to the device) if it cannot open the node `O_RDWR`. A rule granting +your exporter's user or group access to the DUT's vendor ID is the usual fix. -### Run ADB commands +Passing exactly one device into a container also isolates it: that container's ADB +server can only ever see the device you gave it. -All standard adb commands are passed through to the remote ADB server: +### An ADB server already running on the exporter -```bash -# List devices -j adb devices +An ADB server **claims** the USB devices it finds, and only one server can hold a +given device. So if a server is already listening on the driver's port — started by +hand, by udev, by a previous run, or by a developer working on the exporter directly — +a second one does not give a second view of those devices. It gives an *empty* one, +and `adb start-server` reports success either way, so the driver would come up seeing +no devices at all while looking healthy. -# Interactive shell -j adb shell +By default the driver therefore **adopts** a server already on its port, and leaves it +running at teardown rather than killing a server other processes are using. For the +same reason, every `AdbDevice` on a given port shares one server rather than each +starting its own. -# Run a command on the device -j adb shell getprop ro.product.model +If something that is *not* an ADB server holds the port, the driver declines to adopt +it and logs a warning. This matters because `adb start-server` and `adb devices` both +block forever against such a listener rather than failing, so all of the driver's adb +calls are bounded by `connect_timeout`. -# Install an app -j adb install app.apk +### Port assignment -# View device logs -j adb logcat +The exporter runs its ADB server on a non-standard port (default 15037) so it cannot +collide with the standard 5037 — which matters because Android Studio starts and +maintains a server there and will restart it if killed. -# Push/pull files -j adb push local_file.txt /sdcard/ -j adb pull /sdcard/remote_file.txt . -``` +Exporter-side forward ports are **not** configured: each forward is created as +`adb forward tcp:0`, so the ADB server picks a free port and the driver adopts +whatever it chose. Nothing on the exporter has to be kept clear of a guessed range. -### Persistent tunnel +## Usage -The `tunnel` command is the only Jumpstarter-specific command. All other -commands (including `start-server`, `kill-server`, `connect`, `disconnect`, -`reconnect`, `pair`) are passed through to the remote ADB server. +### Attach a device to your own ADB server -```bash -# Create a persistent ADB tunnel (auto-assigned port) -j adb tunnel +```console +$ j dut1.adb attach +attached as 127.0.0.1:41000 -# Create a tunnel on a specific port -j adb tunnel -P 5038 +Your ADB server now lists it; use it with: adb -s 127.0.0.1:41000 shell +Android Studio will list it too. -# Background the tunnel for continued shell use -j adb tunnel & +Press Ctrl+C to detach ``` -When a persistent tunnel is running, subsequent `j adb` commands will -automatically reuse it instead of creating a new ephemeral tunnel. This -makes commands faster and ensures a consistent connection. - -For native `adb` or external tools, export the env vars printed by the -`tunnel` command in another terminal. - -### Unsupported commands +Leave it running for as long as you want the device available. Then, in another +terminal, it is just adb: -The `nodaemon` command is not supported as it would start a local ADB server -process, ignoring the tunnel entirely. - -### Connecting to a remote device +```shell +adb -s 127.0.0.1:41000 shell +adb -s 127.0.0.1:41000 install app.apk +adb -s 127.0.0.1:41000 logcat +adb -s 127.0.0.1:41000 push local_file.txt /sdcard/ +``` -When the Android device is **not** attached to the exporter over USB but is -reachable over the network (for example a virtual device such as -[Cuttlefish](https://source.android.com/docs/devices/cuttlefish), or a device -exposing `adb` over TCP/IP), the exporter's ADB server must `connect` to it -before any `adb` command will see it. +Because `adb connect` is **additive**, the device joins whatever your ADB server +already holds — your own emulator, another bench, a phone — and every Android tool +sees it with no configuration. You do not need to own your ADB server, and you do not +need one at all: `adb connect` starts one if none is running. -The `connect_device` / `disconnect_device` driver methods run -`adb connect ` / `adb disconnect ` on the exporter. The -address is supplied by the caller — this driver does **not** discover or scan -for devices. Timeouts and command failures raise, so callers can react instead -of receiving a silent error string. +### Just give me the address -#### From the CLI +`attach` is a convenience. If you would rather drive adb yourself, or point a tool +that takes a `host:port` at the device: -`connect` and `disconnect` are also plain adb commands, so they pass through the -tunnel like any other: +```console +$ j dut1.adb endpoint +127.0.0.1:41000 -```bash -# Connect the exporter's ADB server to a networked device, then use it -j adb connect 10.0.0.5:6520 -j adb devices -j adb shell getprop ro.product.model -j adb disconnect 10.0.0.5:6520 +Add it to your ADB server with: adb connect 127.0.0.1:41000 +Press Ctrl+C to stop ``` -#### From a parent (composite) driver +No adb runs on your machine at all. This is the primitive the rest is built on. -The intended use case is a higher-level driver that owns the device lifecycle -and knows the address deterministically — no IP discovery needed. For example, -the Cuttlefish driver embeds an `AdbServer` child and connects to a pinned -address derived from its own config (`host` + an ADB port computed from the -instance number) after the virtual device is created: +### Is my device there? -```python -class CuttlefishServer(CompositeInterface, Driver): - def __post_init__(self): - super().__post_init__() - # AdbServer runs on the exporter; the parent drives connect/disconnect - self.children["adb"] = AdbServer(host="127.0.0.1", port=self.adb_server_port) - - def _adb_device(self) -> str: - # Address is known from config, never scanned - return f"{self.host}:{6520 + self.instance_num - 1}" - - def _connect(self): - adb = self.children["adb"] - device = self._adb_device() - try: - adb.connect_device(device) - except (subprocess.CalledProcessError, TimeoutError) as e: - # Device may not be up yet; the boot-wait loop below reconnects. - self.logger.warning("ADB connect to %s failed (%s); retrying while waiting for boot", device, e) - # unexpected exceptions (config/programming errors) propagate - - def _wait_for_boot(self): - adb = self.children["adb"] - device = self._adb_device() - deadline = time.monotonic() + self.boot_timeout - while time.monotonic() < deadline: - try: - adb.connect_device(device) - if self._is_booted(device): - return - except (subprocess.CalledProcessError, TimeoutError): - pass - time.sleep(3) - raise TimeoutError(f"{device} did not come online within {self.boot_timeout}s") +```console +$ j dut1.adb info +transport: usb +adbd_port: 5555 +selector: usb:1-4.2 +serial: HVA1234567 +present: yes ``` -Because `connect_device` raises on failure or timeout, the parent catches only -the *expected* connection failures (letting configuration or programming errors -propagate) and drives its own retry loop rather than parsing return strings. - -### Integration with Android Ecosystem Tools - -#### Forward ADB for external tools - -The `tunnel` command creates a persistent tunnel that other `j adb` commands -reuse automatically. For external tools, export the env vars printed by the -command: - -```bash -# In the jmp shell: -j adb tunnel +A declared device that is powered off reports `present: no` with the reason. That is +normal, not an error — the exporter starts fine with every DUT powered down, and the +device is picked up the moment its relay turns on. + +### Requirements and limits + +- The device's `adbd` must listen on TCP (`persist.adb.tcp.port`, commonly 5555). A + stock phone needs `adb tcpip 5555` first — note this restarts `adbd` and may drop + the USB connection. +- The local address (`127.0.0.1:`) is assigned per session and is not stable + across sessions. Anything that remembers a device by address (a saved Android Studio + run target) needs re-selecting after re-attaching. Use `-P` to pin the port if you + need one address to stay put. +- Direct mode has no lease arbitration, so two clients attaching the same device will + interfere. Use distributed mode for a shared fleet. + +### Power cycles and re-enumeration + +Nothing to configure: the device's serial and its forward are resolved fresh on every +connection. A relay power-cycle re-enumerates USB and can hand the device a different +ADB serial, and the old forward disappears with it — the driver notices, re-resolves +the declared `usb_port` to the new serial, and forwards again. No config edit, no +exporter restart. + +Re-attach after the DUT is back up; the local address will generally be a new port. + +## Transports + +| | `usb` | `tcp` | +| --- | --- | --- | +| The device is | plugged into the exporter over USB | listening on its own TCP address | +| Identified by | `usb_port` (or `serial`) | `address` | +| On the exporter | `adb forward tcp:0 tcp:5555` | `adb connect
` | +| Typical case | a bench DUT on a relay | AAOS head unit, networked or virtual device | + +### There is no serial/UART transport + +adb has none, so neither does this driver. Confirmed in AOSP: `adb.h` defines only +`kTransportUsb` and `kTransportLocal` (where "local" means TCP), and `connect_device()` +coerces every address to `tcp:` — `adb connect serial:/dev/ttyUSB0` fails with +`bad port number '/dev/ttyUSB0'`. The `dev:` and `dev-raw:` specs that appear in +`adb help` are **forward targets executed inside adbd on the device**, not host +transports. Device-side there is no adbd-over-UART property either; +`ttyGS0`/gadget-serial gives a serial console, not an adb transport. + +To reach a serial-only DUT, get it onto TCP and use `transport: tcp`: either use its +console to enable adbd over TCP (`setprop service.adb.tcp.port 5555; stop adbd; start +adbd`), or bridge the UART to a TCP port outside Jumpstarter. Be aware that a raw UART +gives adb no retransmission and no checksum, so hardware flow control is mandatory, +the line must not be shared with a kernel console or getty, and at 115200 baud you get +~11.5 KB/s — enough for a shell, not for `push` or `bugreport`. + +## Pointing your tools at the exporter's ADB server + +The opposite model to `attach`: instead of adding one device to *your* server, aim +your tools at the exporter's server and see its devices instead of your own. Right for +CI, a headless runner, or a container. Requires a declared `AdbServer`. + +```console +$ j adb tunnel +ADB server tunneled to 127.0.0.1:54321 + +To use your own adb or other tools, run: + export ANDROID_ADB_SERVER_ADDRESS=127.0.0.1 + export ANDROID_ADB_SERVER_PORT=54321 + +Press Ctrl+C to stop ``` -```bash -# In another terminal, using the port printed by the tunnel command: -export ANDROID_ADB_SERVER_ADDRESS=127.0.0.1 -export ANDROID_ADB_SERVER_PORT= -adb devices -``` +This replaces your server rather than adding to it, which is exactly wrong when an IDE +is running — Android Studio owns 5037 and respawns its server there within ~3s of +being killed, so the port cannot reliably be taken over. Use `attach` in that case. -#### Android Studio +## Integration with Android ecosystem tools -Android Studio automatically starts and maintains its own ADB server on -port 5037. Because of this, the `tunnel` command uses an auto-assigned port -by default to avoid conflicts. +### How this relates to Android's own remote-device support -To use the tunnel with Android Studio: +`attach` is deliberately the same shape as the remote-device flow Google documents, so +Android Studio needs no Jumpstarter-specific support: -1. Note the port printed by `j adb tunnel` -2. Configure Android Studio to use a custom ADB server port, or: -3. Kill Android Studio's ADB server, bind the tunnel to port 5037, and - restart Android Studio: +- Android's [wireless debugging](https://developer.android.com/tools/adb) has you run + `adb tcpip 5555` then `adb connect :5555`, and the device then appears as a + `host:port` serial alongside your emulators. `attach` does exactly that, except the + `host:port` is a local tunnel endpoint rather than the device's own IP — which is + what makes it work when the device is on a bench network you cannot route to. +- Because the ADB server "manages connections to devices and handles commands from + multiple `adb` clients", remote and local devices coexist and are addressed with + `-s ` (or `$ANDROID_SERIAL`) in the ordinary way. Nothing about a + Jumpstarter-attached device is special to a client. -```bash -adb kill-server -j adb tunnel -P 5037 -# Note: Android Studio may restart the ADB server on 5037 when opened, -# causing a conflict. If this happens, use the auto-assigned port instead. -``` +Two deliberate differences: there is **no pairing** (the tunnel exists only for the +lease, so there is nothing to remember or revoke — lease lifetime is the security +boundary), and **the address is not stable** across sessions. -#### Trade Federation (tradefed) +### Android Studio -tradefed discovers devices through the ADB server via the -`ANDROID_ADB_SERVER_PORT` environment variable: +Run `j dut1.adb attach`. The device appears in Studio's device chooser with **no +configuration**: no `adb.server.port`, no environment variables, no restart. Leave the +command running for as long as you want the device available. -```bash -# Terminal 1: Start the tunnel -j adb tunnel -# Note the port, e.g. 54321 +### Trade Federation (tradefed) -# Terminal 2: Run tradefed with the tunnel port -export ANDROID_ADB_SERVER_PORT=54321 +tradefed discovers devices through the ADB server, so an attached device is visible to +it with no extra setup: + +```shell +j dut1.adb attach # leave running tradefed.sh -# > list devices <-- shows remote devices +# > list devices <-- shows the attached device ``` -#### Python API +To give tradefed the exporter's whole device list instead, use `j adb tunnel` and +export `ANDROID_ADB_SERVER_PORT`. + +### Python API -You can also perform interactions via ADB using the -[`adbutils`](https://github.com/openatx/adbutils) Python package. +Drive a device programmatically with [`adbutils`](https://github.com/openatx/adbutils) +against the endpoint, no CLI involved: ```python # Requires: pip install jumpstarter-driver-adb[python-api] import adbutils -with client.adb.forward_adb(port=0) as (host, port): - adb = adbutils.AdbClient(host=host, port=port) - for device in adb.device_list(): - print(device.serial, device.prop.model) +with client.dut1.adb.endpoint() as target: + host, port = target.rsplit(":", 1) + adb = adbutils.AdbClient(host=host, port=int(port)) + print(adb.device().prop.model) ``` -### CLI +### Connecting the exporter's server to a networked device + +For a device the *exporter* should `adb connect` to — a Cuttlefish instance, say — +`AdbServer` exposes `connect_device` / `disconnect_device`. The address is supplied by +the caller; this driver does not discover or scan for devices. A parent composite +driver that owns the device lifecycle is the intended user: the Cuttlefish driver +embeds an `AdbServer` child and connects to an address derived from its own config. + +For a networked device you want in *your* ADB server, prefer an `AdbDevice` with +`transport: tcp` — it needs no parent driver. -#### Standard ADB commands (passed through) +## CLI -| Usage | Description | -| ----------------------------- | ------------------------------------------------- | -| `j adb [args...]` | Run any adb command against the remote ADB server | -| `j adb devices` | List connected devices | -| `j adb shell [command]` | Open a shell or run a command on the device | -| `j adb install ` | Install an APK | -| `j adb push ` | Push a file to the device | -| `j adb pull ` | Pull a file from the device | -| `j adb logcat` | View device logs | +### Per-device (`j .adb ...`) -#### Jumpstarter-specific commands +| Usage | Description | +| --- | --- | +| `j .adb attach` | Add this device to your own ADB server; holds until Ctrl+C | +| `j .adb endpoint` | Print the device's local adbd address; holds until Ctrl+C | +| `j .adb info` | Show the device's transport, selector and whether it is present | -| Usage | Description | -| ------------------------ | ----------------------------------------------------------------------- | -| `j adb tunnel [-P PORT]` | Create a persistent ADB tunnel (auto-assigned port, or specify with -P) | +Options for `attach` and `endpoint`: -#### Options +| Option | Description | Default | +| --- | --- | --- | +| `-H HOST` | Local address to bind | 127.0.0.1 | +| `-P PORT` | Local port to bind (0=auto) | 0 | +| `--adb PATH` | Path to your local adb (`attach` only) | adb | -| Option | Description | Default | -| ------------ | ------------------------------------ | --------- | -| `-H HOST` | Local address to tunnel ADB to | 127.0.0.1 | -| `-P PORT` | Local port to tunnel ADB to (0=auto) | 0 | -| `--adb PATH` | Path to local adb executable | adb | +### Server-level (`j adb ...`, needs a declared `AdbServer`) + +| Usage | Description | +| --- | --- | +| `j adb devices` | List devices visible to the exporter's ADB server | +| `j adb tunnel [-H HOST] [-P PORT]` | Forward the exporter's ADB server to a local port; holds until Ctrl+C | + +Everything else is your own `adb`, run directly. ## API Reference -### Driver +### Device driver + +```{eval-rst} +.. autoclass:: jumpstarter_driver_adb.driver.AdbDevice() + :members: connect, info +``` + +### Server driver ```{eval-rst} .. autoclass:: jumpstarter_driver_adb.driver.AdbServer() - :members: start_server, kill_server, connect_device, disconnect_device, list_devices + :members: list_devices, start_server, kill_server, connect_device, disconnect_device +``` + +### Device client + +```{eval-rst} +.. autoclass:: jumpstarter_driver_adb.client.AdbDeviceClient() + :members: attach, endpoint, info ``` -### Client +### Server client ```{eval-rst} .. autoclass:: jumpstarter_driver_adb.client.AdbClient() - :members: forward_adb, start_server, kill_server, connect_device, disconnect_device, list_devices + :members: forward_adb, devices, list_devices, connect_device, disconnect_device ``` diff --git a/python/packages/jumpstarter-driver-adb/jumpstarter_driver_adb/client.py b/python/packages/jumpstarter-driver-adb/jumpstarter_driver_adb/client.py index 19d5a917c..6728b2734 100644 --- a/python/packages/jumpstarter-driver-adb/jumpstarter_driver_adb/client.py +++ b/python/packages/jumpstarter-driver-adb/jumpstarter_driver_adb/client.py @@ -1,61 +1,130 @@ -import json -import os +import asyncio import subprocess -import sys -import tempfile from contextlib import contextmanager -from threading import Event from typing import Generator +import anyio import click from jumpstarter_driver_network.adapters import TcpPortforwardAdapter from jumpstarter.client import DriverClient -_UNSUPPORTED_ADB_COMMANDS = frozenset({"nodaemon"}) - -_TUNNEL_STATE_FILE = os.path.join(tempfile.gettempdir(), "jumpstarter-adb-tunnel.json") - - -def _validate_adb_args(args: tuple[str, ...]) -> None: - """Validate adb command arguments, raising UsageError for unsupported commands.""" - for arg in args: - if arg in _UNSUPPORTED_ADB_COMMANDS: - raise click.UsageError(f"'{arg}' is not supported through the Jumpstarter ADB tunnel") - - -def _read_tunnel_state() -> dict | None: - """Read the tunnel state file and verify the tunnel process is still alive.""" +#: Seconds to allow the **local** ``adb connect`` when attaching. +#: +#: Deliberately separate from the driver's ``connect_timeout``, which bounds adb calls +#: on the *exporter*. This one runs on the developer's machine against a local +#: port-forward, so it is not the exporter's business to configure. It is generous +#: because it also covers `adb connect` starting a local ADB server from cold, which +#: is slow on a first run; a healthy connect to a local port returns in milliseconds. +#: Override per call with ``attach(timeout=...)``. +ADB_CONNECT_TIMEOUT = 60.0 + +#: Seconds to allow the local ``adb disconnect`` during teardown. Shorter than the +#: connect: nothing has to be started, and teardown must not hang on a wedged adb. +ADB_DISCONNECT_TIMEOUT = 30.0 + + +def _is_cancelled(exc: BaseException) -> bool: + """Whether *exc* is a task cancellation. + + Checked without ``anyio.get_cancelled_exc_class()``, which resolves the *running* + backend and raises ``NoEventLoopError`` when there is none. These waits run in a + worker thread, off the loop, so asking there would raise from the except arm and + mask the very cancellation being handled -- reproduced as a test failure. + + Both backends' cancellations are matched directly: asyncio's ``CancelledError`` + (which trio's also subclasses on recent versions) and trio's ``Cancelled`` by + name, so trio need not be installed. + """ + if isinstance(exc, asyncio.CancelledError): + return True + return type(exc).__name__ == "Cancelled" and type(exc).__module__.startswith("trio") + + +def _wait_for_interrupt(client: DriverClient) -> None: + """Block until the CLI is interrupted, then return so teardown can run. + + The wait must happen **in the event loop**, not in this thread. + + Driver CLIs run in a worker thread driven by a ``BlockingPortal``, while + ``jmp shell`` handles Ctrl+C with ``anyio.open_signal_receiver`` and cancels + the enclosing task group. A thread-side wait — ``Event().wait()``, + ``time.sleep()``, or ``signal.signal()`` — cannot observe either: Python + delivers signals only to the main thread, and anyio cancellation only unwinds + tasks. So the CLI printed "SIGINT pressed, terminating" while the worker + thread kept waiting, the caller's ``finally`` never ran (leaving a stale + ``adb connect`` entry behind), and a second Ctrl+C hung in + ``threading._shutdown``. + + Sleeping through the portal puts the wait in a real task, so the cancel scope + unwinds it and ``portal.call`` re-raises here, letting teardown proceed. + """ try: - with open(_TUNNEL_STATE_FILE) as f: - state = json.load(f) - # Verify the tunnel process is still running - os.kill(state["pid"], 0) - return state - except (FileNotFoundError, json.JSONDecodeError, KeyError, OSError): - return None - - -def _write_tunnel_state(host: str, port: int) -> None: - """Write the tunnel state file with current process info.""" - with open(_TUNNEL_STATE_FILE, "w") as f: - json.dump({"host": host, "port": str(port), "pid": os.getpid()}, f) - - -def _remove_tunnel_state() -> None: - """Remove the tunnel state file.""" + client.portal.call(anyio.sleep_forever) + except (KeyboardInterrupt, SystemExit, GeneratorExit, RuntimeError): + # RuntimeError covers the portal already being shut down when we ask. + return + except BaseException as e: + # Cancellation derives from BaseException, not Exception, so it needs its + # own arm. + if _is_cancelled(e): + return + raise + + +def _adb_connect(adb: str, target: str, *, timeout: float = ADB_CONNECT_TIMEOUT) -> str: + """Run ``adb connect target``, raising if it did not actually connect. + + The exit status cannot be used: ``adb connect`` returns 0 even when it fails, + reporting the failure on **stdout** instead ("failed to connect to ...", "failed + to resolve host: ...", "bad port number ..."). Verified against adb 1.0.41, for + a refused port, an unresolvable host and an out-of-range port — all rc=0. So a + `check=True` here would silently accept a device that never attached, leaving the + caller to believe it had one. + + A local ADB server is *not* required: if none is running, ``adb connect`` starts + one on 5037 first (verified — it does so even when the connect itself then fails). + That is the whole point of attach — the developer does not have to own, configure, + or even have an ADB server. + + Args: + adb: path to the local adb binary. + target: the local ``host:port`` to connect to. + timeout: seconds to allow. Defaults to ``ADB_CONNECT_TIMEOUT``, a client-side + setting rather than the driver's ``connect_timeout``. + + Returns: + adb's own message, for logging. + + Raises: + RuntimeError: adb reported a failure, or timed out. + """ try: - os.unlink(_TUNNEL_STATE_FILE) - except FileNotFoundError: - pass + result = subprocess.run([adb, "connect", target], check=False, capture_output=True, text=True, timeout=timeout) + except (subprocess.TimeoutExpired, OSError) as e: + raise RuntimeError(f"`adb connect {target}` failed: {e}") from e + + message = (result.stdout or "").strip() or (result.stderr or "").strip() + # Matched against adb's own format strings, which are the only two successes: + # "connected to %s" and "already connected to %s". The failures are + # "failed to connect to ...", "bad port number ...", "cannot connect to daemon ...". + if result.returncode != 0 or not message.startswith(("connected to", "already connected to")): + raise RuntimeError(f"`adb connect {target}` did not connect: {message or 'no output'}") + return message class AdbClient(DriverClient): - """Client for tunneling ADB connections through Jumpstarter.""" + """Client for the exporter's ADB server. + + Use this to point your own tooling *at* the exporter's ADB server, which is + exclusive — you see the exporter's devices instead of your own. To add a single + remote device to an ADB server you already run, use an ``AdbDevice`` and its + ``attach`` instead. + """ @contextmanager def forward_adb(self, host: str = "127.0.0.1", port: int = 0) -> Generator[tuple[str, int], None, None]: - """Forward remote ADB server to a local TCP port. + """Forward the exporter's ADB server to a local TCP port. Args: host: Local bind address (default: 127.0.0.1) @@ -80,7 +149,7 @@ def kill_server(self) -> int: return self.call("kill_server") def connect_device(self, device: str) -> str: - """Connect to an ADB device by address (host:port).""" + """Connect the exporter's ADB server to a device by address (host:port).""" return self.call("connect_device", device) def disconnect_device(self, device: str) -> str: @@ -91,129 +160,183 @@ def list_devices(self) -> str: """List devices visible to the exporter's ADB server.""" return self.call("list_devices") + def devices(self) -> list[str]: + """Return the serials of usable devices on the exporter.""" + serials = [] + for line in self.list_devices().splitlines(): + line = line.strip() + if not line or line.startswith("*") or line.startswith("List of devices"): + continue + fields = line.split() + # Only `device`; offline/unauthorized cannot be forwarded. + if len(fields) >= 2 and fields[1] == "device": + serials.append(fields[0]) + return serials + def cli(self): - @click.command(context_settings={"ignore_unknown_options": True}) - @click.option( - "-H", - "host", - default="127.0.0.1", - show_default=True, - help="Local address to tunnel ADB to", - ) - @click.option( - "-P", - "port", - type=int, - default=0, - show_default=True, - help="Local port to tunnel ADB to (0=auto)", - ) - @click.option( - "--adb", - default="adb", - show_default=True, - help="Path to local adb executable", - ) - @click.argument("args", nargs=-1) - def adb(host: str, port: int, adb: str, args: tuple[str, ...]): - """ADB tunneling and device access. - - Wraps the local adb binary to work against a remote ADB server - tunneled through Jumpstarter. The exporter's ADB server is - automatically tunneled to a local port, and environment variables - ANDROID_ADB_SERVER_ADDRESS and ANDROID_ADB_SERVER_PORT are set so - the local adb binary communicates through the tunnel. - - All standard adb commands (shell, install, push, pull, logcat, - start-server, kill-server, connect, disconnect, etc.) are passed - through directly to the remote ADB server. - - If a persistent tunnel is already running (from a previous - `j adb tunnel`), commands will reuse it instead of creating - a new ephemeral tunnel. - - \b - Jumpstarter-specific commands: - tunnel Create a persistent ADB tunnel to a local port - (auto-assigned by default, use -P to pick a specific - port). Other j adb commands will automatically reuse - the tunnel. For native adb or external tools, export - the env vars printed by the command. - - \b - Unsupported commands: - nodaemon Not supported (would start a local server, ignoring - the tunnel). + """Build the `j adb` command group.""" + + @click.group() + def adb(): + """The exporter's ADB server. + + Jumpstarter does not wrap the adb CLI: use your own adb against the + endpoint these commands give you. + """ + + @adb.command() + def devices(): + """List devices visible to the exporter's ADB server.""" + click.echo(self.list_devices().rstrip()) + + @adb.command() + @click.option("-H", "host", default="127.0.0.1", show_default=True, help="Local address to bind") + @click.option("-P", "port", type=int, default=0, show_default=True, help="Local port to bind (0=auto)") + def tunnel(host: str, port: int): + """Forward the exporter's ADB server to a local port, and hold. + + Point your own adb at it with the environment variables printed below. """ - if not args or (len(args) == 1 and args[0] == "help"): - click.echo(click.get_current_context().get_help()) - click.echo("\n" + "=" * 60) - click.echo("ADB built-in help (from local adb binary):") - click.echo("=" * 60 + "\n") - subprocess.run([adb, "help"], stdin=sys.stdin, stdout=sys.stdout, stderr=sys.stderr) - return 0 - - _validate_adb_args(args) - - if args[0] == "tunnel": - state = _read_tunnel_state() - if state: - # If a specific port was requested, check it matches the running tunnel - if port != 0 and (state["host"] != host or int(state["port"]) != port): - click.echo( - f"Error: tunnel already running (PID {state['pid']}) " - f"on {state['host']}:{state['port']}, " - f"cannot bind to {host}:{port}", - err=True, - ) - return 1 - click.echo(f"Tunnel already running (PID {state['pid']}) on {state['host']}:{state['port']}") - return 0 - - with self.forward_adb(host, port) as addr: - _write_tunnel_state(addr[0], addr[1]) - try: - click.echo(f"ADB server tunneled to {addr[0]}:{addr[1]}") - click.echo("") - click.echo("To use native adb or other tools, run:") - click.echo(f" export ANDROID_ADB_SERVER_ADDRESS={addr[0]}") - click.echo(f" export ANDROID_ADB_SERVER_PORT={addr[1]}") - click.echo("") - click.echo("Press Ctrl+C to stop") - Event().wait() - finally: - _remove_tunnel_state() - return 0 - - # Check if a persistent tunnel is already running - state = _read_tunnel_state() - if state: - env = os.environ | { - "ANDROID_ADB_SERVER_ADDRESS": state["host"], - "ANDROID_ADB_SERVER_PORT": state["port"], - } - process = subprocess.Popen( - [adb, *args], - stdin=sys.stdin, - stdout=sys.stdout, - stderr=sys.stderr, - env=env, - ) - return process.wait() - - # No persistent tunnel - create an ephemeral one with self.forward_adb(host, port) as addr: - env = os.environ | { - "ANDROID_ADB_SERVER_ADDRESS": addr[0], - "ANDROID_ADB_SERVER_PORT": str(addr[1]), - } - process = subprocess.Popen( - [adb, *args], - stdin=sys.stdin, - stdout=sys.stdout, - stderr=sys.stderr, - env=env, - ) - return process.wait() + click.echo(f"ADB server tunneled to {addr[0]}:{addr[1]}") + click.echo("") + click.echo("To use your own adb or other tools, run:") + click.echo(f" export ANDROID_ADB_SERVER_ADDRESS={addr[0]}") + click.echo(f" export ANDROID_ADB_SERVER_PORT={addr[1]}") + click.echo("") + click.echo("Press Ctrl+C to stop") + _wait_for_interrupt(self) + return 0 + + return adb + + +class AdbDeviceClient(DriverClient): + """Client for one declared Android device on the exporter. + + Exposes the device's adbd as a local TCP endpoint. What you do with that endpoint + is up to your own adb: ``attach`` runs a single ``adb connect`` for convenience, + and ``endpoint`` just prints the address so you can drive adb yourself. + """ + + def info(self) -> dict: + """Describe the device: transport, selector, and whether it is present.""" + return self.call("info") + + @contextmanager + def endpoint(self, host: str = "127.0.0.1", port: int = 0) -> Generator[str, None, None]: + """Expose the device's adbd on a local TCP port. + + No adb is involved. This is the primitive: Jumpstarter moves the ADB protocol + between the two machines, and your own tooling does the rest. + + Args: + host: local bind address. + port: local port; 0 lets the OS choose. + + Yields: + The ``host:port`` the device's adbd is reachable at. + """ + with TcpPortforwardAdapter(client=self, local_host=host, local_port=port) as addr: + yield f"{addr[0]}:{addr[1]}" + + @contextmanager + def attach( + self, + *, + adb: str = "adb", + host: str = "127.0.0.1", + port: int = 0, + timeout: float = ADB_CONNECT_TIMEOUT, + ) -> Generator[str, None, None]: + """Add this device to the ADB server your machine already uses. + + Three steps, none of them clever: the exporter streams the device's adbd, + Jumpstarter tunnels it here, and plain ``adb connect`` adds it to the local + server. Because ``adb connect`` is additive, the device lands in the *default* + server — the one Android Studio, tradefed, gradle and a bare ``adb`` all talk + to — with no environment variables and no IDE restart. If you have no ADB + server at all, ``adb connect`` starts one. + + Args: + adb: path to your local adb binary. + host: local bind address. + port: local port to bind; 0 lets the OS choose. The device's address is + whatever this resolves to — deliberately not something this driver + invents, since ADB owns device addressing. + timeout: seconds to allow the local ``adb connect``. A client-side + timeout, distinct from the exporter's ``connect_timeout``, because it + bounds a command on your machine against a local port-forward. + + Yields: + The ``host:port`` the device was attached as. + """ + with self.endpoint(host=host, port=port) as target: + _adb_connect(adb, target, timeout=timeout) + try: + yield target + finally: + # Leave no stale `offline` entry in the developer's ADB server. Bounded + # and swallowed: teardown must not hang, and must not mask the session. + try: + subprocess.run( + [adb, "disconnect", target], + check=False, + capture_output=True, + text=True, + timeout=ADB_DISCONNECT_TIMEOUT, + ) + except (subprocess.SubprocessError, OSError) as e: + self.logger.debug("disconnect %s failed: %s", target, e) + + def cli(self): + """Build the per-device command group.""" + + @click.group() + def adb(): + """One Android device on the exporter. + + Jumpstarter does not wrap the adb CLI. Use `attach` to add this device to + your own ADB server, then run your own `adb -s
...`. + """ + + @adb.command() + def info(): + """Show the device's transport, selector and presence.""" + for key, value in self.info().items(): + click.echo(f"{key}: {value}") + + @adb.command() + @click.option("--adb", "adb_path", default="adb", show_default=True, help="Path to your local adb") + @click.option("-H", "host", default="127.0.0.1", show_default=True, help="Local address to bind") + @click.option("-P", "port", type=int, default=0, show_default=True, help="Local port to bind (0=auto)") + def attach(adb_path: str, host: str, port: int): + """Add this device to your own ADB server, and hold until Ctrl+C.""" + with self.attach(adb=adb_path, host=host, port=port) as target: + click.echo(f"attached as {target}") + click.echo("") + click.echo(f"Your ADB server now lists it; use it with: adb -s {target} shell") + click.echo("Android Studio will list it too.") + click.echo("") + click.echo("Press Ctrl+C to detach") + _wait_for_interrupt(self) + click.echo("detached") + return 0 + + @adb.command() + @click.option("-H", "host", default="127.0.0.1", show_default=True, help="Local address to bind") + @click.option("-P", "port", type=int, default=0, show_default=True, help="Local port to bind (0=auto)") + def endpoint(host: str, port: int): + """Print the device's local adbd address, and hold until Ctrl+C. + + For driving adb yourself, or for tools that take a host:port. + """ + with self.endpoint(host=host, port=port) as target: + click.echo(target) + click.echo("") + click.echo(f"Add it to your ADB server with: adb connect {target}") + click.echo("Press Ctrl+C to stop") + _wait_for_interrupt(self) + return 0 return adb diff --git a/python/packages/jumpstarter-driver-adb/jumpstarter_driver_adb/client_test.py b/python/packages/jumpstarter-driver-adb/jumpstarter_driver_adb/client_test.py new file mode 100644 index 000000000..4abfb5ff7 --- /dev/null +++ b/python/packages/jumpstarter-driver-adb/jumpstarter_driver_adb/client_test.py @@ -0,0 +1,387 @@ +import asyncio +import subprocess +from contextlib import contextmanager +from unittest.mock import MagicMock, patch + +import pytest + +from .client import ( + ADB_CONNECT_TIMEOUT, + ADB_DISCONNECT_TIMEOUT, + AdbClient, + AdbDeviceClient, + _adb_connect, + _wait_for_interrupt, +) + +# ------------------------------------------------------------- `adb connect` +# +# `adb connect` exits 0 even when it fails, printing the reason to STDOUT. Verified +# against adb 1.0.41: a refused port, an unresolvable host and an out-of-range port +# all return 0. So the exit status cannot be used to tell whether a device attached. +# +# This guards the ONE adb invocation left in the client. Jumpstarter no longer wraps +# the adb CLI; `attach` runs `adb connect` and nothing else. + + +def _completed(stdout, returncode=0): + return subprocess.CompletedProcess(args=["adb", "connect", "x"], returncode=returncode, stdout=stdout, stderr="") + + +@pytest.mark.parametrize( + "output", + [ + "failed to connect to '127.0.0.1:59999': Connection refused", + "failed to connect to 127.0.0.1:15055", + "failed to resolve host: 'nope.invalid': nodename nor servname provided", + "bad port number '99999' in '127.0.0.1:99999'", + "cannot connect to daemon at tcp:127.0.0.1:5037: Connection refused", + "", + ], +) +def test_a_failed_connect_is_detected_despite_exit_zero(output): + """This is the whole point: rc=0 with a failure message on stdout.""" + with patch("subprocess.run", return_value=_completed(output, returncode=0)): + with pytest.raises(RuntimeError, match="did not connect"): + _adb_connect("adb", "127.0.0.1:59999") + + +@pytest.mark.parametrize( + "output", + ["connected to 127.0.0.1:16000", "already connected to 127.0.0.1:16000"], +) +def test_a_successful_connect_is_accepted(output): + """adb's only two success strings: `connected to %s`, `already connected to %s`.""" + with patch("subprocess.run", return_value=_completed(output)): + assert _adb_connect("adb", "127.0.0.1:16000") == output + + +def test_a_hung_connect_raises_rather_than_blocking(): + with patch("subprocess.run", side_effect=subprocess.TimeoutExpired("adb connect", 60)): + with pytest.raises(RuntimeError, match="failed"): + _adb_connect("adb", "127.0.0.1:16000") + + +# --------------------------------------------------------- attach and endpoint +# +# The client's whole job: expose the device's adbd locally, and optionally run one +# `adb connect`. Anything more would be wrapping the adb CLI, which is what this +# design deliberately does not do. + +TARGET = "127.0.0.1:41000" + + +@contextmanager +def _fake_endpoint(_client, host="127.0.0.1", port=0): + """Stand in for the port-forward, yielding a fixed local address.""" + yield TARGET + + +def _device_client(): + """An AdbDeviceClient with its transport stubbed out.""" + client = MagicMock(spec=AdbDeviceClient) + client.endpoint = lambda **kwargs: _fake_endpoint(client, **kwargs) + client.logger = MagicMock() + return client + + +def test_attach_runs_exactly_one_adb_connect_and_one_disconnect(): + """A regression here is how the CLI wrapper creeps back in. + + Jumpstarter's contribution is the endpoint; the single `adb connect` exists only + because adding a device to a server the client already owns *is* the feature. + """ + client = _device_client() + with patch("subprocess.run", return_value=_completed("connected to " + TARGET)) as run: + with AdbDeviceClient.attach(client) as target: + assert target == TARGET + connects = [c.args[0] for c in run.call_args_list] + assert connects == [["adb", "connect", TARGET]] + argvs = [c.args[0] for c in run.call_args_list] + + assert argvs == [["adb", "connect", TARGET], ["adb", "disconnect", TARGET]] + + +def test_local_adb_timeouts_are_bounded_and_overridable(): + """The local `adb connect` timeout is a CLIENT setting, not the exporter's. + + It bounds a command on the developer's machine against a local port-forward, so + it is deliberately separate from the driver's `connect_timeout`. Both calls must + be bounded, or a wedged local adb hangs the session (connect) or teardown + (disconnect). + """ + client = _device_client() + + # Default: the documented client-side constant, not the driver's connect_timeout. + with patch("subprocess.run", return_value=_completed("connected to " + TARGET)) as run: + with AdbDeviceClient.attach(client): + pass + defaults = {c.args[0][1]: c.kwargs["timeout"] for c in run.call_args_list} + assert defaults["connect"] == ADB_CONNECT_TIMEOUT + assert defaults["disconnect"] == ADB_DISCONNECT_TIMEOUT + + # ...and overridable per call. + with patch("subprocess.run", return_value=_completed("connected to " + TARGET)) as run: + with AdbDeviceClient.attach(client, timeout=5): + pass + timeouts = {c.args[0][1]: c.kwargs["timeout"] for c in run.call_args_list} + assert timeouts["connect"] == 5, "attach(timeout=...) must reach adb connect" + assert all(t and t > 0 for t in timeouts.values()), timeouts + + +def test_attach_honours_a_custom_adb_path(): + """`--adb` locates the binary for that one call; nothing else shells out.""" + client = _device_client() + with patch("subprocess.run", return_value=_completed("connected to " + TARGET)) as run: + with AdbDeviceClient.attach(client, adb="/opt/sdk/adb"): + pass + assert [c.args[0][0] for c in run.call_args_list] == ["/opt/sdk/adb", "/opt/sdk/adb"] + + +def test_attach_disconnects_even_when_the_body_raises(): + """Otherwise a crash leaves a stale `offline` entry in the developer's server.""" + client = _device_client() + with patch("subprocess.run", return_value=_completed("connected to " + TARGET)) as run: + with pytest.raises(ValueError): + with AdbDeviceClient.attach(client): + raise ValueError("boom") + assert ["adb", "disconnect", TARGET] in [c.args[0] for c in run.call_args_list] + + +def test_a_failed_disconnect_does_not_mask_the_session(): + """Teardown is best-effort: a hung local adb must not turn into a raised error.""" + client = _device_client() + + def run(argv, **kwargs): + if argv[1] == "disconnect": + raise subprocess.TimeoutExpired("adb disconnect", 30) + return _completed("connected to " + TARGET) + + with patch("subprocess.run", side_effect=run): + with AdbDeviceClient.attach(client) as target: + assert target == TARGET + + +def test_attach_does_not_run_adb_when_the_connect_fails(): + """No disconnect for a device that never attached, and the error propagates.""" + client = _device_client() + with patch("subprocess.run", return_value=_completed("failed to connect to " + TARGET)) as run: + with pytest.raises(RuntimeError, match="did not connect"): + with AdbDeviceClient.attach(client): + pass + assert [c.args[0] for c in run.call_args_list] == [["adb", "connect", TARGET]] + + +def test_endpoint_runs_no_adb_at_all(): + """The honest primitive: Jumpstarter moves bytes, the user drives adb.""" + client = MagicMock(spec=AdbDeviceClient) + forwarded = MagicMock() + forwarded.__enter__ = MagicMock(return_value=("127.0.0.1", 41000)) + forwarded.__exit__ = MagicMock(return_value=False) + + with ( + patch("jumpstarter_driver_adb.client.TcpPortforwardAdapter", return_value=forwarded), + patch("subprocess.run", side_effect=AssertionError("endpoint must not run adb")) as run, + ): + with AdbDeviceClient.endpoint(client) as target: + assert target == TARGET + run.assert_not_called() + + +# ------------------------------------------------- waiting inside the event loop +# +# The wait must return rather than propagate, or Ctrl+C leaves a stale `adb connect` +# entry behind and a second Ctrl+C hangs in threading._shutdown. + + +class _Portal: + def __init__(self, raises): + self._raises = raises + + def call(self, *args, **kwargs): + raise self._raises + + +@pytest.mark.parametrize( + "exc", + [KeyboardInterrupt(), SystemExit(), GeneratorExit(), RuntimeError("portal is closed")], +) +def test_an_interrupt_ends_the_wait_without_propagating(exc): + client = MagicMock(portal=_Portal(exc)) + _wait_for_interrupt(client) # must return, so teardown can run + + +def test_anyio_cancellation_ends_the_wait(): + """Cancellation is a BaseException, not an Exception, so it needs its own arm. + + Deliberately synchronous and with no event loop: these waits run in a worker + thread, and `get_cancelled_exc_class()` in the except arm used to raise + NoEventLoopError there, masking the cancellation it was meant to detect. + """ + client = MagicMock(portal=_Portal(asyncio.CancelledError())) + _wait_for_interrupt(client) + + +def test_an_unexpected_error_is_not_swallowed(): + """A real bug must surface, not look like a clean Ctrl+C.""" + client = MagicMock(portal=_Portal(ValueError("something else"))) + with pytest.raises(ValueError): + _wait_for_interrupt(client) + + +def test_ctrl_c_during_attach_still_detaches(): + """The end-to-end teardown path: interrupt the hold, and the device is released.""" + client = _device_client() + client.portal = _Portal(KeyboardInterrupt()) + + with patch("subprocess.run", return_value=_completed("connected to " + TARGET)) as run: + with AdbDeviceClient.attach(client) as target: + _wait_for_interrupt(client) # returns, as a real Ctrl+C would + assert target == TARGET + assert ["adb", "disconnect", TARGET] in [c.args[0] for c in run.call_args_list] + + +# ------------------------------------------------------------------ CLI surface +# +# The CLI is the user-facing contract documented in the README, so it is worth +# pinning: which commands exist, that they run the adb calls they claim to, and that +# `endpoint` runs none. + + +def _cli_device_client(): + """A device client whose transport is stubbed, for driving its CLI.""" + client = MagicMock(spec=AdbDeviceClient) + client.endpoint = lambda **kwargs: _fake_endpoint(client, **kwargs) + client.attach = lambda **kwargs: AdbDeviceClient.attach(client, **kwargs) + client.info = lambda: {"transport": "usb", "selector": "usb:1-4.2", "present": "yes"} + client.logger = MagicMock() + client.portal = _Portal(KeyboardInterrupt()) + return client + + +def test_device_cli_exposes_only_attach_endpoint_info(): + """No `shell`, `install`, `logcat` — Jumpstarter does not wrap the adb CLI.""" + group = AdbDeviceClient.cli(_cli_device_client()) + assert sorted(group.commands) == ["attach", "endpoint", "info"] + + +def test_device_cli_info_prints_the_fields(): + from click.testing import CliRunner + + group = AdbDeviceClient.cli(_cli_device_client()) + result = CliRunner().invoke(group, ["info"]) + assert result.exit_code == 0, result.output + assert "transport: usb" in result.output + assert "selector: usb:1-4.2" in result.output + + +def test_device_cli_attach_connects_and_tells_you_how_to_use_it(): + from click.testing import CliRunner + + client = _cli_device_client() + with patch("subprocess.run", return_value=_completed("connected to " + TARGET)) as run: + result = CliRunner().invoke(group := AdbDeviceClient.cli(client), ["attach"]) + assert group is not None + assert result.exit_code == 0, result.output + assert TARGET in result.output + # It must tell the user to drive their own adb, since we no longer proxy it. + assert f"adb -s {TARGET} shell" in result.output + assert "detached" in result.output + argvs = [c.args[0] for c in run.call_args_list] + assert argvs == [["adb", "connect", TARGET], ["adb", "disconnect", TARGET]] + + +def test_device_cli_endpoint_prints_the_address_and_runs_no_adb(): + from click.testing import CliRunner + + client = _cli_device_client() + with patch("subprocess.run", side_effect=AssertionError("endpoint must not run adb")): + result = CliRunner().invoke(AdbDeviceClient.cli(client), ["endpoint"]) + assert result.exit_code == 0, result.output + assert result.output.splitlines()[0] == TARGET + assert f"adb connect {TARGET}" in result.output + + +def _cli_server_client(): + """A server client with its tunnel stubbed, for driving its CLI.""" + client = MagicMock(spec=AdbClient) + client.list_devices = lambda: "List of devices attached\nHVA1234567\tdevice usb:1-4.2\n" + client.forward_adb = MagicMock() + client.forward_adb.return_value.__enter__ = MagicMock(return_value=("127.0.0.1", 54321)) + client.forward_adb.return_value.__exit__ = MagicMock(return_value=False) + client.portal = _Portal(KeyboardInterrupt()) + return client + + +def test_server_cli_exposes_only_devices_and_tunnel(): + group = AdbClient.cli(_cli_server_client()) + assert sorted(group.commands) == ["devices", "tunnel"] + + +def test_server_cli_devices_lists_them(): + from click.testing import CliRunner + + result = CliRunner().invoke(AdbClient.cli(_cli_server_client()), ["devices"]) + assert result.exit_code == 0, result.output + assert "HVA1234567" in result.output + + +def test_server_cli_tunnel_prints_the_env_vars_to_export(): + """The tunnel's whole purpose: hand the user variables for their own tooling.""" + from click.testing import CliRunner + + client = _cli_server_client() + result = CliRunner().invoke(AdbClient.cli(client), ["tunnel"]) + assert result.exit_code == 0, result.output + assert "ANDROID_ADB_SERVER_ADDRESS=127.0.0.1" in result.output + assert "ANDROID_ADB_SERVER_PORT=54321" in result.output + + +# --------------------------------------------------------- AdbClient call wiring + + +def test_server_client_methods_map_to_driver_calls(): + """Thin wrappers, but cuttlefish and androidemulator depend on these names.""" + client = MagicMock(spec=AdbClient) + client.call = MagicMock(return_value="ok") + + assert AdbClient.start_server(client) == "ok" + assert AdbClient.kill_server(client) == "ok" + assert AdbClient.connect_device(client, "10.0.0.5:5555") == "ok" + assert AdbClient.disconnect_device(client, "10.0.0.5:5555") == "ok" + assert AdbClient.list_devices(client) == "ok" + + assert [c.args for c in client.call.call_args_list] == [ + ("start_server",), + ("kill_server",), + ("connect_device", "10.0.0.5:5555"), + ("disconnect_device", "10.0.0.5:5555"), + ("list_devices",), + ] + + +@pytest.mark.parametrize( + ("output", "expected"), + [ + ("List of devices attached\nA\tdevice\nB\toffline\n", ["A"]), + ("List of devices attached\nA\tunauthorized\n", []), + ("List of devices attached\n", []), + ("* daemon started *\nList of devices attached\nA\tdevice usb:1-1\n", ["A"]), + ("", []), + ], +) +def test_only_forwardable_devices_are_listed(output, expected): + """`offline`/`unauthorized` cannot be forwarded, and adb's noise lines are not devices.""" + client = MagicMock(spec=AdbClient) + client.list_devices = lambda: output + assert AdbClient.devices(client) == expected + + +def test_forward_adb_yields_the_local_listener(): + client = MagicMock(spec=AdbClient) + forwarded = MagicMock() + forwarded.__enter__ = MagicMock(return_value=("127.0.0.1", 54321)) + forwarded.__exit__ = MagicMock(return_value=False) + with patch("jumpstarter_driver_adb.client.TcpPortforwardAdapter", return_value=forwarded): + with AdbClient.forward_adb(client, port=0) as addr: + assert addr == ("127.0.0.1", 54321) diff --git a/python/packages/jumpstarter-driver-adb/jumpstarter_driver_adb/driver.py b/python/packages/jumpstarter-driver-adb/jumpstarter_driver_adb/driver.py index f09f39503..9a1354c5e 100644 --- a/python/packages/jumpstarter-driver-adb/jumpstarter_driver_adb/driver.py +++ b/python/packages/jumpstarter-driver-adb/jumpstarter_driver_adb/driver.py @@ -1,85 +1,155 @@ import math import os import shutil +import socket import subprocess -from dataclasses import dataclass +import threading +from contextlib import asynccontextmanager +from dataclasses import dataclass, field +from anyio import connect_tcp, to_thread from jumpstarter_driver_network.driver import TcpNetwork from jumpstarter.common.exceptions import ConfigurationError -from jumpstarter.driver.decorators import export +from jumpstarter.driver import Driver +from jumpstarter.driver.decorators import export, exportstream +#: Transports an ``AdbDevice`` can use. adb itself has only two: USB and TCP +#: (``adb.h`` defines ``kTransportUsb`` and ``kTransportLocal``, where "local" +#: means TCP). There is deliberately no ``serial`` — adb has no UART transport, +#: and ``dev:``/``dev-raw:`` are forward targets executed inside adbd on the +#: device, not host transports. See the README. +TRANSPORT_USB = "usb" +TRANSPORT_TCP = "tcp" +_TRANSPORTS = (TRANSPORT_USB, TRANSPORT_TCP) -@dataclass(kw_only=True) -class AdbServer(TcpNetwork): - """ADB server driver that tunnels ADB connections over Jumpstarter. +# Transports someone may reasonably reach for that adb cannot do, mapped to what to +# do instead. A bare "unknown transport" sends people looking for a typo. +_UNSUPPORTED_TRANSPORTS = { + "serial": ( + "adb has no serial/UART transport. Bridge the UART to TCP (socat) or use the " + "device's console to enable adbd over TCP, then use transport: tcp." + ), + "uart": ( + "adb has no serial/UART transport. Bridge the UART to TCP (socat) or use the " + "device's console to enable adbd over TCP, then use transport: tcp." + ), + "vsock": "vsock is not implemented yet; use transport: tcp with the device's address.", + "emulator": "emulators are found by the ADB server itself; use jumpstarter-driver-androidemulator.", +} + + +def _adb_env(port: int) -> dict[str, str]: + """Environment pointing adb at the ADB server on *port*.""" + return {**os.environ, "ANDROID_ADB_SERVER_PORT": str(port)} + + +def _resolve_adb_path(adb_path: str) -> str: + """Resolve ``"adb"`` against PATH, and fail early if it is missing or broken.""" + if adb_path == "adb": + resolved = shutil.which("adb") + if not resolved: + raise ConfigurationError("ADB executable not found in PATH") + adb_path = resolved + + try: + subprocess.run( + [adb_path, "version"], + check=True, + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + text=True, + ) + except (subprocess.CalledProcessError, FileNotFoundError) as e: + raise ConfigurationError(f"ADB executable not functional: {e}") from e + return adb_path - Manages an ADB daemon on the exporter and exposes it via TCP tunnel. - Client-side tools (adb, Android Studio, tradefed) connect through - the tunnel as if the ADB server were local. + +def _validate_port(name: str, value) -> None: + """Reject anything that is not a usable TCP port. + + ``bool`` is excluded explicitly: it subclasses ``int``, so ``port: true`` would + otherwise pass as port 1. """ + if not isinstance(value, int) or isinstance(value, bool): + raise ConfigurationError(f"{name} must be an integer: {value}") + if value < 1 or value > 65535: + raise ConfigurationError(f"Invalid {name}: {value}") - adb_path: str = "adb" - host: str = "127.0.0.1" - port: int = 15037 - connect_timeout: float = 30.0 - @classmethod - def client(cls) -> str: - return "jumpstarter_driver_adb.client.AdbClient" - def __post_init__(self): - if hasattr(super(), "__post_init__"): - super().__post_init__() +def _validate_timeout(value) -> None: + """Reject a non-positive or non-finite ``connect_timeout``.""" + if isinstance(value, bool) or not isinstance(value, (int, float)) or not math.isfinite(value) or value <= 0: + raise ConfigurationError(f"connect_timeout must be a positive number: {value}") - if not isinstance(self.port, int): - raise ConfigurationError(f"Port must be an integer: {self.port}") - if self.port < 1 or self.port > 65535: - raise ConfigurationError(f"Invalid port number: {self.port}") - - if ( - isinstance(self.connect_timeout, bool) - or not isinstance(self.connect_timeout, (int, float)) - or not math.isfinite(self.connect_timeout) - or self.connect_timeout <= 0 - ): - raise ConfigurationError(f"connect_timeout must be a positive number: {self.connect_timeout}") - - # Resolve adb binary - if self.adb_path == "adb": - resolved = shutil.which("adb") - if not resolved: - raise ConfigurationError("ADB executable not found in PATH") - self.adb_path = resolved - - # Verify adb works - try: - result = subprocess.run( - [self.adb_path, "version"], - check=True, - stdout=subprocess.PIPE, - stderr=subprocess.PIPE, - text=True, - ) - self.logger.debug(result.stdout.strip()) - except (subprocess.CalledProcessError, FileNotFoundError) as e: - raise ConfigurationError(f"ADB executable not functional: {e}") from e - # Auto-start the ADB server on the configured port - self.start_server() - self.logger.info(f"ADB server running on {self.host}:{self.port}") +class _SharedServer: + """One ADB server on one port, shared by every driver that needs it. + An ADB server **claims** the USB devices it finds, and only one server can hold a + given device. So two drivers must never each start their own on the same port: the + second would see an empty device list while `adb start-server` reported success + (it is silent and exits 0 whether it started a server or found one). Sharing is + therefore a correctness requirement, not an optimisation. - def close(self): - self.kill_server() + Reference-counted so the last user tears it down, and only if *we* started it — + killing a server we merely adopted would drop the device claims of everything else + on the host. + """ - def adb_env(self) -> dict[str, str]: - """Environment with ANDROID_ADB_SERVER_PORT set.""" - return {**os.environ, "ANDROID_ADB_SERVER_PORT": str(self.port)} + def __init__(self, adb_path: str, port: int) -> None: + """Track a not-yet-started server for *adb_path* on *port*.""" + self.adb_path = adb_path + self.port = port + self.refs = 0 + self.owns = False + + def env(self) -> dict[str, str]: + """Environment pointing adb at this server.""" + return _adb_env(self.port) + + def _is_listening(self, connect_timeout: float, logger) -> bool: + """Whether a usable ADB server is already serving this port. + + Two checks, because a listening socket alone is not enough. Something that is + *not* adb holding the port is the dangerous case: `adb start-server` and + `adb devices` both block forever against such a listener rather than failing + (verified against a plain TCP listener), which would hang exporter startup. So + we connect first, then ask the peer something only a server can answer. + + That question has to be `devices`, not `version`: `adb version` reports the + local client's own version without contacting the server at all (verified — it + exits 0 with zero connections to the port), so it would accept any listener. + `devices` does contact the server, which answers it immediately, while a + non-ADB listener leaves it to hit the timeout below. + """ + try: + with socket.create_connection(("127.0.0.1", self.port), timeout=2): + pass + except OSError: + return False - @export - def start_server(self) -> int: - """Start the ADB server on the exporter. Returns the port number.""" - self.logger.info(f"Starting ADB server on port {self.port}") + try: + result = subprocess.run( + [self.adb_path, "devices"], + check=False, + capture_output=True, + text=True, + timeout=min(connect_timeout, 10), + env=self.env(), + ) + except (subprocess.TimeoutExpired, OSError): + logger.warning( + "something is listening on port %d but does not answer as an ADB server; " + "not adopting it. Free the port, or set a different port in the exporter config.", + self.port, + ) + return False + return result.returncode == 0 + + def start(self, connect_timeout: float, logger) -> None: + """Start the ADB server, bounded so a wedged port cannot hang startup.""" + logger.info("Starting ADB server on port %d", self.port) try: result = subprocess.run( [self.adb_path, "start-server"], @@ -87,20 +157,28 @@ def start_server(self) -> int: stdout=subprocess.PIPE, stderr=subprocess.PIPE, text=True, - env=self.adb_env(), + # Bounded: `start-server` blocks forever if a non-ADB process holds the + # port, which would otherwise hang exporter startup. + timeout=connect_timeout, + env=self.env(), ) if result.stdout.strip(): - self.logger.info(result.stdout.strip()) + logger.info(result.stdout.strip()) if result.stderr.strip(): - self.logger.debug(result.stderr.strip()) + logger.debug(result.stderr.strip()) except subprocess.CalledProcessError as e: - self.logger.error(f"Failed to start ADB server: {e}") - return self.port + logger.error("Failed to start ADB server: %s", e) + except subprocess.TimeoutExpired: + logger.error( + "`adb start-server` timed out after %ss on port %d. Something that is not " + "an ADB server may hold that port; free it or configure a different port.", + connect_timeout, + self.port, + ) - @export - def kill_server(self) -> int: - """Kill the ADB server on the exporter. Returns the port number.""" - self.logger.info(f"Killing ADB server on port {self.port}") + def kill(self, connect_timeout: float, logger) -> None: + """Kill the ADB server, bounded because this runs from teardown.""" + logger.info("Killing ADB server on port %d", self.port) try: result = subprocess.run( [self.adb_path, "kill-server"], @@ -108,15 +186,170 @@ def kill_server(self) -> int: stdout=subprocess.PIPE, stderr=subprocess.PIPE, text=True, - env=self.adb_env(), + timeout=connect_timeout, + env=self.env(), ) if result.stdout.strip(): - self.logger.info(result.stdout.strip()) + logger.info(result.stdout.strip()) except subprocess.CalledProcessError as e: - self.logger.error(f"Failed to kill ADB server: {e}") + logger.error("Failed to kill ADB server: %s", e) + except subprocess.TimeoutExpired: + logger.error("`adb kill-server` timed out after %ss", connect_timeout) + + +# One ADB server per (adb_path, port) per exporter process. See `_SharedServer` for +# why sharing is mandatory rather than merely tidy. +_SERVERS: dict[tuple[str, int], _SharedServer] = {} +_SERVERS_LOCK = threading.Lock() + + +def _acquire_server( + adb_path: str, + port: int, + *, + connect_timeout: float, + adopt_existing_server: bool, + logger, +) -> _SharedServer: + """Take a reference to the ADB server on *port*, starting or adopting it once. + + The probe and start happen while holding the lock. That serialises concurrent + first-acquirers, which is the point: without it two callers could both decide no + server was running and both start one. + """ + key = (adb_path, port) + with _SERVERS_LOCK: + entry = _SERVERS.get(key) + if entry is None: + entry = _SharedServer(adb_path, port) + if adopt_existing_server and entry._is_listening(connect_timeout, logger): + logger.info( + "adopting the ADB server already listening on port %d; it owns the " + "connected devices, and this driver will leave it running", + port, + ) + else: + entry.start(connect_timeout, logger) + entry.owns = True + logger.info("ADB server running on port %d", port) + _SERVERS[key] = entry + entry.refs += 1 + return entry + + +def _release_server(adb_path: str, port: int, *, connect_timeout: float, logger) -> None: + """Drop a reference, killing the server only when it is ours and unused.""" + key = (adb_path, port) + with _SERVERS_LOCK: + entry = _SERVERS.get(key) + if entry is None: + return + entry.refs -= 1 + if entry.refs > 0: + return + del _SERVERS[key] + if entry.owns: + entry.kill(connect_timeout, logger) + else: + logger.debug("leaving the adopted ADB server on port %d running", port) + + +@dataclass(kw_only=True) +class AdbServer(TcpNetwork): + """An ADB server on the exporter, tunnelled to the client. + + Point client tooling at *this* server with ``forward_adb``/``j adb tunnel``: the + client then sees the exporter's devices instead of its own. That is exclusive — + the client must own its ADB server — so for adding a single remote device to an + ADB server the client already runs (Android Studio's, say), declare an + :class:`AdbDevice` instead. + + Declaring this driver is optional. ``AdbDevice`` ensures a server on its own, and + both route through the same per-process registry, so an explicitly declared server + is the one a co-located device adopts. + """ + + adb_path: str = "adb" + host: str = "127.0.0.1" + port: int = 15037 + connect_timeout: float = 30.0 + + # Whether to use an ADB server that is already listening on `port` instead of + # insisting on one we started ourselves. See `_SharedServer`: the running server + # owns the USB devices, so a server started alongside it sees nothing. + adopt_existing_server: bool = True + + _server: _SharedServer | None = field(default=None, init=False, repr=False) + + @classmethod + def client(cls) -> str: + """Import path of the matching client class.""" + return "jumpstarter_driver_adb.client.AdbClient" + + def __post_init__(self): + """Validate the config and bring an ADB server up on our port.""" + if hasattr(super(), "__post_init__"): + super().__post_init__() + + _validate_port("port", self.port) + _validate_timeout(self.connect_timeout) + self.adb_path = _resolve_adb_path(self.adb_path) + + # Eager, unlike AdbDevice: this driver *is* the server, and callers such as + # the cuttlefish and androidemulator drivers expect it up after construction. + self._server = _acquire_server( + self.adb_path, + self.port, + connect_timeout=self.connect_timeout, + adopt_existing_server=self.adopt_existing_server, + logger=self.logger, + ) + + @property + def _owns_server(self) -> bool: + """Whether this process started the server, rather than adopting one.""" + return self._server is not None and self._server.owns + + def close(self): + """Release our reference to the shared ADB server.""" + if self._server is not None: + _release_server( + self.adb_path, + self.port, + connect_timeout=self.connect_timeout, + logger=self.logger, + ) + self._server = None + super().close() + + def adb_env(self) -> dict[str, str]: + """Environment with ANDROID_ADB_SERVER_PORT set.""" + return _adb_env(self.port) + + @export + def start_server(self) -> int: + """Start the ADB server on the exporter. Returns the port number. + + Note this is silent and succeeds when a server is already listening, so the + result does not tell you whether the server is ours — see + `adopt_existing_server`. + """ + _SharedServer(self.adb_path, self.port).start(self.connect_timeout, self.logger) + return self.port + + @export + def kill_server(self) -> int: + """Kill the ADB server on the exporter. Returns the port number.""" + _SharedServer(self.adb_path, self.port).kill(self.connect_timeout, self.logger) return self.port - def _connect_device(self, device: str) -> str: + @export + def connect_device(self, device: str) -> str: + """Connect the exporter's ADB server to a device by address (host:port). + + Raises on failure or timeout so callers can react instead of + silently receiving an error string. + """ self.logger.info(f"Connecting to device {device}") try: result = subprocess.run( @@ -139,15 +372,6 @@ def _connect_device(self, device: str) -> str: self.logger.error(f"Failed to connect to device {device}: {stderr or e}") raise - @export - def connect_device(self, device: str) -> str: - """Connect to an ADB device by address (host:port). - - Raises on failure or timeout so callers can react instead of - silently receiving an error string. - """ - return self._connect_device(device) - @export def disconnect_device(self, device: str) -> str: """Disconnect an ADB device by address (host:port). @@ -179,7 +403,11 @@ def disconnect_device(self, device: str) -> str: @export def list_devices(self) -> str: - """List devices visible to the exporter's ADB server.""" + """List devices visible to the exporter's ADB server. + + Read live from the ADB server on every call. Bounded, since `adb devices` + blocks forever if a non-ADB process holds the port. + """ try: result = subprocess.run( [self.adb_path, "devices", "-l"], @@ -187,9 +415,386 @@ def list_devices(self) -> str: stdout=subprocess.PIPE, stderr=subprocess.PIPE, text=True, + timeout=self.connect_timeout, env=self.adb_env(), ) return result.stdout except subprocess.CalledProcessError as e: self.logger.error(f"Failed to list devices: {e}") return f"Error: {e}" + except subprocess.TimeoutExpired as e: + self.logger.error(f"`adb devices` timed out after {self.connect_timeout}s") + return f"Error: {e}" + + +@dataclass(kw_only=True) +class AdbDevice(Driver): + """One declared Android device, exposed as a stream of its adbd. + + Declared rather than discovered, which is what makes it composable: a DUT is a + composite of its power relay, its console and this, so leasing the DUT leases the + right device. It also means a device that is currently powered off is still + *described* — startup does not require it to be present. + + Identity for a USB device is the **bench port** (``usb_port``), not the device's + serial, so hardware can be swapped between benches without a config change. The + serial is looked up from the port on every call, which is exactly what changes + when a relay power-cycles the DUT and USB re-enumerates. + """ + + driver_type = "network" + + #: ``"usb"`` for a USB-attached device, ``"tcp"`` for one whose adbd already + #: listens on TCP (a networked or AAOS head unit, a virtual device). + transport: str = TRANSPORT_USB + + #: For transport usb: the bench USB port (preferred) or an explicit ADB serial. + #: Exactly one of the two. + usb_port: str | None = None + serial: str | None = None + + #: For transport tcp: the device's own adbd endpoint, ``host`` or ``host:port``. + address: str | None = None + + adbd_port: int = 5555 + adb_path: str = "adb" + connect_timeout: float = 30.0 + #: Which ADB server to use. Rarely set: the server is implicit and shared. + server_port: int = 15037 + adopt_existing_server: bool = True + + _lock: threading.Lock = field(init=False, repr=False) + _server: _SharedServer | None = field(default=None, init=False, repr=False) + _forward_port: int | None = field(default=None, init=False, repr=False) + _connected: str | None = field(default=None, init=False, repr=False) + + @classmethod + def client(cls) -> str: + """Import path of the matching client class.""" + return "jumpstarter_driver_adb.client.AdbDeviceClient" + + def __post_init__(self): + """Validate the config and resolve adb. Does not touch the device or a server.""" + if hasattr(super(), "__post_init__"): + super().__post_init__() + + self._lock = threading.Lock() + self._validate_config() + self.adb_path = _resolve_adb_path(self.adb_path) + if self.usb_port is not None: + self.usb_port = self._normalize_usb_port(self.usb_port) + + def _validate_config(self) -> None: + """Reject a config whose fields do not match its transport.""" + if self.transport not in _TRANSPORTS: + hint = _UNSUPPORTED_TRANSPORTS.get(str(self.transport).lower()) + supported = "/".join(_TRANSPORTS) + if hint: + raise ConfigurationError(f"transport {self.transport!r} is not supported: {hint}") + raise ConfigurationError(f"transport must be one of {supported}: {self.transport!r}") + + _validate_port("server_port", self.server_port) + _validate_port("adbd_port", self.adbd_port) + _validate_timeout(self.connect_timeout) + + if self.transport == TRANSPORT_USB: + if self.address is not None: + raise ConfigurationError("'address' only applies to transport: tcp") + if (self.usb_port is None) == (self.serial is None): + raise ConfigurationError( + "transport: usb needs exactly one of 'usb_port' (the bench USB port, preferred) or 'serial'" + ) + if self.usb_port is not None and not str(self.usb_port).strip(): + raise ConfigurationError("'usb_port' must not be empty") + if self.serial is not None and not str(self.serial).strip(): + raise ConfigurationError("'serial' must not be empty") + else: + if self.usb_port is not None or self.serial is not None: + raise ConfigurationError("'usb_port'/'serial' only apply to transport: usb") + if not self.address: + raise ConfigurationError("transport: tcp needs 'address' (the device's adbd endpoint)") + + @staticmethod + def _normalize_usb_port(usb_port: str) -> str: + """Return *usb_port* in the exact form ``adb devices -l`` reports. + + adb prints the devpath as ``usb:`` — on Linux the sysfs bus-port name + (``usb:1-4.2``), on the macOS native backend an IOKit location ID in hex + (``usb:1A320000``). Config may write it with or without the prefix; matching + is exact string equality, so it is normalized once here. + """ + port = str(usb_port).strip() + if not port: + raise ConfigurationError("'usb_port' must not be empty") + return port if port.startswith("usb:") else f"usb:{port}" + + def _ensure_server(self) -> _SharedServer: + """Take a reference to the shared ADB server, starting it on first use. + + Lazy on purpose: an exporter whose DUTs are all powered off should not start a + server it may never need, and startup must not depend on one. + """ + if self._server is None: + self._server = _acquire_server( + self.adb_path, + self.server_port, + connect_timeout=self.connect_timeout, + adopt_existing_server=self.adopt_existing_server, + logger=self.logger, + ) + return self._server + + def _run_adb(self, args: list[str], *, check: bool = True) -> subprocess.CompletedProcess: + """Run adb against our server, bounded by ``connect_timeout``.""" + return subprocess.run( + [self.adb_path, *args], + check=check, + capture_output=True, + text=True, + timeout=self.connect_timeout, + env=_adb_env(self.server_port), + ) + + def _visible_devices(self) -> list[tuple[str, str, str | None]]: + """Return ``(serial, state, devpath)`` for every device the server can see. + + ``adb devices -l`` prints `` [usb:] [product:...] ...``; + emulators carry no ``usb:`` field at all. + """ + try: + result = self._run_adb(["devices", "-l"]) + except (subprocess.CalledProcessError, subprocess.TimeoutExpired, OSError) as e: + raise RuntimeError(f"could not list ADB devices: {e}") from e + + devices: list[tuple[str, str, str | None]] = [] + for line in result.stdout.splitlines(): + line = line.strip() + if not line or line.startswith("*") or line.startswith("List of devices"): + continue + fields = line.split() + if len(fields) < 2: + continue + devpath = next((f for f in fields[2:] if f.startswith("usb:")), None) + devices.append((fields[0], fields[1], devpath)) + return devices + + def _resolve_serial(self) -> str: + """The ADB serial to address this device by, resolved fresh on every call. + + For a ``usb_port``-configured device the serial is looked up from + ``adb devices -l``, so a power cycle that re-enumerates the device (and can + change its serial) is picked up automatically — the bench port is what stays + constant. Using the serial rather than ``-s usb:`` also keeps us on + adb's documented ``-s SERIAL`` contract. + """ + if self.serial is not None: + return self.serial + + for serial, state, devpath in self._visible_devices(): + if devpath != self.usb_port: + continue + if state != "device": + raise RuntimeError( + f"device on {self.usb_port} is '{state}', not ready. " + f"If it is unauthorized, accept the debugging prompt; if offline, power-cycle it." + ) + # A macOS native-backend quirk: when the IOKit location ID cannot be read + # adb sets devpath to the *serial* instead. Matching still works, and using + # the reported serial here is correct either way. + return serial + + raise RuntimeError( + f"no device on USB port {self.usb_port}. It may be powered off — " + f"turn on its power relay — or plugged into a different port." + ) + + def _live_forward_port(self, serial: str) -> int | None: + """The local port ADB currently forwards for *serial*, if any. + + ``adb forward --list`` prints `` tcp: tcp:`` and is the + single source of truth: forwards live in the ADB server, and they vanish with + the device. That is what makes a stale memoized port detectable. + """ + try: + result = self._run_adb(["forward", "--list"]) + except (subprocess.CalledProcessError, subprocess.TimeoutExpired, OSError) as e: + self.logger.warning("could not list adb forwards (%s)", e) + return None + + want_remote = f"tcp:{self.adbd_port}" + for line in result.stdout.splitlines(): + fields = line.split() + if len(fields) < 3 or fields[0] != serial: + continue + if not fields[1].startswith("tcp:") or fields[2] != want_remote: + continue + try: + return int(fields[1].removeprefix("tcp:")) + except ValueError: + continue + return None + + def _create_forward(self, serial: str) -> int: + """Forward this device's adbd to a free exporter port; return the chosen port. + + ``tcp:0`` asks the ADB server to pick the port, so the exporter needs no + configured port range kept clear of whatever else runs there. + """ + try: + result = self._run_adb(["-s", serial, "forward", "tcp:0", f"tcp:{self.adbd_port}"]) + except subprocess.CalledProcessError as e: + stderr = (e.stderr or "").strip() + raise RuntimeError( + f"could not forward {serial}: {stderr or e}. The device may have gone away, " + f"or adbd may not be listening on tcp:{self.adbd_port} " + f"(try `adb -s {serial} tcpip {self.adbd_port}`)." + ) from e + except subprocess.TimeoutExpired as e: + raise RuntimeError(f"forwarding {serial} timed out after {self.connect_timeout}s") from e + except OSError as e: + raise RuntimeError(f"could not run adb to forward {serial}: {e}") from e + + port = self._parse_forwarded_port(result.stdout) + if port is not None: + return port + + # Reporting the port is optional in adb's protocol — AOSP's client prints it + # only when the server sends one ("Server or device may optionally return a + # resolved TCP port number"). A silent server still created the forward, so + # ask what it bound rather than treating this as a failure. + port = self._live_forward_port(serial) + if port is None: + raise RuntimeError( + f"could not forward {serial}: adb reported no forwarded port " + f"(stdout {(result.stdout or '').strip()!r}) and `forward --list` does not show one" + ) + return port + + @staticmethod + def _parse_forwarded_port(stdout: str | None) -> int | None: + """The port `adb forward` printed, or None if it printed no usable port.""" + for line in reversed((stdout or "").strip().splitlines()): + try: + port = int(line.strip()) + except ValueError: + continue + if 0 < port < 65536: + return port + return None + + def _resolve_endpoint(self) -> tuple[str, int]: + """The exporter-side ``(host, port)`` that speaks this device's adbd. + + Resolved on every stream, which is what makes re-enumeration self-healing: + nothing is cached across a power cycle that could go stale unnoticed. + """ + with self._lock: + self._ensure_server() + if self.transport == TRANSPORT_TCP: + return self._resolve_tcp_endpoint() + return "127.0.0.1", self._ensure_forward() + + def _resolve_tcp_endpoint(self) -> tuple[str, int]: + """Connect the server to a TCP device and return the device's own endpoint. + + No forward is involved: adbd is already listening, so the stream goes straight + to it. ``adb connect`` is idempotent ("already connected to ..."), so this is + safe to run per stream. + """ + assert self.address is not None # guaranteed by _validate_config + target = self.address if ":" in self.address else f"{self.address}:{self.adbd_port}" + try: + result = self._run_adb(["connect", target], check=False) + except (subprocess.TimeoutExpired, OSError) as e: + raise RuntimeError(f"`adb connect {target}` failed: {e}") from e + + message = (result.stdout or "").strip() or (result.stderr or "").strip() + # `adb connect` exits 0 even when it fails, reporting the reason on stdout, so + # match adb's own success strings instead of the exit status. + if result.returncode != 0 or not message.startswith(("connected to", "already connected to")): + raise RuntimeError(f"could not connect to {target}: {message or 'no output'}") + self._connected = target + + host, _, port = target.rpartition(":") + return host, int(port) + + def _ensure_forward(self) -> int: + """The local port forwarding this device's adbd, creating it if needed.""" + serial = self._resolve_serial() + + if self._forward_port is not None: + if self._live_forward_port(serial) == self._forward_port: + return self._forward_port + self.logger.info( + "forward tcp:%d for %s is gone (device re-enumerated?); recreating", + self._forward_port, + serial, + ) + self._forward_port = None + + # An earlier forward for this serial from a previous stream is reusable. + existing = self._live_forward_port(serial) + self._forward_port = existing if existing is not None else self._create_forward(serial) + self.logger.info("%s forwarded on tcp:%d", serial, self._forward_port) + return self._forward_port + + @exportstream + @asynccontextmanager + async def connect(self): + """Stream this device's adbd. + + The client port-forwards this and runs ``adb connect`` against the local end, + which adds the device to whatever ADB server the client already uses. + """ + host, port = await to_thread.run_sync(self._resolve_endpoint) + self.logger.debug("streaming adbd via %s:%d", host, port) + async with await connect_tcp(remote_host=host, remote_port=port) as stream: + yield stream + + @export + def info(self) -> dict[str, str]: + """Describe this device: its transport, selector, and whether it is present.""" + result = { + "transport": self.transport, + "adbd_port": str(self.adbd_port), + } + if self.transport == TRANSPORT_TCP: + result["address"] = str(self.address) + return result + + result["selector"] = self.serial or str(self.usb_port) + try: + result["serial"] = self._resolve_serial() + result["present"] = "yes" + except RuntimeError as e: + result["present"] = "no" + result["reason"] = str(e) + return result + + def close(self): + """Drop the forward, disconnect a TCP device, and release the shared server.""" + with self._lock: + forward_port, connected = self._forward_port, self._connected + self._forward_port = self._connected = None + + if forward_port is not None: + try: + self._run_adb(["forward", "--remove", f"tcp:{forward_port}"], check=False) + except (subprocess.SubprocessError, OSError) as e: + self.logger.warning("could not remove forward tcp:%d (%s)", forward_port, e) + + if connected is not None: + try: + self._run_adb(["disconnect", connected], check=False) + except (subprocess.SubprocessError, OSError) as e: + self.logger.debug("could not disconnect %s (%s)", connected, e) + + if self._server is not None: + _release_server( + self.adb_path, + self.server_port, + connect_timeout=self.connect_timeout, + logger=self.logger, + ) + self._server = None + super().close() diff --git a/python/packages/jumpstarter-driver-adb/jumpstarter_driver_adb/driver_test.py b/python/packages/jumpstarter-driver-adb/jumpstarter_driver_adb/driver_test.py index 1112524a3..63b7b83c5 100644 --- a/python/packages/jumpstarter-driver-adb/jumpstarter_driver_adb/driver_test.py +++ b/python/packages/jumpstarter-driver-adb/jumpstarter_driver_adb/driver_test.py @@ -1,27 +1,123 @@ import subprocess +import threading +import time from unittest.mock import MagicMock, patch import pytest -from .driver import AdbServer +from . import driver as adb_driver +from .driver import AdbDevice, AdbServer from jumpstarter.common.exceptions import ConfigurationError +SERIAL = "HVA1234567" +USB_PORT = "usb:1-4.2" + + +@pytest.fixture(autouse=True) +def _reset_server_registry(): + """Clear the process-wide ADB server registry between tests. + + `_SERVERS` is module-level on purpose (one server per port per process), so a test + that leaves an entry behind would make later tests adopt it and pass or fail + depending on ordering. + """ + adb_driver._SERVERS.clear() + yield + adb_driver._SERVERS.clear() + + +class _FakeAdb: + """Stand-in for the adb binary that remembers device and forward state. + + A single canned return value cannot model this driver: it resolves a USB port to a + serial through `devices -l`, then reconciles its forward against `forward --list`. + Both have to reflect what earlier calls did. + + `forward tcp:0` allocates a port and echoes it on stdout, as real adb does — the + driver reads that number to learn where the device landed. + """ + + #: Where the fake starts handing out ports for `tcp:0`. + FIRST_PORT = 41000 + + def __init__(self, devices=((SERIAL, "device", USB_PORT),)): + #: (serial, state, devpath|None); devpath None models an emulator. + self.devices = list(devices) + self.forwards = {} # local port -> serial + self.calls = [] + self._next_port = self.FIRST_PORT + + def _devices_long(self): + lines = ["List of devices attached"] + for serial, state, devpath in self.devices: + extra = f" {devpath}" if devpath else "" + lines.append(f"{serial}\t{state}{extra} product:x model:y device:z") + return "\n".join(lines) + "\n" + + def __call__(self, argv, **kwargs): + self.calls.append(argv) + args = argv[1:] + + if args[:1] == ["version"]: + return MagicMock(stdout="Android Debug Bridge version 1.0.41", stderr="", returncode=0) + if args[:2] == ["devices", "-l"]: + return MagicMock(stdout=self._devices_long(), stderr="", returncode=0) + if args[:1] == ["devices"]: + return MagicMock(stdout="List of devices attached\n", stderr="", returncode=0) + + if "forward" in args: + serial = args[args.index("-s") + 1] if "-s" in args else "" + if "--list" in args: + lines = "".join(f"{s} tcp:{p} tcp:5555\n" for p, s in self.forwards.items()) + return MagicMock(stdout=lines, stderr="", returncode=0) + if "--remove-all" in args: + self.forwards = {p: s for p, s in self.forwards.items() if s != serial} + return MagicMock(stdout="", stderr="", returncode=0) + if "--remove" in args: + self.forwards.pop(int(args[-1].removeprefix("tcp:")), None) + return MagicMock(stdout="", stderr="", returncode=0) + local = int(args[-2].removeprefix("tcp:")) + if local == 0: + local = self._next_port + self._next_port += 1 + self.forwards[local] = serial + # Real adb prints the chosen port, and only that, for tcp:0. + return MagicMock(stdout=f"{local}\n", stderr="", returncode=0) + + return MagicMock(stdout="ok", stderr="", returncode=0) + def _mock_adb_ok(): - """Returns a mock that handles version check + auto-start during __post_init__.""" + """A mock that satisfies the version check and start-server.""" return MagicMock(stdout="ok", stderr="", returncode=0) +def _fake(**kwargs): + """Patch subprocess.run with a fresh `_FakeAdb`, returning it for assertions.""" + fake = _FakeAdb(**kwargs) + return fake, patch("subprocess.run", new=MagicMock(side_effect=fake)) + + +# ================================================================== AdbServer +# +# The server driver is now only about server lifecycle. The cuttlefish and +# androidemulator drivers embed it and call exactly these methods, so this surface is +# a compatibility contract. + + @patch("shutil.which", return_value="/usr/bin/adb") +# Without this the probe opens a real socket to 15037, so the test would depend +# on whether the machine running it happens to have an ADB server there. +@patch("socket.create_connection", side_effect=OSError("refused")) @patch("subprocess.run", return_value=_mock_adb_ok()) -def test_init_validates_adb(mock_run, mock_which): +def test_init_validates_adb(mock_run, mock_conn, mock_which): server = AdbServer() assert server.adb_path == "/usr/bin/adb" assert server.port == 15037 - # Should have called: version check + start-server (auto-start) - assert mock_run.call_count == 2 - assert mock_run.call_args_list[0][0][0] == ["/usr/bin/adb", "version"] - assert mock_run.call_args_list[1][0][0] == ["/usr/bin/adb", "start-server"] + # version check + start-server (the server driver starts eagerly) + argvs = [c.args[0] for c in mock_run.call_args_list] + assert ["/usr/bin/adb", "version"] in argvs + assert ["/usr/bin/adb", "start-server"] in argvs @patch("shutil.which", return_value=None) @@ -48,80 +144,69 @@ def test_invalid_connect_timeout(_, bad): @patch("shutil.which", return_value="/usr/bin/adb") +@patch("socket.create_connection", side_effect=OSError("refused")) @patch("subprocess.run", return_value=_mock_adb_ok()) -def test_start_server(mock_run, _): +def test_start_server(mock_run, mock_conn, _): server = AdbServer() mock_run.reset_mock() - port = server.start_server() - assert port == 15037 - call_args = mock_run.call_args_list[0] - assert call_args[0][0] == ["/usr/bin/adb", "start-server"] - assert call_args[1]["env"]["ANDROID_ADB_SERVER_PORT"] == "15037" + assert server.start_server() == 15037 + call = mock_run.call_args_list[0] + assert call.args[0] == ["/usr/bin/adb", "start-server"] + assert call.kwargs["env"]["ANDROID_ADB_SERVER_PORT"] == "15037" @patch("shutil.which", return_value="/usr/bin/adb") +@patch("socket.create_connection", side_effect=OSError("refused")) @patch("subprocess.run", return_value=_mock_adb_ok()) -def test_kill_server(mock_run, _): +def test_kill_server(mock_run, mock_conn, _): server = AdbServer() mock_run.reset_mock() - port = server.kill_server() - assert port == 15037 - call_args = mock_run.call_args_list[0] - assert call_args[0][0] == ["/usr/bin/adb", "kill-server"] + assert server.kill_server() == 15037 + assert mock_run.call_args_list[0].args[0] == ["/usr/bin/adb", "kill-server"] @patch("shutil.which", return_value="/usr/bin/adb") -@patch("subprocess.run", return_value=_mock_adb_ok()) -def test_list_devices(mock_run, _): - mock_run.side_effect = [ - _mock_adb_ok(), # version check - _mock_adb_ok(), # start-server (auto-start) - MagicMock(stdout="List of devices attached\nHVA1234567\tdevice\n", stderr="", returncode=0), - ] - server = AdbServer() - output = server.list_devices() - assert "HVA1234567" in output +@patch("socket.create_connection", side_effect=OSError("refused")) +def test_list_devices(mock_conn, _): + fake, patcher = _fake() + with patcher: + server = AdbServer() + assert SERIAL in server.list_devices() @patch("shutil.which", return_value="/usr/bin/adb") +@patch("socket.create_connection", side_effect=OSError("refused")) @patch("subprocess.run", return_value=_mock_adb_ok()) -def test_custom_port(mock_run, _): - server = AdbServer(port=5038) - assert server.port == 5038 +def test_custom_port(mock_run, mock_conn, _): + assert AdbServer(port=5038).port == 5038 @patch("shutil.which", return_value="/usr/bin/adb") +@patch("socket.create_connection", side_effect=OSError("refused")) @patch("subprocess.run", return_value=_mock_adb_ok()) -def test_init_no_auto_connect(mock_run, _): +def test_init_does_not_connect_to_any_device(mock_run, mock_conn, _): + """Startup must not reach for hardware; devices are declared, not discovered.""" AdbServer() - assert mock_run.call_count == 2 # version + start-server only + argvs = [c.args[0] for c in mock_run.call_args_list] + assert not any("connect" in argv for argv in argvs) @patch("shutil.which", return_value="/usr/bin/adb") +@patch("socket.create_connection", side_effect=OSError("refused")) @patch("subprocess.run") -def test_connect_device(mock_run, _): - mock_run.side_effect = [ - _mock_adb_ok(), # version check - _mock_adb_ok(), # start-server - MagicMock(stdout="connected to 10.0.0.1:6520\n", stderr="", returncode=0), - ] +def test_connect_device(mock_run, mock_conn, _): + mock_run.return_value = _mock_adb_ok() server = AdbServer() - mock_run.reset_mock() - # reset_mock() does not clear side_effect; clear it so return_value is used. - mock_run.side_effect = None mock_run.return_value = MagicMock(stdout="connected to 10.0.0.2:6520\n", stderr="", returncode=0) - result = server.connect_device("10.0.0.2:6520") - assert result == "connected to 10.0.0.2:6520" - assert mock_run.call_args[0][0] == ["/usr/bin/adb", "connect", "10.0.0.2:6520"] + assert server.connect_device("10.0.0.2:6520") == "connected to 10.0.0.2:6520" + assert mock_run.call_args.args[0] == ["/usr/bin/adb", "connect", "10.0.0.2:6520"] @patch("shutil.which", return_value="/usr/bin/adb") +@patch("socket.create_connection", side_effect=OSError("refused")) @patch("subprocess.run") -def test_connect_device_error(mock_run, _): - mock_run.side_effect = [ - _mock_adb_ok(), # version check - _mock_adb_ok(), # start-server - ] +def test_connect_device_error(mock_run, mock_conn, _): + mock_run.return_value = _mock_adb_ok() server = AdbServer() mock_run.side_effect = subprocess.CalledProcessError(1, "adb connect") with pytest.raises(subprocess.CalledProcessError): @@ -129,42 +214,33 @@ def test_connect_device_error(mock_run, _): @patch("shutil.which", return_value="/usr/bin/adb") +@patch("socket.create_connection", side_effect=OSError("refused")) @patch("subprocess.run") -def test_connect_device_timeout(mock_run, _): - mock_run.side_effect = [ - _mock_adb_ok(), # version check - _mock_adb_ok(), # start-server - ] +def test_connect_device_timeout(mock_run, mock_conn, _): + mock_run.return_value = _mock_adb_ok() server = AdbServer() mock_run.side_effect = subprocess.TimeoutExpired("adb connect", 30.0) with pytest.raises(TimeoutError): server.connect_device("bad:99") - assert mock_run.call_args[0][0] == ["/usr/bin/adb", "connect", "bad:99"] - assert mock_run.call_args[1]["timeout"] == server.connect_timeout + assert mock_run.call_args.kwargs["timeout"] == server.connect_timeout @patch("shutil.which", return_value="/usr/bin/adb") +@patch("socket.create_connection", side_effect=OSError("refused")) @patch("subprocess.run") -def test_disconnect_device(mock_run, _): - mock_run.side_effect = [ - _mock_adb_ok(), # version check - _mock_adb_ok(), # start-server - ] +def test_disconnect_device(mock_run, mock_conn, _): + mock_run.return_value = _mock_adb_ok() server = AdbServer() - mock_run.side_effect = None - mock_run.return_value = MagicMock(stdout="disconnected 10.0.0.1:6520\n", stderr="", returncode=0) - result = server.disconnect_device("10.0.0.1:6520") - assert "disconnected" in result - assert mock_run.call_args[0][0] == ["/usr/bin/adb", "disconnect", "10.0.0.1:6520"] + mock_run.return_value = MagicMock(stdout="disconnected 10.0.0.2:6520\n", stderr="", returncode=0) + assert server.disconnect_device("10.0.0.2:6520") == "disconnected 10.0.0.2:6520" + assert mock_run.call_args.args[0] == ["/usr/bin/adb", "disconnect", "10.0.0.2:6520"] @patch("shutil.which", return_value="/usr/bin/adb") +@patch("socket.create_connection", side_effect=OSError("refused")) @patch("subprocess.run") -def test_disconnect_device_error(mock_run, _): - mock_run.side_effect = [ - _mock_adb_ok(), # version check - _mock_adb_ok(), # start-server - ] +def test_disconnect_device_error(mock_run, mock_conn, _): + mock_run.return_value = _mock_adb_ok() server = AdbServer() mock_run.side_effect = subprocess.CalledProcessError(1, "adb disconnect") with pytest.raises(subprocess.CalledProcessError): @@ -172,15 +248,731 @@ def test_disconnect_device_error(mock_run, _): @patch("shutil.which", return_value="/usr/bin/adb") +@patch("socket.create_connection", side_effect=OSError("refused")) @patch("subprocess.run") -def test_disconnect_device_timeout(mock_run, _): - mock_run.side_effect = [ - _mock_adb_ok(), # version check - _mock_adb_ok(), # start-server - ] +def test_disconnect_device_timeout(mock_run, mock_conn, _): + mock_run.return_value = _mock_adb_ok() server = AdbServer() mock_run.side_effect = subprocess.TimeoutExpired("adb disconnect", 30.0) with pytest.raises(TimeoutError): server.disconnect_device("bad:99") - assert mock_run.call_args[0][0] == ["/usr/bin/adb", "disconnect", "bad:99"] - assert mock_run.call_args[1]["timeout"] == server.connect_timeout + + +@patch("shutil.which", return_value="/usr/bin/adb") +@patch("socket.create_connection", side_effect=OSError("refused")) +@patch("subprocess.run", return_value=_mock_adb_ok()) +def test_adbserver_keeps_the_surface_its_consumers_use(mock_run, mock_conn, _): + """The cuttlefish and androidemulator drivers embed AdbServer and call these. + + A signature-level guard: this refactor removed a lot from AdbServer, and breaking + one of these would surface as a 300s boot timeout on real hardware rather than a + test failure in this package. + """ + server = AdbServer() + for name in ( + "start_server", + "kill_server", + "connect_device", + "disconnect_device", + "list_devices", + "adb_env", + ): + assert callable(getattr(server, name)), name + assert isinstance(server.adb_path, str) + assert server.adb_env()["ANDROID_ADB_SERVER_PORT"] == "15037" + + +# ============================================== the shared, implicit ADB server +# +# An ADB server *claims* the USB devices it finds, and only one server can hold a +# given device. So on a host that already runs one, starting a second does not give +# us "another view" of the devices -- it gives us an empty one, while `start-server` +# reports success. Adopting the running server is the only way to see the hardware, +# and sharing one between drivers is a correctness requirement, not an optimisation. + + +@patch("shutil.which", return_value="/usr/bin/adb") +@patch("socket.create_connection") +@patch("subprocess.run", return_value=_mock_adb_ok()) +def test_adopts_a_server_already_on_our_port(mock_run, mock_conn, _): + """The running server owns the devices; ours would see none.""" + server = AdbServer() + assert server._owns_server is False + argvs = [c.args[0] for c in mock_run.call_args_list] + assert ["/usr/bin/adb", "start-server"] not in argvs + + +@patch("shutil.which", return_value="/usr/bin/adb") +@patch("socket.create_connection") +@patch("subprocess.run", return_value=_mock_adb_ok()) +def test_an_adopted_server_is_left_running_on_close(mock_run, mock_conn, _): + """Killing it would drop the device claims of everything else on the host.""" + server = AdbServer() + server.close() + argvs = [c.args[0] for c in mock_run.call_args_list] + assert ["/usr/bin/adb", "kill-server"] not in argvs + + +@patch("shutil.which", return_value="/usr/bin/adb") +@patch("socket.create_connection", side_effect=OSError("refused")) +@patch("subprocess.run", return_value=_mock_adb_ok()) +def test_starts_a_server_when_the_port_is_free(mock_run, mock_conn, _): + server = AdbServer() + assert server._owns_server is True + argvs = [c.args[0] for c in mock_run.call_args_list] + assert ["/usr/bin/adb", "start-server"] in argvs + + +@patch("shutil.which", return_value="/usr/bin/adb") +@patch("socket.create_connection") +@patch("subprocess.run", return_value=_mock_adb_ok()) +def test_a_server_we_started_is_killed_on_close(mock_run, mock_conn, _): + server = AdbServer(adopt_existing_server=False) + assert server._owns_server is True + server.close() + argvs = [c.args[0] for c in mock_run.call_args_list] + assert ["/usr/bin/adb", "kill-server"] in argvs + + +@patch("shutil.which", return_value="/usr/bin/adb") +@patch("socket.create_connection") +def test_a_non_adb_listener_is_not_adopted(mock_conn, _): + """A plain TCP listener hangs the probe rather than failing it. + + Verified against adb 1.0.41: `start-server` and `devices` both block forever + against a non-ADB listener. Adopting it would wedge every later call, so we + decline and fall through to starting our own. + """ + calls = [] + + def run(argv, **kwargs): + calls.append(argv) + if argv[1:] == ["devices"] and kwargs.get("check") is False: + # The probe: the socket accepted, but nothing answers as ADB. + raise subprocess.TimeoutExpired("adb devices", 10) + return _mock_adb_ok() + + with patch("subprocess.run", side_effect=run): + server = AdbServer() + + # Declined the adoption, so it started its own and owns it. + assert server._owns_server is True + assert ["/usr/bin/adb", "start-server"] in calls + + +@patch("shutil.which", return_value="/usr/bin/adb") +@patch("socket.create_connection") +def test_the_adoption_probe_asks_the_server_not_the_client(mock_conn, _): + """The probe has to be a command the server answers. + + `adb version` reports the local client's own version without contacting the + server at all — verified against adb 1.0.41, where it exits 0 with zero + connections to the port. Probing with it would adopt any listener. + """ + calls = [] + + def run(argv, **kwargs): + calls.append((argv, kwargs.get("check"))) + return _mock_adb_ok() + + with patch("subprocess.run", side_effect=run): + server = AdbServer() + + probes = [argv for argv, check in calls if check is False] + assert probes == [["/usr/bin/adb", "devices"]] + # It answered, so the running server was adopted and left alone. + assert server._owns_server is False + assert ["/usr/bin/adb", "start-server"] not in [argv for argv, _ in calls] + + +@patch("shutil.which", return_value="/usr/bin/adb") +@patch("socket.create_connection", side_effect=OSError("refused")) +def test_start_server_survives_a_hung_port(mock_conn, _): + """`adb start-server` blocks forever on a non-ADB listener; bound it.""" + + def run(argv, **kwargs): + if argv[1:] == ["start-server"]: + assert kwargs.get("timeout"), "start-server must be bounded" + raise subprocess.TimeoutExpired("adb start-server", 30.0) + return _mock_adb_ok() + + with patch("subprocess.run", side_effect=run): + server = AdbServer() # must not hang or raise + assert server.port == 15037 + + +@patch("shutil.which", return_value="/usr/bin/adb") +@patch("socket.create_connection", side_effect=OSError("refused")) +def test_list_devices_is_bounded(mock_conn, _): + """`adb devices` hangs forever on a non-ADB listener.""" + + def run(argv, **kwargs): + if "devices" in argv: + assert kwargs.get("timeout"), "devices must be bounded" + raise subprocess.TimeoutExpired("adb devices", 30.0) + return _mock_adb_ok() + + with patch("subprocess.run", side_effect=run): + server = AdbServer() + assert "Error" in server.list_devices() # reported, not raised + + +@patch("shutil.which", return_value="/usr/bin/adb") +@patch("socket.create_connection", side_effect=OSError("refused")) +def test_two_devices_share_one_server(mock_conn, _): + """Two servers on one port would split the USB device claims. + + The second server would see an empty device list while `adb start-server` + reported success, so the driver would come up blind. + """ + fake, patcher = _fake() + with patcher: + a = AdbDevice(usb_port="1-4.2") + b = AdbDevice(usb_port="1-4.3") + a._ensure_server() + b._ensure_server() + starts = [c for c in fake.calls if c[1:] == ["start-server"]] + assert len(starts) == 1, f"expected one start-server, got {starts}" + assert a._server is b._server + + +@patch("shutil.which", return_value="/usr/bin/adb") +@patch("socket.create_connection", side_effect=OSError("refused")) +def test_no_server_is_started_at_construction(mock_conn, _): + """A bench whose DUTs are all powered off should not start a server it never uses. + + Startup must not depend on the ADB server either, so it is acquired lazily on the + first stream instead. + """ + fake, patcher = _fake() + with patcher: + AdbDevice(usb_port="1-4.2") + assert not [c for c in fake.calls if c[1:] == ["start-server"]] + assert not adb_driver._SERVERS + + +@patch("shutil.which", return_value="/usr/bin/adb") +@patch("socket.create_connection", side_effect=OSError("refused")) +def test_the_last_device_to_close_kills_the_server(mock_conn, _): + """Refcounted: closing one device must not pull the server out from the other.""" + fake, patcher = _fake() + with patcher: + a = AdbDevice(usb_port="1-4.2") + b = AdbDevice(usb_port="1-4.3") + a._ensure_server() + b._ensure_server() + + a.close() + assert not [c for c in fake.calls if c[1:] == ["kill-server"]], "killed while still in use" + + b.close() + assert [c for c in fake.calls if c[1:] == ["kill-server"]], "not killed after last release" + + +@patch("shutil.which", return_value="/usr/bin/adb") +@patch("socket.create_connection") +def test_an_adopted_server_is_not_killed_by_a_device(mock_conn, _): + """It owns other processes' device claims.""" + fake, patcher = _fake() + with patcher: + device = AdbDevice(usb_port="1-4.2") + device._ensure_server() + assert device._server is not None and device._server.owns is False + device.close() + assert not [c for c in fake.calls if c[1:] == ["kill-server"]] + + +@patch("shutil.which", return_value="/usr/bin/adb") +@patch("socket.create_connection", side_effect=OSError("refused")) +def test_a_declared_server_and_a_device_share_one_server(mock_conn, _): + """The androidemulator/cuttlefish coexistence case. + + Both declare an AdbServer on an explicit port; a co-located AdbDevice must adopt + that one rather than starting a second. + """ + fake, patcher = _fake() + with patcher: + server = AdbServer(port=15037) + device = AdbDevice(usb_port="1-4.2", server_port=15037) + device._ensure_server() + assert device._server is server._server + assert len([c for c in fake.calls if c[1:] == ["start-server"]]) == 1 + + +@patch("shutil.which", return_value="/usr/bin/adb") +@patch("socket.create_connection", side_effect=OSError("refused")) +def test_different_server_ports_get_independent_servers(mock_conn, _): + """The registry is keyed per (adb_path, port).""" + fake, patcher = _fake() + with patcher: + a = AdbDevice(usb_port="1-4.2", server_port=15037) + b = AdbDevice(usb_port="1-4.3", server_port=15038) + a._ensure_server() + b._ensure_server() + assert a._server is not b._server + assert len([c for c in fake.calls if c[1:] == ["start-server"]]) == 2 + + +# ================================================================== AdbDevice +# +# One declared device per driver instance. Identity for USB is the BENCH PORT, not the +# serial, so hardware can be swapped between benches without a config change. + + +@patch("shutil.which", return_value="/usr/bin/adb") +@patch("socket.create_connection", side_effect=OSError("refused")) +def test_usb_port_is_normalized(mock_conn, _): + """adb matches the devpath by exact string equality, including the `usb:` prefix.""" + fake, patcher = _fake() + with patcher: + assert AdbDevice(usb_port="1-4.2").usb_port == "usb:1-4.2" + assert AdbDevice(usb_port="usb:1-4.2").usb_port == "usb:1-4.2" + + +@patch("shutil.which", return_value="/usr/bin/adb") +@patch("socket.create_connection", side_effect=OSError("refused")) +def test_usb_port_resolves_to_a_serial_and_forwards(mock_conn, _): + """The whole USB path: bench port -> current serial -> forward -> endpoint.""" + fake, patcher = _fake() + with patcher: + device = AdbDevice(usb_port="1-4.2") + assert device._resolve_endpoint() == ("127.0.0.1", _FakeAdb.FIRST_PORT) + + created = [c for c in fake.calls if "forward" in c and "--list" not in c] + # The documented `-s SERIAL` selector, resolved from the port. Deliberately NOT + # `-s usb:1-4.2`: that works (adb's MatchesTarget falls through to the devpath) + # but it is undocumented, and we do not need it. + assert created[0][1:] == ["-s", SERIAL, "forward", "tcp:0", "tcp:5555"] + assert not any("usb:" in arg for call in fake.calls for arg in call) + + +@patch("shutil.which", return_value="/usr/bin/adb") +@patch("socket.create_connection", side_effect=OSError("refused")) +def test_an_absent_device_is_an_actionable_error(mock_conn, _): + """A DUT powered off by its relay is normal, not a crash — but say so clearly.""" + fake, patcher = _fake(devices=()) + with patcher: + device = AdbDevice(usb_port="1-4.2") + with pytest.raises(RuntimeError, match="no device on USB port usb:1-4.2"): + device._resolve_endpoint() + + +@patch("shutil.which", return_value="/usr/bin/adb") +@patch("socket.create_connection", side_effect=OSError("refused")) +def test_a_device_that_appears_later_needs_no_restart(mock_conn, _): + """Powering the DUT on mid-lease must just work; endpoints resolve per call.""" + fake = _FakeAdb(devices=()) + with patch("subprocess.run", new=MagicMock(side_effect=fake)): + device = AdbDevice(usb_port="1-4.2") + with pytest.raises(RuntimeError): + device._resolve_endpoint() + + fake.devices = [(SERIAL, "device", USB_PORT)] # relay powers it on + assert device._resolve_endpoint() == ("127.0.0.1", _FakeAdb.FIRST_PORT) + + +@patch("shutil.which", return_value="/usr/bin/adb") +@patch("socket.create_connection", side_effect=OSError("refused")) +def test_reenumeration_changes_the_serial_but_not_the_port(mock_conn, _): + """The core regression this design exists to prevent. + + A relay power-cycle re-enumerates USB and can hand the device a different ADB + serial. Because config names the bench PORT, the driver re-resolves and forwards + against the new serial with no config edit and no restart. + """ + fake = _FakeAdb() + with patch("subprocess.run", new=MagicMock(side_effect=fake)): + device = AdbDevice(usb_port="1-4.2") + assert device._resolve_endpoint() == ("127.0.0.1", _FakeAdb.FIRST_PORT) + + # Power cycle: same bench port, new serial, and the old forward is gone. + fake.devices = [("NEWSERIAL999", "device", USB_PORT)] + fake.forwards.clear() + + host, port = device._resolve_endpoint() + assert (host, port) == ("127.0.0.1", _FakeAdb.FIRST_PORT + 1) + assert fake.forwards[port] == "NEWSERIAL999" + + created = [c for c in fake.calls if "forward" in c and "--list" not in c and "--remove" not in c] + assert created[-1][1:3] == ["-s", "NEWSERIAL999"] + + +@patch("shutil.which", return_value="/usr/bin/adb") +@patch("socket.create_connection", side_effect=OSError("refused")) +def test_a_live_forward_is_reused(mock_conn, _): + """Streams are per-connection; re-forwarding on each would churn the ADB server.""" + fake, patcher = _fake() + with patcher: + device = AdbDevice(usb_port="1-4.2") + first = device._resolve_endpoint() + assert device._resolve_endpoint() == first + created = [c for c in fake.calls if "forward" in c and "--list" not in c and "--remove" not in c] + assert len(created) == 1, created + + +@patch("shutil.which", return_value="/usr/bin/adb") +@patch("socket.create_connection", side_effect=OSError("refused")) +def test_a_stale_memoized_forward_is_recreated(mock_conn, _): + """Forwards live in the ADB server and vanish with the device. + + Trusting memory made attach report success while creating no forward, so the + client tunnelled to a dead port and the device sat `offline` with no error + anywhere. Observed on hardware. + """ + fake = _FakeAdb() + with patch("subprocess.run", new=MagicMock(side_effect=fake)): + device = AdbDevice(usb_port="1-4.2") + device._resolve_endpoint() + fake.forwards.clear() # e.g. `adb forward --remove-all`, or a server restart + _, port = device._resolve_endpoint() + assert port in fake.forwards + + +@patch("shutil.which", return_value="/usr/bin/adb") +@patch("socket.create_connection", side_effect=OSError("refused")) +def test_an_unauthorized_device_is_reported_not_forwarded(mock_conn, _): + """`offline`/`unauthorized` cannot be forwarded; say which it is.""" + fake, patcher = _fake(devices=((SERIAL, "unauthorized", USB_PORT),)) + with patcher: + device = AdbDevice(usb_port="1-4.2") + with pytest.raises(RuntimeError, match="unauthorized"): + device._resolve_endpoint() + + +@patch("shutil.which", return_value="/usr/bin/adb") +@patch("socket.create_connection", side_effect=OSError("refused")) +def test_an_explicit_serial_skips_the_port_lookup(mock_conn, _): + """`serial` is the escape hatch for hardware with no usable devpath.""" + fake, patcher = _fake() + with patcher: + device = AdbDevice(serial=SERIAL) + device._resolve_endpoint() + assert not [c for c in fake.calls if c[1:3] == ["devices", "-l"]] + + +@patch("shutil.which", return_value="/usr/bin/adb") +@patch("socket.create_connection", side_effect=OSError("refused")) +def test_a_macos_hex_devpath_matches(mock_conn, _): + """The macOS native backend reports an IOKit location ID, not a port path.""" + fake, patcher = _fake(devices=((SERIAL, "device", "usb:1A320000"),)) + with patcher: + device = AdbDevice(usb_port="1A320000") + assert device._resolve_serial() == SERIAL + + +@patch("shutil.which", return_value="/usr/bin/adb") +@patch("socket.create_connection", side_effect=OSError("refused")) +def test_a_devpath_that_equals_the_serial_still_matches(mock_conn, _): + """A documented macOS native-backend quirk. + + When the location ID cannot be read, adb sets devpath to the *serial* + (`if (devpath.empty()) { devpath = serial; }`). Matching must still work, and must + not select some other device. + """ + fake, patcher = _fake( + devices=( + ("OTHER", "device", "usb:1-1"), + (SERIAL, "device", f"usb:{SERIAL}"), + ) + ) + with patcher: + device = AdbDevice(usb_port=SERIAL) + assert device._resolve_serial() == SERIAL + + +@patch("shutil.which", return_value="/usr/bin/adb") +@patch("socket.create_connection", side_effect=OSError("refused")) +def test_an_emulator_without_a_devpath_is_not_matched(mock_conn, _): + """Emulator lines carry no `usb:` field, so they must never match a bench port.""" + fake, patcher = _fake(devices=(("emulator-5554", "device", None),)) + with patcher: + device = AdbDevice(usb_port="1-4.2") + with pytest.raises(RuntimeError, match="no device on USB port"): + device._resolve_serial() + + +@patch("shutil.which", return_value="/usr/bin/adb") +@patch("socket.create_connection", side_effect=OSError("refused")) +def test_a_silent_forward_falls_back_to_the_forward_list(mock_conn, _): + """Reporting the chosen port is optional in adb's protocol. + + AOSP's client prints it only when the server sends one ("Server or device may + optionally return a resolved TCP port number"), so a server that stays silent + still created the forward and still exits 0. + """ + fake = _FakeAdb() + real = fake.__call__ + + def silent(argv, **kwargs): + result = real(argv, **kwargs) + args = argv[1:] + if "forward" in args and "--list" not in args and "--remove" not in args: + return MagicMock(stdout="", stderr="", returncode=0) + return result + + with patch("subprocess.run", side_effect=silent): + device = AdbDevice(usb_port="1-4.2") + assert device._resolve_endpoint() == ("127.0.0.1", _FakeAdb.FIRST_PORT) + + +@patch("shutil.which", return_value="/usr/bin/adb") +@patch("socket.create_connection", side_effect=OSError("refused")) +def test_a_forward_with_no_discoverable_port_is_an_error(mock_conn, _): + """Better to fail than to hand the client a port that cannot exist.""" + fake, patcher = _fake() + with patcher: + device = AdbDevice(usb_port="1-4.2") + + def blank(argv, **kwargs): + args = argv[1:] + if args[:2] == ["devices", "-l"]: + return MagicMock(stdout=fake._devices_long(), stderr="", returncode=0) + return MagicMock(stdout="", stderr="", returncode=0) + + with patch("subprocess.run", side_effect=blank): + with pytest.raises(RuntimeError, match="reported no forwarded port"): + device._resolve_endpoint() + + +@patch("shutil.which", return_value="/usr/bin/adb") +@patch("socket.create_connection", side_effect=OSError("refused")) +def test_forward_failure_mentions_adb_tcpip(mock_conn, _): + """A device whose adbd is not on TCP is the common failure; say what to do.""" + fake, patcher = _fake() + with patcher: + device = AdbDevice(usb_port="1-4.2") + + def failing(argv, **kwargs): + args = argv[1:] + if args[:2] == ["devices", "-l"]: + return MagicMock(stdout=fake._devices_long(), stderr="", returncode=0) + if "forward" in args and "--list" in args: + return MagicMock(stdout="", stderr="", returncode=0) + if "forward" in args: + raise subprocess.CalledProcessError(1, "adb", stderr="cannot bind") + return _mock_adb_ok() + + with patch("subprocess.run", side_effect=failing): + with pytest.raises(RuntimeError, match="tcpip"): + device._resolve_endpoint() + + +@patch("shutil.which", return_value="/usr/bin/adb") +@patch("socket.create_connection", side_effect=OSError("refused")) +def test_concurrent_streams_create_one_forward(mock_conn, _): + """Streams are opened per client connection, and adb calls run in worker threads. + + Two concurrent resolutions must not each create a forward: the second would + silently strand the first client's port. + """ + fake = _FakeAdb() + real = fake.__call__ + started = threading.Event() + + def slow(argv, **kwargs): + # Hold the first forward-creation open long enough that the other threads are + # definitely inside _resolve_endpoint waiting on the lock. A Barrier cannot be + # used here: the lock means only one thread ever reaches this point, so a + # barrier of 4 would deadlock rather than test anything. + if "forward" in argv and "--list" not in argv and "--remove" not in argv: + started.set() + time.sleep(0.2) + return real(argv, **kwargs) + + with patch("subprocess.run", new=MagicMock(side_effect=slow)): + device = AdbDevice(usb_port="1-4.2") + results = [] + results_lock = threading.Lock() + + def resolve(): + endpoint = device._resolve_endpoint() + with results_lock: + results.append(endpoint) + + threads = [threading.Thread(target=resolve) for _ in range(4)] + for t in threads: + t.start() + assert started.wait(timeout=10), "no forward was ever created" + for t in threads: + t.join(timeout=30) + + assert len(results) == 4, f"a thread did not finish: {results}" + assert len(set(results)) == 1, f"streams disagree on the endpoint: {results}" + assert len(fake.forwards) == 1, f"more than one forward: {fake.forwards}" + + +@patch("shutil.which", return_value="/usr/bin/adb") +@patch("socket.create_connection", side_effect=OSError("refused")) +def test_close_removes_the_forward_and_releases_the_server(mock_conn, _): + fake, patcher = _fake() + with patcher: + device = AdbDevice(usb_port="1-4.2") + device._resolve_endpoint() + assert fake.forwards + device.close() + assert not fake.forwards + assert not adb_driver._SERVERS + + +@patch("shutil.which", return_value="/usr/bin/adb") +@patch("socket.create_connection", side_effect=OSError("refused")) +def test_teardown_completes_when_forward_removal_hangs(mock_conn, _): + """An unresponsive ADB server must not be able to wedge close().""" + fake, patcher = _fake() + with patcher: + device = AdbDevice(usb_port="1-4.2") + device._resolve_endpoint() + + def run(argv, **kwargs): + if "--remove" in argv: + raise subprocess.TimeoutExpired("adb forward --remove", 30.0) + return _mock_adb_ok() + + with patch("subprocess.run", side_effect=run): + device.close() # must not raise + assert device._forward_port is None + + +@patch("shutil.which", return_value="/usr/bin/adb") +@patch("socket.create_connection", side_effect=OSError("refused")) +def test_info_reports_presence(mock_conn, _): + fake = _FakeAdb() + with patch("subprocess.run", new=MagicMock(side_effect=fake)): + device = AdbDevice(usb_port="1-4.2") + info = device.info() + assert info["transport"] == "usb" + assert info["selector"] == USB_PORT + assert info["serial"] == SERIAL + assert info["present"] == "yes" + + fake.devices = [] + absent = device.info() + assert absent["present"] == "no" + assert "powered off" in absent["reason"] + + +# ------------------------------------------------------------- transport: tcp + + +@patch("shutil.which", return_value="/usr/bin/adb") +@patch("socket.create_connection", side_effect=OSError("refused")) +def test_tcp_transport_connects_and_creates_no_forward(mock_conn, _): + """adbd already listens on the DUT, so there is nothing to forward.""" + + def run(argv, **kwargs): + if argv[1:2] == ["connect"]: + return MagicMock(stdout="connected to 10.0.0.5:5555\n", stderr="", returncode=0) + return _mock_adb_ok() + + with patch("subprocess.run", side_effect=run) as mock_run: + device = AdbDevice(transport="tcp", address="10.0.0.5:5555") + assert device._resolve_endpoint() == ("10.0.0.5", 5555) + argvs = [c.args[0] for c in mock_run.call_args_list] + assert ["/usr/bin/adb", "connect", "10.0.0.5:5555"] in argvs + assert not any("forward" in argv for argv in argvs) + + +@patch("shutil.which", return_value="/usr/bin/adb") +@patch("socket.create_connection", side_effect=OSError("refused")) +def test_tcp_address_without_a_port_uses_adbd_port(mock_conn, _): + def run(argv, **kwargs): + if argv[1:2] == ["connect"]: + return MagicMock(stdout="connected to 10.0.0.5:5555\n", stderr="", returncode=0) + return _mock_adb_ok() + + with patch("subprocess.run", side_effect=run) as mock_run: + device = AdbDevice(transport="tcp", address="10.0.0.5") + assert device._resolve_endpoint() == ("10.0.0.5", 5555) + assert ["/usr/bin/adb", "connect", "10.0.0.5:5555"] in [c.args[0] for c in mock_run.call_args_list] + + +@patch("shutil.which", return_value="/usr/bin/adb") +@patch("socket.create_connection", side_effect=OSError("refused")) +def test_tcp_connect_failure_is_detected_despite_exit_zero(mock_conn, _): + """`adb connect` returns 0 on failure and reports it on stdout.""" + + def run(argv, **kwargs): + if argv[1:2] == ["connect"]: + return MagicMock(stdout="failed to connect to 10.0.0.5:5555\n", stderr="", returncode=0) + return _mock_adb_ok() + + with patch("subprocess.run", side_effect=run): + device = AdbDevice(transport="tcp", address="10.0.0.5:5555") + with pytest.raises(RuntimeError, match="could not connect"): + device._resolve_endpoint() + + +@patch("shutil.which", return_value="/usr/bin/adb") +@patch("socket.create_connection", side_effect=OSError("refused")) +def test_tcp_close_disconnects(mock_conn, _): + def run(argv, **kwargs): + if argv[1:2] == ["connect"]: + return MagicMock(stdout="connected to 10.0.0.5:5555\n", stderr="", returncode=0) + return _mock_adb_ok() + + with patch("subprocess.run", side_effect=run) as mock_run: + device = AdbDevice(transport="tcp", address="10.0.0.5:5555") + device._resolve_endpoint() + device.close() + assert ["/usr/bin/adb", "disconnect", "10.0.0.5:5555"] in [c.args[0] for c in mock_run.call_args_list] + + +# ---------------------------------------------------------- config validation + + +@patch("shutil.which", return_value="/usr/bin/adb") +def test_usb_needs_exactly_one_selector(_): + with pytest.raises(ConfigurationError, match="exactly one"): + AdbDevice() + with pytest.raises(ConfigurationError, match="exactly one"): + AdbDevice(usb_port="1-4.2", serial=SERIAL) + + +@patch("shutil.which", return_value="/usr/bin/adb") +def test_transport_field_mismatches_are_rejected(_): + """Silently ignoring a field the transport cannot use hides a config mistake.""" + with pytest.raises(ConfigurationError, match="only applies to transport: tcp"): + AdbDevice(usb_port="1-4.2", address="10.0.0.5") + with pytest.raises(ConfigurationError, match="only apply to transport: usb"): + AdbDevice(transport="tcp", address="10.0.0.5", usb_port="1-4.2") + with pytest.raises(ConfigurationError, match="needs 'address'"): + AdbDevice(transport="tcp") + + +@pytest.mark.parametrize( + ("transport", "expected"), + [ + ("serial", "no serial/UART transport"), + ("uart", "no serial/UART transport"), + ("vsock", "not implemented"), + ("emulator", "androidemulator"), + ], +) +@patch("shutil.which", return_value="/usr/bin/adb") +def test_unsupported_transports_say_what_to_do_instead(_, transport, expected): + """A bare "unknown transport" sends people looking for a typo. + + Serial is the one people will reach for: adb genuinely has no UART transport, so + the message has to point at the actual route rather than imply a spelling error. + """ + with pytest.raises(ConfigurationError, match=expected): + AdbDevice(transport=transport, usb_port="1-4.2") + + +@patch("shutil.which", return_value="/usr/bin/adb") +def test_unknown_transport_lists_the_supported_ones(_): + with pytest.raises(ConfigurationError, match="usb/tcp"): + AdbDevice(transport="carrier-pigeon", usb_port="1-4.2") + + +@pytest.mark.parametrize("bad", [0, -1, 70000, True, "5555", None]) +@patch("shutil.which", return_value="/usr/bin/adb") +def test_invalid_adbd_port(_, bad): + with pytest.raises(ConfigurationError, match="adbd_port"): + AdbDevice(usb_port="1-4.2", adbd_port=bad) + + +@patch("shutil.which", return_value="/usr/bin/adb") +def test_empty_usb_port_is_rejected(_): + with pytest.raises(ConfigurationError, match="usb_port"): + AdbDevice(usb_port=" ") diff --git a/python/packages/jumpstarter-driver-adb/pyproject.toml b/python/packages/jumpstarter-driver-adb/pyproject.toml index 23764530e..ead63df39 100644 --- a/python/packages/jumpstarter-driver-adb/pyproject.toml +++ b/python/packages/jumpstarter-driver-adb/pyproject.toml @@ -33,7 +33,12 @@ addopts = "--cov --cov-report=html --cov-report=xml" log_cli = true log_cli_level = "INFO" testpaths = ["jumpstarter_driver_adb"] -asyncio_mode = "auto" +# No `asyncio_mode`: pytest-asyncio is not a dependency here, so the setting was +# dead ("Unknown config option") while implying a bare `async def test_` would run. +# Async tests in this repo use `@pytest.mark.anyio` with an `anyio_backend` fixture +# -- see packages/jumpstarter/conftest.py. pytest fails an unmarked coroutine test +# outright ("async def functions are not natively supported"), so no extra guard is +# needed; the misleading setting was the whole problem. [build-system] requires = ["hatchling", "hatch-vcs", "hatch-pin-jumpstarter"]