feat(ci): add .gitignore pattern test-coverage gate for #519 - #538
Merged
Conversation
Closes the one remaining gap from #519's retrospective triage (#330): a newly-added .gitignore pattern with no test verifying it. Mirrors gate_skill_rename_lifecycle.py's shape (script + pytest suite + a merge-base-diff-driven workflow). The other three findings #519 cites (#347, #285, #242) are already fixed by prior work -- tests/test_gitignore_worktrees.py's existing 'git check-ignore -v' + source-file assertion, gate_skill_rename_lifecycle.py, and tests/test_check_acm_present_sync.py respectively -- so this PR does not touch them. Refs #519
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
- find_offenders() used a raw substring check, so a pattern's core text matching inside an unrelated word (e.g. "build" inside "rebuild_step") reported an uncovered pattern as covered, defeating the gate. Bound the match with non-alnum/underscore lookarounds so the core must appear as its own token. - main() only caught FileNotFoundError and left the file handle open on success; a directory path or permission-denied file crashed with a raw traceback. Use a context manager and catch OSError generally. Refs #519
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds the one remaining CI/pytest gate #519 asks for (#330: a newly-added
.gitignorepattern must have a test asserting it). The other three findings #519 cites were already fixed by prior, unrelated work -- see Facts below -- so this PR does not touch them.Facts
Verified against the current codebase, not just #519's restated retrospective facts (which predate later fixes):
grep -rn "check-ignore" .finds exactly one hand-written test (tests/test_gitignore_worktrees.py), scoped to a single hardcoded path -- no general "new pattern needs a test" gate exists anywhere in the repo.tests/test_gitignore_worktrees.py:14-33already usesgit check-ignore -vand asserts the reported source resolves toREPO_ROOT/.gitignore(#347's finding -- already fixed, no-qexit-0 check remains)..github/scripts/gate_skill_rename_lifecycle.py+tests/test_gate_skill_rename_lifecycle.py+.github/workflows/skill-rename-lifecycle-gate.ymlalready enforcespec.lifecycle.renamedFromon a removed skill directory (#285's finding -- already fixed, wired onskills/**/SKILL.mddiffs).tests/test_check_acm_present_sync.pyalready discovers all 4 knowncheck_acm_present.py-shaped copies and fails on a diverging header regex unless every diverging copy carries an"intentionally diverged"marker (#242's finding -- already fixed).Assumptions
Risk / blast radius
New CI workflow only triggers on
.gitignorediffs (paths: [".gitignore"]); no effect on any other PR.contents: readonly, no write scopes.Rollback
Delete
.github/workflows/gitignore-pattern-coverage-gate.yml,.github/scripts/gate_gitignore_pattern_coverage.py, andtests/test_gate_gitignore_pattern_coverage.py; no other files touched.Verification
Acceptance Criteria Map from #519, restated row-by-row:
.gitignorepattern needs a test (#330)gate_gitignore_pattern_coverage.py: a scratchadded.txtpattern with no referencing test file exits 1 (FAIL: ... has no test); adding a test file containing the pattern's literal text exits 0 (PASS: test coverage found for all 1 added pattern(s))..gitignore(#347)tests/test_gitignore_worktrees.py(pre-existing, unchanged by this PR).renamedFrom(#285)gate_skill_rename_lifecycle.py+ its workflow (pre-existing, unchanged by this PR).check_acm_present.pycopies share header regex or carry divergence marker (#242)tests/test_check_acm_present_sync.py(pre-existing, unchanged by this PR).Commands run locally:
uv run --frozen pytest tests/test_gate_gitignore_pattern_coverage.py -v-- 19 passed, 97% line coverage on the new script.uv run --frozen pytest(full suite) -- 940 passed.ruff check .github/scripts/gate_gitignore_pattern_coverage.py tests/test_gate_gitignore_pattern_coverage.py-- all checks passed.python3 -c "import yaml; yaml.safe_load(open('.github/workflows/gitignore-pattern-coverage-gate.yml'))"-- valid YAML. (actionlintitself could not run in this sandbox -- no docker daemon -- so the new workflow was instead reviewed line-by-line against the already-CI-validatedskill-rename-lifecycle-gate.ymlit mirrors.)Checklist
Related Issue
Closes #519
Generated by Claude Code