fix: CI never ran tests for 3 of 4 packages, or even triggered on them - #33
Conversation
Multiple independent Opus peer reviews of PRs opened this session flagged
the same underlying gap repeatedly: none of the new regression tests
added to tinyml-tinyverse, tinyml-modelzoo, or
tinyml-modeloptimization/torchmodelopt actually run in CI.
Two separate problems compound this:
1. .github/workflows/test-modelmaker.yml's push/pull_request path filters
only watched 'tinyml-modelmaker/**' -- a PR touching only
tinyml-tinyverse, tinyml-modelzoo, or tinyml-modeloptimization (which is
most of this session's PRs) never triggered the workflow to run at all,
regardless of what test steps existed.
2. Even when the workflow does run, it only ever executes tests under
tinyml-modelmaker/tests/ (four specific files via named Tier 1/2/3
steps). tinyml-tinyverse, tinyml-modelzoo, and torchmodelopt have no
test-running step whatsoever -- any regression test added there is
silently never checked on push or PR.
Fixed by:
- Restoring the tinyml-tinyverse/**, tinyml-modelzoo/**, and
tinyml-modeloptimization/** path filters alongside tinyml-modelmaker/**,
so the workflow actually triggers for PRs touching those packages.
- Adding a test step for each of the three previously-uncovered packages.
Each is guarded by a find-based existence check (not a plain `test -d
tests`) because none of these tests/ directories exist yet on
upstream/main -- an empty or missing directory makes pytest exit
non-zero ("no tests collected"), which would otherwise fail CI on this
package until the first PR adding tests to it merges. The guard lets
this land safely now and start actually running tests the moment any
PR (this session's or otherwise) adds them.
Deliberately NOT adding a broader "run everything under
tinyml-modelmaker/tests/" sweep beyond the existing four named Tier
files, despite tinyml-modelmaker itself having several test files (e.g.
test_constants.py, test_dataset_utils.py, test_nas_support.py) not
wired into any Tier and therefore also never run: a full sweep surfaces
17 pre-existing failures unrelated to anything in this session (verified
even the current, unmodified Tier 1 step alone already fails 2/447 tests
locally), which would immediately turn this workflow red on merge. That
gap is real but is its own, separate, pre-existing issue -- out of scope
for a CI-triggering/coverage fix that should land safely.
Verified locally: the find-based guard correctly skips tinyml-tinyverse,
tinyml-modelzoo, and torchmodelopt on this branch (none of their tests/
directories have any content yet on upstream/main) and would correctly
run pytest once a PR adds real test files there.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Independent Fable peer review of this PR caught an in-scope gap: the pull_request path filters omitted '.github/workflows/test-modelmaker.yml' itself (the push filters have had it all along -- an inherited asymmetry, not one this PR introduced). Consequence: a PR that only changes this workflow -- including this very PR -- never triggers the CI it modifies, so workflow changes merge unvalidated. Given this PR's whole purpose is "CI doesn't trigger when it should," that omission belongs in scope. One added path glob, mirroring the push filter. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
An independent peer review of this PR caught an in-scope gap. Pushed a follow-up commit (9775450): The The same review also verified the load-bearing claims: for |
Both CodeRabbit and Fable reviews flagged that the find-based guard only matched test_*.py, so a package whose only tests use pytest's other default discovery pattern (foo_test.py) would be silently skipped in CI. Extended all three guards to match both patterns; verified the guard runs for a lone foo_test.py and still skips an empty tests/ directory. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Problem
Multiple independent peer reviews of PRs opened this session flagged the same gap repeatedly: none of the new regression tests added to
tinyml-tinyverse,tinyml-modelzoo, ortinyml-modeloptimization/torchmodeloptactually run in CI. Two compounding problems:.github/workflows/test-modelmaker.yml'spush/pull_requestpath filters only watchedtinyml-modelmaker/**— a PR touching onlytinyml-tinyverse,tinyml-modelzoo, ortinyml-modeloptimization(which is most of this session's PRs) never triggered the workflow at all.tinyml-modelmaker/tests/(four specific files via named Tier 1/2/3 steps). The other three packages have no test-running step whatsoever.Fix
tinyml-tinyverse/**,tinyml-modelzoo/**, andtinyml-modeloptimization/**path filters alongsidetinyml-modelmaker/**, so the workflow actually triggers for PRs touching those packages.find-based existence check (not a plaintest -d tests) since none of thesetests/directories exist yet onmain— an empty/missing directory makes pytest exit non-zero ("no tests collected"), which would otherwise fail CI on this package until the first PR adding tests to it merges. The guard lets this land safely now and starts actually running tests the moment any PR adds them.Deliberately out of scope
I considered also adding a broader "run everything under
tinyml-modelmaker/tests/" sweep, since several existing test files there (test_constants.py,test_dataset_utils.py,test_nas_support.py, etc.) aren't wired into any of the three named Tiers and are therefore also never run. I dropped that: it surfaces 17 pre-existing failures unrelated to anything in this session (confirmed even the current, unmodified Tier 1 step alone already fails 2/447 tests locally), which would immediately turn this workflow red on merge. That's a real, separate gap, but out of scope for a fix that should land safely without breaking CI.Testing
Verified locally: the
find-based guard correctly skipstinyml-tinyverse,tinyml-modelzoo, andtorchmodelopton this branch (no test content exists yet onmain) and would correctly run pytest once a PR adds real test files there. YAML syntax validated.🤖 Generated with Claude Code