diff options
| author | hathach <[email protected]> | 2026-08-21 12:41:47 +0700 |
|---|---|---|
| committer | hathach <[email protected]> | 2026-08-21 12:41:47 +0700 |
| commit | e13eff8d4e757ebe7709a58fce44017b8be5a84d (patch) | |
| tree | 32073e11437e057d92ff843290d3a5594c86d528 /test | |
| parent | f96ddbaa1e11a98f8076df48ba73026191c28399 (diff) | |
ci: fix nine ways the selection under-selected or mismatched
Every one of these dropped coverage silently - the worst failure mode here,
because the PR still goes green. Found by review, each reproduced first.
Selection rules:
* class_macros derived the config macro from the class DIRECTORY, so a change to
src/class/midi/midi2_device.c selected the midi_test examples (which do not
compile it) and never examples/device/midi2_device (the only one that enables
CFG_TUD_MIDI2, and the only one that does). The file's own macro is unioned in
where it differs - union, never replace: over-selecting costs a build,
under-selecting merges a break.
* the ${FAMILY_MCUS} fallback added for espressif fired on any family whose
_family_mcus came back empty, and _cmake_sets is if()-blind and keeps the FIRST
definition - so mcx/frdm_mcxn947 answered MCXA15, a token six examples' skip.txt
names, dropping 12 firmware images CMake builds. Limited now to families that
never spell set(FAMILY_MCUS ...) at all.
* lib_examples read only an example's top-level CMakeLists.txt/Makefile;
host/msc_file_explorer_freertos names lib/embedded-cli in src/CMakeLists.txt and
survived by luck. The whole example tree is scanned. (SEGGER_RTT and rt-thread
still resolve to nothing: all three references sit inside a LOGGER=rtt guard no
CI build sets - the documented ruling, not a miss.)
* get_family_boards applied ci_skip_boards/ci_preferred_boards only under
GITHUB_ACTIONS/CIRCLECI, so the selector answered differently on a laptop than
on a runner; _prune_buildable forces CI semantics. Its one-board pick also
abandoned the whole preferred list when entry one could not build the -e set,
and asked skip_example without the build's -D tokens.
* _config_enables and lib_examples still read with the locale encoding - under
LC_ALL=C the selector tracebacked on three tracked tusb_config.h files. The
whole selector and its suite run clean there now.
Workflows:
* the Membrowse Upload step omitted $EX_ARGS, but --one-first now picks the board
from the -e set, so it configured a different, empty build dir and uploaded
--identical for a board never compiled. It takes $EX_ARGS for the BOARD; the
target stays the aggregate, which has no DEPENDS and still records every example.
* blanking FAM_REGEX reset only build_filtered, leaving the build scoped while
code-metrics took the UNSCOPED branch and diffed a 1-family run against the full
averaged baseline. All three drop together now, as CircleCI's fall-open does.
* CircleCI's EX_ARGS had no character screen and is used unquoted, and its
code-metrics job still exit 1'd on an empty metrics set - which a scoped build
makes a legitimate outcome.
* a `ci-full` PR label now turns the scoping off for one PR. A selector bug
under-selects silently, and without a label the only ways back to a full matrix
are accidental.
Performance, since the selector gates every other job: family.cmake texts are read
once rather than per changed directory (a 6,000-file dep bump re-read 84 files
99,892 times) and _scrape_mcu is cached: 2.2s -> 0.29s there, 0.8s -> 0.33s on a
class diff.
Tests: a drift guard for hw/bsp families absent from ci_set_matrix.family_list
(they select zero legs now, where they used to ride the full matrix); the rule-4
port test asserted a SUBSET, which set() satisfies, so it could not fail on the
empty selection it exists to catch; the GITHUB_ENV guard test counted a SUM of two
guards. Drops metrics.py's --only-examples, which nothing called, and applies the
TOTAL scrub to the by-example branch that skipped it.
Diffstat (limited to 'test')
| -rw-r--r-- | test/hil/test/test_ci_metrics.py | 35 | ||||
| -rw-r--r-- | test/hil/test/test_ci_select.py | 24 |
2 files changed, 48 insertions, 11 deletions
diff --git a/test/hil/test/test_ci_metrics.py b/test/hil/test/test_ci_metrics.py index 89d03aaae..6c236e827 100644 --- a/test/hil/test/test_ci_metrics.py +++ b/test/hil/test/test_ci_metrics.py @@ -58,13 +58,16 @@ class TestByExample(unittest.TestCase): '-o', out, os.path.join(td, '*', '*', '*.map.json')], check=True) out2 = os.path.join(td, 'sub') r = subprocess.run([sys.executable, METRICS, 'combine', '-q', '-j', - '--only-examples', 'device/cdc_msc', '-o', out2, out + '_by_example.json'], capture_output=True, text=True) self.assertEqual(r.returncode, 0, r.stderr) sub = json.load(open(out2 + '.json')) names = {f['file'] for f in sub['files']} - self.assertEqual(names, {'usbd.c', 'cdc_device.c'}) # bare_api filtered out + # one data entry per example, not one blob: reading it as an ordinary + # metrics.json would double-count every file + self.assertIn('usbd.c', names) + self.assertIn('cdc_device.c', names) + self.assertNotIn('TOTAL', {n.upper() for n in names}) def test_by_example_expansion_is_keyed_on_the_filename(self): # the '_by_example.json' suffix IS the contract (write_by_example, the CMake @@ -339,9 +342,15 @@ class TestWorkflowSelectionHandOff(unittest.TestCase): # with secrets - and for run_*, flips which rig jobs execute for name in ('EX_ARGS', 'ARTIFACT_TAG'): self.assertIn(f'echo "{name}=', self.util) - self.assertEqual(self.util.count('case "$EX_ARGS" in') + - self.util.count('case "$TAG" in'), 2, - 'both GITHUB_ENV writes must screen their value first') + # per guard, not a sum: `count(a) + count(b) == 2` stays green when one guard is + # deleted and the other duplicated + for guard in ('case "$EX_ARGS" in', 'case "$TAG" in'): + self.assertEqual(self.util.count(guard), 1, + f'{guard}: each GITHUB_ENV write screens its value exactly once') + # CircleCI builds from the same PR-derived map and uses $EX_ARGS unquoted + cci = open(os.path.join(CIRCLECI, 'config2.yml')).read() + self.assertIn('case "$EX_ARGS" in', cci, + 'the CircleCI copy of the example filter needs the same screen') self.assertIn('case "$BUILD_ARGS" in', self.build) self.assertIn('unexpected characters in the " + key', self.build, 'the args_*/run_* emitter must screen each board filter') @@ -429,12 +438,16 @@ class TestWorkflowSelectionHandOff(unittest.TestCase): self.assertEqual(matrix.count('ci_set_matrix: UNSCOPED'), 2, 'every fall-open path must print the marker build.yml greps for') - def test_membrowse_upload_is_not_scoped(self): - # <TARGET>-membrowse-upload has no DEPENDS, so the aggregate rebuilds nothing - - # it records every example, --identical for the ones without an elf. Scoping it - # drops the excluded examples from the dataset instead of marking them unchanged. - upload = self.util[self.util.index('--target examples-membrowse-upload'):] - self.assertNotIn('$EX_ARGS', upload.split('\n')[0]) + def test_membrowse_upload_sees_the_same_board_as_the_build(self): + # $EX_ARGS is passed for the BOARD it selects: --one-first picks a board that can + # build the -e set, so without it membrowse configures a different, empty build + # dir and uploads --identical for a board that was never compiled. It does NOT + # scope the targets - `examples-membrowse-upload` is not `all`, so it passes + # through as the aggregate, which has no DEPENDS and still records every example. + line = [l for l in self.util.splitlines() + if '--target examples-membrowse-upload' in l][0] + self.assertIn('$EX_ARGS', line) + self.assertNotIn('-e ', line.replace('$EX_ARGS', '')) if __name__ == '__main__': diff --git a/test/hil/test/test_ci_select.py b/test/hil/test/test_ci_select.py index 74e5f48e6..031e8e287 100644 --- a/test/hil/test/test_ci_select.py +++ b/test/hil/test/test_ci_select.py @@ -777,6 +777,24 @@ class TestOrphanInvariant(unittest.TestCase): for v in vendors: self.assertTrue(ci_select.mcu_families(v + '/x.c', REPO), f'{v}: resolves to no family') + # hw/bsp families ci_set_matrix's family_list does not map to any toolchain. Before + # scoping these were harmless - the matrix was always every family in family_list, + # so a PR touching one of them still compiled the other 64. Now the selection + # intersects to nothing and every leg skips, so a family landing here by accident is + # a silent hole. espressif is deliberate: its boards are built by hil-build-esp, + # keyed on board name rather than family. + UNBUILT_FAMILIES = {'cxd56', 'efm32', 'espressif', 'f1c100s', 'pic32mz', 'py32f0', + 'same7x'} + + def test_every_bsp_family_is_in_the_ci_matrix(self): + sys.path.insert(0, os.path.join(REPO, '.github/scripts')) + import ci_set_matrix + fams = set(ci_select.all_bsp_families(REPO)) + self.assertEqual(fams - set(ci_set_matrix.family_list), self.UNBUILT_FAMILIES, + 'a hw/bsp family that no toolchain in ci_set_matrix.family_list ' + 'builds: a PR touching only it now selects zero build legs. Wire ' + 'it into family_list, or add it here with a reason.') + def test_every_get_deps_family_token_resolves_or_is_a_known_alias(self): """Same drift guard, dep side. A token naming no hw/bsp dir makes the entry unreachable for its family in get_deps.py itself (`f in entry[2].split()`), and @@ -1132,8 +1150,14 @@ class TestBuildClassifier(unittest.TestCase): # real feather_rp2040_max3421 board) and espressif's component CMakeLists also # references it — so the raw (unpruned) scan legitimately finds both; Task 4's # buildability post-filter is what may later prune either away + # non-empty FIRST: a subset assertion is satisfied by set(), and since ports are + # now empty-means-empty (fail-closed) an unnoticed regression to zero families + # would select no build leg at all and merge an uncompiled HCD + self.assertTrue(s['families'], 'a host-port change must select some family') self.assertLessEqual(set(s['families']), {'espressif', 'rp2040'}) + self.assertTrue(s['family_examples'], 'and must name the examples for them') for exs in s['family_examples'].values(): + self.assertTrue(exs) self.assertFalse(any(e.startswith(('device/', 'typec/')) for e in exs)) def test_port_shared_file_selects_all_examples(self): # rule 5 |
