Skip to content

fix(ci): E2Eテストのリトライ時にDBをリセットするよう修正#6664

Open
dotani1111 wants to merge 1 commit intoEC-CUBE:4.3from
dotani1111:fix/e2e-test-reset-db-on-retry
Open

fix(ci): E2Eテストのリトライ時にDBをリセットするよう修正#6664
dotani1111 wants to merge 1 commit intoEC-CUBE:4.3from
dotani1111:fix/e2e-test-reset-db-on-retry

Conversation

@dotani1111
Copy link
Contributor

@dotani1111 dotani1111 commented Mar 10, 2026

概要(Overview・Refs Issue)

E2Eテストをリトライする際、1回目で変更された共有データがリセットされず、2回目で連鎖的に失敗する問題を修正しました。

方針(Policy)

RetryのブロックにてDB初期化処理を追加しました。
Codeception実行ステップが2つあるため、両方に適用

実装に関する補足(Appendix)

テスト(Test)

相談(Discussion)

マイナーバージョン互換性保持のための制限事項チェックリスト

  • 既存機能の仕様変更はありません
  • フックポイントの呼び出しタイミングの変更はありません
  • フックポイントのパラメータの削除・データ型の変更はありません
  • twigファイルに渡しているパラメータの削除・データ型の変更はありません
  • Serviceクラスの公開関数の、引数の削除・データ型の変更はありません
  • 入出力ファイル(CSVなど)のフォーマット変更はありません

レビュワー確認項目

  • 動作確認
  • コードレビュー
  • E2E/Unit テスト確認(テストの追加・変更が必要かどうか)
  • 互換性が保持されているか
  • セキュリティ上の問題がないか
    • 権限を超えた操作が可能にならないか
    • 不要なファイルアップロードがないか
    • 外部へ公開されるファイルや機能の追加ではないか
    • テンプレートでのエスケープ漏れがないか

Summary by CodeRabbit

リリースノート

  • Tests
    • テスト実行前のデータベース初期化プロセスを改善し、各テストスイートが常に新しい状態から開始されるようにしました。これによりテストの信頼性と安定性が向上します。

- リトライ機構(nick-invision/retry)がcodeceptコマンドのみを再実行するため、
  1回目で変更された共有データがリセットされず2回目で連鎖的に失敗する問題を修正
- retryのcommandブロックにDB初期化処理を追加
- 通常のCodeceptionステップとrestrict-fileupload/change-display-orderステップの両方に適用
@coderabbitai
Copy link

coderabbitai bot commented Mar 10, 2026

📝 Walkthrough

ウォークスルー

Codeception テスト実行前に、doctrine コマンドとキャッシュクリアコマンドをGitHub Actions ワークフローに追加しました。データベーススキーマのリセット、再作成、フィクスチャの読み込み、キャッシュクリアをマトリックスベースの実行と特定グループ実行の前に実行します。

変更内容

コホート / ファイル 概要
GitHub Actions ワークフロー設定
.github/workflows/e2e-test.yml
Codeception テスト実行前に、doctrine:schema:drop --force --env=devdoctrine:schema:create --env=deveccube:fixtures:load --env=devcache:clear --no-warmup --env=dev の4つのコマンドを追加。マトリックスベース実行と特定グループ(restrict-fileupload/change-display-order など)の実行前に同じセットアップコマンドを実行するよう構成。

推定レビュー工数

🎯 2 (Simple) | ⏱️ ~10 minutes

推奨ラベル

bug:Low

推奨レビュアー

  • ji-eunsoo
  • saori-kakiuchi

🐰 dev のデータベース、ぱあっときれい
スキーマ落として、スキーマ作って
フィクスチャ読み込んで、キャッシュも消す
テスト走らせて、心も晴れやか ✨
新しい朝が来た、テストの世界に

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: adding DB reset to E2E test retry mechanism in CI workflow.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

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

📥 Commits

Reviewing files that changed from the base of the PR and between ff712cb and 1b69a77.

📒 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
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

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=codeception

Also 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.

Comment on lines +191 to +192
bin/console eccube:fixtures:load --env=dev
bin/console cache:clear --no-warmup --env=dev
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

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
Copy link

codecov bot commented Mar 10, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 79.33%. Comparing base (44cb414) to head (1b69a77).
⚠️ Report is 94 commits behind head on 4.3.

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     
Flag Coverage Δ
Unit 79.33% <ø> (+0.50%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@dotani1111 dotani1111 added this to the 4.4.0 milestone Mar 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant