Skip to content

Commit 52304d3

Browse files
compare: decouple cross-arch section from branch-over-branch gating (#532)
0.12.25 gated the cross-arch delta on 'len(archs_with_data) >= 2' where archs_with_data was the subset of per-arch sections that had branch-over-branch data. This silently suppressed cross-arch on any PR where one arch lacked a baseline on master -- notably the bootstrap PR that introduces aarch64 CI (seen on RediSearch #9258): x86_64 has a years-long master baseline, aarch64 does not, so aarch64 branch-over-branch correctly rendered the 'no baseline' warning, but the cross-arch section ALSO got skipped even though both archs had data on the PR commit itself -- which is exactly what cross-arch compares. Switch the gate to 'len(multi_archs) >= 2' (user-requested >=2 archs + didn't opt out) and let _render_section's own had_data check drop the section only when cross-arch itself has no comparison points. Adds test_compare_architectures_cross_arch_emits_without_aarch64_baseline to lock this in: simulates the bootstrap scenario and asserts aarch64 branch-over-branch warns AND cross-arch still renders. Bumps pyproject.toml 0.12.25 -> 0.12.26.
1 parent 1596ac2 commit 52304d3

3 files changed

Lines changed: 78 additions & 6 deletions

File tree

pyproject.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
[tool.poetry]
22
name = "redisbench-admin"
3-
version = "0.12.25"
3+
version = "0.12.26"
44
description = "Redis benchmark run helper. A wrapper around Redis and Redis Modules benchmark tools ( ftsb_redisearch, memtier_benchmark, redis-benchmark, aibench, etc... )."
55
authors = ["filipecosta90 <filipecosta.90@gmail.com>","Redis Performance Group <performance@redis.com>"]
66
readme = "README.md"

redisbench_admin/compare/compare.py

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -658,11 +658,17 @@ def _accumulate(totals, detected_regressions_local):
658658
comment_body += _missing_arch_warning(arch, comparison_branch)
659659

660660
# Cross-arch delta section: both sides on the comparison branch,
661-
# architectures differ. Rendered only when we have >=2 archs
662-
# with data AND the user didn't opt out via --no-cross-arch.
663-
archs_with_data = [r[0] for r in per_arch_results if r[3]]
664-
if len(archs_with_data) >= 2 and not getattr(args, "no_cross_arch", False):
665-
xa_baseline, xa_comparison = archs_with_data[0], archs_with_data[1]
661+
# architectures differ. Gated ONLY on "user requested >=2 archs"
662+
# + "didn't opt out"; NOT on whether branch-over-branch had data
663+
# for each arch. The cross-arch comparison is valuable even on
664+
# bootstrap PRs -- e.g. the first PR to introduce aarch64 has no
665+
# aarch64 baseline on master (so that arch's branch-over-branch
666+
# warns empty), but x86 and aarch64 data both exist on the PR
667+
# commit itself, which is exactly what cross-arch wants to
668+
# compare. We delegate the data-presence check to _render_section;
669+
# if it returns had_data=False the section is simply omitted.
670+
if len(multi_archs) >= 2 and not getattr(args, "no_cross_arch", False):
671+
xa_baseline, xa_comparison = multi_archs[0], multi_archs[1]
666672
(
667673
xa_section_md,
668674
_,

tests/test_compare.py

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -387,6 +387,72 @@ def test_compare_architectures_no_cross_arch_flag_skips_cross_section() -> None:
387387
assert "## Cross-arch delta" not in result.comment_body
388388

389389

390+
def test_compare_architectures_cross_arch_emits_without_aarch64_baseline() -> None:
391+
"""Bootstrap case: the PR introducing aarch64 has x86_64 baseline data on
392+
master but NO aarch64 data on master. aarch64 branch-over-branch should
393+
warn (no baseline), but the cross-arch delta MUST still render because
394+
both archs have data on the PR (comparison) branch -- that's the whole
395+
point of cross-arch.
396+
397+
Regression test for the 0.12.25 bug where cross-arch was gated on
398+
len(archs_with_data) >= 2, suppressing it whenever one arch's
399+
branch-over-branch had no baseline.
400+
"""
401+
conn = _get_redis_connection()
402+
if conn is None:
403+
pytest.skip("Redis not available (RTS_DATASINK_HOST or RTS_PORT not set)")
404+
rts, rts_host, rts_port, rts_pass = conn
405+
rts.flushall()
406+
407+
# master: x86_64 only (simulates no ARM CI on master yet)
408+
_export_benchmark_data(
409+
rts_host,
410+
rts_port,
411+
rts_pass,
412+
"master",
413+
"org",
414+
"repo",
415+
architecture="x86_64",
416+
)
417+
# feature branch: BOTH archs (the PR introduces aarch64 CI)
418+
_export_benchmark_data(
419+
rts_host,
420+
rts_port,
421+
rts_pass,
422+
"feature",
423+
"org",
424+
"repo",
425+
architecture="x86_64",
426+
)
427+
_export_benchmark_data(
428+
rts_host,
429+
rts_port,
430+
rts_pass,
431+
"feature",
432+
"org",
433+
"repo",
434+
architecture="aarch64",
435+
)
436+
result = _run_comparison(
437+
rts_host,
438+
rts_port,
439+
rts_pass,
440+
"master",
441+
"feature",
442+
"org",
443+
"repo",
444+
architectures="x86_64,aarch64",
445+
)
446+
# aarch64 branch-over-branch still warns -- no aarch64 baseline on master.
447+
assert "## Architecture: `x86_64` — branch-over-branch" in result.comment_body
448+
assert "## Architecture: `aarch64` — branch-over-branch" in result.comment_body
449+
assert "No `aarch64` benchmark data found" in result.comment_body
450+
# Cross-arch MUST still render -- both archs have data on the feature
451+
# branch so the delta is well-defined.
452+
assert "## Cross-arch delta on `feature`" in result.comment_body
453+
assert "(`x86_64` → `aarch64`)" in result.comment_body
454+
455+
390456
def test_compare_architectures_single_entry_no_cross_arch() -> None:
391457
"""--architectures with a single entry (e.g. 'x86_64') produces one
392458
section and no cross-arch delta -- there's nothing to cross-compare."""

0 commit comments

Comments
 (0)