Feature: Add Tests for Extract Action#178
Merged
Merged
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces comprehensive feature tests for the extraction actions across different sites (Japan, Portal, and Twitrans), verifying content extraction, last modified date parsing, and handler invocation. The review feedback suggests refactoring ExtractActionTest.php to use Mockery instead of manual anonymous classes and state objects, which would make the tests more concise and idiomatic.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
Comment on lines
+17
to
+63
| public function test_invokes_handlers_for_all_sites_when_null_provided(): void | ||
| { | ||
| $state = (object) ['japan' => false, 'portal' => false, 'twitrans' => false]; | ||
|
|
||
| $this->app->bind(Handler::class, function () use ($state) { | ||
| return new class($state) implements HandlerInterface | ||
| { | ||
| public function __construct(private object $state) {} | ||
|
|
||
| public function __invoke(LoggerInterface $logger): void | ||
| { | ||
| $this->state->japan = true; | ||
| } | ||
| }; | ||
| }); | ||
|
|
||
| $this->app->bind(\App\Actions\Extract\Portal\Handler::class, function () use ($state) { | ||
| return new class($state) implements HandlerInterface | ||
| { | ||
| public function __construct(private object $state) {} | ||
|
|
||
| public function __invoke(LoggerInterface $logger): void | ||
| { | ||
| $this->state->portal = true; | ||
| } | ||
| }; | ||
| }); | ||
|
|
||
| $this->app->bind(\App\Actions\Extract\Twitrans\Handler::class, function () use ($state) { | ||
| return new class($state) implements HandlerInterface | ||
| { | ||
| public function __construct(private object $state) {} | ||
|
|
||
| public function __invoke(LoggerInterface $logger): void | ||
| { | ||
| $this->state->twitrans = true; | ||
| } | ||
| }; | ||
| }); | ||
|
|
||
| $action = app(ExtractAction::class); | ||
| $action(null, new NullLogger); | ||
|
|
||
| $this->assertTrue($state->japan); | ||
| $this->assertTrue($state->portal); | ||
| $this->assertTrue($state->twitrans); | ||
| } |
There was a problem hiding this comment.
ハンドラーが呼び出されたことを検証するために、冗長な匿名クラスや手動の $state オブジェクトを使用する代わりに、Mockery を使用してハンドラークラスをモック化し、期待値（once()）を設定することをお勧めします。これにより、テストがより簡潔で読みやすく、かつ慣用的（idiomatic）になります。
public function test_invokes_handlers_for_all_sites_when_null_provided(): void
{
$japanHandler = \Mockery::mock(Handler::class);
$japanHandler->shouldReceive('__invoke')->once();
$this->app->instance(Handler::class, $japanHandler);
$portalHandler = \Mockery::mock(\App\Actions\Extract\Portal\Handler::class);
$portalHandler->shouldReceive('__invoke')->once();
$this->app->instance(\App\Actions\Extract\Portal\Handler::class, $portalHandler);
$twitransHandler = \Mockery::mock(\App\Actions\Extract\Twitrans\Handler::class);
$twitransHandler->shouldReceive('__invoke')->once();
$this->app->instance(\App\Actions\Extract\Twitrans\Handler::class, $twitransHandler);
$action = app(ExtractAction::class);
$action(null, new NullLogger);
}
Comment on lines
+65
to
+111
| public function test_invokes_specific_handler_when_site_provided(): void | ||
| { | ||
| $state = (object) ['japan' => false, 'portal' => false, 'twitrans' => false]; | ||
|
|
||
| $this->app->bind(Handler::class, function () use ($state) { | ||
| return new class($state) implements HandlerInterface | ||
| { | ||
| public function __construct(private object $state) {} | ||
|
|
||
| public function __invoke(LoggerInterface $logger): void | ||
| { | ||
| $this->state->japan = true; | ||
| } | ||
| }; | ||
| }); | ||
|
|
||
| $this->app->bind(\App\Actions\Extract\Portal\Handler::class, function () use ($state) { | ||
| return new class($state) implements HandlerInterface | ||
| { | ||
| public function __construct(private object $state) {} | ||
|
|
||
| public function __invoke(LoggerInterface $logger): void | ||
| { | ||
| $this->state->portal = true; | ||
| } | ||
| }; | ||
| }); | ||
|
|
||
| $this->app->bind(\App\Actions\Extract\Twitrans\Handler::class, function () use ($state) { | ||
| return new class($state) implements HandlerInterface | ||
| { | ||
| public function __construct(private object $state) {} | ||
|
|
||
| public function __invoke(LoggerInterface $logger): void | ||
| { | ||
| $this->state->twitrans = true; | ||
| } | ||
| }; | ||
| }); | ||
|
|
||
| $action = app(ExtractAction::class); | ||
| $action(SiteName::Japan, new NullLogger); | ||
|
|
||
| $this->assertTrue($state->japan); | ||
| $this->assertFalse($state->portal); | ||
| $this->assertFalse($state->twitrans); | ||
| } |
There was a problem hiding this comment.
同様に、Mockery を使用して特定のハンドラーのみが呼び出され（once()）、他のハンドラーは呼び出されないこと（never()）を検証できます。これにより、テストコードが大幅にシンプルになります。
public function test_invokes_specific_handler_when_site_provided(): void
{
$japanHandler = \Mockery::mock(Handler::class);
$japanHandler->shouldReceive('__invoke')->once();
$this->app->instance(Handler::class, $japanHandler);
$portalHandler = \Mockery::mock(\App\Actions\Extract\Portal\Handler::class);
$portalHandler->shouldReceive('__invoke')->never();
$this->app->instance(\App\Actions\Extract\Portal\Handler::class, $portalHandler);
$twitransHandler = \Mockery::mock(\App\Actions\Extract\Twitrans\Handler::class);
$twitransHandler->shouldReceive('__invoke')->never();
$this->app->instance(\App\Actions\Extract\Twitrans\Handler::class, $twitransHandler);
$action = app(ExtractAction::class);
$action(SiteName::Japan, new NullLogger);
}
Owner
Author
|
ご指摘ありがとうございます！ご提案の通り、ExtractActionTest.php を Mockery を利用してリファクタリングし、匿名クラスと状態オブジェクトを削除してシンプルにしました。修正をPushしました。 |
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
2 participants
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.
ExtractActionおよび関連する各サイト（Japan, Portal, Twitrans）のExtractContents、ExtractLastModified処理に対するテストを追加しました。