Skip to content

CI: only run playbook tests whose behavior actually changed - #679

Open
lucbruni-amd wants to merge 3 commits into
mainfrom
ci-test-selection-pathcheck
Open

CI: only run playbook tests whose behavior actually changed#679
lucbruni-amd wants to merge 3 commits into
mainfrom
ci-test-selection-pathcheck

Conversation

@lucbruni-amd

Copy link
Copy Markdown
Collaborator

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.

  • Prose/typo edit -> 0 entries.
  • Edit a test scoped @os:linux + @device:stx -> only the (linux, stx) entry, not Windows or other devices.
  • Edit a shared @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.py gains a per-entry signature helper that reuses the real test extractor (so @require/@setup/@var/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 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 required selector-tests job): 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 under GITHUB_ACTIONS=true.

Known limitation

A composite action under .github/actions/ referenced via uses: ./ would be outside the harness check. None exist today; closable by widening the harness prefixes if one is ever added.

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.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR updates the playbook CI workflow to avoid re-running the full GPU test matrix on PRs when only non-executing content changes, by switching from path-based change detection to per-entry, content-based signatures derived from the real test extractor.

Changes:

  • Replace path-based “changed playbook” detection with a signature-based selector that chooses entries at (playbook, platform, device) granularity.
  • Add a dedicated selector-tests job that regression-tests the selection logic, and gate merges on selector correctness.
  • Introduce new selector/build scripts to materialize the base revision and diff per-entry execution signatures, with safety fallbacks to “run full matrix”.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File Description
.github/workflows/test-playbooks.yml Switches to signature-based matrix building; adds selector regression test job and tighter gating.
.github/scripts/build_test_matrix.py New matrix builder implementing harness-change detection and per-entry signature diffing.
.github/scripts/run_playbook_tests.py Extends extractor APIs and adds signature helpers used by the matrix builder.
.github/scripts/test_build_test_matrix.py New regression tests for selection behavior, including synthetic mutations and commit replay.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread .github/scripts/build_test_matrix.py
Comment thread .github/workflows/test-playbooks.yml
- build_test_matrix.py hard-fails when the matrix exceeds the GitHub Actions
  256-job cap, instead of failing later at workflow expansion with a vague error
- detect-changes now pins Python 3.13 (matching selector-tests) so the builder's
  tarfile filter="data" is always available
- regression test for the cap
@lucbruni-amd
lucbruni-amd marked this pull request as ready for review August 7, 2026 19:31
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.

4 participants