ci(journeys): run the browser probes, so their green stops decaying - #441
Merged
Conversation
`OCU_BROWSER_E2E` was set nowhere — no workflow, no stand script. So the twenty- two frame-egress probes only ever ran when I ran them by hand, and "proven by running probes" was decaying into "proven the day it merged". Twelve of them need a live stand (a pane, a portal, a render frame) and stay opt-in. Four do not: the sink control, the unpoliced-channel control, and both halves of the script-execution probe reproduce the isolation primitives standalone. Measured — they pass on a bare runner with only chromium. The gate is SET in this job on purpose. `_require_browser` fails rather than skips when the gate is set and chromium is missing, so a broken install cannot read as a pass. The vacuity guard is an equality, not a floor, and it is the same failure this file already guards for the journeys: a suite where everything skipped renders green. Four is the measured count; a floor would hide a probe silently dropping out of the selection, which is the regression worth catching. Mutation-checked against a real junit report: with the gate unset, 3 of 4 skip and the guard reds. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. 🗂️ Base branches to auto review (2)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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. Comment |
…rties Fable's ruling, and he was right on both counts. The comment claimed the stand-bound probes "are not deselected silently: they SKIP with their own reason, visible in -rs". Measured: `-k` DESELECTS them, deselected tests never reach `-rs` and never enter the junit, and `-rs` reports zero SKIPPED lines here. It also said twelve; the arithmetic is 22 - 4 = eighteen test ids. A comment that misstates a gate's verification properties is exactly the decay this job exists to stop, so it says the true thing now: the eighteen are deselected, their absence is detectable only through the equality guard, and that guard is the load-bearing half of the job. Also states plainly that this covers 4 of 22 and is not "the N group covered". Two follow-ons he named, both cheap enough to do here rather than queue: A weekly schedule plus workflow_dispatch. Chromium installs at run time, so the probes can rot from upstream drift with no repo change — and that rot would surface as a red on some unrelated deploy PR. Per-PR alone was still "proven the day it merged", only with a longer day. Pinned playwright and pytest. A floating toolchain changes what the probe MEANS between runs. Both pins are the versions the probes were actually measured on (1.62.0 / 9.1.1) — I first wrote 1.58.0 and 8.3.4 from memory, which would have pinned a contract I never ran. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The job died at collection with ModuleNotFoundError: no module named 'yaml' — the journeys conftest imports it, and I had installed only playwright and pytest. My local venv already had it, which is the whole reason the CI run is the one that counts. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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
2 participants
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.
OCU_BROWSER_E2Ewas set nowhere — no workflow, no stand script. So the twenty-two frame-egress probes from #438 only ever ran when I ran them by hand, and "proven by running probes" was quietly decaying into "proven the day it merged".Four probes need no stand
Twelve need a live pane, portal and render frame, and stay opt-in. Four do not — the sink control, the unpoliced-channel control, and both halves of the script-execution probe reproduce the isolation primitives standalone. Measured: they pass on a bare runner with only chromium installed.
The gate is set on purpose
_require_browserfails rather than skips when the gate is set and chromium is missing. A broken install therefore cannot read as a pass — which is the whole reason to set the gate rather than leave the job unconditioned.The vacuity guard is an equality, not a floor
Same failure this workflow already guards for the journeys: a suite where everything skipped renders green. Four is the measured count, and a floor would hide a probe silently dropping out of the
-kselection — which is exactly the regression worth catching.Mutation-checked against a real junit report rather than by reading it:
What this does not cover
The twelve stand-bound probes still need the fleet stand, and they still FAIL rather than skip once a render frame exists — that is deliberate, and it is the tier-2 gate, not this job.