fix: データプロバイダで日時を確定していた不安定なテストを修正 - #4516
Merged
Merged
Conversation
SearchIndexesTableTest::testAllowPublish と ContentsTableTest::testIsPublish で、
データプロバイダ内に date('Y-m-d H:i:s') で日時を生成していたため、
タイミングによってテストが失敗していた。
データプロバイダはテストスイート構築時に一度だけ評価されるため、そこで
「+1 hour」等の日時を確定させると、テスト本体が実行されるまでの経過時間が
1時間を超えた時点で未来日時が過去日時に変わり、判定結果が反転する。
全体テストのように実行時間の長いケースで発生していた。
また、公開開始日時に「現在時刻」を指定して公開中を期待していたケースは、
実装が publish_begin >= 現在日時 を未公開と判定するため、データプロバイダと
テスト本体が同一秒に実行されると失敗する状態だった。
対応として、日時をデータプロバイダに持たせず、テスト実行時点で生成するように
変更した。あわせて境界値ではなく前後1時間の値を用いることで、判定意図を
明確にしている。
- SearchIndexesTableTest: 相対指定を受け取りテスト本体で日時へ変換する方式に変更
- ContentsTableTest: 相対日時のケースを testIsPublishWithPublishPeriod へ分離
- assertEquals の引数順が期待値・実測値で逆になっていた点もあわせて修正
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
ContentsTableTest で FrozenTime::now() を使っているため、テストブートストラップで Chronos::setTestNow() が固定される環境では長時間実行時に再び不安定化する可能性があります。
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
長時間実行時に失敗しうるユニットテスト(公開期間判定)の不安定要因を解消し、期待値と実測値の比較が分かりやすい形になるように調整するPRです。
Changes:
SearchIndexesTableTestのデータプロバイダを「相対日時指定」に変更し、テスト実行時に日時へ変換することで時間経過依存を排除SearchIndexesTableTestのassertEquals()の引数順(期待値→実測値)を修正ContentsTableTestから相対日時ケースを分離し、公開期間指定の専用テストを追加してケースを拡充
File summaries
| File | Description |
|---|---|
| plugins/bc-search-index/tests/TestCase/Model/Table/SearchIndexesTableTest.php | データプロバイダの相対日時化+実行時変換でテストの時間依存を解消し、アサーションの引数順も是正 |
| plugins/baser-core/tests/TestCase/Model/Table/ContentsTableTest.php | 相対日時ケースを専用テストへ分離し、公開期間指定時のケースを追加 |
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
ContentsTableTest::testIsPublishWithPublishPeriod で FrozenTime::now() を 基準に日時を生成していたが、tests/bootstrap.php の Chronos::setTestNow() に より FrozenTime の現在日時はテスト起動時点で固定される。 一方、判定対象の ContentsTable::isPublish() は date() による実時間で比較する ため、テストスイートの実行が1時間を超えると FrozenTime::now()->addHours(1) が 実時間では過去となり、再び不安定になる状態だった。 FrozenTime も実時間から生成した日時文字列を基準とするよう変更し、文字列の ケースと同じ基準時刻で評価されるようにした。 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
BlogPostsTableTest::testAllowPublish のデータプロバイダで
new \Cake\I18n\DateTime('+1 hour') により日時を生成していたため、
CI の Unit Test (8.2) で data set #2 #5 #6 #10 が失敗していた。
原因は2つが重なっている。
1. データプロバイダはテストスイート構築時に一度だけ評価されるため、そこで
確定した「1時間後」は、テスト本体の実行までに1時間以上経過すると
過去日時になる。当該ジョブは全体テストで2時間8分かかっていた。
2. tests/bootstrap.php の Chronos::setTestNow() により Chronos の現在日時は
テスト起動時点で固定される。そのため `new DateTime('+1 hour')` のような
相対指定は起動時点を基準に解釈される。一方 BlogPostsTable::allowPublish()
は time() による実時間で比較するため、両者がズレる。
対応として、日時をデータプロバイダに持たせず、実時間を基準にテスト実行時点で
生成するよう変更した。あわせて各データセットに判定意図のコメントを追加し、
assertEquals の引数順が期待値・実測値で逆になっていた点も修正した。
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
UploaderHelperTest::testIsPublish のデータプロバイダで
new FrozenTime('+1 day') により日時を生成していた。
BlogPostsTableTest と同じアンチパターンで、以下の2点により実時間とズレる。
- データプロバイダはテストスイート構築時に一度だけ評価されるため、そこで
確定した相対日時はテスト本体の実行までの経過時間の影響を受ける
- tests/bootstrap.php の Chronos::setTestNow() により Chronos の現在日時は
テスト起動時点で固定されるため、相対指定は起動時点を基準に解釈される。
一方 UploaderHelper::isPublish() は date() による実時間で比較する
マージンが1日あるため実際の失敗は確認されていないが、同じ不具合の芽を残さない
よう、日時を実時間基準でテスト実行時点に生成する方式へ揃えた。
これにより、データプロバイダ内で日時を確定させる箇所はリポジトリから解消した。
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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
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.
概要
実行タイミングによって失敗することがあったユニットテストを修正しました。データプロバイダ内で日時を確定させていた箇所を、リポジトリ全体で解消しています。
原因
公開期間の判定テストで、データプロバイダ内に相対日時(
+1 hourなど)を生成していたことが原因です。次の2つが重なっています。① データプロバイダは一度だけ評価される
データプロバイダはテストスイート構築時に一度だけ評価されるため、その時点で確定した「1時間後」は、テスト本体が実行されるまでの経過時間が1時間を超えると過去日時になります。全体テストのように実行時間の長いケースで顕在化します。
② Chronos の相対指定は実時間とズレる
tests/bootstrap.php:73のChronos::setTestNow(Chronos::now())により、Chronos の現在日時はテスト起動時点で固定されます。実測すると次のようになります。一方、判定対象の
allowPublish()/isPublish()はdate()やtime()による実時間で比較するため、両者が常にズレます。変更内容
日時をデータプロバイダに持たせず、実時間を基準にテスト実行時点で生成する方式へ統一しました。あわせて各データセットに判定意図のコメントを付け、
assertEquals()の引数順が期待値・実測値で逆になっていた箇所も修正しています。SearchIndexesTableTest::testAllowPublishContentsTableTest::testIsPublishBlogPostsTableTest::testAllowPublishUploaderHelperTest::testIsPublishbc-search-index
データプロバイダは相対指定のみを持ち、テスト本体で日時へ変換します。
baser-core
isPublishDataProvider()から相対日時のケースをtestIsPublishWithPublishPeriod()へ分離し、データプロバイダには固定値のみを残しました。分離にあたり、これまで無かった「公開開始日時が過去」「公開終了日時が過去」「公開期間内」「公開期間終了後」のケースも追加しています。bc-blog / bc-uploader
new DateTime('+1 hour')/new FrozenTime('+1 day')を、実時間から生成した日時文字列を基準に構築する方式へ変更しました。理由は各ヘルパーメソッドの docblock に記載しています。検証
「テスト起動から2時間経過」を再現し、修正前後の挙動を確認しました。
変更した4ファイルのテスト結果です。
※ Incomplete は既存のスキップです。
修正後、データプロバイダ内で日時を確定させている箇所が残っていないことを横断検索で確認しています。
🤖 Generated with Claude Code