Skip to content

ci(cefsrc): run tests against a real cefsrc - #810

Open
wagenet wants to merge 3 commits into
Eyevinn:mainfrom
wagenet:wagenet/ci-cefsrc-job
Open

wagenet wants to merge 3 commits into
Eyevinn:mainfrom
wagenet:wagenet/ci-cefsrc-job

Conversation

@wagenet

@wagenet wagenet commented Sep 13, 2026 •

Copy link
Copy Markdown
Contributor

Opened by Claude (Claude Code) on behalf of @wagenet.

CI has no cefsrc, so nothing guards behaviour of the element behind HTML sources and DSK graphics. A new Test (Linux, cefsrc) job installs the prebuilt gstcefsrc plugin and runs three new tests against it through Strom flows.

The job

It runs on pushes to main, on PRs labelled ci:cef, on PRs touching docker/strom-full/, docker/gstcefsrc/, build-gstcefsrc.yml or the test file, and on workflow_dispatch (new cef input). Main already starts preflight on any ci: label. cef-changes also runs on a ci: label, because that run replaces the one it cancels (see concurrency).

A second job, Build strom-full image, builds docker/strom-full/Dockerfile for amd64 on a pull request that changes that directory, pushes nothing, and then runs gst-inspect-1.0 cefsrc through the entrypoint, so it also fails if cef-env.sh or the plugin install is broken. It reads the publish workflow's layer cache without writing to it. The base is the published eyevinntechnology/strom:latest, not this PR's backend: what it checks is the layers strom-full adds. Before this, the only build was the publish workflow, which runs after merge.

The test job runs in an ubuntu:questing container (the strom-full base), because the plugin is built against questing's glibc and GStreamer 1.26. The tarball is cached with actions/cache, keyed on the version.

The image and CI share one copy of each piece. The version is the Dockerfile's ARG GSTCEFSRC_VERSION. install-gstcefsrc.sh does the download, extract and symlinks, and the Dockerfile bind-mounts it. cef-env.sh holds the Chromium flags, Xvfb, cache directory and mallinfo shim, and the entrypoint sources it. The three path ENV lines are repeated in CI rather than moved, since moving them would change the image for anyone overriding the entrypoint; if they drift, CI fails loudly.

The image is unchanged apart from the added /usr/local/lib/strom/cef-env.sh. Before the rebase onto 0216c1c1, I checked this by building the old and new Dockerfile locally (arm64), diffing the filesystems, and diffing the environment after the entrypoint in all three GPU branches (no GPU, and a stub nvidia-smi with STROM_CEF_GPU=0 and =1).

Tests (backend/tests/cefsrc_test.rs)

Without cefsrc the tests skip, which is what happens locally and in every other CI job. With STROM_REQUIRE_CEF=1, which the new job sets, a missing cefsrc fails them instead. They also skip off Linux, because CEF on macOS needs a main-thread Cocoa run loop the test harness lacks.

Flows go through AppState start/stop. Test pages animate a corner pixel, since Chromium paints only on change.

  1. cefsrc_flow_restarts_without_leaking runs four start / 5 frames / stop cycles of cefsrc -> appsink. Each cycle asserts the pipeline and all elements finalize. Cycles 2-4 must return to cycle 1's thread count (slack 2) and CEF subprocess count.
  2. cefsrc_on_dsk_input_reaches_mixer wires cefsrc -> Video Format (resolution only) -> vision mixer dsk_in_0 (CPU) and asserts the compositor pad linked to the DSK chain receives 10 buffers.
    It does not guard the framerate=0/1 stall yet. The pinned gstcefsrc (b633408) advertises framerate=[1/1, 60/1], and the test still passed with framerate=25/1 pinned in the format block. 0/1 arrived 19 commits later in c89c2ce; until the pin passes it, this is a DSK-path smoke test.
  3. cefsrc_emits_premultiplied_alpha renders background: rgba(255,255,255,0.5) at 320x180 and asserts the centre pixel at cefsrc's output is BGRA ~128,128,128,128 (premultiplied), not 255,255,255,128. The composited on-air value is not asserted: it is 96 on main and feat(vision_mixer): composite premultiplied-alpha DSK inputs correctly #809 changes it, so whichever PR lands second can add that.
    It waits for the page's animated corner square to show red or blue before sampling, since only the page's own script draws that. Waiting instead on a non-zero centre alpha failed once on this PR's CI, on a frame that read opaque black.

Tests 1 and 3 were not reverse-tested: flipping cefsrc's alpha or leaking a CEF process needs a different cefsrc build. The weak-ref detection in test 1 is the mechanism pipeline_lifecycle_test.rs already proves with its negative test.

What ran

Rebased on 0216c1c1. Main's #979 rewrote the CEF cache setup this PR moves into cef_setup_cache; the function now carries #979's version (keep the cache, drop only this host's stale lock). The image-equivalence diff above has not been re-run since the rebase.

After the rebase, at 407a40eb:

  • Local, arm64 questing container: the CI job's steps (install script, cef-env.sh, STROM_REQUIRE_CEF=1): 3 passed in 12.7 s.
  • cargo fmt --check and cargo clippy --workspace --all-targets -D warnings on macOS; actionlint and bash -n on the workflow and scripts.

Before the rebase, at e68a637:

  • This PR's own CI: run 35244345206 — all checks green. Test (Linux, cefsrc) ran all three tests, none skipped (test result: ok. 3 passed), and Build strom-full image built the image and loaded cefsrc through the entrypoint (2 min).
  • Fork CI (amd64): four more workflow_dispatch runs of the cefsrc job, all 3 passed (1, 2, 3, 4). With the PR's own run that is 5 of 5 since the paint-wait change.
  • The flake it fixes: at 167dd1e, before that change, cefsrc_emits_premultiplied_alpha failed once on run 35237502683 with alpha of a 50% CSS fill is 255 (BGRA 0,0,0,255) — an opaque black frame. Three fork runs at that same commit passed, so 1 in 6. Not reproduced locally: the tests skip on macOS and the local Docker daemon is down, so the opaque-black-before-load explanation rests on that one failure message.
  • Local, arm64 questing container: all three passed, plus the test 2 revert experiment above — before the rebase, at 29364e8.
  • cargo fmt --check and cargo clippy --workspace --all-targets -D warnings on macOS after the rebase; the same on Linux through Check (Linux).
  • Not exercised: the ci:cef label, which needs a maintainer.

🤖 Generated with Claude Code

@wagenet
wagenet marked this pull request as ready for review September 13, 2026 04:21
@srperens srperens mentioned this pull request Sep 14, 2026

@srperens srperens left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Verdict: Comment — the new job is the real thing: it defeats the skip guard, and the three tests are confirmed executing in this repository rather than only on the fork. The entrypoint refactor is behaviour-preserving as far as it can be read statically. What holds it short of Approve is that the Dockerfile half ships in an image no check on this pull request builds.

Requested changes

  1. Either extend the cef-changes path trigger to build docker/strom-full/Dockerfile on a pull request that touches it, or state in the body that the Dockerfile change is covered only by the local arm64 build. .github/workflows/docker-publish-full.yml:8 — workflow_run: — and a workflow_dispatch, are its only triggers, so the first CI build of the new bind-mount happens after merge, during a publish.

Claims

Claim Verdict Evidence
All three tests executed here, not only on the fork CONFIRMED job 103644297674 of run 34727553991, whose head_sha is 29364e81e48d403065044c4ff4ae909bc7b273c8: STROM_REQUIRE_CEF: 1 in the job env, then test result: ok. 3 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 6.37s
The Chromium flags are unchanged by the move into cef-env.sh CONFIRMED docker/strom-full/cef-env.sh:13 — CEF_SOFTWARE_FLAGS="no-sandbox,disable-gpu,disable-gpu-compositing,use-gl=disabled,${CEF_COMMON_FLAGS}" expands byte-for-byte to the string the entrypoint set inline at all three branches; docker/strom-full/cef-env.sh:17 — CEF_GPU_FLAGS="no-sandbox,use-gl=angle,use-angle=vulkan,enable-gpu-rasterization,ignore-gpu-blocklist,enable-zero-copy,${CEF_COMMON_FLAGS}" likewise, and the five symlinks moved into install-gstcefsrc.sh resolve to the same paths
The image still builds UNVERIFIED docker/strom-full/Dockerfile:22 — RUN --mount=type=bind,source=install-gstcefsrc.sh,target=/tmp/install-gstcefsrc.sh \ — is reached by no check on this PR. The bind source is correct against context: docker/strom-full, and docker/build-push-action@v7 supplies BuildKit, so I expect it to work — but expecting is not the same as a green build

Diagnosis — Right layer. The gap was that cefsrc is absent everywhere in CI, so an element behind HTML sources and DSK graphics had no guard at all; installing the real plugin in a dedicated job is the only thing that closes it. Coverage is BOUNDED, and the bound is honestly stated: test 2 is a DSK smoke test because the pinned b633408 advertises framerate=[1/1, 60/1], so it cannot yet guard the 0/1 stall.

Radius — SHARED, and it needs arguing because the diff edits a shipped artifact. It changes the startup environment of every strom-full deployment, but no application code, no API surface and no event contract; the blast is confined to whether cefsrc renders in the image. The failure mode if cef-env.sh were wrong is loud — docker/strom-full/entrypoint.sh:32 — . /usr/local/lib/strom/cef-env.sh — sources it unconditionally, so a missing or broken file fails at container start rather than degrading quietly.

Tests & CI — All ten checks green at this head, including Test (Linux, cefsrc) (7m0s). This is the rare PR that satisfies the CLAUDE.md rule most often broken — a test needing a GStreamer element that can actually run in CI — because STROM_REQUIRE_CEF=1 converts the skip into a failure. Worth recording that the three tests still skip in Check (Linux), which is correct, and that .github/workflows/ci.yml:125 — && (github.event_name == 'push' — is what keeps them honest: a PR that breaks cefsrc without touching the trigger paths goes unnoticed until the merge to main runs the job.

Confidence: HIGH

@wagenet
wagenet force-pushed the wagenet/ci-cefsrc-job branch from 29364e8 to 167dd1e Compare September 17, 2026 15:01
@wagenet

wagenet commented Sep 18, 2026

Copy link
Copy Markdown
Contributor Author

Posted by Claude (Claude Code) on behalf of @wagenet.

Rebased on current main and addressed the requested change.

The unbuilt Dockerfile. Taken the first branch: a new Build strom-full image job builds docker/strom-full/Dockerfile for amd64 on a pull request that changes that directory. It pushes nothing, reads the publish workflow's layer cache without writing to it, and then runs gst-inspect-1.0 cefsrc through the entrypoint — so it fails on a broken cef-env.sh or a plugin that did not land where the ENV lines point, not only on a build error. cef-changes gained a second output for this, so a docker/gstcefsrc/ change does not pull the image build in.

It ran green on this PR: Build strom-full image, 2 min, both steps executed. Your UNVERIFIED claim is now covered for amd64. The bound that remains: the base is the published eyevinntechnology/strom:latest, not this PR's backend, and arm64 is still built only by the publish workflow.

A flake the rebase exposed. The first post-rebase run failed cefsrc_emits_premultiplied_alpha with alpha of a 50% CSS fill is 255 (BGRA 0,0,0,255) — opaque black, not a wrong alpha. The test counted any frame with a non-zero centre alpha as painted, and an opaque black frame passes that. It now waits for the page's animated corner square to show red or blue, which only the page's own script draws.

Failure rate before the change was 1 in 6 (one Eyevinn run, three fork runs at the same commit passing). After it, 5 of 5: this PR's run plus four fork dispatches. I could not reproduce it locally — the tests skip on macOS and the local Docker daemon is down — so the opaque-black-before-load explanation rests on that one failure message.

srperens
srperens previously approved these changes Sep 18, 2026

@srperens srperens left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Verdict: Approve — supersedes my review at 29364e81. Its one request is closed in the stronger form: rather than documenting the gap, this now builds the image on a pull request that changes it, and that job has run green at this head. Nothing else in the diff moved — the two new commits are that job and the alpha test's paint wait.

Claims

Claim Verdict Evidence
The image is now built by a check on this PR, and pushes nothing CONFIRMED .github/workflows/ci.yml:228 — if: needs.cef-changes.outputs.image == 'true' — gates it; .github/workflows/ci.yml:243 — push: false — and .github/workflows/ci.yml:249 — cache-from: type=gha,scope=docker-full-amd64 — reads the publish cache with no cache-to, no registry login and no secret in the job
That check exercises cef-env.sh, not just the build CONFIRMED docker/strom-full/Dockerfile:42 — ENTRYPOINT ["/entrypoint.sh"] — with docker/strom-full/entrypoint.sh:78 — exec "$@", so docker run strom-full:ci gst-inspect-1.0 cefsrc sources the env file before loading the plugin
All three tests executed at e68a637, none skipped CONFIRMED job 105280587876 of run 35244345206: STROM_REQUIRE_CEF: 1 in the job env, then running 3 tests and test result: ok. 3 passed; 0 failed; 0 ignored
The paint wait fixes the flake by mechanism, not by luck CONFIRMED backend/tests/cefsrc_test.rs:523 — ` let red_or_blue = g < 32 && ((r > 223 && b < 32)
The pinned b633408 advertises framerate=[1/1, 60/1], so test 2 cannot guard the 0/1 stall EXTERNAL a prebuilt upstream plugin, not readable from this repository; the body states the bound and I am taking it at face value

Diagnosis — Unchanged from my last review and still right: cefsrc was absent from every CI job, so the element behind HTML sources and DSK graphics had no guard at all. Coverage is BOUNDED and the bound is stated rather than glossed — test 2 is a DSK smoke test until the pin passes c89c2ce, and tests 1 and 3 are not reverse-tested because flipping cefsrc's alpha needs a different build of it. CLAUDE.md asks exactly that disclosure of a test that cannot be reverse-tested, and the body makes it.

Radius — SHARED, argued as before: the diff edits a shipped artifact's startup environment but no application code, no API surface and no event contract. What is new since that argument is that the failure mode is no longer only reasoned about — the image now builds and loads cefsrc in CI.

Overlaps — #797, #809 and #838 also edit .github/workflows/ci.yml, in other jobs; this adds its block above check-linux, so the collision is rebase cost rather than a contested decision. None of them owns a design question this merge would settle. The one real interaction is already handled in the body: the composited on-air alpha is deliberately not asserted because #809 changes it, leaving it to whichever lands second. #822 is the open report about premultiplied DSK inputs that test 3 gives a fixed reference point for.

Confidence: HIGH

wagenet added a commit to wagenet/strom that referenced this pull request Sep 29, 2026
… flags

Integration fix-up: upstream Eyevinn#789 adds two structured-logging flags to
EventBroadcaster::new and AppState::new. Tests from Eyevinn#797, Eyevinn#803, Eyevinn#809,
Eyevinn#810 and Eyevinn#844 still call the old signatures.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
wagenet added a commit to wagenet/strom that referenced this pull request Sep 29, 2026
… flags

Integration fix-up: upstream Eyevinn#789 adds two structured-logging flags to
EventBroadcaster::new and AppState::new. Tests from Eyevinn#797, Eyevinn#803, Eyevinn#809,
Eyevinn#810 and Eyevinn#844 still call the old signatures.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
wagenet added a commit to wagenet/strom that referenced this pull request Oct 1, 2026
… flags

Integration fix-up: upstream Eyevinn#789 adds two structured-logging flags to
EventBroadcaster::new and AppState::new. Tests from Eyevinn#797, Eyevinn#803, Eyevinn#809,
Eyevinn#810 and Eyevinn#844 still call the old signatures.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@wagenet
wagenet force-pushed the wagenet/ci-cefsrc-job branch 2 times, most recently from 761b6d9 to fe9a105 Compare October 2, 2026 15:57
@wagenet

wagenet commented Oct 2, 2026

Copy link
Copy Markdown
Contributor Author

Claude (AI), on behalf of @wagenet.

Rebased onto main. docker/strom-full/Dockerfile conflicted with #879, which moved strom-full to Strom's patched build (144.0.21-strom5) and pinned each architecture's archive by sha256. Resolved by keeping both:

Ran: fmt and cargo clippy --workspace --all-targets -D warnings on macOS. In an arm64 ubuntu:questing container with the strom5 arm64 archive installed by the new script (hash OK), cargo test --test cefsrc_test with STROM_REQUIRE_CEF=1 and cef-env.sh set up as the CI job does: 3 passed. A wrong hash makes the script fail before it extracts anything. docker build of strom-full for arm64 succeeds, and gst-inspect-1.0 cefsrc loads through the entrypoint. The amd64 job itself was not run.

wagenet and others added 3 commits October 8, 2026 15:50
Add a Test (Linux, cefsrc) job that installs the prebuilt gstcefsrc plugin
in an ubuntu:questing container, the strom-full base, and runs
backend/tests/cefsrc_test.rs with STROM_REQUIRE_CEF=1, so a missing cefsrc
fails instead of skipping. It runs on main, on ci:cef PRs, on PRs touching
the gstcefsrc pin or the strom-full image, and on dispatch.

The tests cover flow restarts (finalization, threads, CEF subprocesses), a
cefsrc on a vision mixer DSK input reaching the compositor, and cefsrc
emitting premultiplied alpha.

The image and CI share one gstcefsrc version (the Dockerfile ARG), one
install script and one runtime script for the Chromium flags, Xvfb, cache
and mallinfo shim. The image's filesystem gains only cef-env.sh, and the
entrypoint's environment is unchanged.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The publish workflow was the only build of docker/strom-full/Dockerfile,
and it runs after merge. The new job builds the image for amd64 without
pushing, then loads cefsrc through the entrypoint, which fails if
cef-env.sh or the plugin install is broken.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The premultiplied-alpha test counted any frame with a non-zero centre
alpha as painted. On Eyevinn CI it sampled an opaque black frame
(BGRA 0,0,0,255) and failed. It now waits for the animated corner square
to show red or blue, which only the page's script draws.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@wagenet
wagenet force-pushed the wagenet/ci-cefsrc-job branch from fe9a105 to 407a40e Compare October 8, 2026 23:03

@srperens srperens left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Verdict: Comment. This supersedes my review of 2026-09-18 (Approve at e68a637, auto-dismissed by the push). The diff still earns that verdict, but I cannot approve on a red Check (Linux), and that failure is not this PR's. The new head 407a40e is a rebase onto 0216c1c. In git range-diff, commits 2 and 3 are identical. Commit 1 carries main's #979 cache handling into cef-env.sh and main's tarball checksum into install-gstcefsrc.sh.

Needs a maintainer

  1. Re-run the failed job: gh run rerun 37857274288 --failed. If it goes green, this PR is ready to approve.

Claims

Claim Verdict Evidence
The Check (Linux) failure is not from this diff CONFIRMED Job 113584400615 fails in vision_mixer_input_format_test (gpu_i420: failed to reach PLAYING). The diff's only backend file is backend/tests/cefsrc_test.rs, which skips on that job, and it does not change the check-linux job. Main's Check (Linux) at the same base 0216c1c passed (job 113354643183), so this is a flake. Twelve later test binaries never ran (vision_mixer_overlay_upload_test through whip_whep_ice_transport_policy_test)
All three cefsrc tests executed at this head CONFIRMED Test (Linux, cefsrc) job 113584400595: cefsrc_emits_premultiplied_alpha, cefsrc_on_dsk_input_reaches_mixer and cefsrc_flow_restarts_without_leaking all ok. .github/workflows/ci.yml:227 — STROM_REQUIRE_CEF: 1 makes a missing plugin a failure
Moving the download into the script keeps main's checksum CONFIRMED The check is quoted below the table. Build strom-full image (job 113584395037) logs /tmp/gstcefsrc.tar.gz: OK
The image is unchanged apart from cef-env.sh UNVERIFIED The body says the filesystem and environment diff was not re-run after the rebase. #979's cache logic now lives in the sourced file, and I compared that text by reading it only

The checksum line: docker/strom-full/install-gstcefsrc.sh:41 — echo "${GSTCEFSRC_SHA256} ${archive}" | sha256sum -c -

Radius: SHARED. No application code changes, but the PR edits the shipped image's startup environment.

Overlaps

  • #1056 depends on this job. Its stinger_web_test needs adding to the job's --test list and path filter once this lands, so land this first.
  • #809 edits ci.yml in check-linux, and its Xvfb step should go away after #977. It does not collide with this PR.
  • #964 (Linux CI on 1.24.2) is related: this job already runs the production-like questing / 1.26 base. It settles nothing #964 asks.

Tests & CI: Linux builds, WASM, API contract and both cefsrc jobs are green at this head. macOS and Windows skipped, which is right for a Linux-only job. Main has moved four commits since 0216c1c, none of them in docker/ or ci.yml, and a trial merge onto main is clean. I ran nothing locally.

Confidence: HIGH

wagenet added a commit to wagenet/strom that referenced this pull request Oct 9, 2026
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.

2 participants