pkp/pkp-lib#10306 unit tests for queue jobs #10340
Conversation
touhidurabir
force-pushed
the
i10306_main
branch
from
d670e83
to
2fd4f04
Compare
| if (PKPContainer::getInstance()->runningUnitTests()) { | ||
| $client = Registry::get(\PKP\tests\PKPTestCase::MOCKED_GUZZLE_CLIENT_NAME); | ||
| if ($client) { | ||
| return $client; | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
part of functionality to mocking ability for guzzel request .
| @@ -56,7 +56,7 @@ public function updateNotification(PKPRequest $request, ?array $userIds, int $as | |||
| * | |||
| * @copydoc PKPNotificationOperationManager::createNotification() | |||
| */ | |||
| public function createNotification(PKPRequest $request, ?int $userId = null, ?int $notificationType = null, ?int $contextId = Application::SITE_CONTEXT_ID, ?int $assocType = null, ?int $assocId = null, int $level = Notification::NOTIFICATION_LEVEL_NORMAL, ?array $params = null): ?Notification | |||
| public function createNotification(?PKPRequest $request = null, ?int $userId = null, ?int $notificationType = null, ?int $contextId = Application::SITE_CONTEXT_ID, ?int $assocType = null, ?int $assocId = null, int $level = Notification::NOTIFICATION_LEVEL_NORMAL, ?array $params = null): ?Notification | |||
There was a problem hiding this comment.
do we have any use of PKPRequest $request here as it seems this data not used ?
There was a problem hiding this comment.
You're right -- this parameter can be removed.
tests/PKPTestCase.php
Outdated
| @@ -205,8 +220,7 @@ protected function mockRequest($path = 'index/test-page/test-op', $userId = null | |||
| $request->setRouter($router); | |||
|
|
|||
| // Test user. | |||
| $session = $request->getSession(); | |||
| $session->setUserId($userId); | |||
| $request->getSessionGuard()->setUserId($userId); | |||
There was a problem hiding this comment.
a leftover missing change from #9566
| $bulkEmailSenderJob->handle(); | ||
|
|
||
| $this->expectNotToPerformAssertions(); |
There was a problem hiding this comment.
Our previous asserting was like $this->assertNull($bulkEmailSenderJob->handle()) . However the handle method return type set as void which does turn in null and older PHPUnit version did not have a arg type defined . But the latest PHPUnit version has arg type define as mixed which in turn identified as error as return type void does not match with arg time mixed . Though this does not cause exception but modern IDEs identify it as error . So probably better to go with approach here and all other places .
touhidurabir
force-pushed
the
i10306_main
branch
2 times, most recently
from
6677802
to
f3083e3
Compare
| * Override Laravel method; always false. | ||
| * Prevents the undefined method error when the Log Manager tries to determine the driver | ||
| */ | ||
| public function runningUnitTests(): bool |
There was a problem hiding this comment.
This could be removed once we add and configure the Laravel logging service provider, correct? I think this is related to what you're talking about with Erik. It might be worth filing in Github, noting this function in the issue, and adding a FIXME here.
There was a problem hiding this comment.
Not exactly , this has there before this PR and probably will stay . Our Container class extends the laravel core container but We have our very own Application class . Once we able to handle the bootstrapping process to Laravel's foundation Application, we can remove this (as it's reside there) . As few core class required this information, so we have copied it from Laravel's foundation application class to our PKPContainer just to facilitate the dependency.
tests/PKPTestCase.php
Outdated
|
|
||
| /** | ||
| * Mock the mail facade | ||
| * @see https://laravel.com/docs/10.x/mocking |
There was a problem hiding this comment.
Better to link to 11, since that's what we're shipping.
| /** @var NewAnnouncementNotifyUsers $newAnnouncementNotifyUsersJob */ | ||
| $newAnnouncementNotifyUsersJob = unserialize($this->serializedJobData); | ||
|
|
||
| $announcementMock = Mockery::mock(\PKP\announcement\Announcement::class) |
There was a problem hiding this comment.
This is going to require some work once #10328 is merged.
touhidurabir
force-pushed
the
i10306_main
branch
from
994bc97
to
120f66a
Compare
touhidurabir
force-pushed
the
i10306_main
branch
from
120f66a
to
2cad824
Compare
for #10306