From 9eb637e6eb1e17ef6a769a1f0af1aee4e3921504 Mon Sep 17 00:00:00 2001 From: Gareth Morgan Date: Thu, 8 Oct 2026 19:30:31 -0400 Subject: [PATCH 1/2] test(cloudxr): real-browser run_oob_connect() tests against MockCloudXR, with anticipatory reliability-branch coverage Real-browser integration tests for run_oob_connect(), standardized on MockCloudXR (npm run build:app-mock) rather than the real @nvidia/cloudxr SDK, always against a static production build (never a dev-server, to avoid webpack HMR module-duplication and React.StrictMode double-session bugs found during development). Includes anticipatory tests written against #1145/#1146 before they merged, so they fail cleanly until those branches land. Design/decisions are in oob-real-browser-integration-test-plan.md. Rebased onto #1147's current tip (gmorgan/MockADB) after #1147's own history was rewritten by its rebase onto main; squashed from the original 7-commit history to avoid replaying commits whose content already landed on #1147 under different SHAs. Co-Authored-By: Claude Sonnet 5 Signed-off-by: Gareth Morgan --- .../tests/mock/cloudxr-mock-alias.ts | 13 +- deps/cloudxr/webxr_client/webpack.app-mock.js | 13 +- oob-real-browser-integration-test-plan.md | 404 ++++++++++++++++++ tests/python/core/cloudxr/conftest.py | 297 ++++++++++++- tests/python/core/cloudxr/pyproject.toml | 2 + .../core/cloudxr/test_oob_teleop_adb.py | 300 +++++++++++++ 6 files changed, 1023 insertions(+), 6 deletions(-) create mode 100644 oob-real-browser-integration-test-plan.md diff --git a/deps/cloudxr/webxr_client/tests/mock/cloudxr-mock-alias.ts b/deps/cloudxr/webxr_client/tests/mock/cloudxr-mock-alias.ts index 4b1cfd5cca..0ec9edb4ae 100644 --- a/deps/cloudxr/webxr_client/tests/mock/cloudxr-mock-alias.ts +++ b/deps/cloudxr/webxr_client/tests/mock/cloudxr-mock-alias.ts @@ -58,13 +58,20 @@ export function createSession( declare global { interface Window { - __mockCloudXRFail?: (message?: string) => void; + __mockCloudXRFail?: (message?: string, code?: number) => void; __mockCloudXRConnectDelayMs?: number; } } if (typeof window !== 'undefined') { - window.__mockCloudXRFail = (message = 'Mock transient server/tunnel outage') => { - activeSession?.triggerFailure({ name: 'StreamingError', message }); + // code is optional and omitted by default (a recoverable failure, matching the SDK's + // own default): CloudXRComponent.tsx's onStreamStopped auto-reconnects on a recoverable + // error without ever showing #errorMessageBox, exactly as it would for a real transient + // failure. Pass a code in the 0xc0f22300-0xc0f223ff (server-disconnect) range - see + // helpers/streamingErrorClassification.ts's isRecoverable() - for a genuine terminal + // error that surfaces on the DOM, matching CloudXRComponentTest.tsx's own + // NON_RETRYABLE_CODE steps. + window.__mockCloudXRFail = (message = 'Mock transient server/tunnel outage', code) => { + activeSession?.triggerFailure({ name: 'StreamingError', message, code }); }; } diff --git a/deps/cloudxr/webxr_client/webpack.app-mock.js b/deps/cloudxr/webxr_client/webpack.app-mock.js index 7cfaaa943f..9d5b96b6c9 100644 --- a/deps/cloudxr/webxr_client/webpack.app-mock.js +++ b/deps/cloudxr/webxr_client/webpack.app-mock.js @@ -29,8 +29,17 @@ const { merge } = require('webpack-merge'); const common = require('./webpack.common.js'); module.exports = merge(common, { - mode: 'development', - devtool: 'eval-source-map', + // production, not development: src/index.tsx wraps the app in + // , which double-invokes effects in a dev build - that calls + // CloudXRComponent.tsx's establishSession() (and so CloudXR.createSession()) + // twice, leaving tests/mock/cloudxr-mock-alias.ts's activeSession pointing at + // whichever MockCloudXR instance was created last while the mounted component + // may be running against the other one. window.__mockCloudXRFail() then + // silently no-ops against a session nothing is actually using. A production + // build's NODE_ENV=production makes React skip StrictMode's double-invoke + // entirely, so this is the actual fix, not a workaround. + mode: 'production', + devtool: 'source-map', resolve: { alias: { // Exact match ($) only: the shim itself (and MockCloudXR.ts) reach the real SDK via the diff --git a/oob-real-browser-integration-test-plan.md b/oob-real-browser-integration-test-plan.md new file mode 100644 index 0000000000..6564da9d45 --- /dev/null +++ b/oob-real-browser-integration-test-plan.md @@ -0,0 +1,404 @@ + + +# OOB real-browser integration test — plan + +Goal: exercise `run_oob_connect()`'s real orchestration logic against a real +Chromium instance running the real teleop web client (real IWER-emulated +WebXR, real CDP, real client JS), instead of today's fully-scripted +`mock_adb()` / fake-CDP-server unit tests (`gmorgan/oob-connect-tests`, +PR #1148). Those stay as fast unit coverage of the orchestration logic; this +is new, slower, additional integration coverage. + +**Revised: a minimal mock CloudXR runtime is required after all.** §3/§5 +originally concluded none was needed, reasoning that `oob_teleop_adb.py` +never talks to the runtime directly. That's still true, but it missed a +second-order effect: `_cdp_session_click_connect()` (`oob_teleop_adb.py:1584`) +raises `OobAdbError(f"Teleop connection failed: {error_text}")` if the real +client's `#errorMessageBox` shows an explicit error within 30s of the click. +With nothing listening on `backend_port`, `wss.py`'s `proxy_handler()` +accepts the client's signaling WS handshake, then immediately fails to reach +the backend and closes the connection out from under it — the real client +will very likely surface that as a real error, which means +`run_oob_connect()` would raise instead of returning a monitor task, for +real client-behavior reasons, not a gap in the OOB code itself. + +So: something needs to listen on `backend_port` and respond with synthetic +data — not to make signaling "succeed" in any protocol-complete sense, just +enough that the client doesn't explicitly error out within the click-wait +window. Scope stays minimal and empirical: start with the cheapest thing +that might work (accept the WS, hold it open, respond to nothing) and only +add real SDP/ICE handling if the real client's own error/timeout behavior +requires it. Full WebRTC media (real rendering) remains explicitly out of +scope / a separate future project — this is only "enough to not error." + +## 0. Confirmed architecture (superseded — see §8 for what's actually implemented) + +Four real components, one mock: + +1. **Real webxr client** (`App.tsx`, `npm run dev-server`, port 8080) in a + real desktop Chromium — not `MockCloudXR.ts`. Uses real IWER to emulate a + WebXR/OpenXR-backed headset client-side (`loadIWERIfNeeded`). +2. **Real `wss.py` + `OOBControlHub`**, running for real (self-signed cert, + hub wired in), pointed at an unused `backend_port` (49100). No CloudXR + runtime behind it — the resulting connection refusal is handled by + existing production code, not test infra. Required so both the CloudXR + SDK *and* the OOB control channel (`wss://{serverIP}:{port}/oob/v1/ws`) + have a real `serverIP:port` to resolve to. +3. **Real DevTools/CDP** on that real Chromium instance — nothing to fake. +4. **Mock ADB** — the one actual test double, since no physical Android + device exists. Must stay controllable/parametrized like today's + `FakeAdb(device_state=, am_start_rc=, devtools_socket=, forward_rc=)`, so + tests can still deliberately trigger the real error paths (unauthorized/ + offline device, `am start` failure, no devtools socket, forward failure) + — but the success-path commands (`am start`, `adb forward`) now drive + the real browser/CDP port instead of returning canned output (see §3). + +## 1. Exhaustive inventory: everything the OOB code talks to + +### A. ADB (device/OS layer) — driven directly by `oob_teleop_adb.py` + +Read-only queries: + +| adb command | Function | Used for | +|---|---|---| +| `adb get-state` | `adb_device_state()` → `assert_adb_device_online()` | Preflight gate before nearly every other call; drives offline→`adb reconnect` retry and user-facing error text | +| `adb devices` | `assert_exactly_one_adb_device()` | Pins a single device (or validates `ANDROID_SERIAL`) | +| `adb shell ip -o -4 addr show` | `headset_non_loopback_interfaces()` | USB-local WiFi-presence check (`require_headset_non_loopback_network()`); polled every 5s by `monitor_headset_wifi()` during a session | +| `adb shell dumpsys power` | `headset_wakefulness()` | `mWakefulness=` regex; polled by `assert_headset_awake()` after sending a wake key | +| `adb shell getprop ro.product.manufacturer` / `ro.product.brand` | `_adb_getprop()` → `headset_browser_package()` | Vendor string selects which browser package to force | +| `adb shell pm list packages ` | `_adb_pkg_installed()` / `_first_installed_pkg()` | Confirms the chosen vendor browser is actually installed | +| `adb shell cat /proc/net/unix` | `_discover_devtools_socket()` | Regex-scans for `@..._devtools_remote[_pid]`; priority order `weblayer` > `com.oculus.browser` > `chrome` > `webview` | +| `adb reverse --list` | `verify_adb_reverse_rules()` | Confirms USB-local reverse rules survived (rc=0 doesn't guarantee persistence) | + +Actions (side-effecting): + +| adb command | Function | Effect | +|---|---|---| +| `adb reconnect` | `assert_adb_device_online()` | One retry on transient `offline` | +| `adb shell input keyevent KEYCODE_WAKEUP` | `assert_headset_awake()` | Wakes the device | +| `adb shell am start -a android.intent.action.VIEW -d [-p ]` | `open_url_on_headset()` → `run_adb_headset_bookmark()` / `run_oob_connect()` | **Opens the browser to the teleop URL.** Retried in a poll loop inside `run_oob_connect()` until a matching tab appears over CDP | +| `adb reverse tcp: tcp:` | `setup_adb_reverse_ports()`, `setup_adb_reverse_turn()` | USB-local: headset-loopback → PC port mapping (proxy port, backend port, TURN port) | +| `adb reverse --remove tcp:` | `teardown_adb_reverse_ports()`, `teardown_adb_reverse_turn()` | Cleanup | +| `adb forward tcp: localabstract:` | `_adb_forward_cdp()` | Maps the discovered DevTools abstract socket to a local TCP port — this is what makes CDP reachable at all | +| `adb forward --remove tcp:` | `_adb_forward_remove()` / `teardown_adb_forward_cdp()` | Cleanup (also run at startup to clear a stale rule from a crashed prior run) | + +### B. Chrome DevTools Protocol (CDP) — reached over the `adb forward`'d port, driven directly by `oob_teleop_adb.py` + +- `GET http://localhost:/json` (`_cdp_list_tabs()`) — list tabs. Used by + `_close_stale_teleop_tabs()` (closes any tab whose URL contains + `oobEnable=`, freeing XR resources a crashed prior session held) and by + `run_oob_connect()`'s tab-matching loop after `am start`. +- `GET http://localhost:/json/close/` — close a specific tab. +- CDP WebSocket session on a tab's `webSocketDebuggerUrl` + (`_cdp_session_click_connect()`): `Security.setIgnoreCertificateErrors` + (cert bypass), then `Runtime.evaluate` polling a client-side readiness + state machine (`loading` → `initializing` [IWER + capability checks + running] → `ready`/`failed`), then synthesizes the CONNECT click. +- A second, independent CDP WebSocket session, opened by + `_monitor_teleop_error_banner()` once `run_oob_connect()` returns: polls + `document.getElementById('errorMessageBox')` once/second and logs any new + `error`-class banner text. Runs until cancelled or the tab/socket drops; + always tears down its own `adb forward` on exit. + +CDP is purely browser automation — it never touches application/CloudXR +traffic. + +### C. What the client JS itself opens, once loaded — *not* driven by +`run_oob_connect()`'s Python code, but part of the same end-to-end flow + +- **CloudXR streaming signaling websocket.** The real `@nvidia/cloudxr` + client component opens this once CONNECT is clicked and + `requestSession()` succeeds. It terminates at `wss.py`'s real, pure-Python + WSS proxy (TLS termination + WebSocket forwarding), which in turn forwards + to a native CloudXR Runtime backend process + (`CloudXRService._runtime_proc`, `service/_service.py`, launched via + `subprocess.Popen`) — GPU-dependent, and the one piece the existing test + suite (`test_service.py`'s `mock_service_deps()`) always mocks out rather + than launching for real. +- **OOB control-hub websocket** (`/oob/v1/ws`, `oob_teleop_hub.py`, + `OOBControlHub`). Pure Python, no GPU dependency. Headset registers + (`{"type": "register", "role": "headset", ...}`), gets a `hello`, then + sends `streamStatus`/`clientMetrics`; operator-side config pushes flow + back down the same socket. Fully real and runnable with zero mocking. +- `MockCloudXR.ts` (`deps/cloudxr/webxr_client/tests/mock/MockCloudXR.ts`, + behind `CloudXRComponentTest.html` / `npm run dev-server:component-mock`) + is a *third*, unrelated option: it fakes the CloudXR session object + entirely client-side (synthetic stereo render) and opens **no** websocket + at all. Confirmed via grep — zero `WebSocket` usage in that file. Useful + as today's manual dev-server page, but doesn't exercise the real signaling + path, so it's not what we want for this integration test if the goal is + exercising real client↔server behavior. + +## 2. What "real" actually means for this test + +- **ADB layer**: no real device exists in CI/dev-sandbox, so this stays + synthetic — but with real side effects where one exists locally, per the + session's prior discussion. Concretely: + - `am start` → parse the `-d ` out of the real shell command string + and actually navigate a real local Chromium instance to it (not a + canned success). + - `adb forward` / `--remove` → alias the local TCP port directly to + Chromium's own `--remote-debugging-port` (no real abstract socket + exists locally, so there's nothing to really forward — but the *effect*, + a working local CDP port, is real). + - `get-state`, `/proc/net/unix`, `getprop`, `pm list packages` → stay + canned; there is no real device for these to reflect. +- **CDP layer**: fully real. A real Chromium (matching + `deps/cloudxr/webxr_client/playwright.config.js`'s launch args — + `--disable-features=WebXR`, SwiftShader software WebGL, no sandbox) is + reachable over its own real `/json`, `/json/close/`, and debugger + websocket. Nothing to fake here at all. +- **Client-opened sockets layer**: split. + - OOB control-hub (`/oob/v1/ws`) — run the real `OOBControlHub` for real. + No mock needed. + - CloudXR signaling — `wss.py`'s proxy is real Python, runs for real. + **Resolved: no runtime backend needed, real or mocked.** `wss.py` + already treats "nothing listening on `backend_port`" as an anticipated, + supported configuration, not an error to work around — + `_is_backend_connection_refused()` (`wss.py:454`) detects exactly this + and `proxy_handler()` logs it as `"expected when running WSS+hub without + the runtime; teleop signaling uses %s"`, then closes that one signaling + connection and returns; nothing crashes. It also never touches the OOB + hub — `OOB_WS_PATH` is routed to `hub.handle_connection()` *before* + `proxy_handler()` is ever reached (`wss.py:644`), so hub + register/`hello`/`streamStatus`/`clientMetrics` and the + `/api/oob/v1/*` HTTP routes are unaffected either way. So: never start + anything on `backend_port` (49100) for this test. The client will very + likely show a real connection-error banner after CONNECT is clicked + (its own signaling socket gets refused) — a genuine, free signal + `_monitor_teleop_error_banner()` already knows how to read, usable as a + bonus assertion but not required for the test's core scope. + +## 3. Proposed test shape + +New test double(s) in `tests/python/core/cloudxr/conftest.py`, alongside the +existing `FakeAdb`/`mock_adb()`: + +- `RealBrowserAdb` (name TBD): same `subprocess.run`-patching shape as + `FakeAdb`, but `am start` and `adb forward` have the real side effects + described in §2, driven against a Chromium instance the test itself + launches (Playwright, matching `playwright.config.js`'s launch args) and + navigates to the real `webxr_client` bundle served locally (reuse + `npm run dev-server:component-mock` or an equivalent dev-server target + that serves the *real* CloudXR component, not `MockCloudXR.ts`). +- New integration test (separate file or a clearly-marked slow section of + `test_oob_teleop_adb.py`) driving the real `run_oob_connect()` end to end: + device-online → `am start` opens the real tab → real devtools socket + reachable → real CDP tab discovery finds the real tab → real + `_cdp_session_click_connect()` polls the *real* client's readiness state + machine (real IWER capability checks) → real click → real monitor task + reading the real client's error banner. +- Keep this as new, separate, explicitly slower coverage — don't fold it + into the existing fast `mock_adb()`/fake-CDP-server suite from PR #1148. + +## 4. Setup gaps (wiring/plumbing to build — not mocks) + +Confirmed: nothing in §1–3 needs a protocol-level mock. `npm run dev-server` +(`webpack.dev.js`, port 8080) already serves the *real* production client +(`App.tsx`), not `MockCloudXR.ts` — this is "the CloudXRComponent" the test +needs, and it's already runnable today. What's actually missing is wiring: + +1. **`wss.py` isn't started anywhere for this purpose.** Needs launching as + a real process with the OOB hub wired in (`OOBControlHub`) and *some* + `backend_host`/`backend_port` configured for the CloudXR-signaling + forward target — even an unused one is fine, since `/oob/v1/ws` and the + `/api/oob/v1/*` HTTP routes work independently of whether anything + answers on the signaling side. It generates its own self-signed cert; no + action needed there. +2. **No code today launches a real Chromium process directly** (as opposed + to through `adb`). `FakeAdb` only patches `subprocess.run` for synthetic + `adb` calls. Need a small helper that spawns Chrome with + `--remote-debugging-port=`, reusing `playwright.config.js`'s known + working launch args and — critically — its `executablePath` override + (`/usr/bin/google-chrome`): this sandbox cannot reach Playwright's own + browser-download CDN, a constraint already hit once building that config. +3. **URL construction** must go through the existing `build_teleop_url()` / + `build_headset_bookmark_url()` helpers (`oob_teleop_env.py`), pointed at + `localhost:8080` (the client) with `serverIP`/`port` aimed at the local + `wss.py` instance and `oobEnable=1` set, rather than hand-assembling a + query string — this is also what makes `_close_stale_teleop_tabs()`'s + `oobEnable=` tab filter match. +4. Cert trust is already a non-issue: `_cdp_session_click_connect()` already + sends CDP `Security.setIgnoreCertificateErrors` before polling. + +## 5. Exhaustive check: what talks to the CXR runtime directly + +Repo-wide search (Python + vendored `deps/`) for any direct connection to +the runtime — its WS port (`backend_port`, default `49100`) or its +`ipc_cloudxr` Unix socket. Exactly two hits, both already covered above: + +1. **`wss.py:proxy_handler()` → `ws_connect(f"ws://{backend_host}:{backend_port}{path}")`.** + The only WS client of the runtime anywhere in the codebase. A dumb + relay — `_pipe()` forwards every message verbatim, `async for msg in + src: await dst.send(msg)`, never parsing SDP/ICE/JSON content. This is + the one seam a mock runtime would need to satisfy to give the client a + real, successful streaming session (bind `49100`, accept the WS + connection, do real SDP/ICE negotiation, send synthetic media over a + real `RTCPeerConnection` — confirmed via `RTCPeerConnection`/ + `createOffer`/`createAnswer`/`addIceCandidate`/`iceServers` strings in + the vendored `@nvidia/cloudxr` bundle). +2. **`runtime.py:is_runtime_live()` → raw `AF_UNIX` `connect()` to + `/ipc_cloudxr`.** A liveness probe only (no protocol messages), + used solely by `CloudXRService._cleanup_stale_runtime` in `service.py`'s + own launch path. Not touched by the OOB/CDP/client flow this plan covers + — irrelevant to a mock runtime built for the goal above. + +Everything else matching `49100`/`libcloudxr`/`nv_cxr_service` is either the +runtime process *becoming* the listener (`runtime.py:run()`, +`deps/cloudxr/runtime/main.py` — these load `libcloudxr.so` in-process, they +don't connect to anything) or test/comment noise. + +**Conclusion: `run_oob_connect()`'s own code (`oob_teleop_adb.py`) never +touches the runtime, directly or indirectly** — it only ever does ADB and +CDP (§1.A/B). The runtime is entirely orthogonal to what this plan's +integration test exercises. A mock runtime is only relevant if the goal +shifts to proving out a real, successful, rendering CloudXR session — a +separate concern from OOB orchestration coverage, and (per the top-of-file +scope note) tracked as its own separate project, not folded into this one. + +## 6. Open questions before implementation + +1. ~~Does this test need the CloudXR signaling socket to succeed~~ — + **resolved, see §3**: no runtime backend, real or mocked. Never bind + `backend_port`; `wss.py` handles the refused connection as an + anticipated configuration. +2. ~~Where does the local dev-server get started from~~ — **resolved, see + §7**: a Python-side `conftest.py` fixture, reusing the pattern already + used for `live_ipc_socket`/`mock_service_deps`. +3. CI feasibility: deliberately deferred (see §7) — this test runs manually + for now; CI wiring (parallel-worker port collisions, ephemeral ports, + headless-runner Chrome availability) is a future PR. + +## 7. Dev-server launch: where and how (manual-run scope; CI deferred) + +Surveyed two precedents before deciding: + +- **This repo's `playwright.config.js`** uses Playwright's own `webServer` + config (`command: 'npm run dev-server:component-mock'`, fixed port 8083, + `reuseExistingServer: !process.env.CI`) — JS-test-runner machinery, not + usable from a Python pytest test. +- **`~/gitlab/cloudxr-js`'s `tests/playwright/global-setup.js`** spawns + `npx http-server -p 0 --cors` as a child process, scrapes the + ephemeral bound port from stdout, writes it to a side-channel file + (`.http-server-port`) for fixtures to read — built for CI parallelism + (many workers, no fixed-port collisions). That repo also runs a much + heavier **real** (not mock) streaming e2e + (`tests/docker-compose.test-e2e.yaml`): three containers + (`cloudxr-runtime` = the actual GPU-backed CloudXR runtime image, + `webapp`, `playwright`), requiring an NVIDIA GPU + docker + + `nvidia-container-toolkit`. Confirms no lightweight mock runtime exists + anywhere in either repo today — that remains genuinely separate, + greenfield work (per the top-of-file scope note), well outside what this + plan needs. + +**Decision for this plan:** launch the dev server from a new Python-side +`@contextmanager` fixture in `tests/python/core/cloudxr/conftest.py`, +following this repo's own existing convention (`live_ipc_socket`, +`mock_service_deps`) rather than cloudxr-js's Node-side pattern: + +- `subprocess.Popen(["npm", "run", "dev-server"], cwd=/deps/cloudxr/webxr_client, ...)`, + fixed port `8080` (matches `webpack.dev.js`'s default, and what's already + running manually in this session today) — skip ephemeral-port scraping; + not needed without parallel CI workers. +- Mirror `playwright.config.js`'s `reuseExistingServer: !process.env.CI` + idea: probe port 8080 first and reuse an already-running server if found; + only spawn (and later tear down) one if nothing answers. Keeps manual runs + fast and avoids colliding with a dev-server already running for other + purposes. +- Ephemeral-port scraping (`cloudxr-js`'s approach) and any CI-specific + wiring are explicitly deferred to the future CI PR. + +## 8. Final state (as implemented — supersedes §0, §7's dev-server decision) + +Reality diverged from §0/§7 during implementation, for good reasons found +along the way. This is what's actually in `conftest.py`/`test_oob_teleop_adb.py` +now: + +1. **Always `MockCloudXR`, never the real `@nvidia/cloudxr` SDK.** §0 wanted + the real SDK for "real rendering"; in practice the real SDK genuinely + tries to stream against whatever's on `backend_port`, which is + unpredictable and gives a test nothing to assert against. `MockCloudXR` + (via `tests/mock/cloudxr-mock-alias.ts`'s webpack alias) mounts the exact + same `App.tsx`/`CloudXRComponent.tsx` UI (same `#startButton`/ + `#errorMessageBox` DOM `run_oob_connect()`'s own code already polls) with + a fully deterministic, externally-controllable session underneath — + `window.__mockCloudXRFail(message?, code?)`, callable over CDP via + `cdp_evaluate()`. It never opens a socket of its own, so **no + `minimal_mock_runtime()` stand-in exists any more at all** — that whole + piece from §3 is gone. +2. **Never a dev-server — always a static production build** + (`static_webxr_build()`, replacing §7's `real_webxr_dev_server()`). + Two independent, real bugs forced this and are worth knowing about if + this code is ever touched again: + - **Webpack HMR module duplication.** A dev-server's hot-reload + re-executes a module's top-level code on live-reload, creating a + *second* instance of it side by side with the one React already + mounted against. For `cloudxr-mock-alias.ts` this meant + `window.__mockCloudXRFail()` silently bound to a fresh, never-used + `activeSession` while the real, already-running session lived on + untouched in the original instance — callable, no error, just no + effect. A static build has no live-reload runtime at all, so this is + structurally impossible now, not just avoided by luck. + - **`` double-invoking effects in a dev-mode build.** + Independent of HMR: `webpack.app-mock.js` used `mode: 'development'`, + and `src/index.tsx` wraps the app in `StrictMode`, which deliberately + double-invokes effects in dev builds — calling + `CloudXRComponent.tsx`'s `establishSession()` (and so + `CloudXR.createSession()`) twice. Fixed by switching + `webpack.app-mock.js` to `mode: 'production'`, which sets + `NODE_ENV=production` and makes React skip StrictMode's double-invoke + entirely. + - **A third, unrelated trap on the way to diagnosing the above:** + `webpack.common.js`'s persistent filesystem cache + (`cache: {type: 'filesystem'}`) is keyed only on the webpack config + file (`buildDependencies.config`), not on every source file it + compiles — a stale cache entry silently served an old compiled + `cloudxr-mock-alias.ts` that didn't include a real source edit at all + (confirmed by `grep`ping the built bundle for a symbol that wasn't + there). `static_webxr_build()` now deletes + `node_modules/.cache/webpack` before every build; correctness matters + far more than incremental-build speed for a test fixture that already + rebuilds from scratch every run. +3. **`window.__mockCloudXRFail(message?, code?)` needs a `code` in the + `0xc0f22300`–`0xc0f223ff` range to produce a real, DOM-visible error.** + A code-less failure is "recoverable" per + `helpers/streamingErrorClassification.ts`'s `isRecoverable()`, and + `CloudXRComponent.tsx`'s `onStreamStopped` auto-reconnects on those + silently, without ever showing `#errorMessageBox` — exactly like a real + transient failure would. `cloudxr-mock-alias.ts` was extended to accept + an optional `code` (previously message-only) to reach the non-recoverable + path, matching what `tests/mock/CloudXRComponentTest.tsx`'s own + `NON_RETRYABLE_CODE` steps already did internally. +4. **Fire `__mockCloudXRFail()` in a retry loop, not once.** It's a no-op + until MockCloudXR's session actually exists and is + `Connecting`/`Connected`; `run_oob_connect()` returns as soon as the + button text changes, which happens on WebXR session entry — near + instantly, and likely before `CloudXR.createSession()`'s own async chain + has finished. Retrying is safe: once the failure lands the session moves + to `Error` state, so later calls just keep failing the same guard + harmlessly. + +Three tests now exist in `test_oob_teleop_adb.py`: + +- `test_run_oob_connect_real_browser_end_to_end` — the full real + adb→CDP→click path against `MockCloudXR`, proving `run_oob_connect()`'s + own orchestration for real. +- `test_run_oob_connect_real_mock_cxr_crash_surfaces_error_banner` — proves + the crash-trigger mechanism itself: a real, deterministic, DOM-visible + error banner on demand. +- `test_run_oob_connect_real_mock_cxr_crash_triggers_real_relaunch` — + **anticipatory**, written against `gmorgan/oob-error-relaunch` (#1146) + before that branch has merged here. It deliberately does not import any + private function from that branch (so it can't fail on an ImportError + that would depend on exact internal names surviving the merge); instead + it asserts on black-box observable recovery — a second real `am start` + and a second real tab after a genuine triggered crash. It fails today, + correctly, on that assertion after a real 30s wait (not a collection + error), and should turn green the moment #1146's relaunch logic lands on + this branch. `test_build_teleop_url_forwards_reliability_config_from_env` + (added to the plain unit-test section, no browser needed) is the same + "write it now, let it fail until merged" idea applied to + `gmorgan/oob-reliability-config-wiring` (#1145)'s config-forwarding logic. diff --git a/tests/python/core/cloudxr/conftest.py b/tests/python/core/cloudxr/conftest.py index 6d6daec014..30c0f22cb7 100644 --- a/tests/python/core/cloudxr/conftest.py +++ b/tests/python/core/cloudxr/conftest.py @@ -11,15 +11,21 @@ from __future__ import annotations +import asyncio import contextlib import importlib.util +import json import os import shlex +import shutil import socket import subprocess import sys +import time import types -from contextlib import contextmanager +import urllib.error +import urllib.request +from contextlib import asynccontextmanager, contextmanager from pathlib import Path from unittest.mock import MagicMock, patch @@ -336,3 +342,292 @@ def mock_adb(**kwargs): fake = FakeAdb(**kwargs) with patch("subprocess.run", side_effect=fake): yield fake + + +# ============================================================================ +# Real-browser OOB integration test-infra (see /oob-real-browser-integration- +# test-plan.md at the repo root): a real Chromium running the real webxr +# client, driven by run_oob_connect() through a RealBrowserAdb whose +# success-path commands have real side effects instead of canned output. +# ============================================================================ + +_CHROME_EXECUTABLE = os.environ.get("PLAYWRIGHT_CHROME_PATH", "/usr/bin/google-chrome") + +# Matches deps/cloudxr/webxr_client/playwright.config.js's CHROMIUM_ARGS: +# SwiftShader software WebGL (no GPU in this sandbox) and --disable-features=WebXR +# so native/real XR is off and IWER (loaded by the page itself) provides navigator.xr. +_CHROME_LAUNCH_ARGS = [ + "--no-sandbox", + "--disable-setuid-sandbox", + "--ignore-gpu-blocklist", + "--enable-webgl", + "--use-angle=swiftshader", + "--use-gl=angle", + "--disable-features=WebXR", +] + + +def _free_tcp_port() -> int: + """Bind an ephemeral port and immediately release it for a subprocess to reuse.""" + with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as s: + s.bind(("127.0.0.1", 0)) + return s.getsockname()[1] + + +def _wait_for_http(url: str, *, timeout: float) -> None: + deadline = time.monotonic() + timeout + last_exc: Exception | None = None + while time.monotonic() < deadline: + try: + with urllib.request.urlopen(url, timeout=1): + return + except (urllib.error.URLError, ConnectionError, OSError) as exc: + last_exc = exc + time.sleep(0.2) + raise RuntimeError(f"{url} did not respond within {timeout:.0f}s") from last_exc + + +class RealChrome: + """A real, locally-launched Chromium instance reachable over its own CDP port.""" + + def __init__(self, cdp_port: int) -> None: + self.cdp_port = cdp_port + + def open_url(self, url: str) -> dict: + """Open *url* in a new real tab via CDP's HTTP endpoint (``PUT /json/new``).""" + req = urllib.request.Request( + f"http://localhost:{self.cdp_port}/json/new?{url}", method="PUT" + ) + with urllib.request.urlopen(req, timeout=10) as resp: + return json.loads(resp.read()) + + def list_tabs(self) -> list[dict]: + """Real ``GET /json`` — every open tab, with its ``webSocketDebuggerUrl``.""" + with urllib.request.urlopen( + f"http://localhost:{self.cdp_port}/json", timeout=5 + ) as resp: + return json.loads(resp.read()) + + +@contextmanager +def real_chrome(cdp_port: int, *, user_data_dir: Path): + """Launch a real local Chromium reachable over CDP on *cdp_port*, for the block's duration.""" + args = [ + _CHROME_EXECUTABLE, + f"--remote-debugging-port={cdp_port}", + f"--user-data-dir={user_data_dir}", + "--no-first-run", + "--no-default-browser-check", + *_CHROME_LAUNCH_ARGS, + "about:blank", + ] + proc = subprocess.Popen(args, stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL) + try: + _wait_for_http(f"http://localhost:{cdp_port}/json/version", timeout=15.0) + yield RealChrome(cdp_port) + finally: + proc.terminate() + try: + proc.wait(timeout=10) + except subprocess.TimeoutExpired: + proc.kill() + proc.wait(timeout=5) + + +@contextmanager +def static_webxr_build(port: int, *, npm_build_script: str, build_dir_name: str): + """Build webxr_client once via a real ``npm run `` production + build (no dev-server, no HMR), then serve the resulting static directory with a + plain HTTP server for the block's duration. + + Deliberately never a dev-server: webpack's hot-module-replacement re-executes a + module's top-level code on live-reload, creating a *second* instance of it + side by side with the one React already mounted against — for + ``tests/mock/cloudxr-mock-alias.ts`` this means ``window.__mockCloudXRFail()`` + silently binds to a fresh, never-used ``activeSession`` while the real, + already-running session lives on in the original instance untouched. A static + build has no live-reload runtime at all, so this class of bug is structurally + impossible, not just avoided by luck. + + *npm_build_script*/*build_dir_name* pairs (see ``package.json``/the matching + ``webpack.*.js``). Always use the ``build:app-mock`` pair for a real-browser + test: the real SDK genuinely tries to stream against whatever's on + ``backend_port``, which is unpredictable and uncontrollable to assert against. + + * ``"build"`` / ``"build"`` — the real production client against the real + ``@nvidia/cloudxr`` SDK. Not for tests — kept only as a manual sanity + build; see the warning above. + * ``"build:app-mock"`` / ``"build-app-mock"`` — the identical ``App.tsx`` / + ``CloudXRComponent.tsx`` UI (same ``#startButton``/``#errorMessageBox`` + DOM), but with ``@nvidia/cloudxr`` aliased to ``MockCloudXR``, which exposes + ``window.__mockCloudXRFail(message?, code?)`` globally so a CDP + ``Runtime.evaluate`` call can force a deterministic, controllable + mid-session failure — see :func:`cdp_evaluate`. ``MockCloudXR`` opens no + socket of its own, so nothing needs to stand in for a CloudXR runtime at + all. + + Always rebuilds (mirrors ``cloudxr-js``'s ``global-setup.js``: a stale bundle + from a previous branch must never silently pass) rather than reusing whatever + happens to be on disk or already listening on *port*. + """ + webxr_client_dir = repo_root() / "deps" / "cloudxr" / "webxr_client" + # webpack.common.js turns on persistent filesystem caching + # (cache: {type: 'filesystem'}), keyed only on the config file + # (buildDependencies.config), not on every source file it compiles. A stale + # entry there can silently serve an old compiled module for a source file + # that *did* change - hit for real while building this fixture: an edit to + # tests/mock/cloudxr-mock-alias.ts was invisible in the output bundle + # (`grep` for the new symbol found nothing) until this cache was cleared. + # Correctness matters far more than incremental-build speed for a test + # fixture, so always start from a clean cache rather than trust it. + shutil.rmtree( + webxr_client_dir / "node_modules" / ".cache" / "webpack", ignore_errors=True + ) + subprocess.run( + ["npm", "run", npm_build_script], + cwd=webxr_client_dir, + check=True, + stdout=subprocess.DEVNULL, + stderr=subprocess.DEVNULL, + ) + build_dir = webxr_client_dir / build_dir_name + if not (build_dir / "index.html").is_file(): + raise RuntimeError( + f"{build_dir} has no index.html after `npm run {npm_build_script}`" + ) + + from functools import partial + from http.server import ThreadingHTTPServer, SimpleHTTPRequestHandler + from threading import Thread + + handler = partial(SimpleHTTPRequestHandler, directory=str(build_dir)) + httpd = ThreadingHTTPServer(("localhost", port), handler) + thread = Thread(target=httpd.serve_forever, daemon=True) + thread.start() + try: + _wait_for_http(f"http://localhost:{port}/", timeout=15.0) + yield + finally: + httpd.shutdown() + httpd.server_close() + thread.join(timeout=5) + + +async def cdp_evaluate(ws_url: str, expression: str) -> dict: + """Open a one-shot CDP session to *ws_url* and evaluate *expression*. + + Mirrors the ``send()`` helper duplicated in ``_cdp_session_click_connect``/ + ``_monitor_teleop_error_banner`` (``oob_teleop_adb.py``), for a test that + needs to poke a real tab from outside those functions — e.g. calling + ``window.__mockCloudXRFail()`` to force a deterministic mid-session error. + """ + from websockets.asyncio.client import connect as ws_connect # noqa: PLC0415 + + async with ws_connect(ws_url) as ws: + await ws.send( + json.dumps( + { + "id": 1, + "method": "Runtime.evaluate", + "params": {"expression": expression, "returnByValue": True}, + } + ) + ) + while True: + msg = json.loads(await asyncio.wait_for(ws.recv(), timeout=10.0)) + if msg.get("id") == 1: + return msg.get("result", {}) + + +@asynccontextmanager +async def real_wss_proxy(install_dir: Path, *, proxy_port: int, backend_port: int): + """Run the real ``wss.py`` proxy (TLS + OOB hub) for the block's duration. + + ``TELEOP_OOB_HUB_ONLY=1`` makes ``setup_oob=True`` wire up the real + ``OOBControlHub`` without also having ``wss.py`` itself call + ``run_oob_connect()`` on our behalf — the test drives that call directly, + against the real proxy this starts. ``usb_local=False`` here (a ``wss.run()`` + param, independent of the ``usb_local`` a test passes to its own + ``run_oob_connect()`` call) skips ``wss.py``'s own adb-reverse/coturn setup, + which has no real device to run against in this test. + """ + from cloudxr_py_test_ns.wss import run as wss_run # noqa: PLC0415 + + os.environ["CXR_INSTALL_DIR"] = str(install_dir) + prev_hub_only = os.environ.get("TELEOP_OOB_HUB_ONLY") + os.environ["TELEOP_OOB_HUB_ONLY"] = "1" + + stop_future: asyncio.Future = asyncio.get_running_loop().create_future() + listening = asyncio.Event() + + task = asyncio.create_task( + wss_run( + None, + stop_future, + backend_host="localhost", + backend_port=backend_port, + proxy_port=proxy_port, + setup_oob=True, + usb_local=False, + host_client=False, + on_listening=listening.set, + ) + ) + try: + await asyncio.wait_for(listening.wait(), timeout=15.0) + yield + finally: + if not stop_future.done(): + stop_future.set_result(None) + await asyncio.wait_for(task, timeout=10.0) + if prev_hub_only is None: + os.environ.pop("TELEOP_OOB_HUB_ONLY", None) + else: + os.environ["TELEOP_OOB_HUB_ONLY"] = prev_hub_only + + +class RealBrowserAdb(FakeAdb): + """Like :class:`FakeAdb`, but ``am start`` and ``adb forward`` have real side + effects against a real :class:`RealChrome` instead of returning canned output. + + Error-injection kwargs (``device_state``, ``am_start_rc``, ``devtools_socket``, + ``forward_rc``) work exactly as on :class:`FakeAdb`: the fake side of each still + gates whether the real side effect happens at all, so tests can still + deliberately drive the same failure paths ``FakeAdb``-based tests do. + + ``adb forward`` is a real no-op (not a real TCP forward) because *chrome* + is launched with ``--remote-debugging-port`` equal to + ``oob_teleop_adb._CDP_LOCAL_PORT`` already — the port ``run_oob_connect()`` + forwards to is already the real CDP port, so there is nothing to actually + forward. + """ + + def __init__(self, chrome: RealChrome, **kwargs) -> None: + super().__init__(**kwargs) + self._chrome = chrome + + def __call__(self, args: list[str], **kwargs) -> subprocess.CompletedProcess: + if args[:2] == ["adb", "shell"] and any("am start" in a for a in args): + self.calls.append(list(args)) + if self.am_start_rc != 0: + return subprocess.CompletedProcess( + args, self.am_start_rc, "", "am start failed" + ) + shell_cmd = args[2] + tokens = shlex.split(shell_cmd) + url = tokens[tokens.index("-d") + 1] + self._chrome.open_url(url) + return subprocess.CompletedProcess(args, 0, "", "") + if args[:2] == ["adb", "forward"] and "--remove" not in args: + self.calls.append(list(args)) + return subprocess.CompletedProcess(args, self.forward_rc, "", "") + return super().__call__(args, **kwargs) + + +@contextmanager +def real_browser_adb(chrome: RealChrome, **kwargs): + """Patches ``subprocess.run`` with a :class:`RealBrowserAdb` for the block's + duration; yields the instance so a test can inspect ``.calls``.""" + fake = RealBrowserAdb(chrome, **kwargs) + with patch("subprocess.run", side_effect=fake): + yield fake diff --git a/tests/python/core/cloudxr/pyproject.toml b/tests/python/core/cloudxr/pyproject.toml index d9b1072301..d03fbcaef1 100644 --- a/tests/python/core/cloudxr/pyproject.toml +++ b/tests/python/core/cloudxr/pyproject.toml @@ -13,12 +13,14 @@ dev = [ "pytest-asyncio>=1.3.0", "numpy", "websockets>=14", + "cryptography>=3.0", ] test = [ "pytest", "pytest-asyncio>=1.3.0", "numpy", "websockets>=14", + "cryptography>=3.0", ] [tool.pytest.ini_options] diff --git a/tests/python/core/cloudxr/test_oob_teleop_adb.py b/tests/python/core/cloudxr/test_oob_teleop_adb.py index efb131b5af..e1328a2165 100644 --- a/tests/python/core/cloudxr/test_oob_teleop_adb.py +++ b/tests/python/core/cloudxr/test_oob_teleop_adb.py @@ -1711,3 +1711,303 @@ async def test_monitor_without_callback_still_logs_and_keeps_watching() -> None: task.cancel() with pytest.raises(asyncio.CancelledError): await task + + +# ============================================================================ +# run_oob_connect(): real-browser integration test (see +# /oob-real-browser-integration-test-plan.md at the repo root). A real +# Chromium runs the real webxr_client (real IWER-emulated WebXR, real CDP), +# served from a real static production build (never a dev-server — see +# static_webxr_build()'s docstring for why HMR is structurally unsafe for +# this), behind a real wss.py proxy (real OOBControlHub). Only adb itself is +# faked (no physical Android device), via RealBrowserAdb: its `am start` and +# `adb forward` drive the real Chromium instead of returning canned output. +# +# ALWAYS the MockCloudXR build (`build:app-mock`), never the real +# `@nvidia/cloudxr` SDK: the real SDK genuinely tries to stream against +# whatever's on `backend_port`, which is unpredictable and uncontrollable in +# a test (see the plan's §0/§3 history — this was tried and abandoned). +# MockCloudXR gives the same real App.tsx/CloudXRComponent UI (same +# #startButton/#errorMessageBox DOM) with a fully deterministic, externally +# controllable session underneath (window.__mockCloudXRFail(), see +# cdp_evaluate()), and never opens a socket of its own — no +# minimal_mock_runtime() stand-in needed for CloudXR signaling at all. +# +# This replaces the fully-synthetic mock_adb()/fake-CDP-server +# run_oob_connect() tests from PR #1148 — those proved run_oob_connect()'s +# own orchestration logic against scripted responses; this proves the same +# code path against the real client it actually launches. +# +# Slow (real browser + a real webpack production build + real wss.py) and +# requires a system Chrome (PLAYWRIGHT_CHROME_PATH override; defaults to +# /usr/bin/google-chrome) and Node/npm on PATH — run manually for now; not +# yet wired into CI. +# ============================================================================ + +import contextlib # noqa: E402 +from pathlib import Path # noqa: E402 + +from conftest import ( # noqa: E402 + _free_tcp_port, + cdp_evaluate, + real_browser_adb, + real_chrome, + real_wss_proxy, + static_webxr_build, +) + + +async def test_run_oob_connect_real_browser_end_to_end( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """The real run_oob_connect(), driven against a real Chromium running the real + webxr client (MockCloudXR underneath — see module docstring): device online -> + stale-tab scan -> am start opens a real tab -> real devtools socket already + reachable (Chrome's own --remote-debugging-port equals _CDP_LOCAL_PORT, so adb + forward is a real no-op) -> real CDP tab discovery finds the real tab -> real + _cdp_session_click_connect polls the real client's readiness state machine + (real IWER capability checks) and clicks the real CONNECT button. + """ + client_port = _free_tcp_port() + monkeypatch.setenv("TELEOP_WEB_CLIENT_BASE", f"http://localhost:{client_port}") + monkeypatch.delenv("CONTROL_TOKEN", raising=False) + + wss_port = _free_tcp_port() + backend_port = _free_tcp_port() + user_data_dir = tmp_path / "chrome-profile" + user_data_dir.mkdir() + install_dir = tmp_path / "cxr-install" + + with ( + static_webxr_build( + client_port, + npm_build_script="build:app-mock", + build_dir_name="build-app-mock", + ), + real_chrome(_CDP_LOCAL_PORT, user_data_dir=user_data_dir) as chrome, + real_browser_adb(chrome) as fake_adb, + ): + async with real_wss_proxy( + install_dir, proxy_port=wss_port, backend_port=backend_port + ): + task = await run_oob_connect( + resolved_port=wss_port, timeout=30.0, usb_local=True + ) + + # A real task, not None: the click genuinely succeeded and + # _monitor_teleop_error_banner is genuinely running against the real + # client's real tab, not swallowed as "click phase ok, monitor + # failed to spawn" (see run_oob_connect()'s own return-value contract). + assert isinstance(task, asyncio.Task) + task.cancel() + with contextlib.suppress(asyncio.CancelledError): + await task + + assert any(c[:2] == ["adb", "get-state"] for c in fake_adb.calls) + assert any("am start" in " ".join(c) for c in fake_adb.calls) + + +async def test_run_oob_connect_real_mock_cxr_crash_surfaces_error_banner( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """The real run_oob_connect() driven against the real App.tsx/CloudXRComponent + UI with MockCloudXR swapped in for @nvidia/cloudxr (``npm run + build:app-mock``, ``tests/mock/cloudxr-mock-alias.ts``, static build, no + dev-server — see :func:`static_webxr_build`): identical + ``#startButton``/``#errorMessageBox`` DOM to production, but + ``window.__mockCloudXRFail()`` lets this test force a deterministic + mid-session failure instead of guessing at how a real backend failure + would surface over the wire. + + MockCloudXR never opens a socket of its own (zero ``WebSocket`` usage in + ``tests/mock/MockCloudXR.ts``), so nothing needs to stand in for a CloudXR + runtime here; ``wss.py`` + ``OOBControlHub`` still run for real so the URL + ``run_oob_connect()`` builds is a genuine ``wss://.../oob/v1/ws`` endpoint, + matching what a real launch produces. + + Proves the crash-trigger mechanism itself — a real, real-DOM error banner + appears on demand — ahead of reusing it to test + ``_find_and_click_teleop_tab``'s actual recovery once this test infra is + merged onto ``gmorgan/oob-error-relaunch`` (that branch's monitor is + repair-capable; this branch's is not, so this test only proves the crash + signal is real and detectable, not that anything recovers from it). + """ + client_port = _free_tcp_port() + monkeypatch.setenv("TELEOP_WEB_CLIENT_BASE", f"http://localhost:{client_port}") + monkeypatch.delenv("CONTROL_TOKEN", raising=False) + + wss_port = _free_tcp_port() + backend_port = _free_tcp_port() + user_data_dir = tmp_path / "chrome-profile" + user_data_dir.mkdir() + install_dir = tmp_path / "cxr-install" + + with ( + static_webxr_build( + client_port, + npm_build_script="build:app-mock", + build_dir_name="build-app-mock", + ), + real_chrome(_CDP_LOCAL_PORT, user_data_dir=user_data_dir) as chrome, + real_browser_adb(chrome), + ): + async with real_wss_proxy( + install_dir, proxy_port=wss_port, backend_port=backend_port + ): + task = await run_oob_connect( + resolved_port=wss_port, timeout=30.0, usb_local=True + ) + assert isinstance(task, asyncio.Task) + + teleop_tabs = [ + t for t in chrome.list_tabs() if "oobEnable=" in (t.get("url") or "") + ] + assert len(teleop_tabs) == 1 + ws_url = teleop_tabs[0]["webSocketDebuggerUrl"] + + # code in the 0xc0f22300-0xc0f223ff (server-disconnect) range: a + # code in the 0xc0f22300-0xc0f223ff (server-disconnect) range: a + # code-less failure is "recoverable" per + # helpers/streamingErrorClassification.ts's isRecoverable(), and + # CloudXRComponent.tsx's onStreamStopped auto-reconnects on those + # without ever showing #errorMessageBox at all. + # + # Retried inside the poll loop, not fired once beforehand: + # triggerFailure() is a no-op unless MockCloudXR's session is + # already Connecting/Connected (see MockCloudXR.ts), and + # run_oob_connect() returns as soon as the button text changes - + # which happens on WebXR session entry, near-instantly, likely + # before CloudXR.createSession()'s own async chain + # (applyTargetFrameRate/requestReferenceSpace) has finished. A + # call before the session exists silently does nothing, so retry + # until the banner actually appears rather than firing once and + # hoping the timing lines up. Idempotent once it does land: a + # session already in Error state fails the same Connecting/ + # Connected guard, so later retries no-op harmlessly. + # 5s is ample margin above the ~1s this actually takes once the + # session exists (observed) - MockCloudXR's own transitions are + # near-instant, unlike the relaunch test below which waits on + # real am start + tab discovery. + banner = "" + deadline = asyncio.get_running_loop().time() + 5.0 + while asyncio.get_running_loop().time() < deadline: + await cdp_evaluate( + ws_url, + "window.__mockCloudXRFail('simulated crash', 0xc0f22300)", + ) + r = await cdp_evaluate( + ws_url, + """(function() { + const box = document.getElementById('errorMessageBox'); + if (!box || !box.classList.contains('show')) return ''; + if (box.classList.contains('success') || + box.classList.contains('info')) return ''; + return document.getElementById('errorMessageText') + ?.textContent?.trim() || ''; + })()""", + ) + banner = (r.get("result") or {}).get("value") or "" + if banner: + break + await asyncio.sleep(0.5) + + assert "simulated crash" in banner + + task.cancel() + with contextlib.suppress(asyncio.CancelledError): + await task + + +async def test_run_oob_connect_real_mock_cxr_crash_triggers_real_relaunch( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """A real client-side terminal error should trigger a real auto-relaunch: + a second real ``am start``, a second real tab, clicked back to CONNECT-ready. + + Anticipatory, like ``test_build_teleop_url_forwards_reliability_config_from_env`` + above: gmorgan/oob-error-relaunch (#1146) adds this behavior + (``_find_and_click_teleop_tab``, a repair-capable ``_monitor_teleop_error_banner``) + and hasn't merged here yet, so this currently fails - not on an ImportError of a + private function (deliberately not imported here at all, so this stays correct + across internal refactors), but on the black-box assertions below: today's + monitor only logs the banner this test triggers, it never relaunches anything, so + no second tab/``am start`` ever appears and the wait below times out. Exercising + exactly what #1146 promises - a genuine crash recovered by genuine ADB+CDP + automation, not a scripted stand-in for either - is exactly what the real-browser + infrastructure in this module exists to prove, unlike + ``test_build_teleop_url_forwards_reliability_config_from_env``'s plain + env-var/string check. + """ + client_port = _free_tcp_port() + monkeypatch.setenv("TELEOP_WEB_CLIENT_BASE", f"http://localhost:{client_port}") + monkeypatch.delenv("CONTROL_TOKEN", raising=False) + + wss_port = _free_tcp_port() + backend_port = _free_tcp_port() + user_data_dir = tmp_path / "chrome-profile" + user_data_dir.mkdir() + install_dir = tmp_path / "cxr-install" + + with ( + static_webxr_build( + client_port, + npm_build_script="build:app-mock", + build_dir_name="build-app-mock", + ), + real_chrome(_CDP_LOCAL_PORT, user_data_dir=user_data_dir) as chrome, + real_browser_adb(chrome) as fake_adb, + ): + async with real_wss_proxy( + install_dir, proxy_port=wss_port, backend_port=backend_port + ): + task = await run_oob_connect( + resolved_port=wss_port, timeout=30.0, usb_local=True + ) + assert isinstance(task, asyncio.Task) + + teleop_tabs_before = [ + t for t in chrome.list_tabs() if "oobEnable=" in (t.get("url") or "") + ] + assert len(teleop_tabs_before) == 1 + ws_url = teleop_tabs_before[0]["webSocketDebuggerUrl"] + am_start_calls_before = sum( + 1 for c in fake_adb.calls if "am start" in " ".join(c) + ) + + # Same retry-until-it-lands approach as the crash-banner test above: + # triggerFailure() no-ops until MockCloudXR's session actually exists. + # 15s: once #1146 lands, a relaunch has no cold-launch devtools-socket + # wait (browser's already running) - just am start + tab discovery + + # click, which the initial launch above already does in ~5s. + relaunched = False + deadline = asyncio.get_running_loop().time() + 15.0 + while asyncio.get_running_loop().time() < deadline: + await cdp_evaluate( + ws_url, + "window.__mockCloudXRFail('simulated crash', 0xc0f22300)", + ) + am_start_calls_now = sum( + 1 for c in fake_adb.calls if "am start" in " ".join(c) + ) + teleop_tabs_now = [ + t + for t in chrome.list_tabs() + if "oobEnable=" in (t.get("url") or "") + ] + if ( + am_start_calls_now > am_start_calls_before + and len(teleop_tabs_now) >= 2 + ): + relaunched = True + break + await asyncio.sleep(0.5) + + assert relaunched, ( + "expected a second real `am start` and a second real tab after " + "a terminal client error - today's (pre-#1146) monitor only logs " + "the banner, it never relaunches anything" + ) + + task.cancel() + with contextlib.suppress(asyncio.CancelledError): + await task From 97f1cbfaba599ed38f4972d799e2f7f5ad6ed9ee Mon Sep 17 00:00:00 2001 From: Gareth Morgan Date: Fri, 9 Oct 2026 14:29:03 -0400 Subject: [PATCH 2/2] test(cloudxr): fix RealBrowserAdb check=True handling, trim review findings - RealBrowserAdb's am start/forward branches built CompletedProcess directly, bypassing the check=True handling #1147 just added to FakeAdb - a caller passing check=True (as _adb_forward_cdp() does) got no CalledProcessError on a simulated forward/am-start failure. Route both branches through the same check-honoring result() helper FakeAdb uses. - real_wss_proxy() restored TELEOP_OOB_HUB_ONLY on exit but not CXR_INSTALL_DIR, leaking it into later tests in the same process. - Removed a duplicated comment line and two stale docstrings/messages that described #1146 as not yet merged (it has, onto this branch). - Dropped static_webxr_build()'s npm_build_script/build_dir_name parameters: every call site always passed the same pair, and the other pair was explicitly "not for tests." - Extracted the three tests' identical 8-line port/dir/env setup into a shared real_browser_env() helper in conftest.py. - Trimmed oob-real-browser-integration-test-plan.md from a 404-line exploration journal (superseded sections kept alongside the final state) down to the parts that are still useful as reference: what's real vs faked, the two build bugs found, the command inventory, and the test list. Co-Authored-By: Claude Sonnet 5 Signed-off-by: Gareth Morgan --- oob-real-browser-integration-test-plan.md | 469 ++++-------------- tests/python/core/cloudxr/conftest.py | 93 ++-- .../core/cloudxr/test_oob_teleop_adb.py | 106 ++-- 3 files changed, 177 insertions(+), 491 deletions(-) diff --git a/oob-real-browser-integration-test-plan.md b/oob-real-browser-integration-test-plan.md index 6564da9d45..6aade285dd 100644 --- a/oob-real-browser-integration-test-plan.md +++ b/oob-real-browser-integration-test-plan.md @@ -3,402 +3,101 @@ SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All SPDX-License-Identifier: Apache-2.0 --> -# OOB real-browser integration test — plan +# OOB real-browser integration test -Goal: exercise `run_oob_connect()`'s real orchestration logic against a real +Exercises `run_oob_connect()`'s real orchestration logic against a real Chromium instance running the real teleop web client (real IWER-emulated -WebXR, real CDP, real client JS), instead of today's fully-scripted -`mock_adb()` / fake-CDP-server unit tests (`gmorgan/oob-connect-tests`, -PR #1148). Those stay as fast unit coverage of the orchestration logic; this -is new, slower, additional integration coverage. - -**Revised: a minimal mock CloudXR runtime is required after all.** §3/§5 -originally concluded none was needed, reasoning that `oob_teleop_adb.py` -never talks to the runtime directly. That's still true, but it missed a -second-order effect: `_cdp_session_click_connect()` (`oob_teleop_adb.py:1584`) -raises `OobAdbError(f"Teleop connection failed: {error_text}")` if the real -client's `#errorMessageBox` shows an explicit error within 30s of the click. -With nothing listening on `backend_port`, `wss.py`'s `proxy_handler()` -accepts the client's signaling WS handshake, then immediately fails to reach -the backend and closes the connection out from under it — the real client -will very likely surface that as a real error, which means -`run_oob_connect()` would raise instead of returning a monitor task, for -real client-behavior reasons, not a gap in the OOB code itself. - -So: something needs to listen on `backend_port` and respond with synthetic -data — not to make signaling "succeed" in any protocol-complete sense, just -enough that the client doesn't explicitly error out within the click-wait -window. Scope stays minimal and empirical: start with the cheapest thing -that might work (accept the WS, hold it open, respond to nothing) and only -add real SDP/ICE handling if the real client's own error/timeout behavior -requires it. Full WebRTC media (real rendering) remains explicitly out of -scope / a separate future project — this is only "enough to not error." - -## 0. Confirmed architecture (superseded — see §8 for what's actually implemented) - -Four real components, one mock: - -1. **Real webxr client** (`App.tsx`, `npm run dev-server`, port 8080) in a - real desktop Chromium — not `MockCloudXR.ts`. Uses real IWER to emulate a - WebXR/OpenXR-backed headset client-side (`loadIWERIfNeeded`). -2. **Real `wss.py` + `OOBControlHub`**, running for real (self-signed cert, - hub wired in), pointed at an unused `backend_port` (49100). No CloudXR - runtime behind it — the resulting connection refusal is handled by - existing production code, not test infra. Required so both the CloudXR - SDK *and* the OOB control channel (`wss://{serverIP}:{port}/oob/v1/ws`) - have a real `serverIP:port` to resolve to. -3. **Real DevTools/CDP** on that real Chromium instance — nothing to fake. -4. **Mock ADB** — the one actual test double, since no physical Android - device exists. Must stay controllable/parametrized like today's - `FakeAdb(device_state=, am_start_rc=, devtools_socket=, forward_rc=)`, so - tests can still deliberately trigger the real error paths (unauthorized/ - offline device, `am start` failure, no devtools socket, forward failure) - — but the success-path commands (`am start`, `adb forward`) now drive - the real browser/CDP port instead of returning canned output (see §3). - -## 1. Exhaustive inventory: everything the OOB code talks to - -### A. ADB (device/OS layer) — driven directly by `oob_teleop_adb.py` - -Read-only queries: +WebXR, real CDP, real client JS), as additional, slower coverage alongside +the fast, fully-scripted `mock_adb()` / fake-CDP-server unit tests already in +`test_oob_teleop_adb.py`. + +## What's real, what's faked + +- **ADB**: no physical device exists in CI/dev-sandbox, so this stays + synthetic via `RealBrowserAdb` (`conftest.py`) — but `am start` and `adb + forward` have real side effects (navigating a real local Chromium, aliasing + the local TCP port straight to Chromium's own `--remote-debugging-port`) + instead of returning canned output. Error-injection kwargs + (`device_state`, `am_start_rc`, `devtools_socket`, `forward_rc`) work the + same as on `FakeAdb`. +- **CDP**: fully real — a real Chromium (`real_chrome()`, matching + `playwright.config.js`'s launch args: SwiftShader software WebGL, + `--disable-features=WebXR` so IWER provides `navigator.xr`) reachable over + its own `/json`, `/json/close/`, and debugger websocket. +- **OOB control-hub** (`/oob/v1/ws`): the real `OOBControlHub`, via a real + `wss.py` (`real_wss_proxy()`), run for real — pure Python, no GPU + dependency. +- **CloudXR signaling**: never a real or mocked runtime backend. + `wss.py` already treats "nothing listening on `backend_port`" as an + anticipated configuration (`_is_backend_connection_refused()`), not an + error to work around, so the client's own `#errorMessageBox` on a refused + signaling connection is a genuine, free signal — not something this test + infra needs to fake. +- **The web client**: always `MockCloudXR` (`npm run build:app-mock`), never + the real `@nvidia/cloudxr` SDK. The real SDK genuinely tries to stream + against whatever's on `backend_port`, which is unpredictable and + uncontrollable to assert against. `MockCloudXR` mounts the identical + `App.tsx`/`CloudXRComponent.tsx` UI (same `#startButton`/`#errorMessageBox` + DOM) with a fully deterministic, externally-controllable session + underneath — `window.__mockCloudXRFail(message?, code?)`, callable over CDP + via `cdp_evaluate()` — and opens no socket of its own. + +## Two bugs found building this infra (worth knowing if this code is touched again) + +- **Webpack HMR module duplication.** A dev-server's hot-reload re-executes a + module's top-level code on live-reload, creating a *second* instance of it + side by side with the one React already mounted against. For + `cloudxr-mock-alias.ts` this meant `window.__mockCloudXRFail()` silently + bound to a fresh, never-used `activeSession` while the real, + already-running session lived on untouched — callable, no error, just no + effect. Fixed by never using a dev-server: `static_webxr_build()` + (`conftest.py`) always does a real `npm run build:app-mock` production + build and serves the static output, which has no live-reload runtime at + all. +- **`` double-invoking effects in a dev-mode build.** + Independent of HMR: `webpack.app-mock.js` used `mode: 'development'`, and + `src/index.tsx` wraps the app in `StrictMode`, which deliberately + double-invokes effects in dev builds — calling `CloudXRComponent.tsx`'s + `establishSession()` (and so `CloudXR.createSession()`) twice. Fixed by + switching `webpack.app-mock.js` to `mode: 'production'`, which sets + `NODE_ENV=production` and makes React skip StrictMode's double-invoke + entirely. +- A third, unrelated trap hit while diagnosing the above: `webpack.common.js`'s + persistent filesystem cache (`cache: {type: 'filesystem'}`) is keyed only + on the webpack config file, not on every source file it compiles — a stale + cache entry silently served an old compiled bundle that didn't include a + real source edit. `static_webxr_build()` now deletes + `node_modules/.cache/webpack` before every build. + +## ADB/CDP command inventory (what `oob_teleop_adb.py` actually talks to) | adb command | Function | Used for | |---|---|---| -| `adb get-state` | `adb_device_state()` → `assert_adb_device_online()` | Preflight gate before nearly every other call; drives offline→`adb reconnect` retry and user-facing error text | -| `adb devices` | `assert_exactly_one_adb_device()` | Pins a single device (or validates `ANDROID_SERIAL`) | -| `adb shell ip -o -4 addr show` | `headset_non_loopback_interfaces()` | USB-local WiFi-presence check (`require_headset_non_loopback_network()`); polled every 5s by `monitor_headset_wifi()` during a session | -| `adb shell dumpsys power` | `headset_wakefulness()` | `mWakefulness=` regex; polled by `assert_headset_awake()` after sending a wake key | -| `adb shell getprop ro.product.manufacturer` / `ro.product.brand` | `_adb_getprop()` → `headset_browser_package()` | Vendor string selects which browser package to force | -| `adb shell pm list packages ` | `_adb_pkg_installed()` / `_first_installed_pkg()` | Confirms the chosen vendor browser is actually installed | -| `adb shell cat /proc/net/unix` | `_discover_devtools_socket()` | Regex-scans for `@..._devtools_remote[_pid]`; priority order `weblayer` > `com.oculus.browser` > `chrome` > `webview` | -| `adb reverse --list` | `verify_adb_reverse_rules()` | Confirms USB-local reverse rules survived (rc=0 doesn't guarantee persistence) | - -Actions (side-effecting): - -| adb command | Function | Effect | -|---|---|---| -| `adb reconnect` | `assert_adb_device_online()` | One retry on transient `offline` | -| `adb shell input keyevent KEYCODE_WAKEUP` | `assert_headset_awake()` | Wakes the device | -| `adb shell am start -a android.intent.action.VIEW -d [-p ]` | `open_url_on_headset()` → `run_adb_headset_bookmark()` / `run_oob_connect()` | **Opens the browser to the teleop URL.** Retried in a poll loop inside `run_oob_connect()` until a matching tab appears over CDP | -| `adb reverse tcp: tcp:` | `setup_adb_reverse_ports()`, `setup_adb_reverse_turn()` | USB-local: headset-loopback → PC port mapping (proxy port, backend port, TURN port) | -| `adb reverse --remove tcp:` | `teardown_adb_reverse_ports()`, `teardown_adb_reverse_turn()` | Cleanup | -| `adb forward tcp: localabstract:` | `_adb_forward_cdp()` | Maps the discovered DevTools abstract socket to a local TCP port — this is what makes CDP reachable at all | -| `adb forward --remove tcp:` | `_adb_forward_remove()` / `teardown_adb_forward_cdp()` | Cleanup (also run at startup to clear a stale rule from a crashed prior run) | - -### B. Chrome DevTools Protocol (CDP) — reached over the `adb forward`'d port, driven directly by `oob_teleop_adb.py` - -- `GET http://localhost:/json` (`_cdp_list_tabs()`) — list tabs. Used by - `_close_stale_teleop_tabs()` (closes any tab whose URL contains - `oobEnable=`, freeing XR resources a crashed prior session held) and by - `run_oob_connect()`'s tab-matching loop after `am start`. -- `GET http://localhost:/json/close/` — close a specific tab. -- CDP WebSocket session on a tab's `webSocketDebuggerUrl` - (`_cdp_session_click_connect()`): `Security.setIgnoreCertificateErrors` - (cert bypass), then `Runtime.evaluate` polling a client-side readiness - state machine (`loading` → `initializing` [IWER + capability checks - running] → `ready`/`failed`), then synthesizes the CONNECT click. -- A second, independent CDP WebSocket session, opened by - `_monitor_teleop_error_banner()` once `run_oob_connect()` returns: polls - `document.getElementById('errorMessageBox')` once/second and logs any new - `error`-class banner text. Runs until cancelled or the tab/socket drops; - always tears down its own `adb forward` on exit. - -CDP is purely browser automation — it never touches application/CloudXR -traffic. - -### C. What the client JS itself opens, once loaded — *not* driven by -`run_oob_connect()`'s Python code, but part of the same end-to-end flow - -- **CloudXR streaming signaling websocket.** The real `@nvidia/cloudxr` - client component opens this once CONNECT is clicked and - `requestSession()` succeeds. It terminates at `wss.py`'s real, pure-Python - WSS proxy (TLS termination + WebSocket forwarding), which in turn forwards - to a native CloudXR Runtime backend process - (`CloudXRService._runtime_proc`, `service/_service.py`, launched via - `subprocess.Popen`) — GPU-dependent, and the one piece the existing test - suite (`test_service.py`'s `mock_service_deps()`) always mocks out rather - than launching for real. -- **OOB control-hub websocket** (`/oob/v1/ws`, `oob_teleop_hub.py`, - `OOBControlHub`). Pure Python, no GPU dependency. Headset registers - (`{"type": "register", "role": "headset", ...}`), gets a `hello`, then - sends `streamStatus`/`clientMetrics`; operator-side config pushes flow - back down the same socket. Fully real and runnable with zero mocking. -- `MockCloudXR.ts` (`deps/cloudxr/webxr_client/tests/mock/MockCloudXR.ts`, - behind `CloudXRComponentTest.html` / `npm run dev-server:component-mock`) - is a *third*, unrelated option: it fakes the CloudXR session object - entirely client-side (synthetic stereo render) and opens **no** websocket - at all. Confirmed via grep — zero `WebSocket` usage in that file. Useful - as today's manual dev-server page, but doesn't exercise the real signaling - path, so it's not what we want for this integration test if the goal is - exercising real client↔server behavior. - -## 2. What "real" actually means for this test - -- **ADB layer**: no real device exists in CI/dev-sandbox, so this stays - synthetic — but with real side effects where one exists locally, per the - session's prior discussion. Concretely: - - `am start` → parse the `-d ` out of the real shell command string - and actually navigate a real local Chromium instance to it (not a - canned success). - - `adb forward` / `--remove` → alias the local TCP port directly to - Chromium's own `--remote-debugging-port` (no real abstract socket - exists locally, so there's nothing to really forward — but the *effect*, - a working local CDP port, is real). - - `get-state`, `/proc/net/unix`, `getprop`, `pm list packages` → stay - canned; there is no real device for these to reflect. -- **CDP layer**: fully real. A real Chromium (matching - `deps/cloudxr/webxr_client/playwright.config.js`'s launch args — - `--disable-features=WebXR`, SwiftShader software WebGL, no sandbox) is - reachable over its own real `/json`, `/json/close/`, and debugger - websocket. Nothing to fake here at all. -- **Client-opened sockets layer**: split. - - OOB control-hub (`/oob/v1/ws`) — run the real `OOBControlHub` for real. - No mock needed. - - CloudXR signaling — `wss.py`'s proxy is real Python, runs for real. - **Resolved: no runtime backend needed, real or mocked.** `wss.py` - already treats "nothing listening on `backend_port`" as an anticipated, - supported configuration, not an error to work around — - `_is_backend_connection_refused()` (`wss.py:454`) detects exactly this - and `proxy_handler()` logs it as `"expected when running WSS+hub without - the runtime; teleop signaling uses %s"`, then closes that one signaling - connection and returns; nothing crashes. It also never touches the OOB - hub — `OOB_WS_PATH` is routed to `hub.handle_connection()` *before* - `proxy_handler()` is ever reached (`wss.py:644`), so hub - register/`hello`/`streamStatus`/`clientMetrics` and the - `/api/oob/v1/*` HTTP routes are unaffected either way. So: never start - anything on `backend_port` (49100) for this test. The client will very - likely show a real connection-error banner after CONNECT is clicked - (its own signaling socket gets refused) — a genuine, free signal - `_monitor_teleop_error_banner()` already knows how to read, usable as a - bonus assertion but not required for the test's core scope. - -## 3. Proposed test shape - -New test double(s) in `tests/python/core/cloudxr/conftest.py`, alongside the -existing `FakeAdb`/`mock_adb()`: - -- `RealBrowserAdb` (name TBD): same `subprocess.run`-patching shape as - `FakeAdb`, but `am start` and `adb forward` have the real side effects - described in §2, driven against a Chromium instance the test itself - launches (Playwright, matching `playwright.config.js`'s launch args) and - navigates to the real `webxr_client` bundle served locally (reuse - `npm run dev-server:component-mock` or an equivalent dev-server target - that serves the *real* CloudXR component, not `MockCloudXR.ts`). -- New integration test (separate file or a clearly-marked slow section of - `test_oob_teleop_adb.py`) driving the real `run_oob_connect()` end to end: - device-online → `am start` opens the real tab → real devtools socket - reachable → real CDP tab discovery finds the real tab → real - `_cdp_session_click_connect()` polls the *real* client's readiness state - machine (real IWER capability checks) → real click → real monitor task - reading the real client's error banner. -- Keep this as new, separate, explicitly slower coverage — don't fold it - into the existing fast `mock_adb()`/fake-CDP-server suite from PR #1148. - -## 4. Setup gaps (wiring/plumbing to build — not mocks) - -Confirmed: nothing in §1–3 needs a protocol-level mock. `npm run dev-server` -(`webpack.dev.js`, port 8080) already serves the *real* production client -(`App.tsx`), not `MockCloudXR.ts` — this is "the CloudXRComponent" the test -needs, and it's already runnable today. What's actually missing is wiring: - -1. **`wss.py` isn't started anywhere for this purpose.** Needs launching as - a real process with the OOB hub wired in (`OOBControlHub`) and *some* - `backend_host`/`backend_port` configured for the CloudXR-signaling - forward target — even an unused one is fine, since `/oob/v1/ws` and the - `/api/oob/v1/*` HTTP routes work independently of whether anything - answers on the signaling side. It generates its own self-signed cert; no - action needed there. -2. **No code today launches a real Chromium process directly** (as opposed - to through `adb`). `FakeAdb` only patches `subprocess.run` for synthetic - `adb` calls. Need a small helper that spawns Chrome with - `--remote-debugging-port=`, reusing `playwright.config.js`'s known - working launch args and — critically — its `executablePath` override - (`/usr/bin/google-chrome`): this sandbox cannot reach Playwright's own - browser-download CDN, a constraint already hit once building that config. -3. **URL construction** must go through the existing `build_teleop_url()` / - `build_headset_bookmark_url()` helpers (`oob_teleop_env.py`), pointed at - `localhost:8080` (the client) with `serverIP`/`port` aimed at the local - `wss.py` instance and `oobEnable=1` set, rather than hand-assembling a - query string — this is also what makes `_close_stale_teleop_tabs()`'s - `oobEnable=` tab filter match. -4. Cert trust is already a non-issue: `_cdp_session_click_connect()` already - sends CDP `Security.setIgnoreCertificateErrors` before polling. - -## 5. Exhaustive check: what talks to the CXR runtime directly - -Repo-wide search (Python + vendored `deps/`) for any direct connection to -the runtime — its WS port (`backend_port`, default `49100`) or its -`ipc_cloudxr` Unix socket. Exactly two hits, both already covered above: - -1. **`wss.py:proxy_handler()` → `ws_connect(f"ws://{backend_host}:{backend_port}{path}")`.** - The only WS client of the runtime anywhere in the codebase. A dumb - relay — `_pipe()` forwards every message verbatim, `async for msg in - src: await dst.send(msg)`, never parsing SDP/ICE/JSON content. This is - the one seam a mock runtime would need to satisfy to give the client a - real, successful streaming session (bind `49100`, accept the WS - connection, do real SDP/ICE negotiation, send synthetic media over a - real `RTCPeerConnection` — confirmed via `RTCPeerConnection`/ - `createOffer`/`createAnswer`/`addIceCandidate`/`iceServers` strings in - the vendored `@nvidia/cloudxr` bundle). -2. **`runtime.py:is_runtime_live()` → raw `AF_UNIX` `connect()` to - `/ipc_cloudxr`.** A liveness probe only (no protocol messages), - used solely by `CloudXRService._cleanup_stale_runtime` in `service.py`'s - own launch path. Not touched by the OOB/CDP/client flow this plan covers - — irrelevant to a mock runtime built for the goal above. - -Everything else matching `49100`/`libcloudxr`/`nv_cxr_service` is either the -runtime process *becoming* the listener (`runtime.py:run()`, -`deps/cloudxr/runtime/main.py` — these load `libcloudxr.so` in-process, they -don't connect to anything) or test/comment noise. - -**Conclusion: `run_oob_connect()`'s own code (`oob_teleop_adb.py`) never -touches the runtime, directly or indirectly** — it only ever does ADB and -CDP (§1.A/B). The runtime is entirely orthogonal to what this plan's -integration test exercises. A mock runtime is only relevant if the goal -shifts to proving out a real, successful, rendering CloudXR session — a -separate concern from OOB orchestration coverage, and (per the top-of-file -scope note) tracked as its own separate project, not folded into this one. - -## 6. Open questions before implementation - -1. ~~Does this test need the CloudXR signaling socket to succeed~~ — - **resolved, see §3**: no runtime backend, real or mocked. Never bind - `backend_port`; `wss.py` handles the refused connection as an - anticipated configuration. -2. ~~Where does the local dev-server get started from~~ — **resolved, see - §7**: a Python-side `conftest.py` fixture, reusing the pattern already - used for `live_ipc_socket`/`mock_service_deps`. -3. CI feasibility: deliberately deferred (see §7) — this test runs manually - for now; CI wiring (parallel-worker port collisions, ephemeral ports, - headless-runner Chrome availability) is a future PR. - -## 7. Dev-server launch: where and how (manual-run scope; CI deferred) - -Surveyed two precedents before deciding: - -- **This repo's `playwright.config.js`** uses Playwright's own `webServer` - config (`command: 'npm run dev-server:component-mock'`, fixed port 8083, - `reuseExistingServer: !process.env.CI`) — JS-test-runner machinery, not - usable from a Python pytest test. -- **`~/gitlab/cloudxr-js`'s `tests/playwright/global-setup.js`** spawns - `npx http-server -p 0 --cors` as a child process, scrapes the - ephemeral bound port from stdout, writes it to a side-channel file - (`.http-server-port`) for fixtures to read — built for CI parallelism - (many workers, no fixed-port collisions). That repo also runs a much - heavier **real** (not mock) streaming e2e - (`tests/docker-compose.test-e2e.yaml`): three containers - (`cloudxr-runtime` = the actual GPU-backed CloudXR runtime image, - `webapp`, `playwright`), requiring an NVIDIA GPU + docker + - `nvidia-container-toolkit`. Confirms no lightweight mock runtime exists - anywhere in either repo today — that remains genuinely separate, - greenfield work (per the top-of-file scope note), well outside what this - plan needs. - -**Decision for this plan:** launch the dev server from a new Python-side -`@contextmanager` fixture in `tests/python/core/cloudxr/conftest.py`, -following this repo's own existing convention (`live_ipc_socket`, -`mock_service_deps`) rather than cloudxr-js's Node-side pattern: - -- `subprocess.Popen(["npm", "run", "dev-server"], cwd=/deps/cloudxr/webxr_client, ...)`, - fixed port `8080` (matches `webpack.dev.js`'s default, and what's already - running manually in this session today) — skip ephemeral-port scraping; - not needed without parallel CI workers. -- Mirror `playwright.config.js`'s `reuseExistingServer: !process.env.CI` - idea: probe port 8080 first and reuse an already-running server if found; - only spawn (and later tear down) one if nothing answers. Keeps manual runs - fast and avoids colliding with a dev-server already running for other - purposes. -- Ephemeral-port scraping (`cloudxr-js`'s approach) and any CI-specific - wiring are explicitly deferred to the future CI PR. - -## 8. Final state (as implemented — supersedes §0, §7's dev-server decision) +| `adb get-state` | `adb_device_state()` → `assert_adb_device_online()` | Preflight gate before nearly every other call; drives offline→`adb reconnect` retry | +| `adb shell cat /proc/net/unix` | `_discover_devtools_socket()` | Regex-scans for `@..._devtools_remote[_pid]` | +| `adb shell am start -a android.intent.action.VIEW -d ` | `open_url_on_headset()` → `run_adb_headset_bookmark()` / `run_oob_connect()` | Opens the browser to the teleop URL; retried in a poll loop until a matching tab appears over CDP | +| `adb forward tcp: localabstract:` | `_adb_forward_cdp()` | Maps the discovered DevTools abstract socket to a local TCP port — what makes CDP reachable at all | +| `adb forward --remove tcp:` | `_adb_forward_remove()` | Cleanup | -Reality diverged from §0/§7 during implementation, for good reasons found -along the way. This is what's actually in `conftest.py`/`test_oob_teleop_adb.py` -now: +CDP itself (`_cdp_list_tabs()`, `_cdp_session_click_connect()`, +`_monitor_teleop_error_banner()`) is reached over the `adb forward`'d port +and never touches application/CloudXR traffic directly. -1. **Always `MockCloudXR`, never the real `@nvidia/cloudxr` SDK.** §0 wanted - the real SDK for "real rendering"; in practice the real SDK genuinely - tries to stream against whatever's on `backend_port`, which is - unpredictable and gives a test nothing to assert against. `MockCloudXR` - (via `tests/mock/cloudxr-mock-alias.ts`'s webpack alias) mounts the exact - same `App.tsx`/`CloudXRComponent.tsx` UI (same `#startButton`/ - `#errorMessageBox` DOM `run_oob_connect()`'s own code already polls) with - a fully deterministic, externally-controllable session underneath — - `window.__mockCloudXRFail(message?, code?)`, callable over CDP via - `cdp_evaluate()`. It never opens a socket of its own, so **no - `minimal_mock_runtime()` stand-in exists any more at all** — that whole - piece from §3 is gone. -2. **Never a dev-server — always a static production build** - (`static_webxr_build()`, replacing §7's `real_webxr_dev_server()`). - Two independent, real bugs forced this and are worth knowing about if - this code is ever touched again: - - **Webpack HMR module duplication.** A dev-server's hot-reload - re-executes a module's top-level code on live-reload, creating a - *second* instance of it side by side with the one React already - mounted against. For `cloudxr-mock-alias.ts` this meant - `window.__mockCloudXRFail()` silently bound to a fresh, never-used - `activeSession` while the real, already-running session lived on - untouched in the original instance — callable, no error, just no - effect. A static build has no live-reload runtime at all, so this is - structurally impossible now, not just avoided by luck. - - **`` double-invoking effects in a dev-mode build.** - Independent of HMR: `webpack.app-mock.js` used `mode: 'development'`, - and `src/index.tsx` wraps the app in `StrictMode`, which deliberately - double-invokes effects in dev builds — calling - `CloudXRComponent.tsx`'s `establishSession()` (and so - `CloudXR.createSession()`) twice. Fixed by switching - `webpack.app-mock.js` to `mode: 'production'`, which sets - `NODE_ENV=production` and makes React skip StrictMode's double-invoke - entirely. - - **A third, unrelated trap on the way to diagnosing the above:** - `webpack.common.js`'s persistent filesystem cache - (`cache: {type: 'filesystem'}`) is keyed only on the webpack config - file (`buildDependencies.config`), not on every source file it - compiles — a stale cache entry silently served an old compiled - `cloudxr-mock-alias.ts` that didn't include a real source edit at all - (confirmed by `grep`ping the built bundle for a symbol that wasn't - there). `static_webxr_build()` now deletes - `node_modules/.cache/webpack` before every build; correctness matters - far more than incremental-build speed for a test fixture that already - rebuilds from scratch every run. -3. **`window.__mockCloudXRFail(message?, code?)` needs a `code` in the - `0xc0f22300`–`0xc0f223ff` range to produce a real, DOM-visible error.** - A code-less failure is "recoverable" per - `helpers/streamingErrorClassification.ts`'s `isRecoverable()`, and - `CloudXRComponent.tsx`'s `onStreamStopped` auto-reconnects on those - silently, without ever showing `#errorMessageBox` — exactly like a real - transient failure would. `cloudxr-mock-alias.ts` was extended to accept - an optional `code` (previously message-only) to reach the non-recoverable - path, matching what `tests/mock/CloudXRComponentTest.tsx`'s own - `NON_RETRYABLE_CODE` steps already did internally. -4. **Fire `__mockCloudXRFail()` in a retry loop, not once.** It's a no-op - until MockCloudXR's session actually exists and is - `Connecting`/`Connected`; `run_oob_connect()` returns as soon as the - button text changes, which happens on WebXR session entry — near - instantly, and likely before `CloudXR.createSession()`'s own async chain - has finished. Retrying is safe: once the failure lands the session moves - to `Error` state, so later calls just keep failing the same guard - harmlessly. +## Tests -Three tests now exist in `test_oob_teleop_adb.py`: +Three tests in `test_oob_teleop_adb.py`, all slow (real browser + a real +webpack production build + real `wss.py`), requiring a system Chrome +(`PLAYWRIGHT_CHROME_PATH` override; defaults to `/usr/bin/google-chrome`) and +Node/npm on `PATH` — run manually for now, not yet wired into CI: - `test_run_oob_connect_real_browser_end_to_end` — the full real - adb→CDP→click path against `MockCloudXR`, proving `run_oob_connect()`'s - own orchestration for real. + adb→CDP→click path against `MockCloudXR`, proving `run_oob_connect()`'s own + orchestration for real. - `test_run_oob_connect_real_mock_cxr_crash_surfaces_error_banner` — proves the crash-trigger mechanism itself: a real, deterministic, DOM-visible error banner on demand. - `test_run_oob_connect_real_mock_cxr_crash_triggers_real_relaunch` — - **anticipatory**, written against `gmorgan/oob-error-relaunch` (#1146) - before that branch has merged here. It deliberately does not import any - private function from that branch (so it can't fail on an ImportError - that would depend on exact internal names surviving the merge); instead - it asserts on black-box observable recovery — a second real `am start` - and a second real tab after a genuine triggered crash. It fails today, - correctly, on that assertion after a real 30s wait (not a collection - error), and should turn green the moment #1146's relaunch logic lands on - this branch. `test_build_teleop_url_forwards_reliability_config_from_env` - (added to the plain unit-test section, no browser needed) is the same - "write it now, let it fail until merged" idea applied to - `gmorgan/oob-reliability-config-wiring` (#1145)'s config-forwarding logic. + exercises the repair-capable `_monitor_teleop_error_banner` added by + `gmorgan/oob-error-relaunch` (#1146): a genuine crash recovered by genuine + ADB+CDP automation (a second real `am start` and a second real tab), not a + scripted stand-in for either. diff --git a/tests/python/core/cloudxr/conftest.py b/tests/python/core/cloudxr/conftest.py index 30c0f22cb7..eaebe2643f 100644 --- a/tests/python/core/cloudxr/conftest.py +++ b/tests/python/core/cloudxr/conftest.py @@ -26,6 +26,7 @@ import urllib.error import urllib.request from contextlib import asynccontextmanager, contextmanager +from dataclasses import dataclass from pathlib import Path from unittest.mock import MagicMock, patch @@ -435,10 +436,17 @@ def real_chrome(cdp_port: int, *, user_data_dir: Path): @contextmanager -def static_webxr_build(port: int, *, npm_build_script: str, build_dir_name: str): - """Build webxr_client once via a real ``npm run `` production - build (no dev-server, no HMR), then serve the resulting static directory with a - plain HTTP server for the block's duration. +def static_webxr_build(port: int): + """Build webxr_client once via a real ``npm run build:app-mock`` production build + (no dev-server, no HMR), then serve the resulting static directory with a plain + HTTP server for the block's duration. + + Always the mock build (``@nvidia/cloudxr`` aliased to ``MockCloudXR``, same + ``App.tsx``/``CloudXRComponent.tsx`` UI): the real SDK genuinely tries to stream + against whatever's on ``backend_port``, which is unpredictable and uncontrollable + to assert against. ``MockCloudXR`` exposes ``window.__mockCloudXRFail(message?, + code?)`` globally for a deterministic, controllable mid-session failure — see + :func:`cdp_evaluate` — and opens no socket of its own. Deliberately never a dev-server: webpack's hot-module-replacement re-executes a module's top-level code on live-reload, creating a *second* instance of it @@ -449,27 +457,12 @@ def static_webxr_build(port: int, *, npm_build_script: str, build_dir_name: str) build has no live-reload runtime at all, so this class of bug is structurally impossible, not just avoided by luck. - *npm_build_script*/*build_dir_name* pairs (see ``package.json``/the matching - ``webpack.*.js``). Always use the ``build:app-mock`` pair for a real-browser - test: the real SDK genuinely tries to stream against whatever's on - ``backend_port``, which is unpredictable and uncontrollable to assert against. - - * ``"build"`` / ``"build"`` — the real production client against the real - ``@nvidia/cloudxr`` SDK. Not for tests — kept only as a manual sanity - build; see the warning above. - * ``"build:app-mock"`` / ``"build-app-mock"`` — the identical ``App.tsx`` / - ``CloudXRComponent.tsx`` UI (same ``#startButton``/``#errorMessageBox`` - DOM), but with ``@nvidia/cloudxr`` aliased to ``MockCloudXR``, which exposes - ``window.__mockCloudXRFail(message?, code?)`` globally so a CDP - ``Runtime.evaluate`` call can force a deterministic, controllable - mid-session failure — see :func:`cdp_evaluate`. ``MockCloudXR`` opens no - socket of its own, so nothing needs to stand in for a CloudXR runtime at - all. - Always rebuilds (mirrors ``cloudxr-js``'s ``global-setup.js``: a stale bundle from a previous branch must never silently pass) rather than reusing whatever happens to be on disk or already listening on *port*. """ + npm_build_script = "build:app-mock" + build_dir_name = "build-app-mock" webxr_client_dir = repo_root() / "deps" / "cloudxr" / "webxr_client" # webpack.common.js turns on persistent filesystem caching # (cache: {type: 'filesystem'}), keyed only on the config file @@ -513,6 +506,40 @@ def static_webxr_build(port: int, *, npm_build_script: str, build_dir_name: str) thread.join(timeout=5) +@dataclass +class RealBrowserTestEnv: + """Ports/dirs shared by every real-browser ``run_oob_connect()`` test — see + :func:`real_browser_env`.""" + + client_port: int + wss_port: int + backend_port: int + user_data_dir: Path + install_dir: Path + + +@contextmanager +def real_browser_env(tmp_path: Path, monkeypatch): + """Allocate the ports/dirs every real-browser ``run_oob_connect()`` test needs + (client static-build port, wss proxy port, unused CloudXR backend port, a fresh + Chrome profile dir, a fresh install dir) and point ``TELEOP_WEB_CLIENT_BASE`` at + the client port, for the block's duration.""" + client_port = _free_tcp_port() + monkeypatch.setenv("TELEOP_WEB_CLIENT_BASE", f"http://localhost:{client_port}") + monkeypatch.delenv("CONTROL_TOKEN", raising=False) + + user_data_dir = tmp_path / "chrome-profile" + user_data_dir.mkdir() + + yield RealBrowserTestEnv( + client_port=client_port, + wss_port=_free_tcp_port(), + backend_port=_free_tcp_port(), + user_data_dir=user_data_dir, + install_dir=tmp_path / "cxr-install", + ) + + async def cdp_evaluate(ws_url: str, expression: str) -> dict: """Open a one-shot CDP session to *ws_url* and evaluate *expression*. @@ -553,6 +580,7 @@ async def real_wss_proxy(install_dir: Path, *, proxy_port: int, backend_port: in """ from cloudxr_py_test_ns.wss import run as wss_run # noqa: PLC0415 + prev_install_dir = os.environ.get("CXR_INSTALL_DIR") os.environ["CXR_INSTALL_DIR"] = str(install_dir) prev_hub_only = os.environ.get("TELEOP_OOB_HUB_ONLY") os.environ["TELEOP_OOB_HUB_ONLY"] = "1" @@ -584,6 +612,10 @@ async def real_wss_proxy(install_dir: Path, *, proxy_port: int, backend_port: in os.environ.pop("TELEOP_OOB_HUB_ONLY", None) else: os.environ["TELEOP_OOB_HUB_ONLY"] = prev_hub_only + if prev_install_dir is None: + os.environ.pop("CXR_INSTALL_DIR", None) + else: + os.environ["CXR_INSTALL_DIR"] = prev_install_dir class RealBrowserAdb(FakeAdb): @@ -607,20 +639,25 @@ def __init__(self, chrome: RealChrome, **kwargs) -> None: self._chrome = chrome def __call__(self, args: list[str], **kwargs) -> subprocess.CompletedProcess: - if args[:2] == ["adb", "shell"] and any("am start" in a for a in args): + def result( + rc: int, stdout: str = "", stderr: str = "" + ) -> subprocess.CompletedProcess: self.calls.append(list(args)) + completed = subprocess.CompletedProcess(args, rc, stdout, stderr) + if kwargs.get("check", False): + completed.check_returncode() + return completed + + if args[:2] == ["adb", "shell"] and any("am start" in a for a in args): if self.am_start_rc != 0: - return subprocess.CompletedProcess( - args, self.am_start_rc, "", "am start failed" - ) + return result(self.am_start_rc, stderr="am start failed") shell_cmd = args[2] tokens = shlex.split(shell_cmd) url = tokens[tokens.index("-d") + 1] self._chrome.open_url(url) - return subprocess.CompletedProcess(args, 0, "", "") + return result(0) if args[:2] == ["adb", "forward"] and "--remove" not in args: - self.calls.append(list(args)) - return subprocess.CompletedProcess(args, self.forward_rc, "", "") + return result(self.forward_rc) return super().__call__(args, **kwargs) diff --git a/tests/python/core/cloudxr/test_oob_teleop_adb.py b/tests/python/core/cloudxr/test_oob_teleop_adb.py index e1328a2165..8059dcd6ba 100644 --- a/tests/python/core/cloudxr/test_oob_teleop_adb.py +++ b/tests/python/core/cloudxr/test_oob_teleop_adb.py @@ -1748,9 +1748,9 @@ async def test_monitor_without_callback_still_logs_and_keeps_watching() -> None: from pathlib import Path # noqa: E402 from conftest import ( # noqa: E402 - _free_tcp_port, cdp_evaluate, real_browser_adb, + real_browser_env, real_chrome, real_wss_proxy, static_webxr_build, @@ -1768,30 +1768,17 @@ async def test_run_oob_connect_real_browser_end_to_end( _cdp_session_click_connect polls the real client's readiness state machine (real IWER capability checks) and clicks the real CONNECT button. """ - client_port = _free_tcp_port() - monkeypatch.setenv("TELEOP_WEB_CLIENT_BASE", f"http://localhost:{client_port}") - monkeypatch.delenv("CONTROL_TOKEN", raising=False) - - wss_port = _free_tcp_port() - backend_port = _free_tcp_port() - user_data_dir = tmp_path / "chrome-profile" - user_data_dir.mkdir() - install_dir = tmp_path / "cxr-install" - with ( - static_webxr_build( - client_port, - npm_build_script="build:app-mock", - build_dir_name="build-app-mock", - ), - real_chrome(_CDP_LOCAL_PORT, user_data_dir=user_data_dir) as chrome, + real_browser_env(tmp_path, monkeypatch) as env, + static_webxr_build(env.client_port), + real_chrome(_CDP_LOCAL_PORT, user_data_dir=env.user_data_dir) as chrome, real_browser_adb(chrome) as fake_adb, ): async with real_wss_proxy( - install_dir, proxy_port=wss_port, backend_port=backend_port + env.install_dir, proxy_port=env.wss_port, backend_port=env.backend_port ): task = await run_oob_connect( - resolved_port=wss_port, timeout=30.0, usb_local=True + resolved_port=env.wss_port, timeout=30.0, usb_local=True ) # A real task, not None: the click genuinely succeeded and @@ -1826,36 +1813,21 @@ async def test_run_oob_connect_real_mock_cxr_crash_surfaces_error_banner( matching what a real launch produces. Proves the crash-trigger mechanism itself — a real, real-DOM error banner - appears on demand — ahead of reusing it to test - ``_find_and_click_teleop_tab``'s actual recovery once this test infra is - merged onto ``gmorgan/oob-error-relaunch`` (that branch's monitor is - repair-capable; this branch's is not, so this test only proves the crash - signal is real and detectable, not that anything recovers from it). + appears on demand — independent of whether anything recovers from it; see + :func:`test_run_oob_connect_real_mock_cxr_crash_triggers_real_relaunch` + for the recovery assertion. """ - client_port = _free_tcp_port() - monkeypatch.setenv("TELEOP_WEB_CLIENT_BASE", f"http://localhost:{client_port}") - monkeypatch.delenv("CONTROL_TOKEN", raising=False) - - wss_port = _free_tcp_port() - backend_port = _free_tcp_port() - user_data_dir = tmp_path / "chrome-profile" - user_data_dir.mkdir() - install_dir = tmp_path / "cxr-install" - with ( - static_webxr_build( - client_port, - npm_build_script="build:app-mock", - build_dir_name="build-app-mock", - ), - real_chrome(_CDP_LOCAL_PORT, user_data_dir=user_data_dir) as chrome, + real_browser_env(tmp_path, monkeypatch) as env, + static_webxr_build(env.client_port), + real_chrome(_CDP_LOCAL_PORT, user_data_dir=env.user_data_dir) as chrome, real_browser_adb(chrome), ): async with real_wss_proxy( - install_dir, proxy_port=wss_port, backend_port=backend_port + env.install_dir, proxy_port=env.wss_port, backend_port=env.backend_port ): task = await run_oob_connect( - resolved_port=wss_port, timeout=30.0, usb_local=True + resolved_port=env.wss_port, timeout=30.0, usb_local=True ) assert isinstance(task, asyncio.Task) @@ -1865,7 +1837,6 @@ async def test_run_oob_connect_real_mock_cxr_crash_surfaces_error_banner( assert len(teleop_tabs) == 1 ws_url = teleop_tabs[0]["webSocketDebuggerUrl"] - # code in the 0xc0f22300-0xc0f223ff (server-disconnect) range: a # code in the 0xc0f22300-0xc0f223ff (server-disconnect) range: a # code-less failure is "recoverable" per # helpers/streamingErrorClassification.ts's isRecoverable(), and @@ -1924,44 +1895,23 @@ async def test_run_oob_connect_real_mock_cxr_crash_triggers_real_relaunch( """A real client-side terminal error should trigger a real auto-relaunch: a second real ``am start``, a second real tab, clicked back to CONNECT-ready. - Anticipatory, like ``test_build_teleop_url_forwards_reliability_config_from_env`` - above: gmorgan/oob-error-relaunch (#1146) adds this behavior - (``_find_and_click_teleop_tab``, a repair-capable ``_monitor_teleop_error_banner``) - and hasn't merged here yet, so this currently fails - not on an ImportError of a - private function (deliberately not imported here at all, so this stays correct - across internal refactors), but on the black-box assertions below: today's - monitor only logs the banner this test triggers, it never relaunches anything, so - no second tab/``am start`` ever appears and the wait below times out. Exercising - exactly what #1146 promises - a genuine crash recovered by genuine ADB+CDP - automation, not a scripted stand-in for either - is exactly what the real-browser - infrastructure in this module exists to prove, unlike - ``test_build_teleop_url_forwards_reliability_config_from_env``'s plain - env-var/string check. + Exercises the repair-capable ``_monitor_teleop_error_banner`` added by + gmorgan/oob-error-relaunch (#1146): a genuine crash recovered by genuine + ADB+CDP automation, not a scripted stand-in for either. Asserts on + black-box observables only (a second ``am start``/tab), not on any + private function, so it stays correct across internal refactors. """ - client_port = _free_tcp_port() - monkeypatch.setenv("TELEOP_WEB_CLIENT_BASE", f"http://localhost:{client_port}") - monkeypatch.delenv("CONTROL_TOKEN", raising=False) - - wss_port = _free_tcp_port() - backend_port = _free_tcp_port() - user_data_dir = tmp_path / "chrome-profile" - user_data_dir.mkdir() - install_dir = tmp_path / "cxr-install" - with ( - static_webxr_build( - client_port, - npm_build_script="build:app-mock", - build_dir_name="build-app-mock", - ), - real_chrome(_CDP_LOCAL_PORT, user_data_dir=user_data_dir) as chrome, + real_browser_env(tmp_path, monkeypatch) as env, + static_webxr_build(env.client_port), + real_chrome(_CDP_LOCAL_PORT, user_data_dir=env.user_data_dir) as chrome, real_browser_adb(chrome) as fake_adb, ): async with real_wss_proxy( - install_dir, proxy_port=wss_port, backend_port=backend_port + env.install_dir, proxy_port=env.wss_port, backend_port=env.backend_port ): task = await run_oob_connect( - resolved_port=wss_port, timeout=30.0, usb_local=True + resolved_port=env.wss_port, timeout=30.0, usb_local=True ) assert isinstance(task, asyncio.Task) @@ -1976,8 +1926,8 @@ async def test_run_oob_connect_real_mock_cxr_crash_triggers_real_relaunch( # Same retry-until-it-lands approach as the crash-banner test above: # triggerFailure() no-ops until MockCloudXR's session actually exists. - # 15s: once #1146 lands, a relaunch has no cold-launch devtools-socket - # wait (browser's already running) - just am start + tab discovery + + # 15s: a relaunch has no cold-launch devtools-socket wait (browser's + # already running) - just am start + tab discovery + # click, which the initial launch above already does in ~5s. relaunched = False deadline = asyncio.get_running_loop().time() + 15.0 @@ -2004,8 +1954,8 @@ async def test_run_oob_connect_real_mock_cxr_crash_triggers_real_relaunch( assert relaunched, ( "expected a second real `am start` and a second real tab after " - "a terminal client error - today's (pre-#1146) monitor only logs " - "the banner, it never relaunches anything" + "a terminal client error - the monitor should relaunch, not just " + "log the banner" ) task.cancel()