test(webxr): cover client-side missing-UI-state failure modes - #1137
Open
gareth-morgan-nv wants to merge 8 commits into
Open
gareth-morgan-nv wants to merge 8 commits into
gareth-morgan-nv wants to merge 8 commits into
Conversation
Contributor
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository: NVIDIA/IsaacCapture/.coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
gareth-morgan-nv
requested review from
AndreusNvidius,
jiwenc-nv and
yanziz-nvidia
September 28, 2026 21:06
3 tasks done
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
force-pushed
the
gmorgan/client-ui-mock-tests
branch
from
September 28, 2026 21:32
ba26ced to
9b12df0
Compare
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
changed the base branch from
gmorgan/client-ui-mock-tests
to
gmorgan/passthrough-only-detection
September 29, 2026 03:03
gareth-morgan-nv
force-pushed
the
gmorgan/client-ui-state-tests
branch
from
September 29, 2026 03:03
30e5050 to
ffdcc00
Compare
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>
gareth-morgan-nv
force-pushed
the
gmorgan/client-ui-state-tests
branch
from
September 29, 2026 03:30
2f0948e to
a6437e0
Compare
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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 fromApp.tsx/CloudXR2DUI.tsxalone againstMockCloudXR(stale-tab and certificate-interstitial detection/repair live inoob_teleop_adb.py's host-side CDP orchestration and are tracked separately):#errorMessageText/console.warn.Connectingindefinitely via a newwindow.__mockCloudXRConnectDelayMstest hook (mirrors the existing__mockCloudXRFailhook incloudxr-mock-alias.ts) and confirms the app has no signal today distinguishing "connecting" from "connected but not streaming."panelHiddenAtStartURL 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 inupdateConnectButtonState(), noted in a test comment rather than fixed here).Test plan
npx playwright test(all 5 specs, default parallel workers) — passing, verified 3xClientUIStatesTest.spec.jsalone, serial, verified 3x for determinismnpm test(jest) — 107/107npm run typecheck/npm run lint:check/npm run prettier:check— no new failures vs.mainSKIP=check-copyright-year pre-commit run --files ...on changed files — all hooks pass🤖 Generated with Claude Code