Skip to content

Commit 0c74096

Browse files
committed
Name a real regression even when another condition outranks it
The verdict chain picks the most severe condition, so a run with a skew-excused crash on one bucket and a genuine BLOCK on another described itself as "CI image looks stale" and never mentioned the regression. Same for a BLOCK alongside a bucket that never reported. The status colour was right in both cases, but the description misattributed the cause -- the same failure mode the skew excuse already risks, and the one a reviewer acts on. The description is now additive: the most severe condition still sets the verdict, and a blocking-level regression is named alongside it.
1 parent bf5e6a3 commit 0c74096

2 files changed

Lines changed: 49 additions & 0 deletions

File tree

tools/perf_smoke_test/aggregate.py

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -525,6 +525,13 @@ def _verdict_outputs(
525525
verdict = OracleVerdict.PASS
526526
description = f"perf-smoke: no meaningful regression across {len(rows)} buckets"
527527

528+
# A run can be several things at once -- a stale image on one bucket and a
529+
# genuine regression on another. The verdict takes the most severe, but the
530+
# description must not misattribute: reporting a real BLOCK as "CI image
531+
# looks stale" is the same misattribution the skew excuse already risks.
532+
if has_block and verdict != OracleVerdict.BLOCK:
533+
description += "; a blocking-level regression was also detected"
534+
528535
state = "success" if verdict in (OracleVerdict.PASS, OracleVerdict.WARN) else "failure"
529536
if not blocking and state == "failure":
530537
description += " (advisory)"

tools/perf_smoke_test/test/test_advisory_exit.py

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -367,3 +367,45 @@ def test_complete_matrix_is_clean_on_both_surfaces(tmp_path, monkeypatch) -> Non
367367
assert outputs["status_state"] == "success"
368368
assert "did not report" not in summary
369369
assert "coverage is incomplete" not in summary
370+
371+
372+
def test_a_real_block_is_not_misreported_as_a_stale_image() -> None:
373+
"""A BLOCK alongside a skew-excused crash must still be named."""
374+
rows = _rows(OracleVerdict.HARD_FAILURE, OracleVerdict.BLOCK)
375+
out = aggregate._verdict_outputs(
376+
rows,
377+
has_block=True,
378+
has_hard_failure=False, # cleared by detect_dependency_skew
379+
blocking=False,
380+
missing=[],
381+
expected_total=2,
382+
)
383+
384+
assert out["status_state"] == "failure"
385+
assert "stale" in out["status_description"]
386+
assert "blocking-level regression was also detected" in out["status_description"]
387+
388+
389+
def test_a_real_block_is_not_hidden_behind_a_bucket_shortfall() -> None:
390+
"""A BLOCK alongside missing buckets must still be named."""
391+
out = aggregate._verdict_outputs(
392+
_rows(OracleVerdict.BLOCK),
393+
has_block=True,
394+
has_hard_failure=False,
395+
blocking=False,
396+
missing=["Task-X/physx"],
397+
expected_total=2,
398+
)
399+
400+
assert "only 1 of 2" in out["status_description"]
401+
assert "blocking-level regression was also detected" in out["status_description"]
402+
403+
404+
def test_a_lone_block_is_not_double_reported() -> None:
405+
"""The additive note must not duplicate when BLOCK is already the verdict."""
406+
out = aggregate._verdict_outputs(
407+
_rows(OracleVerdict.BLOCK), has_block=True, has_hard_failure=False, blocking=False, missing=[], expected_total=1
408+
)
409+
410+
assert out["overall_verdict"] == "BLOCK"
411+
assert out["status_description"].count("blocking-level") == 1

0 commit comments

Comments
 (0)