Skip to content

Fix browser CI capture readiness and macOS fixture permissions - #155

Open
katulevskiy wants to merge 6 commits into
mirror/upstream-mainfrom
mirror/upstream-pr/563
Open

katulevskiy wants to merge 6 commits into
mirror/upstream-mainfrom
mirror/upstream-pr/563

Conversation

@katulevskiy

Copy link
Copy Markdown
Owner

Upstream mirror

Source: zeronsh#563
Original author: @jsgrrchg
Original revision: 4441c1761f1fb42f62b7b1016d265aec90b17697

This PR is an isolated Contribution Manager pilot mirror. Its reviews, checks and merge state are separate from upstream. An empty [skip ci] commit suppresses automatic bulk CI; the code tree matches the imported upstream revision.


The first Linux browser screenshot could fail with fixture window not visible: capture performed a single PID-based lookup while Openbox startup and the video recorder's window wait ran independently. This change makes capture readiness explicit and retains enough evidence to distinguish mapping, display, and lookup failures.

Closes zeronsh#561.

  • Wait for Openbox's valid _NET_SUPPORTING_WM_CHECK before launching the fixture.
  • Obtain the native GPUI X11 window ID, wait asynchronously for a viewable window with valid dimensions (10-second deadline), and atomically publish the same ID for screenshots, native pointer input, and recording. X11 queries run off the UI thread, with an outer deadline for an unresponsive server.
  • Preserve Wayland's enclosing Weston X11 capture target. Weston discovery still uses WM_CLASS, since it does not reliably publish _NET_WM_PID; now both its visible window and Wayland socket must be ready.
  • Save window attributes/tree, PID properties, lookup status/stdout/stderr, display variables, timestamps, revision/environment, and server/WM process status before teardown. Add real-X11 readiness tests to CI and document local validation.

The macOS fixture wrapper is also now tracked as executable (100755). Direct Python subprocess.run() callers previously failed with PermissionError before starting the fixture, including the storage test's normal and cleanup paths. This permission change fixes that launch failure without changing the wrapper's contents.

Validation

Tested patch 43a0398d against unmodified baseline 03b67beb on Fedora 44, Rust 1.98.0, Xvfb 21.1.24, Openbox 3.6.1, Weston 15.0.1, Mesa 26.2.2 / LLVM 22.1.8, and WebKitGTK 2.52.5. Local FFmpeg lacks libx264, so recordings used the existing BROWSER_FIXTURE_VIDEO_CODEC=mpeg4 override.

  • macOS wrapper permission fix (0dcda0c1): reproduced PermissionError locally before the change; after the change a direct Python subprocess launch reaches Bash. bash -n passed. This launch smoke check ran on Linux and stops at argument validation, before macOS-specific commands.
  • Release build with --locked: passed.
  • xvfb-run -a cargo test --release --locked -p zeron-ui --example browser-fixture --features browser-fixture linux_capture::tests -- --include-ignored: 4 passed, none skipped. Covers delayed mapping without PID metadata, permanent unmapping, destroyed windows, and override validation.
  • Failure injection: an unmapped capture target failed at 10,000 ms with diagnostics retained and no ready-window acknowledgement. An unavailable WM failed within its deadline without launching the fixture.
  • Four final full-harness attempts: two default-renderer runs (one with Openbox delayed by 3 seconds) and two with VK_DRIVER_FILES=/dev/null. X11 passed 4/4, each retaining all 15 screenshots, browser results, and verified 1000×680 video. All 8 initial X11/Wayland screenshots succeeded.
Revision / runs X11 Wayland
Baseline: default + forced GL 2/2 passed Both failed later at menu is opaque instead of showing the live page backdrop
Patch: default + delayed Openbox + forced GL twice 4/4 passed All reached the later blur checks and failed menu tint did not blur checkerboard edges

The complete Wayland suite is not green locally. The baseline also fails blur validation on this Fedora/Weston stack, but the exact failing assertion differs. Screenshots, blur measurements, logs, videos, and every attempt's outcome are retained locally in /tmp/issue-561-validation; no blur, navigation, video, or forced-GL checks were weakened. Ubuntu CI still needs to confirm the full suite in its own environment. The historical CI startup failure was not spontaneously reproduced; controlled readiness failures exercise the new contract.


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.

@contribution-manager-katulevskiy contribution-manager-katulevskiy Bot added the review:needs-triage Managed by Contribution Manager label Oct 6, 2026
@contribution-manager-katulevskiy

contribution-manager-katulevskiy Bot commented Oct 6, 2026 •

Copy link
Copy Markdown

Contribution Manager

needs triage · medium risk · untriaged

Open review workspace · Revision 9f78faf0

Next actions:

  • Direction decision needed
  • Scope, risk, and platform triage needed
  • Independent code review needed
  • Required check: core-tests (missing)

This summary is maintained by the review app. Decisions and test evidence are recorded in the workspace; the app does not merge PRs.

@contribution-manager-katulevskiy contribution-manager-katulevskiy Bot added cm:inbox Contribution Manager: waiting for triage and removed review:needs-triage Managed by Contribution Manager labels Oct 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cm:inbox Contribution Manager: waiting for triage

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants