test: smoke tests for update.py — last uncovered module#12
Open
bipinhcs11 wants to merge 3 commits into
Open
Conversation
update.py had zero test coverage despite being the Phase-2 hot path. Adds 29 stdlib-only tests across four test classes: TestBumpVersion — version parsing + fallback when field absent TestDomainFromSkill — class/XML/SQL/shell extraction; dedup invariant TestResolveSkillsDir — .github/skills/ preferred; None when absent TestMapFilesToFeatures — basename matching; multi-feature; unmatched Plus seven ingest_responses integration tests (validate_schema=False): version bump to existing+1, last_updated rewritten to today, failed[] entries for missing response / missing skill / empty response, markdown- fence stripping, no-skills-dir guard, and multi-feature batch update. Total suite: 172 → 201 tests. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> https://claude.ai/code/session_01V3GMvToFpDMsnSbSe2Fsgp
…verage PR #12 added 29 smoke tests for update.py. This commit adds 4 more that cover two safety-critical code paths which were not yet exercised: Force-correction regressions (TestIngestResponsesForceCorrections): - AI response says "version: 99" but existing skill is version: 3 → ingest must write version: 4, not 99 or 100. Verifies the forced re.sub always wins regardless of what the AI wrote. - AI response has a stale last_updated (e.g. 2020-06-15) → ingest always rewrites to today. Verifies the second forced re.sub. Feature-filter isolation (TestIngestResponsesFeatureFilter): - feature="alpha" with two response files → only alpha updated, beta untouched - No feature kwarg → both features updated (batch path) These cover the two `re.sub` force-overwrite lines in ingest_responses (lines 224-237 of update.py) and the feature= conditional on line 203, none of which were reached by the previous test set. Suite: 29 → 33 tests for update.py; 201 → 205 total, all green. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The existing test_update.py (added in this branch) covered all pure
helpers and the ingest integration path, but left _git_changed_files
untested because it calls subprocess. Add TestGitChangedFiles (5 tests)
using unittest.mock to keep the suite hermetic:
- success path returns the diff file list, stripping blank lines
- CalledProcessError on git-diff triggers the git-status fallback
- empty diff output (shallow clone / single-commit repo) also triggers
the fallback rather than silently returning []
- git status --porcelain 3-char prefix is parsed correctly
Total suite: 210 tests (was 205 before this change, 172 before the branch).
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This was referenced May 31, 2026
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.
What
Adds
tests/test_update.py— 38 stdlib-only smoke tests fortools/skill_generator/update.py, which was the only module in the package with zero test coverage.Why
update.pyis the Phase-2 hot path: it maps changed files to features, bumps versions, rewriteslast_updated, and writes refreshed SKILL.mds. Silent bugs here corrupt the skills that every AI session depends on. The existing suite coveredcrawler,plan,generate,link,validate, anddoctor—updatewas the gap.What is tested
TestBumpVersion(5 tests)TestDomainFromSkill(8 tests)feature_idused asidandname; description always""; Java class names extracted; XML/SQL/shell sources extracted with": from prior skill"suffix; empty text → empty classes; duplicate class names deduplicatedTestResolveSkillsDir(3 tests).github/skills/preferred overskills/;skills/used when.github/skills/absent;Nonewhen neither existsTestMapFilesToFeatures(6 tests){}TestIngestResponses(7 tests)existing+1;last_updatedrewritten to today; missing response →failed[]; missing existing SKILL.md →failed[]; empty/whitespace response →failed[]; markdown fence stripped before write; two-feature batch updateTestIngestResponsesForceCorrections(2 tests)existing+1; stalelast_updatedin AI response replaced with todayTestIngestResponsesFeatureFilter(2 tests)feature=kwarg restricts updates to one feature; no filter updates allTestGitChangedFiles(5 tests)CalledProcessErrortriggersgit statusfallback; empty diff output also triggers fallback (not silent[]);--porcelain3-char prefix parsed correctly — usesunittest.mockto stay hermeticAll ingest tests use
validate_schema=Falseto isolate update logic from the validator (which has its own 40-test suite intest_validate.py).Suite size
172 → 210 tests, all green.
Follow-ups noted (not in this PR)
emit_promptsintegration test (requires a crawled index + skills dir) — could be added alongside thegenerate-emitintegration tests if those are ever added._normalize_java_versionedge case for"V11"(capital-V prefix, notVERSION_) is worth adding totest_crawler.py.