ci: GitHub で回っていなかったユニットテストを回す - #1909
Conversation
develop_v2.0.5 には、テスト一式を持ちながら GitHub 上で一度も実行されていない テストが 390 本あった。いずれも**テストコード側は健全**で、CI の設定に載って いないだけだった。 ## 1. unit-tests.yml のマトリクスから漏れていた3モジュール (283本) modules/ には49ディレクトリあるが、マトリクスは44しか列挙していなかった。 差分5つのうち resources / cookiecutter-weko-module はモジュールではないが、 残る3つは tox.ini と tests/ を持つ実モジュールで、単に列挙漏れだった。 weko-notifications 39 passed, 1 skipped weko-signposting 5 passed weko-workspace 239 passed 3モジュールとも CI と同じ経路(scripts/ci/run-module-tests.sh 経由の tox)で 実行して確認済み。いずれも `c1: commands succeeded`。 requirements-weko-modules.txt には最初から入っているので、イメージ側の 変更は要らない。テストコードにも手を入れていない。 コメントの「45個のマトリクス」も実数(47)に直した。 ## 2. api-inventory のツールテスト (107本) api-inventory-tests.yml を tools/api-inventory/ci/ に置いたまま .github/workflows/ に配置していなかったため、一度も動いていなかった。配置する。 Docker も Secret も台帳も要らず、数秒で終わる。 あわせて push と pull_request のパス指定を YAML アンカーで共有した。 別々に書くと片方だけ古びて「PR では回るが push では回らない」という 説明のつかない差になるため。ci/README.md の設置手順にも2本目を追記した (.github/workflows/ が実体で ci/ 配下は原本、という関係を明記)。 ## 確認の際に踏んだ落とし穴(記録) ローカルで既存イメージを流用して回すと、イメージに焼き付いた古い egg-info の entry_point (weko_theme.bundles:js_preview_widget など。現行の setup.py には無い) を invenio_assets が読みにいって 191件が ImportError になる。CI は ci-images.yml がブランチからイメージを作り直すので発生しない。ローカルで 再現するときは全モジュールに `python setup.py egg_info` を掛けてから回すこと。 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UFWPMjL6mrvQ2NHBvNgviy
Reviewer's GuideGitHub Actionsで未実行だった390本のユニットテストを有効化するため、unit-testsのマトリクスに3モジュールを追加し、api-inventoryの単体テストワークフローを配置する。後者ではpushとpull_requestの対象パスを共有し、pytestに加えて静的経路検知の最低件数チェックも実行する。 File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Team Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 2 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path=".github/workflows/api-inventory-tests.yml" line_range="19-24" />
<code_context>
+# 「PR では回るが push では回らない」といった説明のつかない差になる)。
+on:
+ pull_request:
+ paths: &paths
+ - 'tools/api-inventory/**'
+ - '.github/workflows/api-inventory-tests.yml'
+ push:
+ branches: ['**']
+ paths: *paths
+ workflow_dispatch:
+
</code_context>
<issue_to_address>
**issue (bug_risk):** The workflow relies on a YAML anchor defined under `pull_request.paths` and an alias under `push.paths`; GitHub Actions rejects YAML anchors/aliases in workflow syntax, so the workflow fails validation before any job starts.
**Suggested fix:** Use a duplicated `paths` list, or use a GitHub Actions-supported reusable configuration mechanism instead of YAML anchors.
```suggestion
paths:
- 'tools/api-inventory/**'
- '.github/workflows/api-inventory-tests.yml'
push:
branches: ['**']
paths:
- 'tools/api-inventory/**'
- '.github/workflows/api-inventory-tests.yml'
```
</issue_to_address>
### Comment 2
<location path=".github/workflows/api-inventory-tests.yml" line_range="14-24" />
<code_context>
+
+name: API Inventory Tests
+
+# 対象は tools/api-inventory/ だけなので、そこを触ったときだけ回す。
+# push と pull_request でパスの並びを揃えること(片方だけ古びると、
+# 「PR では回るが push では回らない」といった説明のつかない差になる)。
+on:
+ pull_request:
+ paths: &paths
+ - 'tools/api-inventory/**'
+ - '.github/workflows/api-inventory-tests.yml'
+ push:
+ branches: ['**']
+ paths: *paths
+ workflow_dispatch:
+
</code_context>
<issue_to_address>
**issue (broader_impact):** Changes made only to the canonical workflow at `tools/api-inventory/ci/api-inventory-tests.yml` do not match the deployed workflow's path filters, so the newly added CI workflow does not run to validate those source changes. The documented requirement to keep the source and deployed copies synchronized is therefore not enforced by CI.
**Triggers:** When a contributor edits only `tools/api-inventory/ci/api-inventory-tests.yml`.
**Suggested fix:** Include `tools/api-inventory/ci/api-inventory-tests.yml` in the deployed workflow's path filters, or eliminate the copied-source arrangement by using the deployed workflow as the single source of truth.
</issue_to_address>Sourcery assessment
Approval pending. 2 findings to address first.
Blocking findings: .github/workflows/api-inventory-tests.yml:24, .github/workflows/api-inventory-tests.yml:24
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| paths: &paths | ||
| - 'tools/api-inventory/**' | ||
| - '.github/workflows/api-inventory-tests.yml' | ||
| push: | ||
| branches: ['**'] | ||
| paths: *paths |
There was a problem hiding this comment.
issue (bug_risk): The workflow relies on a YAML anchor defined under pull_request.paths and an alias under push.paths; GitHub Actions rejects YAML anchors/aliases in workflow syntax, so the workflow fails validation before any job starts.
Suggested fix: Use a duplicated paths list, or use a GitHub Actions-supported reusable configuration mechanism instead of YAML anchors.
| paths: &paths | |
| - 'tools/api-inventory/**' | |
| - '.github/workflows/api-inventory-tests.yml' | |
| push: | |
| branches: ['**'] | |
| paths: *paths | |
| paths: | |
| - 'tools/api-inventory/**' | |
| - '.github/workflows/api-inventory-tests.yml' | |
| push: | |
| branches: ['**'] | |
| paths: | |
| - 'tools/api-inventory/**' | |
| - '.github/workflows/api-inventory-tests.yml' |
PR Summary by QodoRun omitted module and API inventory tests in GitHub Actions
AI Description
Diagram
High-Level Assessment
Files changed (4)
|
Code Review by Qodo🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)
Great, no issues found!Qodo reviewed your code and found no material issues that require reviewTip of the day💡 Did you know, you can turn on the rule miner and Qodo learns your standards from review history |
API インベントリ差分（件数のみ）
ベースラインとの差分API インベントリ差分レポート
判定: ✅ PASS (FAIL 0 / WARN 1)サマリ
[WARN] W6 依存パッケージの版が変化した — 40件
台帳との突き合わせスナップショット ↔ インベントリ 突き合わせ
判定: ✅ 一致 (0件)
|
ローカルと CI で違う回し方をすると、どちらかでしか出ない失敗が生まれ、結果を
突き合わせられなくなる。実際、直前の作業で**テストは正常なのに落ちる**事象を
2件踏んだ。
- 手元にあった無関係な weko-web イメージを流用したところ、イメージに焼き付いた
古い egg-info の entry_point (weko_theme.bundles:js_preview_widget。現行の
setup.py には無い) を invenio_assets が読みにいって 191件が ImportError。
CI は ci-images.yml が modules/*/setup.py を含むハッシュでタグを決めるので
起きない。
- invenio の venv で直接 pytest を叩いたところ pytest-mock / mock が無く
「fixture 'mocker' not found」。CI は tox が requirements2.txt から入れる。
どちらも原因の切り分けに時間を取られただけで、実装にもテストにも問題は無かった。
README-TEST.md が案内していたのも CI とは別経路(Python 3.5 + venv + run-tests.sh)
で、Python も依存も tox の有無も違う。ここで通っても CI で通る保証がない。
## scripts/ci/run-local.sh
CI の Unit Tests ジョブと**同じ部品をそのまま呼ぶ**。分岐させないことが要点。
compose docker-compose2.yml:docker-compose.ci.yml (CI と同一)
起動サービス postgresql / elasticsearch / redis / rabbitmq のみ (同一)
起動待ち scripts/ci/wait-for-services.sh (同一)
テスト実行 scripts/ci/run-module-tests.sh (= tox) (同一)
モジュール一覧 .github/workflows/unit-tests.yml の matrix (同一)
イメージは CI と同じ入力ファイル集合(modules/*/setup.py を含む)のハッシュで
タグ付けし、無ければビルドする。GHCR から引けない代わりに、setup.py が変われば
必ず作り直されるので、古い egg-info を使い回す事故が起きない。
CI と完全に同一のイメージで確かめたいときは WEKO_IMAGE で明示する。
起動前後に2つの事前確認を入れた。どちらも今回実際に踏んだもの:
- 別の WEKO スタックが 29201/26301/24301 を掴んでいないか
- invenio_assets.bundles の entry_point が壊れていないか(= egg-info が古い)
## scripts/ci/matrix.sh
モジュール一覧が「ワークフローの中」と「手順書」に二重に書かれると必ず片方が
古びる。マトリクスを唯一の正として読み、食い違いを検出する。
tests/ と tox.ini があるのにマトリクスに無い → 失敗
ジョブが立たない = 赤くもならない静かな漏れ。v2.0.5 まで3モジュール283本が
この状態だった。unit-tests.yml に matrix-check ジョブとして組み込む。
マトリクスにあるが tests/ か tox.ini が無い → 警告
ジョブは立って赤くなるので見えている。消すか足すかは人の判断。
現状 weko-redis が該当(tox.ini の c1 が `--cov=weko_rediss tests` を指すが
tests/ が無く、CI でも常に失敗している)。
## 残る差: ホストのアーキテクチャ
CI は x86_64。ARM では ES 6.8 の seccomp が x86_64 専用のため bootstrap check に
失敗する。ホストが x86_64 でないときだけ scripts/ci/compose.arm64.yml を重ねて
discovery.type=single-node にし、その旨を表示する。テストの内容には影響しない。
Dockerfile.arm64 は nodesource の setup_4.x が消えており現在ビルドできないので
使わない(標準の Dockerfile は aarch64 でもビルドできる)。
aarch64 のこのホストで scripts/ci/run-local.sh weko-signposting を通しで実行し、
イメージのビルドから 5 passed / congratulations まで確認済み。
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UFWPMjL6mrvQ2NHBvNgviy
🔍 Claude レビュー統合他レビューの指摘 2 件 → ✅ 妥当 0 ／ ❌ 誤検知 1 ／ 🔎 要文脈 0 ／ ☑️ 対応済み 1 Claude の追加指摘 2 件 — 🔴 高 2 ／ 🟠 中 0 ／ 🟡 低 0
1. ☑️ 対応済み Canonical workflow copy under tools/api-inventory/ci/ not covered by deployed workflow's path filters
解決済みとマークされているが、実際にコードを読んで確認した。デプロイ済みワークフローの pull_request.paths に 'tools/api-inventory/**' が含まれ、これは tools/api-inventory/ci/api-inventory-tests.yml にもマッチするグロブなので、原本のみを編集した場合もワークフローが起動する。両ファイルは diff コマンドでバイト単位の同一を確認した。 根拠確認: .github/workflows/api-inventory-tests.yml:17-21 と tools/api-inventory/ci/api-inventory-tests.yml を diff で比較し IDENTICAL を確認 2. ❌ 誤検知 YAML anchor/alias in
|
前のコミットは aarch64 のときだけ Elasticsearch の設定を変えていた。これだと
開発機が AMD か ARM かで手元の条件が変わり、「片方の CPU でしか再現しない失敗」を
自分で作ることになる。ローカルと CI を揃えるという目的に反する。
分岐をやめ、scripts/ci/compose.local.yml を**常に**重ねる形にした
(compose.arm64.yml から改名)。調整点は install.sh と同じく COMPOSE_FILE ひとつ。
ES 6.8 は非ループバックアドレスに bind した時点で bootstrap check を強制するが、
これはホストのカーネルと sysctl に依存するので、開発機では環境しだいで落ちる。
確認できたものだけでも:
- ARM: seccomp の実装が x86_64 専用で
「seccomp unavailable: CONFIG_SECCOMP not compiled into kernel」で失敗する
- vm.max_map_count が 262144 未満のホスト: max_map_count の検査で失敗する
後者は AMD でも起こる。アーキで分岐しても解決しない類の問題なので、
discovery.type=single-node で bootstrap check ごと省く。ES はテストが使う単一
ノードなので意味は変わらない(リポジトリの docker-compose.arm64.yml と同じ扱い)。
CI はこのオーバレイを読まないため、CI 側は素の設定のまま。
Dockerfile は両アーキとも CI と同じものを使う(標準の Dockerfile /
elasticsearch/Dockerfile は aarch64 でもビルドできる)。
aarch64 の実機で scripts/ci/run-local.sh weko-signposting を通しで実行し、
5 passed / congratulations / exit=0 を確認。x86_64 の実機は手元に無いため未検証。
## AGENTS.md
- テスト節が `python manage.py test` (Django のコマンド) を案内していた。
本プロジェクトは Flask/Invenio なので、run-local.sh / matrix.sh /
api-inventory のテストに書き換えた。
- 同じくセキュリティ節の「Django のバリデーション機構」を実態
(Flask-WTF / marshmallow / JSON Schema) に直した。
- ベースラインに既存の失敗が残っている旨を明記した。「赤い = 自分のせい」と
取り違えると、無い原因を探すことになるため。
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UFWPMjL6mrvQ2NHBvNgviy
API インベントリ差分（件数のみ）
ベースラインとの差分API インベントリ差分レポート
判定: ✅ PASS (FAIL 0 / WARN 1)サマリ
[WARN] W6 依存パッケージの版が変化した — 40件
台帳との突き合わせスナップショット ↔ インベントリ 突き合わせ
判定: ✅ 一致 (0件)
|
API インベントリ差分（件数のみ）
ベースラインとの差分API インベントリ差分レポート
判定: ✅ PASS (FAIL 0 / WARN 1)サマリ
[WARN] W6 依存パッケージの版が変化した — 40件
台帳との突き合わせスナップショット ↔ インベントリ 突き合わせ
判定: ✅ 一致 (0件)
|
1 participant
GitHub 上で一度も実行されていないユニットテストが 390本 あった。いずれもテストコード側は健全で、CI の設定に載っていないだけだった。テストコードは変更していない。
1.
unit-tests.ymlのマトリクスから漏れていた3モジュール（283本）modules/には49ディレクトリあるが、マトリクスは44しか列挙していなかった。差分5つのうちresources/cookiecutter-weko-moduleはモジュールではないが、残る3つはtox.iniとtests/を持つ実モジュールで、単に列挙漏れだった。weko-notificationsweko-signpostingweko-workspace3モジュールとも CI と同じ経路（
scripts/ci/run-module-tests.sh経由の tox）でローカル実行して確認済み。いずれもc1: commands succeeded。requirements-weko-modules.txtには最初から入っているので、イメージ側の変更は不要。コメントの「45個のマトリクス」も実数（47）に修正した。
2. api-inventory のツールテスト（107本）
api-inventory-tests.ymlをtools/api-inventory/ci/に置いたまま.github/workflows/に配置していなかったため、一度も動いていなかった。配置する。Docker も Secret も台帳も要らず数秒で終わる。あわせて
pushとpull_requestのパス指定を YAML アンカーで共有した。別々に書くと片方だけ古びて「PR では回るが push では回らない」という説明のつかない差になるため。ci/README.mdの設置手順にも2本目を追記した（.github/workflows/が実体、ci/配下が原本という関係を明記）。確認の際に踏んだ落とし穴
ローカルで既存イメージを流用して回すと、イメージに焼き付いた古い egg-info の entry_point（
weko_theme.bundles:js_preview_widgetなど。現行のsetup.pyには無い）をinvenio_assetsが読みにいって 191件が ImportError になる。CI はci-images.ymlがブランチからイメージを作り直すので発生しない。ローカルで再現するときは全モジュールにpython setup.py egg_infoを掛けてから回すこと。レビュー時の確認点
unit-tests.ymlのジョブが 44 → 47 に増え、追加3モジュールが緑になることAPI Inventory Testsワークフローが起動し、107本が緑になること🤖 Generated with Claude Code
https://claude.ai/code/session_01UFWPMjL6mrvQ2NHBvNgviy
Summary by Sourcery
Ensure GitHub Actions executes all testable module and API inventory unit tests while preventing future CI coverage gaps.
New Features:
Bug Fixes:
Enhancements:
CI:
Documentation:
Tests: