diff options
| author | hathach <[email protected]> | 2026-08-21 22:17:51 +0700 |
|---|---|---|
| committer | hathach <[email protected]> | 2026-08-21 22:17:51 +0700 |
| commit | 03a329eeda09e073d7de9be84df55d65392e4013 (patch) | |
| tree | b59db5c7ecb4dc5c9f1e87d35634c9fc58a3d462 | |
| parent | fd70160a2f5fd23de1abfbaefb6399746a90b588 (diff) | |
ci_select: classify the 254 files that were reaching rule 17
Rule 17 (unclassified -> full on both axes) is the fail-open net for paths nobody
anticipated, and it must stay that way: a wrong `full` costs runner minutes and is
visible in the run, a wrong `empty` costs a merged regression and is invisible.
But nothing in the tree should REACH it, and 254 tracked files did.
The cost was real. PR #3842 changed a skill, a README and .gitignore; .gitignore
matched no rule, so both axes went full and 74 cmake legs span up runners to do
checkout + toolchain + get_deps before skipping the build, plus the whole 30-board
rig. Three changes, none of which touch rule 17 itself:
1. _META_RE - repo metadata and tooling no Build step reads: .gitignore,
.gitattributes, .clang-format, .codespellrc, .pre-commit-config.yaml,
.readthedocs.yaml, .PVS-Studio/, .idea/, sonar-project.properties, the
packaging manifests, CMakePresets, udev rules, test/{fuzz,unit-test} (their own
jobs build those), the non-build .github/ files, and the tools/*.py scripts no
build invokes. Deliberately NOT included, and still full: .circleci/**,
.github/workflows/build*.yml, .github/actions/**, .github/scripts/**. The line
is "does a Build step read this", not "is it source".
2. Rules 15 and 16 now match what they already claimed. Row 15 names
examples/<role>/CMakeLists.txt and the regex never had it; row 16 says
tools/build*.py but anchored tools/build\.py$. Both got the right answer only
because rule 17 caught them on the way past. Also names their siblings -
family_support.mk, family_rules.mk, src/CMakeLists.txt, src/tinyusb.mk - and
.circleci/**, which generates the whole CircleCI matrix and was in no row at all.
3. src/typec/** gets row 12b. It is listed unconditionally by both build systems
but its body is `#if CFG_TUC_ENABLED`, which only examples/typec/power_delivery
sets - the same shape as the class rule, so the same answer: the examples that
enable it (stm32g4 and stm32u5 after the buildability prune), and nothing on the
rig, which runs no typec test. It was force-fulling 82 families and all 30 boards.
TestNoTrackedFileIsUnclassified walks every tracked file and asserts none reaches
rule 17, on both axes - 254 -> 0. Verified it fails when a new unclassified path
appears. That turns 17 into what it should be: unreachable for anything in the
tree, so it fires only for genuinely new shapes, and the author is told to write
the row rather than letting the fall-through pick an answer for them.
test_full_paths used sonar-project.properties as its stand-in for "unclassified";
that is now metadata, so the case moved to the new
test_repo_metadata_is_not_a_build_input, with test_the_build_machinery_is_still_full
pinning the other side of the line.
| -rw-r--r-- | docs/superpowers/specs/2026-08-19-ci-build-family-filter-design.md | 48 | ||||
| -rw-r--r-- | test/hil/test/test_ci_select.py | 62 | ||||
| -rwxr-xr-x | tools/ci_select.py | 79 |
3 files changed, 160 insertions, 29 deletions
diff --git a/docs/superpowers/specs/2026-08-19-ci-build-family-filter-design.md b/docs/superpowers/specs/2026-08-19-ci-build-family-filter-design.md index 8f77dc50a..b10f5b4ae 100644 --- a/docs/superpowers/specs/2026-08-19-ci-build-family-filter-design.md +++ b/docs/superpowers/specs/2026-08-19-ci-build-family-filter-design.md @@ -46,29 +46,31 @@ never inflates one axis with another's breadth. `FAM` = the families whose `family.cmake` references the changed path (CMake only — see below). "roster boards" = boards on `test/hil/{tinyusb,hfp}.json`. -| # | Changed path | Build families | Build examples | HIL boards → tests | -| --- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | --------------------------------------------------------------- | ----------------------------------------------------------------- | ---------------------------------------------------------------------------------------------- | -| 1 | `docs/`, `.claude/`, `*.md`, `*.rst`, `LICENSE` | — | — | — | -| 2 | `test/hil/**` | — | — | all boards → all tests | -| 2b | `tools/metrics.py`, `.github/scripts/metrics_*.py` | `ALL` (unchanged — `tinyusb_metrics` runs `metrics.py` as a build target) | `ALL` | — (nothing on the rig runs it) | -| 3 | `src/portable/<port>/dcd_*`, `*_device.[ch]` | `FAM` | `DEV`+`DUAL` | `FAM`'s device-role boards → device+dual tests | -| 4 | `src/portable/<port>/hcd_*`, `*_host.[ch]` | `FAM` | `HOST`+`DUAL` | `FAM`'s host-role boards → host+dual tests | -| 5 | `src/portable/<port>/**` (anything else) | `FAM` | `ALL` | `FAM`'s boards → all their tests | -| 5b | `src/portable/<port>/**` where `FAM` is empty | — | — | — (empty resolves to nothing on BOTH axes) | -| 6 | `hw/bsp/<family>/**` | that family | `ALL` | that family's boards → all tests (a `boards/<board>/` path narrows to that board) | -| 7 | `hw/mcu/<vendor>/**` | `FAM` — empty resolves to nothing (maintainer ruling) | `ALL` | `FAM`'s boards → all tests; empty resolves to nothing (maintainer ruling) ⚠ *see below* | -| 8 | `src/class/<cls>/*_device.[ch]` | `ALL` | examples enabling `CFG_TUD_<CLS>` | device-role boards → HIL tests enabling `CFG_TUD_<CLS>` | -| 9 | `src/class/<cls>/*_host.[ch]` | `ALL` | examples enabling `CFG_TUH_<CLS>` | host-role boards → HIL tests enabling `CFG_TUH_<CLS>` | -| 10 | `src/class/<cls>/**` (shared header) | `ALL` | either, **plus include-edge classes** | both roles → same, plus include-edge classes | -| 11 | `src/device/**` | `ALL` | `DEV`+`DUAL` | device-role boards → device+dual tests | -| 12 | `src/host/**` | `ALL` | `HOST`+`DUAL` | host-role boards → host+dual tests | -| 13 | `examples/<role>/<name>/**` | `ALL` | just `<name>` | if `<name>` is a HIL test: all boards → that test; else nothing | -| 14 | `examples/device/board_test/**` | `ALL` | just `board_test` | all boards → all tests (HIL parking firmware) | -| 15 | `examples/build_system/**`, `examples/CMakeLists.txt`, `examples/<role>/CMakeLists.txt` | `ALL` | `ALL` | all boards → all tests | -| 16 | `src/common/`, `src/osal/`, `src/tusb.[ch]`, `src/tusb_option.h`, `tools/build*.py`, `tools/cmake/**`, `hw/bsp/{family_support.cmake,board.c,board_api.h,ansi_escape.h}`, `.github/**` | `ALL` | `ALL` | all boards → all tests | -| 16a | `lib/<name>/**` | `ALL` | examples whose own `CMakeLists.txt`/`Makefile` names `lib/<name>` | those examples that are HIL tests, on all boards; empty resolves to nothing | -| 16b | `tools/get_deps.py` | families whose `deps_mandatory`/`deps_optional` entries changed | `ALL` | those families' boards → all tests; a logic change, an `'all'` entry, no base content or a changed token naming no family → full | -| 17 | anything unclassified | `ALL` | `ALL` | all boards → all tests (fail-open) | +| # | Changed path | Build families | Build examples | HIL boards → tests | +| --- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------------------------------------------------------------------------- | ----------------------------------------------------------------- | -------------------------------------------------------------------------------------------------------------------------------- | +| 1 | `docs/`, `.claude/`, `*.md`, `*.rst`, `LICENSE` | — | — | — | +| 1b | `.gitignore`, `.clang-format`, `.idea/**`, `test/{fuzz,unit-test}/**`, non-build `.github/**`, packaging manifests | — | — | — | +| 2 | `test/hil/**` | — | — | all boards → all tests | +| 2b | `tools/metrics.py`, `.github/scripts/metrics_*.py` | `ALL` (unchanged — `tinyusb_metrics` runs `metrics.py` as a build target) | `ALL` | — (nothing on the rig runs it) | +| 3 | `src/portable/<port>/dcd_*`, `*_device.[ch]` | `FAM` | `DEV`+`DUAL` | `FAM`'s device-role boards → device+dual tests | +| 4 | `src/portable/<port>/hcd_*`, `*_host.[ch]` | `FAM` | `HOST`+`DUAL` | `FAM`'s host-role boards → host+dual tests | +| 5 | `src/portable/<port>/**` (anything else) | `FAM` | `ALL` | `FAM`'s boards → all their tests | +| 5b | `src/portable/<port>/**` where `FAM` is empty | — | — | — (empty resolves to nothing on BOTH axes) | +| 6 | `hw/bsp/<family>/**` | that family | `ALL` | that family's boards → all tests (a `boards/<board>/` path narrows to that board) | +| 7 | `hw/mcu/<vendor>/**` | `FAM` — empty resolves to nothing (maintainer ruling) | `ALL` | `FAM`'s boards → all tests; empty resolves to nothing (maintainer ruling) ⚠ *see below* | +| 8 | `src/class/<cls>/*_device.[ch]` | `ALL` | examples enabling `CFG_TUD_<CLS>` | device-role boards → HIL tests enabling `CFG_TUD_<CLS>` | +| 9 | `src/class/<cls>/*_host.[ch]` | `ALL` | examples enabling `CFG_TUH_<CLS>` | host-role boards → HIL tests enabling `CFG_TUH_<CLS>` | +| 10 | `src/class/<cls>/**` (shared header) | `ALL` | either, **plus include-edge classes** | both roles → same, plus include-edge classes | +| 11 | `src/device/**` | `ALL` | `DEV`+`DUAL` | device-role boards → device+dual tests | +| 12 | `src/host/**` | `ALL` | `HOST`+`DUAL` | host-role boards → host+dual tests | +| 12b | `src/typec/**` | `ALL` | examples enabling `CFG_TUC_ENABLED` | — (no rig board runs a typec test) | +| 13 | `examples/<role>/<name>/**` | `ALL` | just `<name>` | if `<name>` is a HIL test: all boards → that test; else nothing | +| 14 | `examples/device/board_test/**` | `ALL` | just `board_test` | all boards → all tests (HIL parking firmware) | +| 15 | `examples/build_system/**`, `examples/CMakeLists.txt`, `examples/<role>/CMakeLists.txt` | `ALL` | `ALL` | all boards → all tests | +| 16 | `src/common/`, `src/osal/`, `src/tusb.[ch]`, `src/tusb_option.h`, `tools/{build,build_utils,ci_select}.py`, `tools/cmake/**`, `src/CMakeLists.txt`, `src/tinyusb.mk`, `hw/bsp/{family_support.{cmake,mk},family_rules.mk,zephyr_board_aliases.cmake,board.c,board_api.h,ansi_escape.h}`, `.github/**`, `.circleci/**` | `ALL` | `ALL` | all boards → all tests | +| 16a | `lib/<name>/**` | `ALL` | examples whose own `CMakeLists.txt`/`Makefile` names `lib/<name>` | those examples that are HIL tests, on all boards; empty resolves to nothing | +| 16b | `tools/get_deps.py` | families whose `deps_mandatory`/`deps_optional` entries changed | `ALL` | those families' boards → all tests; a logic change, an `'all'` entry, no base content or a changed token naming no family → full | +| 17 | anything unclassified (no tracked file reaches this — TestNoTrackedFileIsUnclassified) | `ALL` | `ALL` | all boards → all tests (fail-open) | **Rule 2 is deliberately asymmetric.** A `test/hil/**` change is invisible to the family matrix but is exactly what the rig exercises, so it builds nothing and runs everything. diff --git a/test/hil/test/test_ci_select.py b/test/hil/test/test_ci_select.py index 8f1841531..66b20b2e4 100644 --- a/test/hil/test/test_ci_select.py +++ b/test/hil/test/test_ci_select.py @@ -840,6 +840,45 @@ class TestRostersDoNotOverlap(unittest.TestCase): seen[b['name']] = b.get('tests') +class TestNoTrackedFileIsUnclassified(unittest.TestCase): + """Rule 17 (unclassified -> full on both axes) is the fail-open net for paths nobody + anticipated. It must stay that way - a wrong `full` costs runner minutes and is + visible in the run, a wrong `empty` costs a merged regression and is invisible - but + nothing in the tree should REACH it. Every tracked file is classified by a rule, so + 17 fires only for genuinely new shapes, and this test is what tells the author to + write the row instead of letting the fall-through pick an answer for them. + + Before this guard, 254 tracked files reached 17: .gitignore took a docs-only PR to + 74 cmake legs and the whole rig, while examples/<role>/CMakeLists.txt got the RIGHT + answer from the wrong rule - row 15 names it, the regex never matched it.""" + + def _unclassified(self, axis): + import subprocess as sp + r = sp.run(['git', 'ls-files'], cwd=REPO, capture_output=True, text=True) + if r.returncode != 0: + self.skipTest('not a git checkout') + files = r.stdout.split() + self.assertGreater(len(files), 1000, 'suspiciously few tracked files') + out = [] + for f in files: + s = (ci_select.classify_build([f], REPO) if axis == 'build' + else ci_select.classify([f], REPO, real_rosters())) + if any('unclassified' in why for why in s['reasons']): + out.append(f) + return out + + def test_build_axis(self): + left = self._unclassified('build') + self.assertEqual(left, [], f'{len(left)} tracked files fall through to rule 17 on ' + f'the build axis, e.g. {left[:5]} - classify them, or ' + f'add the pattern to _META_RE if no build reads them') + + def test_hil_axis(self): + left = self._unclassified('hil') + self.assertEqual(left, [], f'{len(left)} tracked files fall through to rule 17 on ' + f'the HIL axis, e.g. {left[:5]}') + + class TestLibRule(unittest.TestCase): """lib/** is not a full-matrix path: only the examples that build the lib need it.""" @@ -1241,7 +1280,28 @@ class TestBuildClassifier(unittest.TestCase): 'tools/build.py', 'tools/cmake/cpu/cortex-m4.cmake', 'examples/CMakeLists.txt', 'examples/device/CMakeLists.txt', 'examples/build_system/cmake/cpu.cmake', '.github/workflows/build.yml', - 'sonar-project.properties', 'some/unknown/path.c'): + '.circleci/config.yml', 'src/CMakeLists.txt', 'src/tinyusb.mk', + 'hw/bsp/family_support.mk', 'tools/build_utils.py', + 'some/unknown/path.c'): + self.assertTrue(self.b([p])['full'], p) + + def test_repo_metadata_is_not_a_build_input(self): + # these used to reach `full` through rule 17: a PR touching only .gitignore and a + # README created 74 cmake legs and booked the whole rig. No Build step reads them. + for p in ('sonar-project.properties', '.gitignore', '.gitattributes', + '.clang-format', '.idea/misc.xml', 'version.yml', 'library.json', + 'examples/CMakePresets.json', 'test/fuzz/fuzz.cc', + 'test/unit-test/project.yml', '.github/workflows/pr_comment.yml', + 'tools/gen_doc.py'): + s = self.b([p]) + self.assertFalse(s['full'], p) + self.assertEqual(s['families'], [], p) + + def test_the_build_machinery_is_still_full(self): + # the other side of the same line: these DECIDE what gets built + for p in ('.circleci/config.yml', '.github/workflows/build.yml', + '.github/scripts/ci_set_matrix.py', 'tools/ci_select.py', + 'tools/build_utils.py', 'tools/metrics.py'): self.assertTrue(self.b([p])['full'], p) def test_mixed_diff_unions_per_family(self): diff --git a/tools/ci_select.py b/tools/ci_select.py index ced3bbbc0..cf5a0da5b 100755 --- a/tools/ci_select.py +++ b/tools/ci_select.py @@ -54,6 +54,34 @@ def _read(path: str) -> str: _NONCODE_RE = re.compile( r'^(docs/|\.claude/|.*\.(md|rst)$|LICENSE)') +# Repo metadata and tooling that no CI build reads. Enumerated rather than left to +# rule 17, which widens BOTH axes: a PR touching only .gitignore and a README was +# creating 74 cmake legs (each a runner doing checkout + toolchain + get_deps before +# skipping the build) and booking the whole 30-board rig. +# +# Deliberately NOT here, and still full: .circleci/**, .github/workflows/build*.yml, +# .github/actions/**, .github/scripts/** - those decide what gets built. The line is +# "does any Build step read this file", not "is it source". +# +# test/{fuzz,unit-test} have their own jobs (cifuzz.yml, the unit-test pre-commit hook +# and workflow); the Build matrix never compiles them, and test/hil is rule 2. +_META_RE = re.compile( + r'^(' + r'\.(gitignore|gitattributes|clang-format|codespellrc|readthedocs\.yaml)$|' + r'\.pre-commit-config\.yaml$|\.PVS-Studio/|\.idea/|\.vscode/|' + r'sonar-project\.properties$|library\.json$|pkg\.yml$|repository\.yml$|' + r'version\.yml$|SConscript$|' + r'.*CMakePresets\.json$|hw/bsp/BoardPresets\.json$|examples/west\.yml$|' + r'.*/[0-9]+-tinyusb[^/]*\.rules$|tools/usb_drivers/|tools/codespell/|' + r'test/(fuzz|unit-test)/|' + # .github, minus the build machinery named in _FULL_RE + r'\.github/(FUNDING\.yml|labeler\.yml|membrowse_pr_message\.j2|ISSUE_TEMPLATE/|' + r'workflows/(cifuzz|claude|claude-code-review|labeler|membrowse-comment|' + r'membrowse-onboard|pr_comment|pre-commit|static_analysis|trigger)\.yml$)|' + # tools/ scripts no build invokes (tools/build*.py and metrics are handled above) + r'tools/(build_doc|check_example_pids|file2carray|gen_doc|gen_presets|iar_gen|' + r'make_release|mksunxi|pcapng_to_corpus)\.py$|tools/iar_template\.ipcf$' + r')') # Build-size metrics tooling. HIL axis ONLY: nothing on the rig runs any of it, and # without this rule these paths are unclassified, so a metrics-only PR booked an # exclusive full 30-board sweep to validate a script no board executes. @@ -66,9 +94,20 @@ _METRICS_RE = re.compile( _FULL_RE = re.compile( r'^(src/common/|src/osal/|src/tusb\.c$|src/tusb\.h$|src/tusb_option\.h$|' r'test/hil/|\.github/workflows/build.*\.yml$|\.github/actions/|\.github/scripts/|' - r'tools/build\.py$|tools/cmake/|' - r'hw/bsp/(family_support\.cmake|board_api\.h|board\.c|ansi_escape\.h)$|' + # generates the whole CircleCI matrix, same authority as .github/** + r'\.circleci/|' + # rule 16 says `tools/build*.py`; name the two siblings the glob implies. Both + # decide what gets built, so neither can be trusted to narrow its own change. + r'tools/(build|build_utils|ci_select)\.py$|tools/cmake/|' + # the make twins of family_support.cmake are the same authority for the make legs + r'hw/bsp/(family_support\.(cmake|mk)|family_rules\.mk|zephyr_board_aliases\.cmake|' + r'board_api\.h|board\.c|ansi_escape\.h)$|' + # rule 15 lists examples/<role>/CMakeLists.txt - it registers every target in that + # role, so it was only ever reaching `full` through rule 17's fall-through r'examples/build_system/|examples/CMakeLists\.txt$|' + r'examples/[^/]+/CMakeLists\.txt$|' + # every firmware compiles these unconditionally (src/CMakeLists.txt, src/tinyusb.mk) + r'src/CMakeLists\.txt$|src/tinyusb\.mk$|' # board_test is HIL infrastructure, not a test: hil_test.py flashes it to park # every board (variant boundary + end-of-board teardown), so every board depends on it r'examples/device/board_test/)') @@ -538,7 +577,7 @@ class _Sel: def _classify_one(path, repo_root, roster_boards, extras: set, s: _Sel, get_deps_families=None): base = os.path.basename(path) - if _NONCODE_RE.match(path): + if _NONCODE_RE.match(path) or _META_RE.match(path): s.reasons.append(f'{path}: non-code, no contribution') return if _METRICS_RE.match(path): @@ -673,6 +712,11 @@ def _classify_one(path, repo_root, roster_boards, extras: set, s: _Sel, s.add(boards, sorted(tests), f'{path}: lib {lib} -> {sorted(tests)} on all boards') return + if re.match(r'src/typec/', path): + # only examples/typec enables CFG_TUC_ENABLED, and no rig board runs a typec + # test (see _HIL_EX_ROLES) - so the build axis covers it and the rig cannot + s.reasons.append(f'{path}: typec, no HIL contribution') + return m = _BUILD_EX_RE.match(path) if m: if m.group(1) not in _HIL_EX_ROLES: @@ -935,7 +979,8 @@ class _BSel: def _classify_build_one(path, repo_root, s: _BSel, get_deps_families=None): base = os.path.basename(path) - if _NONCODE_RE.match(path): # rule 1 + if _NONCODE_RE.match(path) or _META_RE.match(path): # rule 1 + s.reasons.append(f'{path}: non-code, no build contribution') return if re.match(r'test/hil/', path): # rule 2 s.reasons.append(f'{path}: HIL harness, no build contribution') @@ -1006,6 +1051,18 @@ def _classify_build_one(path, repo_root, s: _BSel, get_deps_families=None): # CMakeLists (rule 15) is what forces the full matrix s.reasons.append(f'{path}: not an example dir, no build contribution') return + if re.match(r'src/typec/', path): # rule 12b + # listed unconditionally by src/CMakeLists.txt and src/tinyusb.mk, but the whole + # body is `#if CFG_TUC_ENABLED` - so it is PARSED by every build and COMPILED + # only for examples that enable it. Same shape as the class rule, same answer: + # the examples whose tusb_config.h turns it on, and empty means empty. + exs = examples_enabling(role_examples(repo_root, ('typec',)), + ('CFG_TUC_ENABLED',), repo_root) + if not exs: + s.reasons.append(f'{path}: typec enabled by no example config, no contribution') + return + s.add(all_bsp_families(repo_root), exs, f'{path}: typec -> {sorted(exs)}') + return m = re.match(r'lib/([^/]+)/', path) if m: # lib rule lib = m.group(1) @@ -1018,7 +1075,19 @@ def _classify_build_one(path, repo_root, s: _BSel, get_deps_families=None): return s.add(all_bsp_families(repo_root), exs, f'{path}: lib {lib} -> {sorted(exs)}') return - s.force_full(f'{path}: unclassified -> full build matrix') # rules 15-17 + if _METRICS_RE.match(path): + # HIL-suppressed above; on this axis they stay full - tools/metrics.py runs as + # the `tinyusb_metrics` build target, so a break in it fails the build + s.force_full(f'{path}: metrics tooling runs in the build -> full build matrix') + return + if _FULL_RE.match(path): # rules 15-16 + # attribution, not behaviour: these already reached `full` through the + # fall-through below. Naming them means a future narrowing of rule 17 cannot + # silently change what they do. Deliberately last, so every earlier rule keeps + # priority - examples/device/board_test is rule 14 (just board_test), not ALL. + s.force_full(f'{path}: core/infra -> full build matrix') + return + s.force_full(f'{path}: unclassified -> full build matrix') # rule 17 @contextlib.contextmanager |
