Skip to content

test(webxr): cover client-side missing-UI-state failure modes - #1137

Open
gareth-morgan-nv wants to merge 8 commits into
gmorgan/passthrough-only-detectionfrom
gmorgan/client-ui-state-tests
Open

gareth-morgan-nv wants to merge 8 commits into
gmorgan/passthrough-only-detectionfrom
gmorgan/client-ui-state-tests

Conversation

@gareth-morgan-nv

Copy link
Copy Markdown
Contributor

Summary

Stacked on #1136. Adds ClientUIStatesTest.spec.js, covering the three of the reliability backlog's five "missing client UI state" failure modes that are reproducible from App.tsx/CloudXR2DUI.tsx alone against MockCloudXR (stale-tab and certificate-interstitial detection/repair live in oob_teleop_adb.py's host-side CDP orchestration and are tracked separately):

  • browser-launched-but-client-not-loaded: blocks the IWER CDN script to force the loader's failure path, and confirms it's reliably surfaced via #errorMessageText/console.warn.
  • passthrough-only: holds the mock session in Connecting indefinitely via a new window.__mockCloudXRConnectDelayMs test hook (mirrors the existing __mockCloudXRFail hook in cloudxr-mock-alias.ts) and confirms the app has no signal today distinguishing "connecting" from "connected but not streaming."
  • missing-panel: confirms the existing panelHiddenAtStart URL param doesn't break the connect flow; the panel itself is a world-anchored WebXR scene object with no host-observable signal either way, so this is necessarily shallow coverage.

