fix(ci): status-reaper rev4 reads per-context "status" key not "state" (compensation was unreachable since rev1) #652
@ -222,9 +222,20 @@ def is_red(status: dict) -> tuple[bool, list[dict]]:
|
||||
combined = status.get("state")
|
||||
statuses = status.get("statuses") or []
|
||||
red_states = {"failure", "error"}
|
||||
# Schema asymmetry: top-level combined uses `state`, but per-entry
|
||||
# items in `statuses[]` use `status` in Gitea 1.22.6. Prefer
|
||||
# `status`; fall back to `state` defensively. Verified empirically
|
||||
# 2026-05-12 03:42Z. Pre-rev4 code only read `state` from per-entry
|
||||
# items → failed[] always empty → render_body always showed the
|
||||
# "no per-context entries were in a red state" fallback even when
|
||||
# the combined-state correctly flagged red. See
|
||||
# `feedback_smoke_test_vendor_truth_not_shape_match`.
|
||||
def _entry_state(s: dict) -> str:
|
||||
return s.get("status") or s.get("state") or ""
|
||||
|
||||
failed = [
|
||||
s for s in statuses
|
||||
if isinstance(s, dict) and s.get("state") in red_states
|
||||
if isinstance(s, dict) and _entry_state(s) in red_states
|
||||
]
|
||||
return (combined in red_states or bool(failed), failed)
|
||||
|
||||
@ -313,7 +324,9 @@ def render_body(sha: str, failed: list[dict], debug: dict) -> str:
|
||||
else:
|
||||
for s in failed:
|
||||
ctx = s.get("context", "(no context)")
|
||||
state = s.get("state", "(no state)")
|
||||
# Per-entry key is `status` in Gitea 1.22.6, not `state`
|
||||
# (see _entry_state in is_red). Fallback for forward-compat.
|
||||
state = s.get("status") or s.get("state") or "(no state)"
|
||||
url = s.get("target_url") or ""
|
||||
desc = (s.get("description") or "").strip()
|
||||
entry = f"- **{ctx}** — `{state}`"
|
||||
@ -546,7 +559,11 @@ def run_once(*, dry_run: bool = False) -> int:
|
||||
"combined_state": status.get("state"),
|
||||
"failed_contexts": [s.get("context") for s in failed],
|
||||
"all_contexts": [
|
||||
{"context": s.get("context"), "state": s.get("state")}
|
||||
# Per-entry key is `status` in Gitea 1.22.6, not `state`.
|
||||
# Pre-rev4 debug output reported `state: None` for every
|
||||
# context, making run logs useless for triage.
|
||||
{"context": s.get("context"),
|
||||
"state": s.get("status") or s.get("state")}
|
||||
for s in (status.get("statuses") or [])
|
||||
if isinstance(s, dict)
|
||||
],
|
||||
|
||||
@ -452,7 +452,18 @@ def reap(
|
||||
if not isinstance(s, dict):
|
||||
continue
|
||||
context = s.get("context") or ""
|
||||
state = s.get("state") or ""
|
||||
# Schema asymmetry: Gitea 1.22.6 returns the TOP-LEVEL combined
|
||||
# aggregate as `combined.state` but each per-context entry in
|
||||
# `combined.statuses[]` uses the key `status`, NOT `state`.
|
||||
# Prefer `status`; fall back to `state` so a future Gitea
|
||||
# version (or a test fixture written against the wrong key)
|
||||
# still flows through the compensation path. Verified empirically
|
||||
# via direct API probe 2026-05-12 03:42Z:
|
||||
# /repos/.../commits/{sha}/status entries → key is "status".
|
||||
# Pre-rev4 code read "state" only → returned "" → bypassed the
|
||||
# `state != "failure"` guard → compensation path unreachable.
|
||||
# See `feedback_smoke_test_vendor_truth_not_shape_match`.
|
||||
state = s.get("status") or s.get("state") or ""
|
||||
|
||||
# Only `failure` is the bug shape. `error`/`pending`/`success`
|
||||
# left alone — they have other meanings.
|
||||
|
||||
@ -189,6 +189,78 @@ def test_is_red_no_statuses(wd_module):
|
||||
assert failed == []
|
||||
|
||||
|
||||
# --------------------------------------------------------------------------
|
||||
# Per-entry vendor-truth key (rev4) — see status-reaper rev4 sibling
|
||||
#
|
||||
# Gitea 1.22.6 returns per-entry items in combined.statuses[] with key
|
||||
# `status`, not `state`. Pre-rev4 code only read `state` → failed[]
|
||||
# was always empty → render_body always emitted the fallback "no
|
||||
# per-context entries were in a red state". These tests use the
|
||||
# canonical Gitea shape to lock the fix in.
|
||||
# --------------------------------------------------------------------------
|
||||
def test_is_red_vendor_truth_status_key_under_pending(wd_module):
|
||||
"""Real Gitea 1.22.6 shape: per-entry uses `status`. A single failed
|
||||
context counts as red even when combined is `pending`. Pre-rev4
|
||||
this returned `(False, [])` because `s.get("state")` was None."""
|
||||
red, failed = wd_module.is_red({
|
||||
"state": "pending",
|
||||
"statuses": [
|
||||
{"context": "ci/lint", "status": "success"},
|
||||
{"context": "ci/test", "status": "failure"},
|
||||
{"context": "ci/build", "status": "pending"},
|
||||
],
|
||||
})
|
||||
assert red is True
|
||||
assert [s["context"] for s in failed] == ["ci/test"]
|
||||
|
||||
|
||||
def test_is_red_status_takes_precedence_over_state(wd_module):
|
||||
"""If both keys present (defensive), `status` (vendor truth) wins."""
|
||||
red, failed = wd_module.is_red({
|
||||
"state": "pending",
|
||||
"statuses": [
|
||||
# `status=failure` is truth even though `state=success` is
|
||||
# stale. Locking in the precedence prevents a hypothetical
|
||||
# future Gitea release that emits both from re-introducing
|
||||
# the bug under a different shape.
|
||||
{"context": "ci/test", "status": "failure", "state": "success"},
|
||||
],
|
||||
})
|
||||
assert red is True
|
||||
assert len(failed) == 1
|
||||
|
||||
|
||||
def test_is_red_state_only_fallback_still_works(wd_module):
|
||||
"""Backward-compat: a legacy fixture or future Gitea variant that
|
||||
only emits `state` still trips the red detection via the fallback
|
||||
chain. Keeps pre-rev4 fixtures green during the rev4 rollout."""
|
||||
red, failed = wd_module.is_red({
|
||||
"state": "pending",
|
||||
"statuses": [
|
||||
{"context": "ci/test", "state": "failure"}, # legacy shape
|
||||
],
|
||||
})
|
||||
assert red is True
|
||||
assert len(failed) == 1
|
||||
|
||||
|
||||
def test_render_body_uses_status_key_for_per_entry_state(wd_module):
|
||||
"""render_body must surface the per-entry `status` value in the
|
||||
issue body. Pre-rev4 it read `state` (always None on real Gitea) →
|
||||
every issue body said `(no state)`, defeating the diagnostic."""
|
||||
failed = [
|
||||
{"context": "ci/test", "status": "failure",
|
||||
"target_url": "https://example.test/run/1",
|
||||
"description": "broke"},
|
||||
]
|
||||
body = wd_module.render_body("deadbeefcafe1234", failed, {})
|
||||
assert "`failure`" in body, (
|
||||
"render_body did not surface per-entry status — likely still "
|
||||
"reading `state` key only (rev1-3 bug)."
|
||||
)
|
||||
assert "(no state)" not in body
|
||||
|
||||
|
||||
# --------------------------------------------------------------------------
|
||||
# Happy path — main is green, no issue created
|
||||
# --------------------------------------------------------------------------
|
||||
|
||||
@ -544,6 +544,156 @@ def test_reap_unparseable_push_context_preserved(sr_module, monkeypatch):
|
||||
assert counters["preserved_unparseable"] == 1
|
||||
|
||||
|
||||
# --------------------------------------------------------------------------
|
||||
# Per-context status-key vendor-truth (rev4)
|
||||
#
|
||||
# Gitea 1.22.6 returns commit-status entries with key `status` per entry,
|
||||
# NOT `state`. The TOP-LEVEL combined aggregate uses `state`. This schema
|
||||
# asymmetry caused rev1-3 to take the compensation path 0 times despite
|
||||
# triggering on real failures: `s.get("state")` returned None → state
|
||||
# evaluated to "" → `"" != "failure"` guard preserved every entry.
|
||||
#
|
||||
# These tests explicitly use the vendor-truth shape (`status` per entry),
|
||||
# proving the rev4 fix routes the failure entry through compensation.
|
||||
# Fixtures in rev1-3 tests above use `state` (the pre-fix bug shape) —
|
||||
# we keep them for backward-compat coverage via the fallback in
|
||||
# `s.get("status") or s.get("state")`, but the canonical Gitea shape
|
||||
# uses `status`. Logged under
|
||||
# `feedback_smoke_test_vendor_truth_not_shape_match`.
|
||||
# --------------------------------------------------------------------------
|
||||
def test_reap_per_context_uses_status_key_not_state(sr_module, monkeypatch):
|
||||
"""Empirical Gitea 1.22.6 shape: per-entry uses `status`, top-level
|
||||
uses `state`. The rev4 fix MUST detect failure via `status`."""
|
||||
calls = []
|
||||
|
||||
def fake_api(method, path, *, body=None, query=None, expect_json=True):
|
||||
calls.append((method, path, body))
|
||||
return (201, {})
|
||||
|
||||
monkeypatch.setattr(sr_module, "api", fake_api)
|
||||
|
||||
workflow_map = {"staging-smoke": False} # no push trigger → Class-O
|
||||
# Real Gitea-shaped response: top-level `state`, per-entry `status`.
|
||||
# No `state` key on the per-entry item.
|
||||
combined = {
|
||||
"state": "failure",
|
||||
"statuses": [
|
||||
{
|
||||
"context": "staging-smoke / smoke (push)",
|
||||
"status": "failure", # ← vendor-truth key
|
||||
"target_url": "https://example.test/run/1",
|
||||
"description": "smoke job failed",
|
||||
}
|
||||
],
|
||||
}
|
||||
counters = sr_module.reap(workflow_map, combined, SHA, dry_run=False)
|
||||
# The bug-class assertion: pre-rev4 this would have been 0, with
|
||||
# preserved_non_failure=1. Rev4 reads `status` → routes to compensate.
|
||||
assert counters["compensated"] == 1, (
|
||||
"Compensation path unreachable: status-reaper still reads `state` "
|
||||
"instead of `status` on per-entry combined.statuses[] items "
|
||||
"(rev1-3 bug)."
|
||||
)
|
||||
assert counters["preserved_non_failure"] == 0
|
||||
assert len(calls) == 1
|
||||
assert calls[0][0] == "POST"
|
||||
assert calls[0][1] == f"/repos/owner/repo/statuses/{SHA}"
|
||||
|
||||
|
||||
def test_reap_per_context_status_key_takes_precedence_over_state(
|
||||
sr_module, monkeypatch
|
||||
):
|
||||
"""Defensive: if both `status` and `state` are present (e.g. a
|
||||
hypothetical Gitea version emits both), `status` (the canonical
|
||||
Gitea 1.22.6 key) wins. Guards against a future regression where
|
||||
a fixture or future Gitea release emits stale `state="success"`
|
||||
while `status="failure"` is the truth."""
|
||||
calls = []
|
||||
|
||||
def fake_api(method, path, *, body=None, query=None, expect_json=True):
|
||||
calls.append((method, path, body))
|
||||
return (201, {})
|
||||
|
||||
monkeypatch.setattr(sr_module, "api", fake_api)
|
||||
|
||||
workflow_map = {"staging-smoke": False}
|
||||
combined = {
|
||||
"state": "failure",
|
||||
"statuses": [
|
||||
{
|
||||
"context": "staging-smoke / smoke (push)",
|
||||
# Both keys present — vendor-truth `status` MUST win.
|
||||
"status": "failure",
|
||||
"state": "success",
|
||||
"target_url": "https://example.test/run/2",
|
||||
"description": "smoke job failed",
|
||||
}
|
||||
],
|
||||
}
|
||||
counters = sr_module.reap(workflow_map, combined, SHA, dry_run=False)
|
||||
assert counters["compensated"] == 1
|
||||
assert counters["preserved_non_failure"] == 0
|
||||
assert len(calls) == 1
|
||||
|
||||
|
||||
def test_reap_per_context_state_only_fallback(sr_module, monkeypatch):
|
||||
"""Backward-compat: a test fixture or older Gitea variant that emits
|
||||
only `state` (no `status`) must still flow through compensation.
|
||||
Belt-and-suspenders against future fixture drift. Keeps rev1-3
|
||||
`state`-using fixtures green."""
|
||||
calls = []
|
||||
|
||||
def fake_api(method, path, *, body=None, query=None, expect_json=True):
|
||||
calls.append((method, path, body))
|
||||
return (201, {})
|
||||
|
||||
monkeypatch.setattr(sr_module, "api", fake_api)
|
||||
|
||||
workflow_map = {"staging-smoke": False}
|
||||
combined = {
|
||||
"state": "failure",
|
||||
"statuses": [
|
||||
{
|
||||
"context": "staging-smoke / smoke (push)",
|
||||
"state": "failure", # legacy fixture shape only
|
||||
"target_url": "https://example.test/run/3",
|
||||
}
|
||||
],
|
||||
}
|
||||
counters = sr_module.reap(workflow_map, combined, SHA, dry_run=False)
|
||||
assert counters["compensated"] == 1
|
||||
assert len(calls) == 1
|
||||
|
||||
|
||||
def test_reap_per_context_missing_both_keys_preserves(sr_module, monkeypatch):
|
||||
"""A per-entry item lacking BOTH `status` and `state` must be
|
||||
preserved (counted under preserved_non_failure). This is the only
|
||||
correctly-behaving leg of the pre-rev4 bug — exercising it ensures
|
||||
the fallback chain doesn't accidentally over-compensate on
|
||||
malformed entries."""
|
||||
monkeypatch.setattr(
|
||||
sr_module, "api",
|
||||
lambda *a, **kw: (_ for _ in ()).throw(
|
||||
AssertionError("api should not be called")
|
||||
),
|
||||
)
|
||||
|
||||
workflow_map = {"staging-smoke": False}
|
||||
combined = {
|
||||
"state": "failure",
|
||||
"statuses": [
|
||||
{
|
||||
"context": "staging-smoke / smoke (push)",
|
||||
# No status, no state — neither key present.
|
||||
"target_url": "https://example.test/run/4",
|
||||
}
|
||||
],
|
||||
}
|
||||
counters = sr_module.reap(workflow_map, combined, SHA, dry_run=False)
|
||||
assert counters["compensated"] == 0
|
||||
assert counters["preserved_non_failure"] == 1
|
||||
|
||||
|
||||
# --------------------------------------------------------------------------
|
||||
# ApiError propagation
|
||||
# --------------------------------------------------------------------------
|
||||
|
||||
Loading…
Reference in New Issue
Block a user