ci(lint): detect TAP groups that no workflow can select - #5992
Conversation
lint_group_coverage.py already answered "does this group have a CI
workflow", but two narrowing assumptions meant it could not see a whole
class of gap:
1. classify_group() only considered infra-backed groups:
if concrete and not missing and not workflow_exists(group):
A group whose infras.lst is empty or absent was exempt by construction.
But needing no backend does not mean needing no workflow -- such a group
still needs one to ever run in CI.
2. workflow_exists() only looked for a .github/workflows/CI-<group>.yml
filename on this branch. That misses groups selected by an env or matrix
entry inside a larger workflow (e.g. 'TAP_GROUP: mysqlx-tsan-g1'), and it
cannot see the reusable half of a workflow pair, which lives on
GH-Actions.
Together these hid no-infra-g1: created in ffb610f (2026-03-19), it has an
empty infras.lst and no wiring on either branch, so its 5 tests have never
run in CI -- and the linter meant to catch exactly this stayed silent.
A group is now considered wired if any of: a CI-<group>.yml caller exists;
its name appears in a workflow on this branch; its name appears in a workflow
on origin/GH-Actions; or it belongs to a dynamically discovered family.
cluster_sim_* is the one such family today -- CI-cluster-simulator.yml builds
its matrix from `cluster-simulator-ci.bash discover`, which selects every
groups.json entry with that prefix, so those groups are wired without any
static reference.
When origin/GH-Actions is unavailable (shallow clone, no remote), the
workflow check is skipped with an explicit NOTE rather than reporting gaps it
cannot verify. CI-lint-groups-json.yml now fetches that ref at --depth=1,
since actions/checkout fetches a single branch and the check would otherwise
skip itself silently.
Widening the check surfaced 21 further pre-existing unwired groups. They are
allowlisted as tracked debt so the linter reports only genuinely NEW gaps.
no-infra-g1 is deliberately NOT allowlisted: it is real and actionable, and
is now the single NEW finding.
Still warn-only (exit 0); --strict exits 1 for a future enforcement step.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📜 Recent review details🧰 Additional context used🪛 ast-grep (0.45.0)test/tap/groups/lint_group_coverage.py[warning] 135-135: File path is request-/variable-derived; validate and normalize to prevent path traversal. (open-filename-from-request) [error] 151-154: Command coming from incoming request (subprocess-from-request) [error] 162-165: Command coming from incoming request (subprocess-from-request) 🪛 Ruff (0.16.1)test/tap/groups/lint_group_coverage.py[error] 153-153: Starting a process with a partial executable path (S607) [error] 163-163: (S603) [error] 164-164: Starting a process with a partial executable path (S607) 🔇 Additional comments (4)
📝 WalkthroughWalkthroughThe CI workflow fetches ChangesWorkflow coverage validation
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant CIWorkflow
participant GitOriginGHActions
participant lint_coverage
participant classify_group
CIWorkflow->>GitOriginGHActions: shallow fetch GH-Actions
lint_coverage->>GitOriginGHActions: inspect workflow files
lint_coverage->>classify_group: pass local and remote workflow data
classify_group-->>lint_coverage: return coverage diagnostics
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## v3.0 #5992 +/- ##
==========================================
- Coverage 52.42% 52.42% -0.01%
==========================================
Files 472 472
Lines 143105 143105
Branches 36164 36164
==========================================
- Hits 75022 75020 -2
- Misses 51237 51254 +17
+ Partials 16846 16831 -15
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
1 participant
Adds the check you asked for — except a linter for this already existed, so this widens it rather than adding a second one.
Why it didn't catch anything
test/tap/groups/lint_group_coverage.pyalready ran inCI-lint-groups-jsonand already asked "does this group have a workflow". Two narrowing assumptions made it blind to a whole class of gap:1. Infra-less groups were exempt by construction.
A group with an empty or absent
infras.lstcould never be flagged. But needing no backend doesn't mean needing no workflow.2. Only the filename
CI-<group>.ymlon this branch counted.That misses groups selected by an env/matrix entry inside a larger workflow (
TAP_GROUP: mysqlx-tsan-g1), and it cannot see the reusable half of a pair, which lives onGH-Actions.Together these hid
no-infra-g1— created inffb610f4b(2026-03-19), emptyinfras.lst, no wiring on either branch. Its 5 tests have never run in CI, and the linter meant to catch exactly this stayed quiet for five months.What changed
A group is wired if any of:
.github/workflows/CI-<group>.ymlexistsorigin/GH-Actions(4) matters and was easy to get wrong:
CI-cluster-simulator.ymlbuilds its matrix fromcluster-simulator-ci.bash discover, which selects everygroups.jsonentry starting withcluster_sim_. Those 6 groups have no static reference anywhere and are genuinely wired — a naive check reports them as gaps. I verified this by readingdiscover_groups_json()rather than assuming.When
origin/GH-Actionsis unavailable the workflow check skips with an explicit NOTE instead of reporting gaps it can't verify.actions/checkoutfetches a single branch, soCI-lint-groups-json.ymlnow fetches that ref at--depth=1— without it the check would silently no-op in CI, which is the same failure mode this PR exists to fix.Result
Widening the check surfaced 21 further pre-existing unwired groups (the
mysql-<variant>-g*families,mysql91-gr,mysql92-gr,mysqlx-e2e,pgsql-repl,todo). Those are allowlisted as tracked debt so the signal stays clean and only genuinely NEW gaps appear.no-infra-g1is deliberately not allowlisted — it's real and actionable, and is now the single NEW finding.Still warn-only (exit 0).
--strictexits 1, for a future enforcement step.Verification
CI-no-infra-g1.ymldrops NEW from 1 → 0, then removed--depth=1fetch confirmed sufficient:git ls-tree -r origin/GH-Actions .github/workflowsreturns all 67 files--strictexits 1; default exits 0Follow-up, not in this PR
no-infra-g1still needs its actual wiring —CI-no-infra-g1.ymlhere plusci-no-infra-g1.ymlonGH-Actions. It's cheap to run (the whole group is ~73s with no backend), and it currently gatesreg_test_5363_admin_monitor_caching_sha2-tfrom #5991.Summary by CodeRabbit