Close the last three: doctor + readiness coverage, one shared test helper - #65
Merged
Conversation
doctor's whole job is telling a user why /idstack:<skill> is missing, so every branch it prints is one someone acts on. It had no execution coverage — smoke-test only checked the file exists and parses. Redone from #55 rather than merged; that branch was conflicting and its harness had two faults that would have made the suite lie: - It copied only bin/idstack-doctor. doctor sources bin/lib/version-classify.sh and bin/lib/plugin-status.sh (idstack-doctor:21-23), both added after #55 was written, so every assertion would fail on a missing source file rather than on the behavior under test. Fixture now copies the whole bin/ tree. - It did `export PATH="$MOCK_BIN:$PATH"`, leaving the real PATH reachable. The "claude not found" case then passes on CI (no claude) and fails on any machine with Claude Code installed. PATH is now replaced, not prepended, with explicit symlinks to what doctor and its libs invoke. 13 cases: healthy install, both missing manifests, unparseable plugin.json (doctor parses it as JSON, so a regex-shaped test would miss this), missing SKILL.md, absent claude as WARNING not failure, installed-but-disabled, not-installed, all three legacy conflict shapes, and an unrecognized directory warning rather than being claimed as a broken idstack. Also pins the v3.3.0.0 regression at the doctor level, not just in test-plugin-status.sh: a neighbouring plugin's "enabled" line must not be attributed to idstack. Verified mutation-effective before committing — breaking one branch takes the suite to 11/13. Added as mutation 7b, and hooked into smoke-test as well as CI because release.yml's gate runs only smoke-test.sh. smoke 371 -> 372, mutations 14 -> 15. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
bin/idstack-status --readiness (:193-294) decides whether a course is fit to ship. Roughly 100 lines with no coverage. Redone from #54 rather than merged, for three reasons: - #54 deleted the existing idstack-status block from integration-test.sh and moved it into the new file. Kept both instead; integration-test stays 48/48. - Two of its assertions used the bare /needs-analysis and /learning-objectives spelling, which v3.3.0.0 banned because it resolves in neither CLI. Merging them would have re-admitted the form the mutation suite exists to keep out. - Its fixture sat below all three thresholds at once, so a wrong threshold constant still produced NOT-READY via some other unmet condition and the suite passed. Verdict-state coverage without boundary coverage. Each threshold is now probed at its own boundary with the other two held passing, so only the constant under test can flip the verdict: quality 70 vs 69, accessibility 80 vs 79, red-team 0 vs 1 critical. The WCAG Level-A override is exercised too, and is asserted twice on purpose. It is implemented in access_tier() for the per-skill row and again in verdict() for the overall line; I removed each in turn and found a single assertion only caught one of them. Level-AA is asserted NOT to block, so the override can't be widened by accident. Verified mutation-effective, added as mutations 7c and 7d. smoke 372 -> 373, mutations 15 -> 17, new suite 22 assertions. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Nine suites each carried their own PASS/FAIL/TOTAL and their own copy of check(). They had already drifted, which is the actual argument for doing this rather than tidiness: - two spelled it `assert` (test-manifest-merge, test-status) - test-manifest-merge ran assertions with `>/dev/null 2>&1` and printed a bare FAIL, so a red CI run told you something broke and not what - test-doctor needed exit-code and output matching and had grown a fourth variant to get it test/test-helper.sh now owns the counters and one check() whose optional 3rd and 4th arguments cover the exit-code/output form. Two suites keep a local wrapper because their assertion genuinely differs in shape — comparing a version string to a classification, and feeding a listing to plugin_is_enabled. Those are named check_version and check_listing rather than check, so they cannot shadow the shared one and drift again. Redone rather than merging #50, which reasoned from its own head blob: it restored the local classify_version() mirror that main had already deleted in favour of sourcing bin/lib/version-classify.sh, and it replaced main's diagnostic check() body with the silent version. The rename touched 45 call sites (23 manifest-merge, 22 status) plus 36 in the two domain-specific suites. Verification is the per-suite counts, which are identical before and after: 373/48/17/23/27/9/6/13/22. Mutations still 17/17 guarded, so no suite lost its teeth in the move. Two new guards so this cannot regrow: smoke-test asserts every suite sources the helper and defines no local counters, and mutation 7e reintroduces a private counter block with a silent check() and asserts smoke-test fails. smoke 373 -> 393, mutations 17 -> 18. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
This was referenced Aug 6, 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
1 participant
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.
Resolves the final three
needs-reworkPRs — #55, #54, #50 — each redone rather than merged, because each had a fault that would have made the suite lie.#55 → doctor coverage
bin/idstack-doctorhad no execution coverage; smoke-test only checked the file exists and parses. Its whole job is telling a user why/idstack:<skill>is missing, so every branch is one someone acts on.Two faults in the original harness:
bin/idstack-doctor. doctor sourcesbin/lib/version-classify.shandbin/lib/plugin-status.sh(both added after 🧪 Add tests for idstack-doctor #55 was written), so every assertion would have failed on a missing source file rather than on behavior.export PATH="$MOCK_BIN:$PATH"left the real PATH reachable, so "claude not found" passes on CI and fails on any machine with Claude Code installed.13 cases. Verified mutation-effective before committing — breaking one branch takes it to 11/13.
#54 → readiness coverage
--readiness(bin/idstack-status:193-294) is the pre-export gate. ~100 lines, no coverage./needs-analysisspelling that v3.3.0.0 banned — merging them would have re-admitted the form the mutation suite exists to keep out.The WCAG Level-A override is asserted twice on purpose — it's implemented in
access_tier()and again inverdict(), and I confirmed by removing each in turn that a single assertion only catches one.#50 → shared test helper
Nine suites each carried their own counters and copy of
check(), and they had already drifted: two spelled itassert, and test-manifest-merge ran with>/dev/null 2>&1and printed a bare FAIL.#50 reasoned from its own head blob — it restored the local
classify_version()mirror main had already deleted, and replaced main's diagnosticcheck()with the silent one.45 call sites renamed. Verification is the per-suite counts being identical before and after, and mutations staying 17/17 guarded through the move. Two suites keep a local wrapper where the assertion genuinely differs in shape, named
check_version/check_listingso they can't shadow the shared one.Guards against regrowth: smoke-test asserts every suite sources the helper and defines no local counters; mutation 7e reintroduces a private silent counter block and asserts smoke-test fails.
Verification
Supersedes #55, #54, #50.
🤖 Generated with Claude Code