Skip to content

ci: add shellcheck workflow for shell script linting - #16

Merged
RyodoTanaka merged 1 commit into
developfrom
ci/shellcheck-and-tests
Oct 15, 2026
Merged

RyodoTanaka merged 1 commit into
developfrom
ci/shellcheck-and-tests

Conversation

@RyodoTanaka

Copy link
Copy Markdown
Member

変更点

GitHub Actions ワークフロー .github/workflows/shellcheck.yml を追加しました。

Refs

ローカル実行結果

shellcheck -S warning

  • リポジトリルート .sh/.bash ファイル: 問題なし
  • tests/ .sh/.bash ファイル: 問題なし
  • tests/fakes/* (拡張子なし bash スクリプト): 問題なし
  • 既知の warning: tests/test_lib_iface.sh line 100 (SC2034: CAN_RUN_BOUNDED_PID 未使用警告)

bash tests/run.sh

Results: 4 passed, 0 failed
  - Fake tests: 36 passed
  - Core tests: 49 passed
  - Iface tests: 17 passed
  - Setup tests: 17 passed

すべてのテストが通過しました。

🤖 Generated with Claude Code

Add GitHub Actions workflow that runs shellcheck -S warning on shell scripts
and bash tests. Includes repo-root .sh/.bash files and tests/ directory.
Excludes PR-A2/B files (will be removed in PR-D #9).
Includes tests/fakes/* scripts for comprehensive linting coverage.

Runs shellcheck -S warning and bash tests/run.sh on each push/PR.

@RyodoTanaka RyodoTanaka left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

レビュー(Opus): マージ可

Issue #9 §5/§6 の PR-C(shellcheck workflow)部分と、#14 レビューの CI 低指摘への対応を確認した。

確認できたこと

  • 対象・除外が §5/§6 どおり: repo 直下の *.sh/*.bash(-maxdepth 1 なので can-utils/・openimu/ は構造上入らない。! -path は冗長だが無害)、-S warning。A2/B の 4 ファイル(canhat_fd_setup.bash・canhat_fd_rename.bash・enable_canhat_fd.sh・can_monitor.sh)は除外し、「PR-D で外す」コメントあり。
  • 拡張子なしの fakes を拾う: 同じ条件で手元で試し、tests/fakes/{docker,ip,journalctl,systemctl,mk_sysfs.sh} と tests/test_*.sh の 4 本が対象になることを確認した。
  • 失敗が job に伝わる: Actions の run: は既定で bash -e -o pipefail なので、shellcheck が非 0 ならその場で step が落ちる(rc=$? 以降には来ないが結果は同じ)。tests/run.sh は失敗が 1 件でもあれば exit 1 で、step も落ちる。
  • A2(#13)・B(#15) のマージ後: 直下に新しく入る install.sh(A2)と can_watchdog.sh(B)は除外されず対象になるが、両方とも shellcheck 0.11 -S warning で警告 0(rc=0)だった。新しい tests/test_*.sh は対象になり、警告 0 は確認済み。B の can_monitor.sh 削除も除外判定は basename 比較なので問題ない。
  • 安全: secrets は使っていない。外部 action は actions/checkout@v4 だけ。CI は緑(push と pull_request で 2 回)。

低(任意。PR-D でまとめて直してもよい)

  1. permissions: contents: read を明記していない。リポジトリ/組織の既定が write だと、GITHUB_TOKEN に必要以上の権限が付く。
  2. tests/ 直下は test_* で絞っているので、tests/run.sh は対象外になる(§5 の「tests/」より狭い)。今は警告 0 なので、対象に足しても通る。
  3. tests/fakes/ 配下は拡張子を見ずにすべて渡している。シェルでないデータ(JSON の雛形など)を置くと誤検出で落ちる。逆に、can_diag 用の fakes を tests/diag_fakes/ のような別ディレクトリに置くと対象から漏れる。fakes は tests/fakes/ にシェルだけ置く、という運用を守るか、shebang で判定するとよい。
  4. on: [push, pull_request] なので、同じリポジトリ内の PR では 2 回走る。push を branches: [develop, main] に絞ると重複がなくなる。
  5. ubuntu-latest の apt 版 shellcheck は手元の 0.11 より古い。ローカル(契約)と CI で結果がずれることがある。

クリティカル・高・中の指摘なし。マージ可。

@RyodoTanaka
RyodoTanaka merged commit 78a6fd7 into develop Oct 15, 2026
2 checks passed
@RyodoTanaka
RyodoTanaka deleted the ci/shellcheck-and-tests branch October 15, 2026 04:49
RyodoTanaka added a commit that referenced this pull request Oct 15, 2026
Addresses the PR-A2 3rd review (pullrequestreview-5528306698) per
coordinator decision: HAT+ is out of scope for Issue #9 §2b, so instead
of fixing the --plus flag (which enabled canhat_plus_* *alongside* the
FD units, reproducing the exact race the earlier C-new1 fix was meant
to prevent - M-r3), remove it entirely.

- install.sh never installs or enables canhat_plus_rename.service /
  canhat_plus_setup.service, under any flag or env var
  (CAN_TOOLS_INSTALL_PLUS and --plus are both gone). The canhat_plus_*
  scripts are still copied to /opt/can_tools (harmless, never run on
  their own).
- README's "CAN HAT Plus" section now documents the manual setup
  instead (disable the FD units first, copy+enable the plus units by
  hand), and notes that can_watchdog (once it exists, PR-B) is FD-only
  for now and not meant to run on a HAT+ machine - flagged for PR-B to
  pick up.
- L-r3a (optional, done): _wait_inactive now polls
  `systemctl show -p ActiveState --value` instead of `is-active`, since
  a running oneshot reports ActiveState=activating (is-active's exit
  status alone can't tell "still running" apart from "never ran"/
  "finished"/"failed" the way checking the value for
  activating/deactivating can).
- Merged origin/develop (c63da4d..78a6fd7, PR #16 shellcheck CI
  workflow) via a merge commit, no rebase.

tests/test_install_idempotent.sh: Scenario 6 now asserts canhat_plus_*
is never installed/enabled for any of install.sh's invocations (no
args, a stray "--plus" - now just an ignored unrecognized argument -
and "--check"); scenario 4's can_monitor assertion checks for the new
`show -p ActiveState --value` call instead of `is-active`.

can_tools_lib.bash and tests/fakes/ are untouched.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant