Skip to content

fix: merge test/test-full leaf variants per-cell instead of wholesale - #80

Merged
HereThereBeDragons merged 3 commits into
developfrom
users/cgoea/merge_test_leaf_variants_per_cell
Aug 17, 2026
Merged

fix: merge test/test-full leaf variants per-cell instead of wholesale#80
HereThereBeDragons merged 3 commits into
developfrom
users/cgoea/merge_test_leaf_variants_per_cell

Conversation

@cgoea

@cgoea cgoea commented Aug 14, 2026

Copy link
Copy Markdown
Contributor

Motivation

upsert_leaf's test/test-full path replaced a per-arch test leaf (and its full variants list) wholesale whenever the do-not-downgrade guard accepted the update. The build phase already avoided this via _merge_variant_leaf, which merges matrix cells one at a time, but test/test-full had no equivalent.

Technical Details

  • Added merge_matrix_test_leaf(existing, new) to therock_status_document.py, mirroring _merge_variant_leaf. Beyond per-cell variant merging, its leaf header (run_id/run_attempt) advances monotonically via the same should_replace guard, so a race-loser snapshot cannot regress the header.
  • Wired StatusDocument.upsert_leaf's test/test-full branch: when the incoming leaf carries variants, it now merges into the existing leaf via merge_matrix_test_leaf and always reports the write as accepted.
  • Wired _refresh_same_run_fanout_tests in therock_update_status_json.py to merge cell-by-cell via merge_matrix_test_leaf instead of overwriting the leaf's variants wholesale. After merging develop (prevent cross-architectural leakeage in matrix_job_re #76), this composes with prevent cross-architectural leakeage in matrix_job_re #76's per-arch scoping: variants are re-derived per architecture into a candidate, and that per-arch candidate is what gets merged cell-by-cell, so per-cell preservation and cross-arch isolation both hold.

Test Plan

  • Updated test_upsert_test_replaces_whole_leaf_with_variants_atomically and renamed test_upsert_test_with_variants_rejects_lower_attempt_wholesale -> test_upsert_test_with_variants_rejects_lower_attempt_per_cell.
  • Added test_upsert_test_with_variants_always_returns_true, mirroring the build-side coverage.
  • Added test_upsert_test_merge_does_not_regress_completed_cell: the core push-race regression test.
  • Added 3 leaf-header identity tests (test_matrix_merge_leaf_header_keeps_newer_run_over_stale_loser, ..._advances_to_newer_run, ..._takes_higher_attempt_within_run) covering the monotonic run_id/run_attempt pick.

Test Result

All tests pass (the 3 pre-existing therock_workflow_registry_test.py env failures aside).

@cgoea
cgoea requested review from a team and HereThereBeDragons August 14, 2026 13:57
@cgoea
cgoea changed the base branch from main to develop August 14, 2026 14:29
…e_matrix_test_leaf. add test for it. small fix for code readability.

@HereThereBeDragons HereThereBeDragons left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

directly added code changes as review due to OOO

@HereThereBeDragons
HereThereBeDragons merged commit a4ea034 into develop Aug 17, 2026
3 checks passed
@HereThereBeDragons
HereThereBeDragons deleted the users/cgoea/merge_test_leaf_variants_per_cell branch August 17, 2026 16:41
quartz-sync-github-app Bot pushed a commit that referenced this pull request Aug 17, 2026
a4ea034, fix: merge test/test-full leaf variants per-cell instead of wholesale (#80), Ciprian Goea (ciprian.goea@amd.com), Mon Aug 17 19:41:41 2026 +0300
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants