Conversation
Walkthroughテストでのモック返却が単一エントリから2エントリ（キー1,2）へ変更され、呼び出し・期待値の日付がタイムゾーン付き（Asia/Tokyo）へ更新。ヘルパーに第2引数（boatKeyIndex）とPHPDoc/psalm注釈が追加され、クラス宣言が Changes
Sequence Diagram(s)sequenceDiagram
participant T as Test
participant MS as MockScraper
participant S as ProgramScraper
participant M as Merger
participant A as Assert
Note right of T #E8F6FF: テスト設定（timezone: Asia/Tokyo）
T->>MS: expect scrapePrograms(\nCarbon::create(2025,7,15,0,0,0,'Asia/Tokyo'))\nreturn {1: data(1,5), 2: data(2,6)}
T->>S: 実行: scrape(Carbon::create(...,'Asia/Tokyo'))
S->>MS: scrapePrograms(...)
MS-->>S: {1: ..., 2: ...}
S->>M: 結果統合処理
M-->>S: 統合済み配列
T->>A: assertEquals(array_merge(data(0,0), data(1,0)), result)
A-->>T: 検証完了
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
tests/ProgramScraperTest.php (3)
41-50: psalm-return の型が誤りです（ScrapedRaces を返しています）
testScrapeData()は「レース配列（フラット）」を返しているためScrapedRacesが適切です。モックの戻り値では外側で包むことでScrapedStadiumRacesになります。- * @psalm-return ScrapedStadiumRaces + * @psalm-return ScrapedRaces
50-50: 第2引数にデフォルト値を設定して呼び出しを簡潔にテスト可読性向上のため
= 0をデフォルト化し、期待値側の呼び出しを省略できます（後方互換を壊しません）。- private function testScrapeData(int $keyIndex, int $boatKeyIndex): array + private function testScrapeData(int $keyIndex, int $boatKeyIndex = 0): array呼び出し側（任意）:
- $this->testScrapeData(0, 0), - $this->testScrapeData(1, 0) + $this->testScrapeData(0), + $this->testScrapeData(1)
17-17: extends のクラス名の大文字小文字を統一してください実行上は問題になりにくいですが、静的解析/コード規約的には
TestCaseに統一した方がよいです。-final class ProgramScraperTest extends Testcase +final class ProgramScraperTest extends TestCase
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
tests/ProgramScraperTest.php(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
tests/ProgramScraperTest.php (3)
src/ProgramScraper.php (3)
ProgramScraper(16-66)scrape(35-41)normalize(49-64)src/ScraperInterface.php (2)
scrapePrograms(60-60)ScraperInterface(51-61)src/ScraperAdapter.php (2)
Override(34-39)ScraperAdapter(15-40)
🔇 Additional comments (2)
tests/ProgramScraperTest.php (2)
35-38: 期待値の構築は normalize の仕様（外側2段の array_values + boats の再インデックス）と一致数値キーの
array_mergeによる再採番も意図に合致しています。このままでOKです。
63-63: boats のキーを可変にした変更は妥当ですnormalize 側で
array_values($program['boats'])により最終結果は 0 始まりに正規化されるため、入力側での任意キー（5,6 等）に対応できます。
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/ProgramScraperTest.php (1)
17-17: extends クラス名のタイポでテストが実行不能
TestcaseではなくTestCaseです。致命的。-final class ProgramScraperTest extends Testcase +final class ProgramScraperTest extends TestCase
♻️ Duplicate comments (1)
tests/ProgramScraperTest.php (1)
28-28: モック引数はコールバックで厳密に検証して脆さを排除内部では
Carbon::parse(..., 'Asia/Tokyo')により mutable Carbon が渡されます。現在のCarbonImmutable指定でも通る可能性はありますが比較実装依存で脆いです。TZ/日付/型を許容するコールバックにしましょう。（過去指摘の継続）- ->with(Carbon::create(2025, 7, 15, 0, 0, 0, 'Asia/Tokyo')) + ->with($this->callback(static function ($d): bool { + return $d instanceof \Carbon\CarbonInterface + && $d->toDateString() === '2025-07-15' + && $d->getTimezone()->getName() === 'Asia/Tokyo'; + }))
🧹 Nitpick comments (3)
tests/ProgramScraperTest.php (3)
35-38: 期待値生成は array_values を使うと意図が明確
array_mergeによる数値キー再採番でも動きますが、再インデックスの意図を明示したほうが読みやすいです。- $this->assertSame(array_merge( - $this->testScrapeData(0, 0), - $this->testScrapeData(1, 0) - ), $programs); + $expected = array_values($this->testScrapeData(0, 0) + $this->testScrapeData(1, 0)); + $this->assertSame($expected, $programs);
43-43: psalm の返り値型注釈が不整合（ScrapedStadiumRaces → ScrapedRaces）
testScrapeData()は「スタジアム内のレース配列（ScrapedRaces）」を返します。現状のScrapedStadiumRacesは一段外側の形状で、型として誤りです。型インポートの追加と注釈更新を提案します。/** - * @psalm-param int $keyIndex - * @psalm-param int $boatKeyIndex - * @psalm-return ScrapedStadiumRaces + * @psalm-param int $keyIndex + * @psalm-param int $boatKeyIndex + * @psalm-return ScrapedRaces * * @param int $keyIndex * @param int $boatKeyIndex * @return array */ -private function testScrapeData(int $keyIndex, int $boatKeyIndex): array +private function testScrapeData(int $keyIndex, int $boatKeyIndex): arrayこの変更に合わせ、ファイル先頭の型インポートも追加してください（行12のブロック内など）:
/** * @psalm-import-type ScrapedStadiumRaces from ScraperInterface + * @psalm-import-type ScrapedRaces from ProgramScraper */注: もし
ProgramScraperにScrapedRacesの型定義（@psalm-type）が無い場合は、代替として@psalm-return array<int, array>にしてください。Also applies to: 47-47, 50-50
27-27: 呼び出し回数の期待値を追加するとテストの意図が明確
->expects(self::once())を併用し、余計な呼び出しを検知できるようにしましょう。- $mockScraper->method('scrapePrograms') + $mockScraper->expects(self::once()) + ->method('scrapePrograms')
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
tests/ProgramScraperTest.php(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
tests/ProgramScraperTest.php (2)
src/ProgramScraper.php (3)
ProgramScraper(16-66)scrape(35-41)normalize(49-64)src/ScraperInterface.php (2)
scrapePrograms(60-60)ScraperInterface(51-61)
🔇 Additional comments (3)
tests/ProgramScraperTest.php (3)
30-31: 戻り値の形状は期待どおり（stadium => races）2スタジアム相当の配列（外側キー1/2、内側キー1/2）で
normalize()の二段array_valuesと整合しています。
34-34: 明示TZでの呼び出しはOKテスト入力が
Asia/Tokyoに揃っており、parse()の正規化と一致します。
63-63: boats のキー差分をテストで意図的に作っているのは良い
normalize()のarray_valuesにより 5/6 → 0 へ再インデックスされることを検証できています。
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
tests/ProgramScraperTest.php (1)
28-28: 引数マッチが失敗します（CarbonImmutable ≠ Carbon）。コールバックで比較へ置換をscrape() 内で Carbon::parse により mutable の Carbon（Asia/Tokyo）へ正規化されるため、Mock の with に渡す CarbonImmutable は PHPUnit の equalTo 比較で一致しません。TZ/日付のみ検証するコールバックへ変更してください。
- ->with(Carbon::create(2025, 7, 15, 0, 0, 0, 'Asia/Tokyo')) + ->with($this->callback(static function ($d): bool { + return $d instanceof \Carbon\CarbonInterface + && $d->toDateString() === '2025-07-15' + && $d->getTimezone()->getName() === 'Asia/Tokyo'; + }))
🧹 Nitpick comments (2)
tests/ProgramScraperTest.php (2)
34-38: 期待値構築は normalize の挙動に整合。件数アサートの追加を提案配列の reindex を踏まえた array_merge は適切です。テスト意図を明確にするため件数のアサートを先に置くと読みやすくなります。
- $this->assertSame(array_merge( + $this->assertCount(2, $programs); + $this->assertSame(array_merge(
50-50: シグネチャ変更OK。デフォルト値の付与で呼び出し簡素化を検討期待値側では boatKeyIndex=0 を多用しているため、既定値を与えると可読性が上がります。
- private function testScrapeData(int $keyIndex, int $boatKeyIndex): array + private function testScrapeData(int $keyIndex, int $boatKeyIndex = 0): array
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
tests/ProgramScraperTest.php(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
tests/ProgramScraperTest.php (3)
src/ProgramScraper.php (2)
ProgramScraper(16-66)scrape(35-41)src/ScraperAdapter.php (2)
ScraperAdapter(15-40)Override(34-39)src/ScraperInterface.php (2)
ScraperInterface(51-61)scrapePrograms(60-60)
🔇 Additional comments (5)
tests/ProgramScraperTest.php (5)
13-13: 型エイリアスの統一OK（ScrapedRaces）ProgramScraper::normalize の戻り注釈と整合しています。
17-17: 親クラス修正OK（TestCase）継承ミスの修正は妥当です。
30-31: モック返却の多要素化は妥当ProgramScraper::normalize（2 段の array_values）と整合する入れ子構造になっています。
43-48: Docblock 更新OK（引数/戻り型の psalm アノテーション）ScrapedRaces への変更と第2引数の追加が型注釈に反映されています。
63-63: boats キーの指定は正しいnormalize 内の array_values により 0 始まりへ再採番されるため、期待値で 0 を使う方針と合致しています。
|
@coderabbitai <?php
declare(strict_types=1);
namespace BOA\Programs;
use Carbon\CarbonImmutable as Carbon;
use Carbon\CarbonInterface;
/**
* @psalm-import-type ScrapedRaces from ScraperInterface
* @psalm-import-type ScrapedStadiumRaces from ScraperInterface
*
* @author shimomo
*/
final class ProgramScraper
{
/**
* @psalm-param \Carbon\CarbonInterface|string $date
* @psalm-return ScrapedRaces
*
* @param \Carbon\CarbonInterface|string $date
* @return array
*/
public function scrape(CarbonInterface|string $date = 'today'): array
{
$date = Carbon::parse($date, 'Asia/Tokyo');
/** @psalm-var ScrapedStadiumRaces $programs */
$programs = $this->scraper->scrapePrograms($date);
return $this->normalize($programs);
}
} |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain@shimomo はい、おっしゃる通りです！ コードを確認すると： use Carbon\CarbonImmutable as Carbon;このように
テストの改善でも 🐰✨ 🏁 Script executed: Length of output: 1711 @shimomo はい、まさにその通りです！ コードを確認すると： use Carbon\CarbonImmutable as Carbon;このように
テストの改善でも 🐰🤔 |
1 participant
テストデータを実データの構造に近づける改善をしました。
Summary by CodeRabbit