From 49c107888a48387afa04275bf84d47e7983a9de1 Mon Sep 17 00:00:00 2001 From: Gareth Morgan Date: Thu, 8 Oct 2026 19:30:31 -0400 Subject: [PATCH 1/6] 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 0af35b8e66..b76763dd9a 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 @@ -337,3 +343,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 32219dc409e7808c14e052b03ae3827886c5f85b Mon Sep 17 00:00:00 2001 From: Gareth Morgan Date: Fri, 9 Oct 2026 14:29:03 -0400 Subject: [PATCH 2/6] 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 b76763dd9a..7d7e0761ed 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 @@ -436,10 +437,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 @@ -450,27 +458,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 @@ -514,6 +507,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*. @@ -554,6 +581,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" @@ -585,6 +613,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): @@ -608,20 +640,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() From 92c43d024a37cd9a3e8831b9841414574f361be4 Mon Sep 17 00:00:00 2001 From: Gareth Morgan Date: Fri, 23 Oct 2026 20:03:32 -0400 Subject: [PATCH 3/6] test(cloudxr): end-to-end fragment token delivery through fake adb and a real browser Real build_teleop_url -> run_oob_connect -> FakeAdb am start -> Chrome opens the exact recorded URL -> the real web client (MockCloudXR build) registers with the real wss.run + OOBControlHub. Covers a generated and an awkward configured token, URL cleanup, no token in request targets or Referer, reload with a new registration and no second am start, and rejection of a wrong fragment token. Browser cases are opt-in (RUN_OOB_BROWSER_TESTS=1, -m oob_browser); a fast /bin/sh test checks the -d argument survives shell quoting. Co-Authored-By: Claude Sonnet 5.5 Signed-off-by: Gareth Morgan --- tests/python/core/cloudxr/pyproject.toml | 3 + .../cloudxr/test_oob_token_integration.py | 465 ++++++++++++++++++ 2 files changed, 468 insertions(+) create mode 100644 tests/python/core/cloudxr/test_oob_token_integration.py diff --git a/tests/python/core/cloudxr/pyproject.toml b/tests/python/core/cloudxr/pyproject.toml index d03fbcaef1..bd11e20670 100644 --- a/tests/python/core/cloudxr/pyproject.toml +++ b/tests/python/core/cloudxr/pyproject.toml @@ -29,5 +29,8 @@ python_files = ["test_*.py"] python_classes = ["Test*"] python_functions = ["test_*"] asyncio_mode = "auto" +markers = [ + "oob_browser: real Chrome + npm build; needs RUN_OOB_BROWSER_TESTS=1", +] # Prevent pytest from recursing into parent directories norecursedirs = [".git", ".venv", "build", "dist", "*.egg", "__pycache__"] diff --git a/tests/python/core/cloudxr/test_oob_token_integration.py b/tests/python/core/cloudxr/test_oob_token_integration.py new file mode 100644 index 0000000000..114d31b389 --- /dev/null +++ b/tests/python/core/cloudxr/test_oob_token_integration.py @@ -0,0 +1,465 @@ +# SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 + +"""The OOB control token travels in the URL fragment from the host to the hub. + +``build_teleop_url`` -> ``run_oob_connect`` -> :class:`conftest.FakeAdb` ``am start`` -> +real Chrome opens the exact recorded URL -> the real web client (``npm run +build:app-mock``: real ``App.tsx``/``CloudXRComponent``, MockCloudXR as the SDK) registers +with the real ``wss.run`` + ``OOBControlHub``. + +The browser cases need Chrome and an npm build, so they run only with +``RUN_OOB_BROWSER_TESTS=1`` (``-m oob_browser``); nothing runs them automatically. +Chrome here ignores TLS errors: certificate trust is bypassed, not covered. +""" + +from __future__ import annotations + +import asyncio +import contextlib +import json +import os +import shutil +import socket +import ssl +import subprocess +import threading +import time +import urllib.request +from functools import partial +from http.server import SimpleHTTPRequestHandler, ThreadingHTTPServer +from pathlib import Path +from unittest.mock import patch +from urllib.parse import parse_qs, urlencode, urlsplit + +import pytest +from websockets.exceptions import ConnectionClosed +from websockets.sync.client import connect as ws_connect + +from cloudxr_py_test_ns import oob_teleop_adb as adb_module +from cloudxr_py_test_ns import oob_teleop_env as env_module +from conftest import FakeAdb, mock_adb +from repo_paths import repo_root + +# Unpatched, for helpers that run while ``subprocess.run`` is faked. +_REAL_RUN = subprocess.run + +# A token no URL builder may pass through unencoded. +_AWKWARD_TOKEN = "tok&a=b?c#d+e f'\"g" + +_CHROME = os.environ.get("PLAYWRIGHT_CHROME_PATH", "/usr/bin/google-chrome") +_CHROME_ARGS = [ + "--no-sandbox", + "--ignore-gpu-blocklist", + "--use-angle=swiftshader", + "--use-gl=angle", + "--disable-features=WebXR", + "--ignore-certificate-errors", + "--no-first-run", + "--no-default-browser-check", +] + + +def _intent_url(shell_cmd: str) -> str: + """The ``-d`` argument of an ``am start`` command, as the headset shell would pass it.""" + script = ( + 'am() { while [ "$#" -gt 0 ]; do ' + '[ "$1" = "-d" ] && { printf "%s" "$2"; return; }; shift; done; }; ' + shell_cmd + ) + return _REAL_RUN( + ["/bin/sh", "-c", script], capture_output=True, text=True, check=True + ).stdout + + +def test_launch_command_keeps_the_whole_url_through_sh(monkeypatch): + """``&``, ``?``, ``#`` and quotes in the token must not split the ``-d`` argument.""" + monkeypatch.setenv("TELEOP_CLIENT_ROUTE", "/sim/isaaclab") + monkeypatch.setenv("TELEOP_WEB_CLIENT_BASE", "http://localhost:8080") + monkeypatch.setenv("CONTROL_TOKEN", _AWKWARD_TOKEN) + url = adb_module.build_teleop_url(resolved_port=48322, usb_local=True) + + with mock_adb() as adb: + assert adb_module.open_url_on_headset(url) == (0, "") + shell_cmd = next( + c[2] for c in adb.calls if c[:2] == ["adb", "shell"] and "am start" in c[2] + ) + + assert _intent_url(shell_cmd) == url + fragment = urlsplit(url).fragment + assert fragment.startswith("/sim/isaaclab?controlToken=") + assert parse_qs(fragment.split("?", 1)[1])["controlToken"] == [_AWKWARD_TOKEN] + + +# --------------------------------------------------------------------------- +# Browser harness +# --------------------------------------------------------------------------- + + +def _free_port() -> int: + """An ephemeral loopback port; released immediately for the subprocess to take.""" + with socket.socket() as s: + s.bind(("127.0.0.1", 0)) + return s.getsockname()[1] + + +def _wait_http(url: str, timeout: float) -> None: + """Block until *url* answers, or raise.""" + deadline = time.monotonic() + timeout + while True: + try: + with urllib.request.urlopen(url, timeout=1): + return + except OSError: + if time.monotonic() > deadline: + raise RuntimeError(f"{url} did not answer within {timeout}s") from None + time.sleep(0.2) + + +async def _wait_for(check, what: str, timeout: float = 30.0): + """Poll the async *check* until it returns something truthy; fail with *what*.""" + deadline = time.monotonic() + timeout + while True: + value = await check() + if value: + return value + if time.monotonic() > deadline: + raise AssertionError(f"timed out after {timeout}s waiting for {what}") + await asyncio.sleep(0.25) + + +class _Recorder(SimpleHTTPRequestHandler): + """Static file handler that records every request target and Referer.""" + + def do_GET(self): + self.server.seen.append((self.path, self.headers.get("Referer") or "")) + super().do_GET() + + def log_message(self, *args): + pass + + +class _Tab: + """Opens a URL in Chrome over CDP, keeping console output from before navigation. + + ``/json/new?`` would drop a ``#fragment``, so the tab is created blank, console + capture is enabled, and only then navigated to the exact URL. + """ + + def __init__(self, cdp_port: int) -> None: + self.cdp_port = cdp_port + self.console: list[str] = [] # console lines and uncaught exceptions + self.navigations: list[str] = [] # main-frame URLs, fragment dropped + self._ws = None + self._id = 0 + + def _call(self, method, params=None, session=None): + self._id += 1 + msg = {"id": self._id, "method": method, "params": params or {}} + if session: + msg["sessionId"] = session + self._ws.send(json.dumps(msg)) + while True: + reply = json.loads(self._ws.recv(timeout=10)) + if reply.get("id") == self._id: + return reply.get("result", {}) + self._note(reply) + + def _note(self, event): + method, params = event.get("method"), event.get("params", {}) + if method == "Runtime.consoleAPICalled": + self.console.append( + " ".join(str(a.get("value", "")) for a in params["args"]) + ) + elif method == "Runtime.exceptionThrown": + detail = params["exceptionDetails"] + self.console.append( + "EXCEPTION " + + str(detail.get("exception", {}).get("description", detail["text"])) + ) + elif method == "Page.frameNavigated" and "parentId" not in params["frame"]: + self.navigations.append(params["frame"]["url"].split("#")[0]) + + def _pump(self): + while True: + try: + self._note(json.loads(self._ws.recv(timeout=1))) + except TimeoutError: + continue + except (ConnectionClosed, OSError): + return + + def open(self, url: str) -> None: + with urllib.request.urlopen( + f"http://127.0.0.1:{self.cdp_port}/json/version", timeout=5 + ) as resp: + browser_ws = json.load(resp)["webSocketDebuggerUrl"] + self._ws = ws_connect(browser_ws, max_size=None) + target = self._call("Target.createTarget", {"url": "about:blank"})["targetId"] + session = self._call( + "Target.attachToTarget", {"targetId": target, "flatten": True} + )["sessionId"] + self._call("Runtime.enable", session=session) + self._call("Page.enable", session=session) + self._call("Page.navigate", {"url": url}, session=session) + self.pump = threading.Thread(target=self._pump, daemon=True) + self.pump.start() + + def close(self) -> None: + if self._ws is not None: + self._ws.close() + + +class _BrowserAdb(FakeAdb): + """FakeAdb whose accepted ``am start`` opens its exact ``-d`` URL in real Chrome.""" + + def __init__(self, tab: _Tab, *, fragment_token: str | None = None) -> None: + super().__init__() + self.tab = tab + self.fragment_token = fragment_token # fault injection: replaces the real token + self.launched: list[str] = [] + self.monitor: asyncio.Task | None = None + + async def stop_monitor(self) -> None: + """Cancel the connect monitor while ``subprocess.run`` is still faked.""" + if self.monitor is not None: + self.monitor.cancel() + with contextlib.suppress(asyncio.CancelledError): + await self.monitor + + def __call__(self, args, **kwargs): + result = super().__call__(args, **kwargs) # validates and records the command + if args[:2] == ["adb", "shell"] and args[2].startswith("am start"): + url = _intent_url(args[2]) + if self.fragment_token is not None: + head, _, fragment = url.partition("#") + route = fragment.split("?", 1)[0] + url = ( + f"{head}#{route}?{urlencode({'controlToken': self.fragment_token})}" + ) + self.launched.append(url) + self.tab.open(url) + return result + + +@pytest.fixture(scope="session") +def app_mock_build() -> Path: + """Build the MockCloudXR web client once, from this checkout, with a clean cache.""" + client = repo_root() / "deps" / "cloudxr" / "webxr_client" + shutil.rmtree(client / "node_modules" / ".cache" / "webpack", ignore_errors=True) + subprocess.run( + ["npm", "run", "build:app-mock"], cwd=client, check=True, capture_output=True + ) + build = client / "build-app-mock" + assert (build / "index.html").is_file() + return build + + +@pytest.fixture +def static_server(app_mock_build): + """Serve the build on a free loopback port, recording requests.""" + server = ThreadingHTTPServer( + ("localhost", 0), partial(_Recorder, directory=str(app_mock_build)) + ) + server.seen = [] + threading.Thread(target=server.serve_forever, daemon=True).start() + yield server + server.shutdown() + server.server_close() + + +@pytest.fixture +def chrome(tmp_path): + """A real Chrome with its own profile and CDP port.""" + if not os.path.exists(_CHROME): + pytest.fail(f"Chrome not found at {_CHROME}; set PLAYWRIGHT_CHROME_PATH") + port = _free_port() + proc = subprocess.Popen( + [ + _CHROME, + f"--remote-debugging-port={port}", + f"--user-data-dir={tmp_path / 'profile'}", + *_CHROME_ARGS, + "about:blank", + ], + stdout=subprocess.DEVNULL, + stderr=subprocess.DEVNULL, + ) + try: + _wait_http(f"http://127.0.0.1:{port}/json/version", 15) + yield port + finally: + proc.terminate() + try: + proc.wait(timeout=10) + except subprocess.TimeoutExpired: + proc.kill() + + +@pytest.fixture +def token(request, monkeypatch) -> str: + """The hub's control token: ``request.param`` as ``CONTROL_TOKEN``, or generated if None. + + Fixed before the hub starts, since the hub resolves it once at start-up. + """ + if request.param is None: + monkeypatch.delenv("CONTROL_TOKEN", raising=False) + monkeypatch.setattr(env_module, "_generated_control_token", None) + else: + monkeypatch.setenv("CONTROL_TOKEN", request.param) + return env_module.resolve_control_token() + + +@pytest.fixture +async def hub(tmp_path, monkeypatch, token): + """The real ``wss.run`` with the OOB hub; yields a function reading its HTTP state.""" + from cloudxr_py_test_ns.wss import run as wss_run # noqa: PLC0415 + + monkeypatch.setenv("CXR_INSTALL_DIR", str(tmp_path / "install")) + # Hub only: this test drives run_oob_connect itself. + monkeypatch.setenv("TELEOP_OOB_HUB_ONLY", "1") + port = _free_port() + stop = asyncio.get_running_loop().create_future() + listening = asyncio.Event() + task = asyncio.create_task( + wss_run( + None, + stop, + backend_port=_free_port(), + proxy_port=port, + setup_oob=True, + on_listening=listening.set, + ) + ) + ready = asyncio.create_task(listening.wait()) + + def fetch() -> list[dict]: + request = urllib.request.Request( + f"https://localhost:{port}/api/oob/v1/state", + headers={"X-Control-Token": token}, + ) + insecure = ssl.create_default_context() + insecure.check_hostname = False + insecure.verify_mode = ssl.CERT_NONE + with urllib.request.urlopen(request, timeout=5, context=insecure) as resp: + return json.load(resp)["headsets"] + + async def headsets() -> list[dict]: + return await asyncio.to_thread(fetch) + + headsets.port = port + try: + await asyncio.wait( + {ready, task}, timeout=30, return_when=asyncio.FIRST_COMPLETED + ) + if task.done(): # startup failed: surface why, not a bare timeout + task.result() + raise AssertionError("wss.run exited before listening") + assert ready.done(), "wss.run did not start listening within 30s" + yield headsets + finally: + ready.cancel() + if not task.done(): + stop.set_result(None) + await asyncio.wait_for(task, timeout=15) + + +@contextlib.asynccontextmanager +async def _session(monkeypatch, chrome, static_server, hub, *, fragment_token=None): + """Run the real ``run_oob_connect`` against Chrome; yield the adapter, then tear down.""" + monkeypatch.setenv("TELEOP_CLIENT_ROUTE", "/sim/isaaclab") + monkeypatch.setenv( + "TELEOP_WEB_CLIENT_BASE", f"http://localhost:{static_server.server_port}" + ) + monkeypatch.setattr(adb_module, "_CDP_LOCAL_PORT", chrome) + + tab = _Tab(chrome) + adb = _BrowserAdb(tab, fragment_token=fragment_token) + try: + with patch("subprocess.run", side_effect=adb): + adb.monitor = await adb_module.run_oob_connect( + resolved_port=hub.port, timeout=30.0, usb_local=True + ) + try: + yield adb + finally: + # Its cleanup issues `adb forward --remove`, which must hit the fake. + await adb.stop_monitor() + finally: + tab.close() + + +def _tab_url(cdp_port: int) -> str: + """URL of the page tab, from Chrome's own ``/json`` list.""" + with urllib.request.urlopen(f"http://127.0.0.1:{cdp_port}/json", timeout=5) as resp: + pages = [t for t in json.load(resp) if t.get("type") == "page"] + return next(t["url"] for t in pages if "oobEnable=1" in t["url"]) + + +@pytest.mark.oob_browser +@pytest.mark.skipif( + os.environ.get("RUN_OOB_BROWSER_TESTS") != "1", + reason="set RUN_OOB_BROWSER_TESTS=1 to run the real-browser token tests", +) +class TestFragmentTokenInRealBrowser: + """Launch, authenticate, clean the URL; and reject a wrong token. + + Reload persistence is covered by the jest and Playwright tests, not here. + """ + + @pytest.mark.parametrize( + "token", [None, _AWKWARD_TOKEN], ids=["generated", "awkward"], indirect=True + ) + async def test_launch_authenticates_via_fragment( + self, monkeypatch, chrome, static_server, hub, token + ): + async with _session(monkeypatch, chrome, static_server, hub) as adb: + expected = token + + # The URL handed to `am start`: token only in the fragment, after the route. + (url,) = adb.launched + parts = urlsplit(url) + assert "controlToken" not in parts.query + route, _, fragment_query = parts.fragment.partition("?") + assert route == "/sim/isaaclab" + assert parse_qs(fragment_query)["controlToken"] == [expected] + + # The real hub accepted the real client, so the fragment token was lifted. + await _wait_for(hub, "an authenticated headset registration") + + # The tab's URL no longer carries the token but keeps route and endpoint. + page_url = _tab_url(chrome) + assert expected not in page_url and "controlToken" not in page_url + assert urlsplit(page_url).fragment == "/sim/isaaclab" + assert f"port={hub.port}" in urlsplit(page_url).query + + # The mock session really connected, not just registered. + async def streaming(): + return [h for h in await hub() if h["streaming"]] + + await _wait_for(streaming, "the mock session to report streaming") + + # Stopping the monitor under the fake runs its `adb forward --remove`. + await adb.stop_monitor() + assert any(c[1:3] == ["forward", "--remove"] for c in adb.calls) + + # No HTTP request or Referer ever carried the token, raw or encoded. + seen = " ".join(f"{path} {referer}" for path, referer in static_server.seen) + assert expected not in seen + assert urlencode({"t": expected})[2:] not in seen + + @pytest.mark.parametrize("token", [_AWKWARD_TOKEN], indirect=True) + async def test_wrong_fragment_token_is_rejected( + self, monkeypatch, chrome, static_server, hub, token + ): + async with _session( + monkeypatch, chrome, static_server, hub, fragment_token="not-the-hub-token" + ) as adb: + (url,) = adb.launched + sent = parse_qs(urlsplit(url).fragment.split("?", 1)[1])["controlToken"] + assert sent == ["not-the-hub-token"] != [token] + + async def rejected(): + return any("Terminal close (1008" in line for line in adb.tab.console) + + await _wait_for(rejected, "the client's 1008 auth rejection in the console") + assert await hub() == [] From c4acaa61b7493e3517aef5f50c7d8d8685f77759 Mon Sep 17 00:00:00 2001 From: Gareth Morgan Date: Fri, 23 Oct 2026 21:19:16 -0400 Subject: [PATCH 4/6] test(cloudxr): combine the real-browser OOB tests; add fragment token and attach-timeout cases Fold the fake-adb + MockCloudXR token launch test into the real-browser run_oob_connect() tests, and add a case where a stream-attach timeout set in the environment reaches the page through the bookmark. Drop the relaunch test written against reliability branches that have not landed, and the plan document describing it. Co-Authored-By: Claude Sonnet 5.5 Signed-off-by: Gareth Morgan --- oob-real-browser-integration-test-plan.md | 103 ------------------ tests/python/core/cloudxr/conftest.py | 3 +- ...ion.py => test_oob_browser_integration.py} | 55 +++++++++- .../core/cloudxr/test_oob_teleop_adb.py | 84 +------------- 4 files changed, 53 insertions(+), 192 deletions(-) delete mode 100644 oob-real-browser-integration-test-plan.md rename tests/python/core/cloudxr/{test_oob_token_integration.py => test_oob_browser_integration.py} (89%) diff --git a/oob-real-browser-integration-test-plan.md b/oob-real-browser-integration-test-plan.md deleted file mode 100644 index 6aade285dd..0000000000 --- a/oob-real-browser-integration-test-plan.md +++ /dev/null @@ -1,103 +0,0 @@ - - -# OOB real-browser integration test - -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), 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 | -| `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 | - -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. - -## Tests - -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. -- `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` — - 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 7d7e0761ed..a0bf236717 100644 --- a/tests/python/core/cloudxr/conftest.py +++ b/tests/python/core/cloudxr/conftest.py @@ -347,8 +347,7 @@ def mock_adb(**kwargs): # ============================================================================ -# 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 +# Real-browser OOB integration test-infra: 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. # ============================================================================ diff --git a/tests/python/core/cloudxr/test_oob_token_integration.py b/tests/python/core/cloudxr/test_oob_browser_integration.py similarity index 89% rename from tests/python/core/cloudxr/test_oob_token_integration.py rename to tests/python/core/cloudxr/test_oob_browser_integration.py index 114d31b389..ec0d396d53 100644 --- a/tests/python/core/cloudxr/test_oob_token_integration.py +++ b/tests/python/core/cloudxr/test_oob_browser_integration.py @@ -1,7 +1,10 @@ # SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. # SPDX-License-Identifier: Apache-2.0 -"""The OOB control token travels in the URL fragment from the host to the hub. +"""Real-browser OOB tests: the headset launch path, the hub, and the web client together. + +The fragment token travels from the host to the hub, and client timeouts set in the +bookmark reach the page. ``build_teleop_url`` -> ``run_oob_connect`` -> :class:`conftest.FakeAdb` ``am start`` -> real Chrome opens the exact recorded URL -> the real web client (``npm run @@ -188,7 +191,7 @@ def _pump(self): except (ConnectionClosed, OSError): return - def open(self, url: str) -> None: + def open(self, url: str, init_script: str | None = None) -> None: with urllib.request.urlopen( f"http://127.0.0.1:{self.cdp_port}/json/version", timeout=5 ) as resp: @@ -200,6 +203,12 @@ def open(self, url: str) -> None: )["sessionId"] self._call("Runtime.enable", session=session) self._call("Page.enable", session=session) + if init_script: # runs in every new document, before the page's own scripts + self._call( + "Page.addScriptToEvaluateOnNewDocument", + {"source": init_script}, + session=session, + ) self._call("Page.navigate", {"url": url}, session=session) self.pump = threading.Thread(target=self._pump, daemon=True) self.pump.start() @@ -212,9 +221,16 @@ def close(self) -> None: class _BrowserAdb(FakeAdb): """FakeAdb whose accepted ``am start`` opens its exact ``-d`` URL in real Chrome.""" - def __init__(self, tab: _Tab, *, fragment_token: str | None = None) -> None: + def __init__( + self, + tab: _Tab, + *, + fragment_token: str | None = None, + init_script: str | None = None, + ) -> None: super().__init__() self.tab = tab + self.init_script = init_script self.fragment_token = fragment_token # fault injection: replaces the real token self.launched: list[str] = [] self.monitor: asyncio.Task | None = None @@ -237,7 +253,7 @@ def __call__(self, args, **kwargs): f"{head}#{route}?{urlencode({'controlToken': self.fragment_token})}" ) self.launched.append(url) - self.tab.open(url) + self.tab.open(url, self.init_script) return result @@ -364,7 +380,9 @@ async def headsets() -> list[dict]: @contextlib.asynccontextmanager -async def _session(monkeypatch, chrome, static_server, hub, *, fragment_token=None): +async def _session( + monkeypatch, chrome, static_server, hub, *, fragment_token=None, init_script=None +): """Run the real ``run_oob_connect`` against Chrome; yield the adapter, then tear down.""" monkeypatch.setenv("TELEOP_CLIENT_ROUTE", "/sim/isaaclab") monkeypatch.setenv( @@ -373,7 +391,7 @@ async def _session(monkeypatch, chrome, static_server, hub, *, fragment_token=No monkeypatch.setattr(adb_module, "_CDP_LOCAL_PORT", chrome) tab = _Tab(chrome) - adb = _BrowserAdb(tab, fragment_token=fragment_token) + adb = _BrowserAdb(tab, fragment_token=fragment_token, init_script=init_script) try: with patch("subprocess.run", side_effect=adb): adb.monitor = await adb_module.run_oob_connect( @@ -463,3 +481,28 @@ async def rejected(): await _wait_for(rejected, "the client's 1008 auth rejection in the console") assert await hub() == [] + + @pytest.mark.parametrize("token", [None], indirect=True) + async def test_stream_attach_timeout_from_the_bookmark_reaches_the_page( + self, monkeypatch, chrome, static_server, hub, token + ): + """A timeout set in the environment travels in the bookmark and fires.""" + monkeypatch.setenv("TELEOP_CLIENT_STREAM_ATTACH_TIMEOUT_MS", "3000") + async with _session( + monkeypatch, + chrome, + static_server, + hub, + # Hold the mock session in Connecting so the attach deadline expires. + init_script="window.__mockCloudXRConnectDelayMs = 3600000;", + ) as adb: + (url,) = adb.launched + assert parse_qs(urlsplit(url).query)["streamAttachTimeoutMs"] == ["3000"] + + async def timed_out(): + return any( + "stream did not attach within 3000ms" in line + for line in adb.tab.console + ) + + await _wait_for(timed_out, "the client's stream-attach timeout", timeout=20) diff --git a/tests/python/core/cloudxr/test_oob_teleop_adb.py b/tests/python/core/cloudxr/test_oob_teleop_adb.py index 8059dcd6ba..43047fe034 100644 --- a/tests/python/core/cloudxr/test_oob_teleop_adb.py +++ b/tests/python/core/cloudxr/test_oob_teleop_adb.py @@ -1714,8 +1714,7 @@ async def test_monitor_without_callback_still_logs_and_keeps_watching() -> None: # ============================================================================ -# run_oob_connect(): real-browser integration test (see -# /oob-real-browser-integration-test-plan.md at the repo root). A real +# run_oob_connect(): real-browser integration test. 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 @@ -1813,9 +1812,7 @@ 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 — independent of whether anything recovers from it; see - :func:`test_run_oob_connect_real_mock_cxr_crash_triggers_real_relaunch` - for the recovery assertion. + appears on demand — independent of whether anything recovers from it. """ with ( real_browser_env(tmp_path, monkeypatch) as env, @@ -1857,8 +1854,7 @@ async def test_run_oob_connect_real_mock_cxr_crash_surfaces_error_banner( # 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. + # near-instant. banner = "" deadline = asyncio.get_running_loop().time() + 5.0 while asyncio.get_running_loop().time() < deadline: @@ -1887,77 +1883,3 @@ async def test_run_oob_connect_real_mock_cxr_crash_surfaces_error_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. - - 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. - """ - with ( - 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( - env.install_dir, proxy_port=env.wss_port, backend_port=env.backend_port - ): - task = await run_oob_connect( - resolved_port=env.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: 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 - the monitor should relaunch, not just " - "log the banner" - ) - - task.cancel() - with contextlib.suppress(asyncio.CancelledError): - await task From 39cd7c886349bb7b8cbdfc4fc6f97b79103ab67f Mon Sep 17 00:00:00 2001 From: Gareth Morgan Date: Fri, 23 Oct 2026 21:40:59 -0400 Subject: [PATCH 5/6] test(cloudxr): one real-browser suite; port the error-banner case, drop the old harness Every real-browser case now shares one build, browser, hub and adb cleanup and the same RUN_OOB_BROWSER_TESTS opt-in. The older conftest harness (which opened launch URLs without their fragment) and its two tests are removed; the mock-failure banner case moves to the new suite and waits for the session to stream first. Co-Authored-By: Claude Sonnet 5.5 Signed-off-by: Gareth Morgan --- tests/python/core/cloudxr/conftest.py | 333 +----------------- .../cloudxr/test_oob_browser_integration.py | 58 ++- .../core/cloudxr/test_oob_teleop_adb.py | 172 --------- 3 files changed, 57 insertions(+), 506 deletions(-) diff --git a/tests/python/core/cloudxr/conftest.py b/tests/python/core/cloudxr/conftest.py index a0bf236717..0af35b8e66 100644 --- a/tests/python/core/cloudxr/conftest.py +++ b/tests/python/core/cloudxr/conftest.py @@ -11,22 +11,15 @@ 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 -import urllib.error -import urllib.request -from contextlib import asynccontextmanager, contextmanager -from dataclasses import dataclass +from contextlib import contextmanager from pathlib import Path from unittest.mock import MagicMock, patch @@ -344,327 +337,3 @@ def mock_adb(**kwargs): fake = FakeAdb(**kwargs) with patch("subprocess.run", side_effect=fake): yield fake - - -# ============================================================================ -# Real-browser OOB integration test-infra: 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): - """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 - 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. - - 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 - # (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) - - -@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*. - - 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 - - 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" - - 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 - 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): - """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: - 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 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 result(0) - if args[:2] == ["adb", "forward"] and "--remove" not in args: - return result(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/test_oob_browser_integration.py b/tests/python/core/cloudxr/test_oob_browser_integration.py index ec0d396d53..aa0a67e907 100644 --- a/tests/python/core/cloudxr/test_oob_browser_integration.py +++ b/tests/python/core/cloudxr/test_oob_browser_integration.py @@ -413,13 +413,33 @@ def _tab_url(cdp_port: int) -> str: return next(t["url"] for t in pages if "oobEnable=1" in t["url"]) +def _evaluate(cdp_port: int, expression: str): + """Value of *expression* in the page tab, over its own CDP socket.""" + with urllib.request.urlopen(f"http://127.0.0.1:{cdp_port}/json", timeout=5) as resp: + tab = next(t for t in json.load(resp) if "oobEnable=1" in t.get("url", "")) + with ws_connect(tab["webSocketDebuggerUrl"], max_size=None) as ws: + ws.send( + json.dumps( + { + "id": 1, + "method": "Runtime.evaluate", + "params": {"expression": expression, "returnByValue": True}, + } + ) + ) + while True: + reply = json.loads(ws.recv(timeout=10)) + if reply.get("id") == 1: + return reply["result"]["result"].get("value") + + @pytest.mark.oob_browser @pytest.mark.skipif( os.environ.get("RUN_OOB_BROWSER_TESTS") != "1", reason="set RUN_OOB_BROWSER_TESTS=1 to run the real-browser token tests", ) -class TestFragmentTokenInRealBrowser: - """Launch, authenticate, clean the URL; and reject a wrong token. +class TestOobRealBrowser: + """Launch and auth, wrong-token rejection, bookmark timeouts, terminal errors. Reload persistence is covered by the jest and Playwright tests, not here. """ @@ -506,3 +526,37 @@ async def timed_out(): ) await _wait_for(timed_out, "the client's stream-attach timeout", timeout=20) + + @pytest.mark.parametrize("token", [None], indirect=True) + async def test_terminal_client_error_shows_the_error_banner( + self, monkeypatch, chrome, static_server, hub, token + ): + """A server-disconnect-range failure in the mock session reaches the DOM banner.""" + async with _session(monkeypatch, chrome, static_server, hub): + + async def streaming(): + return [h for h in await hub() if h["streaming"]] + + await _wait_for(streaming, "the mock session to report streaming") + # A code in 0xc0f22300..ff is a server disconnect; a code-less failure is + # recoverable and reconnects without ever showing the banner. + await asyncio.to_thread( + _evaluate, + chrome, + "window.__mockCloudXRFail('simulated crash', 0xc0f22300)", + ) + + async def banner(): + return await asyncio.to_thread( + _evaluate, + chrome, + "(() => { 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() || ''; })()", + ) + + text = await _wait_for(banner, "the error banner", timeout=15) + assert "simulated crash" in text diff --git a/tests/python/core/cloudxr/test_oob_teleop_adb.py b/tests/python/core/cloudxr/test_oob_teleop_adb.py index 43047fe034..efb131b5af 100644 --- a/tests/python/core/cloudxr/test_oob_teleop_adb.py +++ b/tests/python/core/cloudxr/test_oob_teleop_adb.py @@ -1711,175 +1711,3 @@ 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. 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 - cdp_evaluate, - real_browser_adb, - real_browser_env, - 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. - """ - with ( - 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( - env.install_dir, proxy_port=env.wss_port, backend_port=env.backend_port - ): - task = await run_oob_connect( - resolved_port=env.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 — independent of whether anything recovers from it. - """ - with ( - 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( - env.install_dir, proxy_port=env.wss_port, backend_port=env.backend_port - ): - task = await run_oob_connect( - resolved_port=env.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-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. - 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 From 319f41eb8cad01564bde9f63406b17665f539522 Mon Sep 17 00:00:00 2001 From: Gareth Morgan Date: Fri, 23 Oct 2026 21:51:54 -0400 Subject: [PATCH 6/6] test(cloudxr): trim unused navigation capture and shorten mock comments Co-Authored-By: Claude Sonnet 5.5 Signed-off-by: Gareth Morgan --- AGENTS.md | 4 +++- .../webxr_client/tests/mock/cloudxr-mock-alias.ts | 9 ++------- deps/cloudxr/webxr_client/webpack.app-mock.js | 11 ++--------- .../core/cloudxr/test_oob_browser_integration.py | 7 +++---- 4 files changed, 10 insertions(+), 21 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 0113244048..7d1f2cfb15 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -265,7 +265,9 @@ pre-commit install --hook-type commit-msg - **`--all-files` means all *tracked* files.** pre-commit enumerates through `git ls-files`, so a file you have created but not yet `git add`ed is skipped and the run passes for the wrong reason — REUSE and `ruff format` alike. - Stage first (`git add -A`), then run the hooks. + Stage first (`git add -A`), then run the hooks; read `git status` first so a local + artifact (a `node_modules` symlink, a log) is not staged with it — a symlink whose + target exists only on your machine passes locally and fails CI's `check-symlinks`. - Format touched Python source with `ruff format` before staging. If the pre-commit hook rewrites a file, restage it and rerun the hooks. - **REUSE:** files covered by the REUSE hook need **`SPDX-FileCopyrightText`** and **`SPDX-License-Identifier`** in the form the repo already uses (for example the HTML comment block at the top of `README.md` also applies to **`AGENTS.md`** and similar docs). 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 0ec9edb4ae..ca284f04c6 100644 --- a/deps/cloudxr/webxr_client/tests/mock/cloudxr-mock-alias.ts +++ b/deps/cloudxr/webxr_client/tests/mock/cloudxr-mock-alias.ts @@ -64,13 +64,8 @@ declare global { } if (typeof window !== 'undefined') { - // 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. + // Without a code the failure is recoverable and reconnects silently; a code in the + // server-disconnect range (0xc0f22300-0xc0f223ff) is terminal and shows the error banner. 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 9d5b96b6c9..0385e67d2d 100644 --- a/deps/cloudxr/webxr_client/webpack.app-mock.js +++ b/deps/cloudxr/webxr_client/webpack.app-mock.js @@ -29,15 +29,8 @@ const { merge } = require('webpack-merge'); const common = require('./webpack.common.js'); module.exports = merge(common, { - // 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. + // Production React semantics: StrictMode's development effect replay would create + // duplicate mock sessions. mode: 'production', devtool: 'source-map', resolve: { diff --git a/tests/python/core/cloudxr/test_oob_browser_integration.py b/tests/python/core/cloudxr/test_oob_browser_integration.py index aa0a67e907..44b083949e 100644 --- a/tests/python/core/cloudxr/test_oob_browser_integration.py +++ b/tests/python/core/cloudxr/test_oob_browser_integration.py @@ -151,7 +151,6 @@ class _Tab: def __init__(self, cdp_port: int) -> None: self.cdp_port = cdp_port self.console: list[str] = [] # console lines and uncaught exceptions - self.navigations: list[str] = [] # main-frame URLs, fragment dropped self._ws = None self._id = 0 @@ -179,8 +178,6 @@ def _note(self, event): "EXCEPTION " + str(detail.get("exception", {}).get("description", detail["text"])) ) - elif method == "Page.frameNavigated" and "parentId" not in params["frame"]: - self.navigations.append(params["frame"]["url"].split("#")[0]) def _pump(self): while True: @@ -202,6 +199,8 @@ def open(self, url: str, init_script: str | None = None) -> None: "Target.attachToTarget", {"targetId": target, "flatten": True} )["sessionId"] self._call("Runtime.enable", session=session) + # Keep the Page domain enabled: without it the new-document script below did + # not run in the attach-timeout case. self._call("Page.enable", session=session) if init_script: # runs in every new document, before the page's own scripts self._call( @@ -436,7 +435,7 @@ def _evaluate(cdp_port: int, expression: str): @pytest.mark.oob_browser @pytest.mark.skipif( os.environ.get("RUN_OOB_BROWSER_TESTS") != "1", - reason="set RUN_OOB_BROWSER_TESTS=1 to run the real-browser token tests", + reason="set RUN_OOB_BROWSER_TESTS=1 to run the OOB browser tests", ) class TestOobRealBrowser: """Launch and auth, wrong-token rejection, bookmark timeouts, terminal errors.