This branch is tests-only, no product code changes (one exploratory finding along the way: #startButton's disabled state races an unrelated bug in updateConnectButtonState(), noted in a test comment rather than fixed here).

Test plan

  • npx playwright test (all 5 specs, default parallel workers) — passing, verified 3x
  • ClientUIStatesTest.spec.js alone, serial, verified 3x for determinism
  • npm test (jest) — 107/107
  • npm run typecheck / npm run lint:check / npm run prettier:check — no new failures vs. main
  • SKIP=check-copyright-year pre-commit run --files ... on changed files — all hooks pass

🤖 Generated with Claude Code

@coderabbitai

coderabbitai Bot commented Sep 28, 2026 •

Copy link
Copy Markdown
Contributor

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository: NVIDIA/IsaacCapture/.coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 299c47d9-b5b4-4fed-ba81-a77344de74be

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Comment @coderabbitai help to get the list of available commands.

@gareth-morgan-nv gareth-morgan-nv self-assigned this Sep 28, 2026
gareth-morgan-nv added a commit that referenced this pull request Sep 28, 2026
…p_tabs

Adds coverage for the "stale-tab" state from the web-client UI-state
reliability backlog on the host-orchestration side (oob_teleop_adb.py),
following up on the client-side coverage in #1136/#1137.

_discover_devtools_socket's candidate-matching and priority logic is
exercised via mocked _run_adb output. _close_stale_teleop_tabs is driven
against a real local HTTP server standing in for Chromium's CDP /json
endpoint (only the adb forward/remove calls either side are mocked, since
there's no real device here), so the oobEnable= tab-matching and
close-request logic itself is exercised for real rather than asserted via
mocked call arguments.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Signed-off-by: Gareth Morgan <gmorgan@nvidia.com>
@gareth-morgan-nv
gareth-morgan-nv force-pushed the gmorgan/client-ui-mock-tests branch from ba26ced to 9b12df0 Compare September 28, 2026 21:32
gareth-morgan-nv added a commit that referenced this pull request Sep 28, 2026
…diness poll

Extends the fake-CDP pattern from the stale-tab tests to a WebSocket
responder (_CdpScript/_fake_cdp_ws), since _cdp_session_click_connect
speaks CDP entirely over one WebSocket rather than the HTTP /json endpoint
_close_stale_teleop_tabs uses. Covers the remaining two named states from
the web-client UI-state reliability backlog that are host-side
(oob_teleop_adb.py), completing the follow-up from #1136/#1137:

- certificate: both bypass layers - primary (Page.navigate re-navigate)
  and DOM fallback (details-button -> proceed-link click-through, forced
  by a chrome-error: interstitial URL, the same branch condition the real
  function checks).
- browser-launched-but-client-not-loaded (host-side half; the client-side
  half is in #1137): the #startButton readiness-poll state machine's
  ready path (multi-step initializing -> ready) and failed path (raises
  OobAdbError before ever dispatching a click).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Signed-off-by: Gareth Morgan <gmorgan@nvidia.com>
@gareth-morgan-nv
gareth-morgan-nv changed the base branch from gmorgan/client-ui-mock-tests to gmorgan/passthrough-only-detection September 29, 2026 03:03
@gareth-morgan-nv
gareth-morgan-nv force-pushed the gmorgan/client-ui-state-tests branch from 30e5050 to ffdcc00 Compare September 29, 2026 03:03
gareth-morgan-nv and others added 8 commits September 28, 2026 23:29
Add AppMockTest.spec.js, a minimal Playwright smoke test that drives the
actual production App.tsx (not the scripted CloudXRComponentTest.tsx
harness) through webpack.app-mock.js (:8082) under IWER emulation, and
confirms a session connects. This is the foundation for upcoming
client-UI error-state coverage.

playwright.config.js now brings up both mock dev servers (:8082 app-mock,
:8083 component-mock) before tests run.

The test waits for IWER's last-emitted ready log ("IWER DevUI initialized
with XR device.") before clicking Connect. #startButton can report enabled
before IWER's device.installRuntime() actually finishes - a real race in
App.tsx's capability-check gating - so clicking on the earlier "IWER loaded
as fallback." log is nondeterministic: it can land before navigator.xr is
usable and permanently wedge the UI with no retry path.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Signed-off-by: Gareth Morgan <gmorgan@nvidia.com>
Add ClientUIStatesTest.spec.js covering the three of the reliability doc's
five "missing client UI state" failure modes that are reproducible from
App.tsx/CloudXR2DUI.tsx alone (stale-tab and certificate belong to
oob_teleop_adb.py's host-side CDP orchestration and are out of scope here):

- browser-launched-but-client-not-loaded: blocks the IWER CDN script to
  force loadIWERIfNeeded()'s failure path, and confirms it's surfaced via
  #errorMessageText/console.warn ("Detected at launch" per the doc).
  #startButton's disabled state is deliberately not asserted: it races an
  unrelated bug in updateConnectButtonState() (CloudXR2DUI.tsx:961), which
  unconditionally re-enables the button from resolution/grid validity alone
  whenever its text is exactly 'CONNECT', with no awareness of
  capabilities/IWER state.
- passthrough-only: holds MockCloudXR in Connecting indefinitely via a new
  window.__mockCloudXRConnectDelayMs test hook (cloudxr-mock-alias.ts,
  mirrors the existing __mockCloudXRFail hook) and confirms the app has no
  signal distinguishing "connecting" from "connected but not streaming" -
  the doc's "not detected today" gap.
- missing-panel: confirms the existing panelHiddenAtStart URL param doesn't
  break the connect flow; the in-headset panel itself is a world-anchored
  WebXR scene object with no host-observable signal either way, so deeper
  assertion isn't possible without new detection code (out of scope here
  per discussion - this branch is tests only).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Signed-off-by: Gareth Morgan <gmorgan@nvidia.com>
CloudXRUI.tsx now logs "[CloudXRUI] panel visibility: hidden|visible" on
every panelHidden transition - the only host-observable signal that exists
today for the in-headset panel (a world-anchored WebXR scene object, not a
DOM element). Lets ClientUIStatesTest.spec.js's missing-panel test assert
that panelHiddenAtStart is actually reflected, instead of only confirming
the connect flow doesn't break.

This does not add detection/recovery for the panel becoming unreachable
mid-session (still out of scope per prior discussion) - only the existence
of a visibility signal to build that on top of later.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Signed-off-by: Gareth Morgan <gmorgan@nvidia.com>
Real coverage now lives in ControlPanelPositionTest.spec.js
(gmorgan/reset-panel-tests, #1141), against the actual fix
(gmorgan/reset-panel-key, #1140). This test only ever checked that
panelHiddenAtStart correctly drove the panelHidden state - it never
touched the real failure mode (panel/handle out of reach after the
operator moves), and its own comment already said as much. Keeping it
under this file's "state reproduction" framing was misleading about what
it actually covered.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Signed-off-by: Gareth Morgan <gmorgan@nvidia.com>
Add AppMockTest.spec.js, a minimal Playwright smoke test that drives the
actual production App.tsx (not the scripted CloudXRComponentTest.tsx
harness) through webpack.app-mock.js (:8082) under IWER emulation, and
confirms a session connects. This is the foundation for upcoming
client-UI error-state coverage.

playwright.config.js now brings up both mock dev servers (:8082 app-mock,
:8083 component-mock) before tests run.

The test waits for IWER's last-emitted ready log ("IWER DevUI initialized
with XR device.") before clicking Connect. #startButton can report enabled
before IWER's device.installRuntime() actually finishes - a real race in
App.tsx's capability-check gating - so clicking on the earlier "IWER loaded
as fallback." log is nondeterministic: it can land before navigator.xr is
usable and permanently wedge the UI with no retry path.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Signed-off-by: Gareth Morgan <gmorgan@nvidia.com>
…oad log

Human review on #1136 (yanziz-nvidia) found two real issues with the
smoke test's synchronization, both confirmed against LoadIWER.ts's actual
source:

- "IWER DevUI initialized with XR device." logs BEFORE
  device.installRuntime() (not after, as the prior comment claimed), and
  is skipped entirely on the supported path where DevUI fails to load
  but the runtime still installs fine - a usable app would falsely fail
  the test waiting on it.
- test.setTimeout(30000) was shorter than the cumulative budget of the
  poll timeouts that follow it (15s + button actionability + 15s, plus
  navigation), so a slow-but-successful run could be killed by Playwright
  before its own declared timeouts even elapsed.

LoadIWER.ts now logs "IWER runtime installed." right after
installRuntime() actually succeeds, unconditionally (not gated on the
DevUI branch) - the test waits on that instead, and the 30s override is
removed so the test inherits the configured 60s default.

Verified 3/3 passing runs.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Signed-off-by: Gareth Morgan <gmorgan@nvidia.com>
… too

Same fix as #1136 (commit 271a009): "IWER DevUI initialized with XR
device." logs before installRuntime() and is skipped on the supported
no-DevUI path - wait for LoadIWER.ts's "IWER runtime installed." instead
(merged in from gmorgan/client-ui-mock-tests). Verified 2/2 passing.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Signed-off-by: Gareth Morgan <gmorgan@nvidia.com>
…cted

This test previously documented a known gap (reliability doc Row 8):
a session stuck in Connecting forever looked identical to one
connecting normally, with no error and no way to tell them apart.
Rebasing onto gmorgan/passthrough-only-detection means that gap is
closed - CloudXRComponent's streamAttachTimeoutMs now fires a
synthetic, recoverable error when a stream never attaches - so the
test needed to flip from "assert nothing happens" to "assert it's
actually detected."

Also fixes two issues found while updating it:
- reconnectEnabled defaults to on in this environment (index.html
  default and/or persisted localStorage from an earlier test in the
  same browser context), which routes the synthetic error through 3
  retries instead of the immediate give-up path this test wants.
  Forced off explicitly via the reconnectEnabled=false URL param
  rather than relying on an incidental default.
- Checking the live #errorMessageBox DOM state raced an unrelated
  capability/performance "info" notice that can legitimately
  overwrite the shared status box right after the error renders.
  Switched to checking console output instead, which showStatus()
  always mirrors and which isn't subject to that race.

Signed-off-by: Gareth Morgan <gmorgan@nvidia.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Signed-off-by: Gareth Morgan <gmorgan@nvidia.com>

This branch was successfully deployed

1 active (outdated) deployment
dev — c6f6bdb9 Deployed Sep 28, 2026 by gareth-morgan-nv via publish-wheel #5047
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants