CI: only run playbook tests whose behavior actually changed - #679
Draft
lucbruni-amd wants to merge 1 commit into
Draft
CI: only run playbook tests whose behavior actually changed#679lucbruni-amd wants to merge 1 commit into
lucbruni-amd wants to merge 1 commit into
Conversation
Path-based detection re-ran a playbook's whole GPU matrix for any file change, including prose-only README edits. This replaces it with content-based per-entry (playbook, platform, device) selection. run_playbook_tests.py gains a per-entry signature helper that reuses the real test extractor, so @require inlining, @setup/@var resolution and os/device filtering are all reflected. build_test_matrix.py materialises the base revision with git archive and diffs signatures computed by the head extractor against both trees. A byte-wise harness path check (deny-by-default over .github/scripts and test-playbooks.yml) forces the full matrix on any selector/runner/workflow change, which is what makes applying one extractor to both trees safe. Every failure to establish a trustworthy base falls back to the full matrix, and the nightly cron still runs everything. A new selector-tests job runs 22 regression tests (synthetic mutations plus replay of real historical commits via an independent diff-derived classifier), and the test gate now fails if detection did not succeed.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
A PR that only edits prose in a playbook README (a typo, a reworded sentence) currently re-runs that playbook's entire GPU test matrix, because detection is path-based: any file under
playbooks/<cat>/<id>/changing triggers all of that playbook's entries. These runs are slow and sometimes flaky.What this does
Replaces path-based detection with content-based selection at per-entry
(playbook, platform, device)granularity. On a PR, an entry runs only if what it would actually execute changed.@os:linux+@device:stx-> only the(linux, stx)entry, not Windows or other devices.@required dependency -> exactly its consumer playbooks.The nightly cron still runs the full 194-entry matrix, so anything wrongly skipped on a PR surfaces within 24h.
How
run_playbook_tests.pygains a per-entry signature helper that reuses the real test extractor (so@require/@setup/@var/device filtering are all reflected).build_test_matrix.pymaterialises the base revision withgit archiveand diffs signatures computed by the head extractor against both trees. A byte-wise harness path check (deny-by-default over.github/scripts/andtest-playbooks.yml) forces the full matrix on any change to the selector/runner/workflow, which is what makes applying one extractor to both trees safe. Every failure to establish a trustworthy base falls back to the full matrix.Safety
The dangerous direction is skipping an entry that changed. Guards: null signature -> run; new/vanished entry -> run; harness changed -> full matrix; unresolvable base -> full matrix; empty matrix or >256 jobs -> hard fail; nightly backstop.
Tests
test_build_test_matrix.py(22 tests, run as a requiredselector-testsjob): synthetic mutations for each selection rule, plus replay of 9 real historical commits judged by an independent diff-derived classifier (e.g. #571 "add instructions to clarify usage" -> 0 selected; a real comfyui test edit -> 8/194). Verified green underGITHUB_ACTIONS=true.Known limitation
A composite action under
.github/actions/referenced viauses: ./would be outside the harness check. None exist today; closable by widening the harness prefixes if one is ever added.