test(webclient): Adds e2e web client test#418
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .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:
📝 WalkthroughWalkthroughThis pull request introduces a comprehensive end-to-end testing pipeline for the CloudXR web client. It adds a new GitHub Actions workflow job that builds the web application, followed by a self-hosted GPU-based E2E test job that uses Docker Compose to orchestrate a CloudXR runtime and Playwright test container. The test validates SDK initialization, UI state transitions, WebSocket connectivity, and user input handling via hand controller simulation. The publishing workflow is gated on successful E2E test completion. Supporting infrastructure includes Docker Compose configuration, test runner script, Playwright test suite with configuration, and documentation. Sequence Diagram(s)sequenceDiagram
participant GHA as GitHub Actions
participant Build as build-webapp job
participant Artifact as Artifact Storage
participant E2E as test-cloudxr-web-e2e job
participant Docker as Docker Compose
participant CXR as CloudXR Runtime
participant PW as Playwright
participant Publish as publish-wheel job
GHA->>Build: trigger
Build->>Build: npm install & build
Build->>Artifact: upload webxr-client-build
GHA->>E2E: trigger (on build success)
E2E->>Artifact: download webxr-client-build
E2E->>E2E: prepare environment
E2E->>Docker: docker compose up
Docker->>CXR: start cloudxr-runtime service
CXR->>CXR: health check polling
Docker->>PW: start playwright service
PW->>CXR: connect to runtime (ws://)
PW->>PW: run cloudxr-connect.spec.ts
PW->>CXR: validate WebSocket frames
PW->>Artifact: upload test-results
E2E->>Docker: docker compose down
GHA->>Publish: trigger (on E2E success)
Publish->>Publish: publish Python wheel
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
yanziz-nvidia
force-pushed
the
yanziz/ci-for-web
branch
3 times, most recently
from
5bc5dd9 to
b48546a
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
deps/cloudxr/docker-compose.test-e2e.yaml (1)
87-90: Keep Playwright image tag in sync with package.json.The image tag
v1.59.1-noble(line 90) must match@playwright/test: 1.59.1intests/web_e2e/package.json. A mismatch breaks browser path resolution. Consider documenting this coupling or adding a CI check.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@deps/cloudxr/docker-compose.test-e2e.yaml` around lines 87 - 90, The Playwright Docker image tag (image: mcr.microsoft.com/playwright:v1.59.1-noble) must be kept identical to the `@playwright/test` version declared in tests/web_e2e/package.json; update the image tag to match the `@playwright/test` version (e.g., 1.59.1) or parameterize the tag so both are driven from a single source, and add a short comment or CI validation that compares the Docker image tag against the `@playwright/test` entry to prevent future mismatches.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@scripts/run_test_e2e_with_web.sh`:
- Around line 92-104: The case pattern "*healthy*" currently matches "unhealthy"
making the unhealthy branch unreachable; update the case in the loop that
inspects the variable status to either check the "*unhealthy*" pattern first or
replace the glob patterns with exact matches like "healthy") and "unhealthy") so
that the unhealthy check in the case block for status triggers correctly; locate
the case statement handling variable status in the script (the patterns
"*healthy*" and "*unhealthy*" inside the loop) and apply one of these fixes.
---
Nitpick comments:
In `@deps/cloudxr/docker-compose.test-e2e.yaml`:
- Around line 87-90: The Playwright Docker image tag (image:
mcr.microsoft.com/playwright:v1.59.1-noble) must be kept identical to the
`@playwright/test` version declared in tests/web_e2e/package.json; update the
image tag to match the `@playwright/test` version (e.g., 1.59.1) or parameterize
the tag so both are driven from a single source, and add a short comment or CI
validation that compares the Docker image tag against the `@playwright/test` entry
to prevent future mismatches.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: b6b6315e-b44c-4c4f-aa40-8700036508bc
📒 Files selected for processing (8)
.github/workflows/build-ubuntu.yml.gitignoredeps/cloudxr/docker-compose.test-e2e.yamlscripts/run_test_e2e_with_web.shtests/web_e2e/README.mdtests/web_e2e/cloudxr-connect.spec.tstests/web_e2e/package.jsontests/web_e2e/playwright.config.ts
yanziz-nvidia
force-pushed
the
yanziz/ci-for-web
branch
from
b48546a to
04d37e9
Compare
yanziz-nvidia
requested review from
aristarkhovNV,
blongs-nv,
gareth-morgan-nv and
jiwenc-nv
gareth-morgan-nv
left a comment
gareth-morgan-nv
left a comment
There was a problem hiding this comment.
Wouldn't it be better to start with a simple playwright test that assumes no server?
| @@ -0,0 +1,113 @@ | |||
| #!/bin/bash | |||
There was a problem hiding this comment.
Do we need the script? Could the logic be contained in Playwright?
There was a problem hiding this comment.
It's useful to run it locally imo.
yanziz-nvidia
force-pushed
the
yanziz/ci-for-web
branch
from
04d37e9 to
5eb357e
Compare
then that wouldn't be called e2e right? I would want to make sure it actually connects. This PR also lacks an actual server running. Open for ideas what to run. |
yanziz-nvidia
force-pushed
the
yanziz/ci-for-web
branch
2 times, most recently
from
9791623 to
725ae5b
Compare
| # GPU runner required: the CloudXR runtime needs an NVIDIA GPU even when | ||
| # only one E2E spec is running. Single matrix entry — the web client | ||
| # behavior is independent of the matrix dims used by other test jobs. | ||
| runs-on: [self-hosted, linux, gpu, x64] |
There was a problem hiding this comment.
may skip the arch tag so it'd use any available runner
yanziz-nvidia
force-pushed
the
yanziz/ci-for-web
branch
from
725ae5b to
4c4917c
Compare
5 participants
Example run: https://github.com/NVIDIA/IsaacTeleop/actions/runs/24949804846/job/73058257235
Note we don't have an IL or other app connected. So it shows empty frames. Also it's odd react control panel isn't shown. But at least we can get a connected session, a video recording (from artifacts), and some logs for e2e.
Download artifact https://github.com/NVIDIA/IsaacTeleop/actions/runs/24949804846/artifacts/6645351244 and see.
Summary by CodeRabbit
Tests
Chores
Documentation