Conversation
- リトライ機構(nick-invision/retry)がcodeceptコマンドのみを再実行するため、 1回目で変更された共有データがリセットされず2回目で連鎖的に失敗する問題を修正 - retryのcommandブロックにDB初期化処理を追加 - 通常のCodeceptionステップとrestrict-fileupload/change-display-orderステップの両方に適用
📝 WalkthroughウォークスルーCodeception テスト実行前に、doctrine コマンドとキャッシュクリアコマンドをGitHub Actions ワークフローに追加しました。データベーススキーマのリセット、再作成、フィクスチャの読み込み、キャッシュクリアをマトリックスベースの実行と特定グループ実行の前に実行します。 変更内容
推定レビュー工数🎯 2 (Simple) | ⏱️ ~10 minutes 推奨ラベル
推奨レビュアー
詩
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/e2e-test.yml:
- Line 167: The workflow clears the wrong Symfony environment: update the cache
clear invocations that run "bin/console cache:clear --no-warmup --env=dev" to
target the E2E runtime environment ("codeception") so that var/cache/codeception
is removed between retries; ensure any other occurrences (e.g., the similar
command around the other reported location) are changed consistently so both
cache:clear commands use --env=codeception.
- Around line 191-192: The fixtures reload step (bin/console
eccube:fixtures:load and cache:clear) is being run while
ECCUBE_RESTRICT_FILE_UPLOAD is still set for matrix.group ==
'change-display-order', causing restrictive settings to be written into the DB;
change the workflow so ECCUBE_RESTRICT_FILE_UPLOAD is only set for the
restrict-fileupload scenario (either move the ECCUBE_RESTRICT_FILE_UPLOAD
environment variable into the restrict-fileupload job/step or wrap its addition
with a conditional on matrix.group == 'restrict-fileupload'), or create a
dedicated environment key for the restrict-fileupload job so the fixture reload
step uses the correct env only for that group. Ensure you modify the job/env
blocks that define ECCUBE_RESTRICT_FILE_UPLOAD and the steps running
eccube:fixtures:load / cache:clear accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 28018b7e-ce99-41a4-86fc-05b12a26e05f
📒 Files selected for processing (1)
.github/workflows/e2e-test.yml
| bin/console doctrine:schema:drop --force --env=dev | ||
| bin/console doctrine:schema:create --env=dev | ||
| bin/console eccube:fixtures:load --env=dev | ||
| bin/console cache:clear --no-warmup --env=dev |
There was a problem hiding this comment.
cache:clear の対象 env が実際の E2E 実行環境とずれています。
HTTP 側のアプリは APP_ENV=codeception で起動しているのに、ここで消しているのは dev だけです。これだとリトライ間で var/cache/codeception が残り続けるので、状態リセットの意図を満たせません。
差分案
- bin/console cache:clear --no-warmup --env=dev
+ bin/console cache:clear --no-warmup --env=codeceptionAlso applies to: 192-192
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/e2e-test.yml at line 167, The workflow clears the wrong
Symfony environment: update the cache clear invocations that run "bin/console
cache:clear --no-warmup --env=dev" to target the E2E runtime environment
("codeception") so that var/cache/codeception is removed between retries; ensure
any other occurrences (e.g., the similar command around the other reported
location) are changed consistently so both cache:clear commands use
--env=codeception.
| bin/console eccube:fixtures:load --env=dev | ||
| bin/console cache:clear --no-warmup --env=dev |
There was a problem hiding this comment.
change-display-order のリトライで restrict-fileupload 用の設定が DB に混ざります。
このステップは matrix.group == 'change-display-order' でも Line 203 の ECCUBE_RESTRICT_FILE_UPLOAD='1' を持ったまま Line 191 の fixture 再投入を実行します。初期セットアップ時とは別のベースラインを作ってしまいますし、PHP サーバ側は同グループで ECCUBE_RESTRICT_FILE_UPLOAD='0' なので、DB 設定と実行時設定も食い違います。restrict-fileupload 専用の env に分離するか、この変数はそのグループのときだけ付与した方が安全です。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/e2e-test.yml around lines 191 - 192, The fixtures reload
step (bin/console eccube:fixtures:load and cache:clear) is being run while
ECCUBE_RESTRICT_FILE_UPLOAD is still set for matrix.group ==
'change-display-order', causing restrictive settings to be written into the DB;
change the workflow so ECCUBE_RESTRICT_FILE_UPLOAD is only set for the
restrict-fileupload scenario (either move the ECCUBE_RESTRICT_FILE_UPLOAD
environment variable into the restrict-fileupload job/step or wrap its addition
with a conditional on matrix.group == 'restrict-fileupload'), or create a
dedicated environment key for the restrict-fileupload job so the fixture reload
step uses the correct env only for that group. Ensure you modify the job/env
blocks that define ECCUBE_RESTRICT_FILE_UPLOAD and the steps running
eccube:fixtures:load / cache:clear accordingly.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## 4.3 #6664 +/- ##
============================================
+ Coverage 78.83% 79.33% +0.50%
- Complexity 6631 6638 +7
============================================
Files 475 475
Lines 26539 26568 +29
============================================
+ Hits 20922 21078 +156
+ Misses 5617 5490 -127
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
概要(Overview・Refs Issue)
E2Eテストをリトライする際、1回目で変更された共有データがリセットされず、2回目で連鎖的に失敗する問題を修正しました。
方針(Policy)
RetryのブロックにてDB初期化処理を追加しました。
Codeception実行ステップが2つあるため、両方に適用
実装に関する補足(Appendix)
テスト(Test)
相談(Discussion)
マイナーバージョン互換性保持のための制限事項チェックリスト
レビュワー確認項目
Summary by CodeRabbit
リリースノート