fix(ci): GitHub で回っていなかったユニットテストを回し、ローカルと経路を揃える - #1910
Closed
mhaya wants to merge 3 commits into
Closed
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
ローカルと 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
前のコミットは 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
|
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 |
Reviewer's GuideGitHub Actions が従来未実行だった390件のユニットテスト（3モジュール283件と api-inventory 107件）を実行するようにし、マトリクス漏れの自動検出と、CI と同じ経路で安全にローカルテストを実行する仕組みおよび関連ドキュメントを追加した。 Sequence diagram for CI module unit testssequenceDiagram
participant Actions as GitHub Actions
participant Matrix as unit-tests.yml matrix
participant Services as Docker Compose services
participant Wait as wait-for-services.sh
participant Runner as run-module-tests.sh
participant Tox as tox
Actions->>Matrix: Select module
Actions->>Services: Start test services
Actions->>Wait: Wait for services
Wait-->>Actions: Services ready
Actions->>Runner: Run module tests
Runner->>Tox: Execute tox
Tox-->>Actions: Test result
Sequence diagram for CI-aligned local module testingsequenceDiagram
participant User
participant Local as run-local.sh
participant Image as Local test images
participant Compose as Docker Compose
participant Wait as wait-for-services.sh
participant Runner as run-module-tests.sh
participant Tox as tox
User->>Local: Run module test command
Local->>Image: Build or reuse hash-tagged images
Local->>Compose: Start PostgreSQL, Elasticsearch, Redis, RabbitMQ
Local->>Wait: Wait for services
Wait-->>Local: Services ready
Local->>Compose: Validate entry points
Local->>Runner: Run selected module
Runner->>Tox: Execute tox
Tox-->>User: Test result
Flow diagram for matrix coverage validationflowchart TD
Start[Run matrix.sh check] --> Matrix[Read unit-tests.yml matrix]
Matrix --> Modules[Find modules with tests/ and tox.ini]
Modules --> Compare[Compare module lists]
Compare --> Missing{Unregistered test module?}
Missing -->|Yes| Fail[Fail matrix-check]
Missing -->|No| Stale{Registered module lacks tests?}
Stale -->|Yes| Warn[Warn about stale entry]
Stale -->|No| Pass[Coverage check passes]
Warn --> Pass
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
PR Summary by QodoCI未実行テストの有効化とローカル実行経路の統一
AI Description
Diagram
High-Level Assessment
Files changed (9)
|
![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 reviewed your changes and they look great!
Sourcery assessment
Needs a human reviewer. この変更はCIのテスト経路、対象モジュール、静的検知器の合否条件を追加・変更するため、誤っていればテスト漏れを緑のまま通したり、逆にマージを継続的に阻害したりします。ワークフローを戻せば将来のCI挙動は元に戻せますが、その間に誤ったテスト結果を信頼してマージされたコードの修正は再実行だけでは取り消せません。.
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Code Review by Qodo
1. ARM Elasticsearch build fails
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
Comment on lines
+22
to
+25
| awk ' | ||
| /^[[:space:]]+module:[[:space:]]*$/ { inlist = 1; next } | ||
| inlist && /^[[:space:]]+- [A-Za-z0-9_-]+[[:space:]]*$/ { | ||
| gsub(/^[[:space:]]+- |[[:space:]]+$/, ""); print; next |
There was a problem hiding this comment.
1. matrix.sh lacks tests 📘 Rule violation ▣ Testability
The PR adds matrix parsing and validation logic in matrix.sh without adding or modifying a corresponding automated test. Parser or set-comparison regressions could silently omit testable modules or incorrectly fail CI.
Agent Prompt
## Issue description
Add automated tests for the new matrix extraction and validation behavior in `matrix.sh`.
## Issue Context
The script parses workflow YAML and compares its module list with filesystem contents, but this PR includes no corresponding test-file changes. Cover successful listing, missing modules, stale modules, strict mode, and malformed or absent workflow input.
## Fix Focus Areas
- scripts/ci/matrix.sh[19-80]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+60
to
+63
| while [ $# -gt 0 ]; do | ||
| case "$1" in | ||
| --all) MODULES=(__ALL__) ;; | ||
| --list) exec "$ROOT/scripts/ci/matrix.sh" list ;; |
There was a problem hiding this comment.
2. run-local.sh lacks tests 📘 Rule violation ▣ Testability
The PR adds a substantial local CI runner with option parsing, validation, image selection, cleanup, and failure aggregation without corresponding automated tests. Regressions could produce results that diverge from CI or leave local services running.
Agent Prompt
## Issue description
Add automated tests for the new `run-local.sh` behavior.
## Issue Context
The runner contains extensive executable logic but this PR includes no corresponding test-file changes. Use mocked or stubbed Docker and helper commands to cover option parsing, matrix validation, image build decisions, cleanup, and aggregated module failures without launching the complete stack.
## Fix Focus Areas
- scripts/ci/run-local.sh[60-90]
- scripts/ci/run-local.sh[129-175]
- scripts/ci/run-local.sh[199-220]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+101
to
+102
| DOCKERFILE_WEB=Dockerfile | ||
| DOCKERFILE_ES=elasticsearch/Dockerfile |
There was a problem hiding this comment.
3. Arm elasticsearch build fails 🐞 Bug ≡ Correctness
run-local.sh always builds elasticsearch/Dockerfile, whose Elasticsearch 6.8.23 base image is amd64-only, despite explicitly claiming native aarch64 support. On an ARM Docker host, image resolution fails before any tests run unless amd64 emulation is separately configured.
Agent Prompt
## Issue description
The local test runner selects an amd64-only Elasticsearch 6.8.23 image on ARM hosts, preventing the documented ARM workflow from building.
## Issue Context
The standard Elasticsearch Dockerfile uses `docker.elastic.co/elasticsearch/elasticsearch:6.8.23`, whose published architecture is amd64. The repository already contains a separate ARM Elasticsearch Dockerfile.
## Fix Focus Areas
- scripts/ci/run-local.sh[97-107]
- elasticsearch/Dockerfile[21-37]
- elasticsearch/Dockerfile.arm64[21-60]
- README-TEST.md[41-68]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+161
to
+162
| echo "▶ サービス起動 (postgresql / elasticsearch / redis / rabbitmq)" | ||
| docker compose up -d --no-build postgresql elasticsearch redis rabbitmq || exit 1 |
There was a problem hiding this comment.
4. Failed startup leaks services 🐞 Bug ☼ Reliability
The cleanup trap is registered only after docker compose up succeeds, so a partial startup followed by a nonzero Compose result exits without stopping containers or deleting volumes. Those leaked resources can block or contaminate later local test runs.
Agent Prompt
## Issue description
A partially failed Compose startup exits before the cleanup trap is registered, leaving containers and volumes behind.
## Issue Context
GitHub CI guarantees `docker compose down -v` through an `if: always()` step, but the local equivalent does not install its trap until after startup succeeds.
## Fix Focus Areas
- scripts/ci/run-local.sh[160-175]
- .github/workflows/unit-tests.yml[197-203]
- .github/workflows/unit-tests.yml[233-235]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+147
to
+149
| # docker-compose2.yml は 29201 / 26301 / 24301 を publish する。別の WEKO を | ||
| # 動かしたままだと起動に失敗するか、最悪そちらのサービスを掴む。 | ||
| for p in 29201 26301 24301; do |
There was a problem hiding this comment.
5. Port precheck misses conflicts 🐞 Bug ☼ Reliability
The preflight check omits Elasticsearch port 29301 and RabbitMQ port 45601 even though the selected services publish both ports. If either is occupied, preflight succeeds and Compose can partially start the stack before failing on the unvalidated binding.
Agent Prompt
## Issue description
The local runner validates only three of the five fixed host ports published by the services it starts.
## Issue Context
Elasticsearch publishes 29201 and 29301, while RabbitMQ publishes 24301 and 45601. Both additional ports can independently cause Compose startup failure.
## Fix Focus Areas
- scripts/ci/run-local.sh[146-158]
- docker-compose2.yml[344-364]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+149
to
+152
| for p in 29201 26301 24301; do | ||
| if (exec 3<>"/dev/tcp/127.0.0.1/$p") 2>/dev/null; then | ||
| exec 3<&- 2>/dev/null | ||
| running=$(docker ps --filter "publish=$p" --format '{{.Names}}' | head -1) |
There was a problem hiding this comment.
6. Keep mode prevents reuse 🐞 Bug ≡ Correctness
After a successful --keep run, the next invocation rejects the ports occupied by that same retained Compose project and exits before it can reuse the services. This contradicts the option's documented purpose of keeping services available for subsequent runs.
Agent Prompt
## Issue description
The port-conflict guard treats services intentionally retained by `--keep` as an unrelated WEKO stack, preventing subsequent test invocations.
## Issue Context
The usage text explicitly presents `--keep` for continued runs, and cleanup deliberately leaves the current Compose project running. Conflict detection should distinguish containers belonging to the current project from unrelated listeners and reuse or validate the former.
## Fix Focus Areas
- scripts/ci/run-local.sh[44-56]
- scripts/ci/run-local.sh[146-173]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Contributor
Author
|
#1911 に移行のためクローズ |
API インベントリ差分（件数のみ）
ベースラインとの差分API インベントリ差分レポート
判定: ✅ PASS (FAIL 0 / WARN 1)サマリ
[WARN] W6 依存パッケージの版が変化した — 40件
台帳との突き合わせスナップショット ↔ インベントリ 突き合わせ
判定: ✅ 一致 (0件)
|
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.
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 all testable modules and API Inventory tooling run reliably in CI while providing a matching, validated local test workflow.
New Features:
Bug Fixes:
Enhancements:
CI:
Documentation:
Tests:
ローカルと CI の経路を揃える（
06a072d15,7ecb5521d）上記の作業中、テストは正常なのに落ちる事象を2件踏んだ。ローカルと CI で回し方が違うことが原因で、実装にもテストにも問題は無かった。
weko-webイメージを流用 → イメージに焼き付いた古い egg-info の entry_point（weko_theme.bundles:js_preview_widget）をinvenio_assetsが読みにいって 191件が ImportErrorpytest→pytest-mock/mockが無くfixture 'mocker' not foundREADME-TEST.mdが案内していたのも CI とは別経路（Python 3.5 + venv +run-tests.sh）だった。scripts/ci/run-local.shCI の Unit Tests ジョブと同じ部品をそのまま呼ぶ（compose オーバレイ /
wait-for-services.sh/run-module-tests.sh= tox / マトリクス）。イメージは CI と同じ入力（modules/*/setup.py含む）のハッシュでタグ付けし、無ければビルドするので、古い egg-info を使い回す事故が起きない。起動前後に、実際に踏んだ2つの罠の事前確認を入れた（どちらも発動を実機確認済み）。
invenio_assets.bundlesの entry_point が壊れていないか（= egg-info が古い）scripts/ci/matrix.sh— 再発防止マトリクスを唯一の正として読み、食い違いを検出する。
matrix-checkジョブとして CI に組み込み済み。weko-redisが該当（tox.iniの c1 が--cov=weko_rediss testsを指すがtests/が無く、CI で常に失敗している）CPU アーキ
AMD / ARM で分岐しない。 ES 6.8 の bootstrap check が落ちる条件は ARM 固有ではなく（
vm.max_map_count< 262144 は AMD でも起こる）、分岐しても解決しないため、scripts/ci/compose.local.ymlを常に重ねてdiscovery.type=single-nodeにする。CI はこのオーバレイを読まない。AGENTS.md
テスト節が
python manage.py test（Django のコマンド。本プロジェクトは Flask/Invenio）を案内していたので書き換え、隣接する「Django のバリデーション機構」も実態に修正した。検証の範囲
run-local.sh weko-signpostingを通しで実行し 5 passed /exit=0レビュー時の確認点
unit-tests.ymlのジョブが 44 → 47 に増え、追加3モジュールが緑になることmatrix-checkとAPI Inventory Testsが緑になること