ci: shellcheck + bats on push and PR#25
Conversation
Runs shellcheck on bin/revive and install.sh, plus the full bats suite, on push to main and on every PR. Fixes one SC2012 warning in purpose_from_gemspec (replace `ls *.gemspec | head` with `find`). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
apt-installed bats on ubuntu-latest is too old — `run` does not merge stderr into $output, which broke the suggest stdout-vs-stderr test. Use the official bats-core GitHub action to pin a current release.
There was a problem hiding this comment.
Pull request overview
Adds GitHub Actions CI to automatically lint and test this bash-based CLI on pushes to main and on pull requests, and adjusts gemspec PURPOSE extraction to satisfy ShellCheck.
Changes:
- Add
.github/workflows/ci.ymlwithshellcheckandbatsjobs for PR/push validation. - Replace
ls *.gemspec | headwith afind-based approach inpurpose_from_gemspecto address SC2012.
Reviewed changes
Copilot reviewed 1 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
bin/revive |
Updates gemspec discovery logic used by PURPOSE extraction. |
.github/workflows/ci.yml |
Introduces CI jobs for ShellCheck linting and Bats test execution. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| push: | ||
| branches: [main] | ||
| pull_request: | ||
|
|
There was a problem hiding this comment.
Consider explicitly setting workflow permissions (e.g., contents: read) at the top-level to follow GitHub Actions least-privilege defaults. This limits the default GITHUB_TOKEN scope for both jobs since they only need to check out code and run local commands.
| permissions: | |
| contents: read |
| shellcheck: | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - uses: actions/checkout@v4 |
There was a problem hiding this comment.
The shellcheck job assumes shellcheck is already available on ubuntu-latest. To make CI more deterministic (and consistent with the bats job that installs its dependencies), consider installing shellcheck explicitly or using a dedicated ShellCheck action so runner image changes don’t break linting.
| - uses: actions/checkout@v4 | |
| - uses: actions/checkout@v4 | |
| - name: install shellcheck | |
| run: sudo apt-get update && sudo apt-get install -y shellcheck |
Test 71 asserted that meta-comments (which `cmd_suggest` writes to stderr) don't leak into stdout. But it checked the merged $output, which always contains both — so the assertion only "passed" because bats default errexit behavior ignores all but the last [[ ]] in a test. Stricter bats environments (CI) caught the bug. Switch the test to `run --separate-stderr` so $output really is stdout-only, and add explicit `|| return 1` on each assertion so intermediate failures aren't silently swallowed regardless of errexit semantics. Pin bats >= 1.5.0 for the flag.
- Add top-level `permissions: contents: read` for least-privilege GITHUB_TOKEN scope (both jobs only read the repo). - Install shellcheck explicitly via apt instead of relying on the ubuntu-latest runner image — keeps CI deterministic if the image ever drops the preinstalled binary.
Summary
.github/workflows/ci.ymlwith two jobs:shellcheck(lintsbin/revive+install.sh) andbats(runs the 111-test suite).purpose_from_gemspec(ls *.gemspec | head→find -maxdepth 1). Behavior unchanged.Why
Repo is installed via
curl | bash, so a regression ininstall.shships directly to users. CI on PRs catches that before merge. Cheapest insurance available — both tools are already part of the documented dev loop in CLAUDE.md.Test plan
shellcheck bin/revive install.shclean locallybats tests/— 111/111 pass locally🤖 Generated with Claude Code