Skip to content

test/browser: headless-browser smoke client (phase 2)#1542

Merged
kixelated merged 2 commits into
mainfrom
claude/peaceful-grothendieck-10c13b
May 29, 2026
Merged

test/browser: headless-browser smoke client (phase 2)#1542
kixelated merged 2 commits into
mainfrom
claude/peaceful-grothendieck-10c13b

Conversation

@kixelated
Copy link
Copy Markdown
Collaborator

Summary

Phase 2 of the cross-language smoke test (#1529 was phase 1). Adds a headless-browser client so the smoke test covers the independent JS/WebCodecs implementation interoperating with the native Rust/Python clients over WebTransport.

The full rust × python × js-browser matrix passes in every direction:

rust       → rust ✓   python ✓   js-browser ✓
python     → rust ✓   python ✓   js-browser ✓
js-browser → rust ✓   python ✓   js-browser ✓

How the browser client works

  • nix-provisioned Chromium: flake.nix pulls playwright-driver.browsers and sets PLAYWRIGHT_BROWSERS_PATH (npm playwright pinned to the driver's 1.58.2) — no Playwright browser download, reproducible.
  • test/browser/: a standalone Vite page using the real <moq-publish> / <moq-watch> elements, driven by a Playwright runner (driver.ts):
    • Launches full Chromium with channel: "chromium" (the new headless mode — the default chrome-headless-shell lacks WebTransport/WebCodecs/mediaDevices).
    • publish: a fake camera (--use-fake-device-for-media-stream) → WebCodecs H.264 encode → streams until killed.
    • subscribe: exits 0 once <moq-watch>'s stats signal reports a decoded frame.
    • Served as a prebuilt static bundle (not a live Vite server) to avoid concurrent dep-optimizer deadlocks.

Things that needed solving (documented in code)

  • <moq-watch> only subscribes to/decodes video when it has a render target, and @moq/publish encodes lazily on demand — so the subscriber page adds a <canvas>.
  • smoke.py now waits for a catalog update that carries a video track (the browser publisher announces video in a later catalog, not the first snapshot).
  • The browser watch gives up if it subscribes to the catalog before the publisher announces it (RESET_STREAM), so test/smoke.sh adds a publisher warmup and the driver reloads once mid-timeout to recover from the race.

Defaults & CI

  • Default just test smoke stays rust,python (fast; browser cells spin Chromium).
  • js-browser is opt-in via --publishers/--subscribers; the nightly smoke.yml runs the full matrix (--timeout 30).

Notes for reviewers

  • Validated locally on macOS (full matrix green). The nightly will be the first run on Linux CI Chromium — it's non-blocking (manual + schedule), so any Linux-headless tweaks surface there without gating PRs.
  • shfmt / shellcheck / ruff / biome / actionlint / nixfmt all clean.

Test plan

  • just test smoke --publishers rust,python,js-browser --subscribers rust,python,js-browser --timeout 30 → all 9 cells PASS
  • default just test smoke (rust,python) still green
  • nightly smoke.yml on Linux CI

🤖 Generated with Claude Code

(Written by Claude)

Add a js-browser publisher/subscriber so the smoke test covers the independent
JS/WebCodecs implementation, interoperating with the native Rust/Python clients
over WebTransport. Full rust/python/js-browser matrix passes in every direction.

- flake.nix: provide headless Chromium via nixpkgs playwright-driver.browsers
  and set PLAYWRIGHT_BROWSERS_PATH (pin npm playwright to the driver's 1.58.2).
- test/browser: a standalone Vite page using <moq-publish>/<moq-watch>, driven by
  a Playwright runner. Launches full Chromium (channel "chromium" / new headless
  for WebTransport + WebCodecs) with a fake camera; publish streams H.264, subscribe
  exits once the watch element's stats signal reports a decoded frame. Served as a
  prebuilt static bundle to avoid concurrent Vite dep-optimizer deadlocks.
- test/smoke.sh: js-browser dispatch + a publisher warmup so the browser watch
  doesn't race the catalog (RESET_STREAM). The driver also reloads once mid-timeout
  to recover from an early catalog race.
- smoke.py: wait for a catalog update that carries a video track (the browser
  publisher encodes lazily, so video appears in a later catalog, not the first).
- smoke.yml: nightly runs the full matrix incl. js-browser (--timeout 30); the
  default `just test smoke` stays rust,python for speed.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 29, 2026

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: e96bcbb8-68ab-4c19-9c81-2ac698028ffa

📥 Commits

Reviewing files that changed from the base of the PR and between a881ac3 and 2c69f87.

📒 Files selected for processing (1)
  • py/moq-rs/examples/smoke.py
🚧 Files skipped from review as they are similar to previous changes (1)
  • py/moq-rs/examples/smoke.py

Walkthrough

This PR adds browser-based smoke testing to the MOQ interop test suite. It introduces a new test/browser workspace with a TypeScript test client and Playwright-driven CLI harness, integrates the browser harness into test/smoke.sh (including a WARMUP delay), updates Nix to provide Playwright browser binaries, makes the Python smoke client wait for a catalog with video, and updates CI to include js-browser in the smoke test matrix.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately and specifically describes the main change: adding a headless-browser smoke client for the cross-language test suite.
Description check ✅ Passed The description is comprehensive and directly related to the changeset, explaining the browser client implementation, matrix testing, nix provisioning, and race condition solutions.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
✨ Simplify code
  • Create PR with simplified code
  • Commit simplified code in branch claude/peaceful-grothendieck-10c13b

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
py/moq-rs/examples/smoke.py (1)

46-53: ⚡ Quick win

Add type hints for consistency with the file's existing style.

All other functions in this file include type hints (publish, subscribe, main), but _catalog_with_video does not. Adding type hints would improve consistency and provide better IDE/tooling support.

♻️ Suggested type hints
-async def _catalog_with_video(consumer):
+async def _catalog_with_video(consumer: moq.BroadcastConsumer) -> moq.Catalog:
     # The catalog is a live track. A lazy publisher (e.g. the browser, which only

Note: Use the appropriate type names from the moq library's interface. If the exact types are not exported, consider using typing.Any or Protocol types as appropriate.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@py/moq-rs/examples/smoke.py` around lines 46 - 53, Add missing type hints to
the _catalog_with_video function: annotate the consumer parameter with the
appropriate type (e.g., moq.Consumer or typing.Any if moq.Consumer/Catalog types
are not exported) and add a return type annotation (e.g., -> moq.Catalog or ->
typing.Any). Update the signature of _catalog_with_video to include these
annotations so it matches the other functions (publish, subscribe, main) in the
file and improves IDE/tooling support.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@py/moq-rs/examples/smoke.py`:
- Around line 46-53: Add missing type hints to the _catalog_with_video function:
annotate the consumer parameter with the appropriate type (e.g., moq.Consumer or
typing.Any if moq.Consumer/Catalog types are not exported) and add a return type
annotation (e.g., -> moq.Catalog or -> typing.Any). Update the signature of
_catalog_with_video to include these annotations so it matches the other
functions (publish, subscribe, main) in the file and improves IDE/tooling
support.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: b9444f51-8dcb-476c-b2dc-747b65c5993f

📥 Commits

Reviewing files that changed from the base of the PR and between ab08b0c and a881ac3.

⛔ Files ignored due to path filters (1)
  • bun.lock is excluded by !**/*.lock
📒 Files selected for processing (10)
  • .github/workflows/smoke.yml
  • flake.nix
  • package.json
  • py/moq-rs/examples/smoke.py
  • test/browser/driver.ts
  • test/browser/index.html
  • test/browser/package.json
  • test/browser/src/main.ts
  • test/browser/vite.config.ts
  • test/smoke.sh

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@kixelated kixelated enabled auto-merge (squash) May 29, 2026 17:13
@kixelated kixelated merged commit 414a10b into main May 29, 2026
1 check passed
@kixelated kixelated deleted the claude/peaceful-grothendieck-10c13b branch May 29, 2026 17:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant