feat: gate crucible-ci on multiplex's own unit tests - #110
Conversation
unittest.yaml previously ran standalone on every PR but its result was never enforced -- branch protection only requires crucible-ci-complete, not unittest-complete, so a failing test suite had zero effect on mergeability. Converts unittest.yaml to workflow_call-only and wires it into crucible-ci.yaml the same way as toolbox/rickshaw/crucible: call-unittest shares call-real-core-release-crucible-ci's if: condition, is added to that job's needs so a failing gate skips the expensive integration run, and is added directly to crucible-ci-complete's needs so a skip (not just a failure) still fails the required check. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
PR Review: multiplex#110 — feat: gate crucible-ci on multiplex's own unit testsSummary: Converts Documentation
File Coverage
Missing from diff:
LimitationsCould not verify GitHub's badge behavior on multiplex itself post-merge (no time-travel), only inferred from toolbox's identical, already-merged conversion as a direct precedent. Did not re-verify the underlying VerdictApprove with comments — the workflow wiring is correct and CI-verified live; the only finding is a non-blocking README staleness issue the author can fix in this PR or a fast follow-up. 🤖 Generated with Claude Code |
unittest.yaml lost its pull_request trigger in this same branch, so GitHub no longer tracks run history under its own badge -- it's now a workflow_call-only sub-job of crucible-ci.yaml. Same fix already applied in toolbox's identical conversion (#130). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
Fixed in a7840c2. Repointed the README CI badge from `workflows/unittest/badge.svg` to `workflows/crucible-ci/badge.svg`, matching the same fix toolbox applied for its identical conversion (#130). `unittest.yaml` no longer has a `pull_request` trigger, so its badge would otherwise silently regress to "no status" once this merges. 🤖 Generated with Claude Code |
atheurer
left a comment
There was a problem hiding this comment.
Approved! This is a high-quality PR that correctly gates crucible-ci on local unittest runs:
- Enforcement & Security: Directly chains
call-unittesttocrucible-ci-complete'sneeds, closing a critical branch-protection bypass where test failures could previously be skipped or ignored. - Resource Optimization: Correctly chains
call-real-core-release-crucible-cito depend oncall-unittestso that expensive integration runs do not start if local unit tests fail. - Workflow Simplification: Cleans up redundant trigger filters and status aggregation jobs in
unittest.yaml, adding safetimeout-minutesand secure permissions. - Badge Repointing: The README.md update (a7840c2) successfully prevents the CI badge from showing "no status" after the PR trigger removal.
Matches the ecosystem design established in toolbox#130 and rickshaw#864. Solid work!
Summary
unittest.yamlpreviously ran standalone on every PR (pull_requesttrigger, its ownunittest-completejob) but its result was never actually enforced: branch protection only requirescrucible-ci-complete, notunittest-complete, so a failing test suite had zero effect on mergeability.unittest.yamltoworkflow_call/workflow_dispatch-only, keeping just themultiplex-unittestsjob (pytest + jsonschema venv, unchanged invocation).crucible-ci.yamlwith the same 3-part gating already merged in toolbox (#130) and rickshaw (#864), and just added to crucible itself:call-unittestsharescall-real-core-release-crucible-ci'sif:condition, is added to that job'sneeds:so a failing gate prevents the expensive integration run from starting, and is added directly tocrucible-ci-complete'sneeds:too — a job skipped because a dependency failed reportsskipped(notfailure), andcrucible-ci-complete's own check only looks forfailure/cancelled, so without this direct dependency a failing gate would still produce a passing required check..github/workflows/unittest.yamlfromcrucible-ci.yaml's docs-only skip-file list — it's the actual test payload now, not orchestration.gh api repos/perftool-incubator/multiplex/rulesets/3847755) thatunittest-completewas never a required check to begin with, so this closes a real enforcement gap rather than needing a ruleset migration.Test plan
pytest/pytest-html/jsonschema— all passcall-unittestpasses beforecall-real-core-release-crucible-cistarts,call-faux-core-release-crucible-cistill correctly skips for non-docs changes🤖 Generated with Claude Code