Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Set cookie lifetime on test #4730

Merged
merged 1 commit into from
Oct 12, 2020
Merged

Conversation

okazy
Copy link
Contributor

@okazy okazy commented Oct 9, 2020

概要(Overview・Refs Issue)

Cookieの持続時間は ECCUBE_COOKIE_LIFETIME の環境変数で指定できる。
デフォルトでは0、つまりブラウザ終了でcookieが失効して再ログインが必要な設定となっている。

env(ECCUBE_COOKIE_LIFETIME): 0

テストで利用しているSymfony BrowserKitのcookieの失効条件が見直され、 $this->expires < time() から $this->expires <= time() となった。
つまり有効期限が現在時刻のcookieは無効となった。

symfony/symfony#38360

一般的なブラウザでアクセス時はブラウザを終了しない限りcookieは保持されるが、テストで利用しているSymfony BrowserKitは リクエスト -> 確認 -> リクエスト -> 確認 -> ... を繰り返し、リクエストのたびにcookieの有効性が評価されるようになっている。
cookieの有効期限が現在時刻だとセッションが保持されないためテストが通らなくなっていた。

影響があったテストケース

Eccube\Tests\Web\ProductControllerTest::testProductFavoriteAddThroughLogin

影響があるのは以下

  • Symfony 3.4.46以降(まだリリースしていない)
  • Symfony 4.4.15以降

方針(Policy)

testの環境にcookieの有効期限を設定

実装に関する補足(Appendix)

Symfony3.4.46はまだリリースされていないため現時点でテストは落ちていませんが、いずれ対応が必要になるためプルリクしました。

テスト(Test)

相談(Discussion)

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

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

レビュワー確認項目

  • 動作確認
  • コードレビュー
  • E2E/Unit テスト確認(テストの追加・変更が必要かどうか)
  • 互換性が保持されているか
  • セキュリティ上の問題がないか

@okazy okazy added the improvement 機能改善 label Oct 9, 2020
@okazy okazy added this to the 4.0.x milestone Oct 9, 2020
@kiy0taka kiy0taka merged commit e3d7ebd into EC-CUBE:4.0 Oct 12, 2020
@okazy okazy modified the milestones: 4.0.x, 4.0.6 Oct 12, 2020
@chihiro-adachi chihiro-adachi modified the milestones: 4.0.6, 4.1 Jul 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
improvement 機能改善
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants