test: close 3 real test-coverage gaps + drop stale shellcheck exclusions - #99
Merged
Conversation
path-keys.sh doesn't exist anymore (removed in dotfiles-sync's opt-in SSH keys refactor) and sync-files.sh now passes shellcheck clean (verified directly: shellcheck -S warning src/dotfiles-sync/sync-files.sh, no output). Only gh-auth.sh legitimately needs the exclusion — it's sourced from /etc/profile.d/ and can't carry a shebang (SC2148). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
✅ PR Validation Passed
📋 Pipeline Status
🤖 Generated by @helpers4 CI • 2026-09-09 |
baxyz
force-pushed
the
test/close-coverage-gaps
branch
from
September 9, 2026 21:30
d59dcbf to
0120158
Compare
This feature had no test/ directory at all and no CI matrix entry in either workflow — the published feature had never once been exercised by CI. Adds a test.sh that asserts the documented behavior for both cases it can hit: the gh copilot extension installed when gh is present, or the graceful skip when it isn't (copilot-dev has no dependsOn on github-dev by design, so the latter is what a standalone install actually gets by default). No version bump — test-only, invisible to consumers. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
test.sh was a no-op — two echo lines regardless of what install.sh actually did. The feature's one real, build-time artifact (git config --system alias.fixup/alias.polish) was never verified. Now asserts both aliases resolve to their expected values. No version bump — test-only, invisible to consumers. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Same gap class claude-dev just got a scenario for: install.sh already degrades gracefully (warns instead of aborting) on a failed npm install, which is correct — but installCli defaults to false and test.sh only ever asserted the CLI's *absence* under default options. The true-install path had zero CI coverage. Verified directly: devcontainer features test --features cline-dev --skip-autogenerated . installs cline 3.0.61 correctly. No version bump — test-only, invisible to consumers. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Same gap class claude-dev just got a scenario for. install.sh's uv branch in particular trusts uv tool install without verifying the binary actually landed afterward — exactly the class of gap already found once in vite-plus — and installCli defaults to false with zero prior CI coverage of the true-install path. Verified directly: devcontainer features test --features mistral-dev --skip-autogenerated . installs vibe (via pip, Python 3.12) correctly. No version bump — test-only, invisible to consumers. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Counterpart to pr-validation.yml's matrix updates from the shellcheck-fix commit — this repo's push-to-main workflow needs the same entries. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…changes Corrects an over-broad rule from #97: test/<name>/test.sh itself was kept as a bump trigger there (reasoning: "the one file CI runs against every consumer's build"), only exempting scenario files. That's still wrong — test.sh is exactly as invisible to a consumer as a scenario file. A version bump publishes a new GHCR tag and triggers a website docs rebuild; doing that for a change nobody running the feature can see is pure noise, not a safety measure. Narrows the check to src/<name>/ (minus README) only. Nothing under test/<name>/ ever requires a bump anymore. Updated AGENTS.md's "Modifying an existing feature" and "Testing non-default options" sections to state this explicitly, and reverted the version bumps this branch had already added to copilot-dev and essential-dev for test-only changes. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
baxyz
force-pushed
the
test/close-coverage-gaps
branch
from
September 9, 2026 21:54
0120158 to
e119823
Compare
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.
Summary
Verified findings from a review report before acting — one claim didn't hold up (see below), the rest did:
path-keys.shdoesn't exist anymore (removed in dotfiles-sync's opt-in SSH keys refactor);sync-files.shnow passesshellcheck -S warningclean (verified directly, no output). Onlygh-auth.shlegitimately needs the exclusion (sourced from/etc/profile.d/, can't carry a shebang — SC2148).copilot-dev: zero test coverage — notest/directory, absent from both CI matrices. The published feature (v1.0.4) had never once been exercised by CI. Added a test asserting the documented behavior in both cases (gh copilot extension installed whenghis present, graceful skip when it isn't).essential-dev/test.shwas a no-op — twoecholines regardless of whatinstall.shdid. Now asserts the feature's real build-time artifact: the two git aliases (alias.fixup,alias.polish).cline-dev/mistral-devmissinginstallCli:truescenarios — the exact same silent-degradation gap classclaude-devjust got a scenario for (test: add scenarios.json testing pattern, close claude-dev's installCli gap #97). Both install scripts already degrade gracefully and correctly, but the true-install path had zero CI coverage. Verified directly: installscline 3.0.61andvibe(via pip, Python 3.12) correctly through both real installers.One claim in the report was a false positive: "helpers4's self-heal" phrasing supposedly leaking into 15 READMEs — checked every occurrence, all 15 are inside Version History changelog entries describing the old wording that was replaced, not live in the callout text. No fix needed there.
Two items from the report are left as backlog, not bundled here: alerting on
test.yml'scontinue-on-errorfailures (bigger, separate notification mechanism) andtypescript-dev/test.sh's minimal no-op (low priority, IDE-config-only feature).Test plan
shellcheck -S warningclean on all touched scriptscline-dev/mistral-devscenarios directly against the real installers in a containeressential-dev's enriched test.sh against the real feature