From 8a43297e247a67965d57f54557bff9e438a707f6 Mon Sep 17 00:00:00 2001 From: hamed <15238-genius.ha@users.noreply.drupalcode.org> Date: Tue, 18 Aug 2026 09:57:29 +0330 Subject: [PATCH] test: remove the two real sources of full-run flakiness, and the mock notices MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit createUser() drew a random mobile and recovered from a collision by catching the unique-constraint violation and calling resetManager(). That hands back a brand-new EntityManager, which detaches every entity the running test had built so far; its next flush died with "Multiple non-persisted new entities were found", always in a different test and never reproducible in isolation. The number is now checked before the insert, so the collision never reaches the database and the manager stays open. testParentIdAddsNoQueryPerSpecialty counted queries on the first request of each size, so one-shot per-process caches — site config, subscription plan, Doctrine metadata — landed inside the count or not depending on which tests had run before it. Both requests are now warmed first; the assertion measures steady-state growth, which is what it was always about. The 23 PHPUnit notices were all one complaint: doubles created with createMock() that never had an expectation. The ones that only stub return values became createStub(); in SmsServiceLookupOnlyTest the provider and the bus got the expectations they were missing, since "dispatch does not touch the provider" and "sendNow does not enqueue" are exactly what that suite is there to prove. Co-Authored-By: Claude Opus 5 (1M context) --- tests/ApiTestCase.php | 35 +++++++++----------- tests/Doctor/DoctorClaimTest.php | 2 +- tests/Doctor/DoctorSpecialtySearchTest.php | 7 ++++ tests/Payment/MellatGatewayTest.php | 6 ++-- tests/Shared/HealthControllerTest.php | 4 +-- tests/Sms/SmsServiceLookupOnlyTest.php | 21 ++++++++---- tests/Subscription/ActivateTrialTest.php | 10 +++--- tests/Subscription/GrantSubscriptionTest.php | 22 ++++++------ 8 files changed, 59 insertions(+), 48 deletions(-) diff --git a/tests/ApiTestCase.php b/tests/ApiTestCase.php index 5f52b421..45f8e9e1 100644 --- a/tests/ApiTestCase.php +++ b/tests/ApiTestCase.php @@ -11,7 +11,6 @@ use App\Doctor\Entity\Doctor; use App\Payment\Entity\Payment; use App\Shared\Context\EntityContext; use App\Subscription\Entity\SubscriptionPlan; -use Doctrine\DBAL\Exception\UniqueConstraintViolationException; use Doctrine\ORM\EntityManagerInterface; use Lexik\Bundle\JWTAuthenticationBundle\Services\JWTTokenManagerInterface; use Symfony\Bundle\FrameworkBundle\KernelBrowser; @@ -126,27 +125,23 @@ abstract class ApiTestCase extends WebTestCase } // 9 random digits after 09 (full ^09\d{9}$ space). db_test is never reset and - // already holds tens of thousands of users, so a draw does collide now and - // then; retry rather than fail an unrelated test on a birthday collision. + // already holds tens of thousands of users, so a draw does collide now and then. + // + // شماره پیش از insert بررسی می‌شود، نه بعد از شکستنِ قید یکتا. برخوردِ واقعی + // EntityManager را می‌بست و راه ترمیمش `resetManager()` بود — که مدیر تازه‌ای + // می‌سازد و همهٔ موجودیت‌هایی را که تستِ جاری تا آن لحظه ساخته بود detach + // می‌کند. اولین flush بعدی با «Multiple non-persisted new entities were found» + // می‌ترکید؛ همان خطای تصادفی که هر بار روی تستِ دیگری می‌افتاد و در اجرای تکی + // هرگز تکرار نمی‌شد. حالا برخورد اصلاً به دیتابیس نمی‌رسد. for ($attempt = 0; ; $attempt++) { - try { - return $this->persistUser( - '09' . str_pad((string) random_int(0, 999_999_999), 9, '0', STR_PAD_LEFT), - $roles, - ); - } catch (UniqueConstraintViolationException $e) { - if ($attempt >= 4) { - throw $e; - } + $mobile = '09' . str_pad((string) random_int(0, 999_999_999), 9, '0', STR_PAD_LEFT); - // The failed INSERT closes the EntityManager, and asking the container - // for it again hands back the *same closed instance* — Doctrine only - // builds a fresh one when the registry is reset. Without this the retry - // throws EntityManagerClosed, and every later test in the same process - // inherits a dead manager: the intermittent, always-somewhere-else - // failure that made full runs flaky. - static::getContainer()->get('doctrine')->resetManager(); - $this->em = static::getContainer()->get(EntityManagerInterface::class); + if ($this->em->getRepository(User::class)->findOneBy(['mobileNumber' => $mobile]) === null) { + return $this->persistUser($mobile, $roles); + } + + if ($attempt >= 20) { + throw new \RuntimeException('could not draw a free test mobile number after 20 attempts'); } } } diff --git a/tests/Doctor/DoctorClaimTest.php b/tests/Doctor/DoctorClaimTest.php index 25b998ee..0e784ae0 100644 --- a/tests/Doctor/DoctorClaimTest.php +++ b/tests/Doctor/DoctorClaimTest.php @@ -22,7 +22,7 @@ class DoctorClaimTest extends ApiTestCase private function mockApiIr(bool $shahkar = true, ?array $person = ['firstName' => 'تست', 'lastName' => 'ایمپورت', 'alive' => true]): void { - $mock = $this->createMock(ApiIrService::class); + $mock = $this->createStub(ApiIrService::class); $mock->method('isConfigured')->willReturn(true); $mock->method('shahkarMatch')->willReturn($shahkar); $mock->method('personInfo')->willReturn($person); diff --git a/tests/Doctor/DoctorSpecialtySearchTest.php b/tests/Doctor/DoctorSpecialtySearchTest.php index 8ec1791b..ff6d7317 100644 --- a/tests/Doctor/DoctorSpecialtySearchTest.php +++ b/tests/Doctor/DoctorSpecialtySearchTest.php @@ -222,6 +222,13 @@ class DoctorSpecialtySearchTest extends ApiTestCase $this->newDoctor('دکتر شمارش ' . uniqid(), [$t['root'], $t['childA'], $t['childB']]); } + // اجرای گرم‌کننده پیش از شمارش: کوئری‌های یک‌بارهٔ هر پردازه — پیکربندی سایت، + // پلن اشتراک، متادیتای Doctrine — بسته به اینکه کدام تست‌ها زودتر اجرا شده‌اند + // پر یا خالی‌اند. بدون این گرم‌کردن، همان کوئری‌ها در اجرای کامل suite داخل + // شمارش می‌افتادند و تست به‌صورت تصادفی می‌شکست. اینجا رشدِ حالت پایدار مهم است. + $this->get('specialty_id=' . $t['root']->getId() . '&limit=2'); + $this->get('specialty_id=' . $t['root']->getId() . '&limit=6'); + $qSmall = $this->countQueries(fn() => $this->get('specialty_id=' . $t['root']->getId() . '&limit=2')); $qLarge = $this->countQueries(fn() => $this->get('specialty_id=' . $t['root']->getId() . '&limit=6')); diff --git a/tests/Payment/MellatGatewayTest.php b/tests/Payment/MellatGatewayTest.php index 9ac2eb0c..23077542 100644 --- a/tests/Payment/MellatGatewayTest.php +++ b/tests/Payment/MellatGatewayTest.php @@ -12,9 +12,9 @@ class MellatGatewayTest extends TestCase { public function testConstructorAcceptsNullCredentials(): void { - $httpClient = $this->createMock(HttpClientInterface::class); - $configRepo = $this->createMock(SiteConfigRepository::class); - $logger = $this->createMock(LoggerInterface::class); + $httpClient = $this->createStub(HttpClientInterface::class); + $configRepo = $this->createStub(SiteConfigRepository::class); + $logger = $this->createStub(LoggerInterface::class); $configRepo->method('get')->willReturn(null); diff --git a/tests/Shared/HealthControllerTest.php b/tests/Shared/HealthControllerTest.php index f7d7207b..84f6dea7 100644 --- a/tests/Shared/HealthControllerTest.php +++ b/tests/Shared/HealthControllerTest.php @@ -20,9 +20,9 @@ class HealthControllerTest extends TestCase { private function em(): EntityManagerInterface { - $conn = $this->createMock(Connection::class); + $conn = $this->createStub(Connection::class); $conn->method('executeQuery'); // SELECT 1 → موفق - $em = $this->createMock(EntityManagerInterface::class); + $em = $this->createStub(EntityManagerInterface::class); $em->method('getConnection')->willReturn($conn); return $em; diff --git a/tests/Sms/SmsServiceLookupOnlyTest.php b/tests/Sms/SmsServiceLookupOnlyTest.php index 0a4eceaf..7f87bcba 100644 --- a/tests/Sms/SmsServiceLookupOnlyTest.php +++ b/tests/Sms/SmsServiceLookupOnlyTest.php @@ -11,6 +11,7 @@ use App\Sms\Repository\SmsMessageTemplateRepository; use App\Sms\Service\SmsService; use App\Sms\Service\SmsTextResolver; use PHPUnit\Framework\MockObject\MockObject; +use PHPUnit\Framework\MockObject\Stub; use PHPUnit\Framework\TestCase; use Psr\Log\LoggerInterface; use Symfony\Component\Messenger\Envelope; @@ -24,8 +25,8 @@ class SmsServiceLookupOnlyTest extends TestCase private KavehNegarProvider&MockObject $kavenegar; private SmsLogRepository&MockObject $logRepo; private MessageBusInterface&MockObject $bus; - private SmsMessageTemplateRepository&MockObject $messageTemplateRepo; - private SmsTextResolver&MockObject $textResolver; + private SmsMessageTemplateRepository&Stub $messageTemplateRepo; + private SmsTextResolver&Stub $textResolver; private SmsService $service; protected function setUp(): void @@ -34,17 +35,17 @@ class SmsServiceLookupOnlyTest extends TestCase $this->kavenegar->method('getName')->willReturn('kavenegar'); $this->logRepo = $this->createMock(SmsLogRepository::class); $this->bus = $this->createMock(MessageBusInterface::class); - $this->messageTemplateRepo = $this->createMock(SmsMessageTemplateRepository::class); - $this->textResolver = $this->createMock(SmsTextResolver::class); + $this->messageTemplateRepo = $this->createStub(SmsMessageTemplateRepository::class); + $this->textResolver = $this->createStub(SmsTextResolver::class); $this->service = new SmsService( $this->kavenegar, - $this->createMock(RanginehProvider::class), + $this->createStub(RanginehProvider::class), $this->logRepo, $this->bus, $this->messageTemplateRepo, $this->textResolver, - $this->createMock(LoggerInterface::class), + $this->createStub(LoggerInterface::class), ); } @@ -52,6 +53,8 @@ class SmsServiceLookupOnlyTest extends TestCase { $this->kavenegar->expects($this->never())->method('send'); $this->kavenegar->expects($this->never())->method('sendTemplate'); + // sendNow مسیر همزمان است؛ نباید چیزی به صف بیندازد. + $this->bus->expects($this->never())->method('dispatch'); $captured = null; $this->logRepo->expects($this->once())->method('save') @@ -68,6 +71,7 @@ class SmsServiceLookupOnlyTest extends TestCase public function testSendNowWithTemplateCodeUsesLookup(): void { $this->kavenegar->expects($this->never())->method('send'); + $this->bus->expects($this->never())->method('dispatch'); $this->kavenegar->expects($this->once())->method('sendTemplate') ->with('09120000000', 'clinicpro-otp', ['token' => '1234']) ->willReturn(true); @@ -91,6 +95,9 @@ class SmsServiceLookupOnlyTest extends TestCase // TAG_GLOBAL در DEFAULTS نیست و در DB هم چیزی نداریم → باید رد شود، نه ارسال خام. $this->messageTemplateRepo->method('findByTag')->willReturn(null); $this->textResolver->method('resolve')->willReturn('any body'); + // صف‌کردن نباید در همان لحظه سراغ provider برود — نه خام و نه lookup. + $this->kavenegar->expects($this->never())->method('send'); + $this->kavenegar->expects($this->never())->method('sendTemplate'); $this->bus->expects($this->never())->method('dispatch'); $this->logRepo->expects($this->once())->method('save'); @@ -102,6 +109,8 @@ class SmsServiceLookupOnlyTest extends TestCase // TAG_OTP در DEFAULTS الگو + token_map دارد → باید با templateCode به صف برود. $this->messageTemplateRepo->method('findByTag')->willReturn(null); $this->textResolver->method('resolve')->willReturn('کد شما: 1234'); + $this->kavenegar->expects($this->never())->method('send'); + $this->kavenegar->expects($this->never())->method('sendTemplate'); $captured = null; $this->bus->expects($this->once())->method('dispatch') diff --git a/tests/Subscription/ActivateTrialTest.php b/tests/Subscription/ActivateTrialTest.php index 17007497..4c2b915c 100644 --- a/tests/Subscription/ActivateTrialTest.php +++ b/tests/Subscription/ActivateTrialTest.php @@ -25,16 +25,16 @@ class ActivateTrialTest extends TestCase ?SubscriptionPlan $basicPlan, mixed $trialPeriod, ): SubscriptionService { - $subscriptionRepo = $this->createMock(ClinicSubscriptionRepository::class); + $subscriptionRepo = $this->createStub(ClinicSubscriptionRepository::class); $subscriptionRepo->method('hasUsedTrial')->willReturn($usedTrial); - $planRepo = $this->createMock(SubscriptionPlanRepository::class); + $planRepo = $this->createStub(SubscriptionPlanRepository::class); $planRepo->method('findByName')->willReturn($basicPlan); - $periodRepo = $this->createMock(SubscriptionPeriodRepository::class); + $periodRepo = $this->createStub(SubscriptionPeriodRepository::class); $periodRepo->method('findTrialPeriodForPlan')->willReturn($trialPeriod); - $configRepo = $this->createMock(SiteConfigRepository::class); + $configRepo = $this->createStub(SiteConfigRepository::class); $configRepo->method('get')->willReturn($trialEnabled); return new SubscriptionService($subscriptionRepo, $planRepo, $periodRepo, $configRepo); @@ -45,7 +45,7 @@ class ActivateTrialTest extends TestCase $service = $this->service( usedTrial: false, trialEnabled: '1', - basicPlan: $this->createMock(SubscriptionPlan::class), + basicPlan: $this->createStub(SubscriptionPlan::class), trialPeriod: null, ); diff --git a/tests/Subscription/GrantSubscriptionTest.php b/tests/Subscription/GrantSubscriptionTest.php index de239536..b032df12 100644 --- a/tests/Subscription/GrantSubscriptionTest.php +++ b/tests/Subscription/GrantSubscriptionTest.php @@ -26,27 +26,27 @@ class GrantSubscriptionTest extends TestCase private function service(?SubscriptionPeriod $period, ?ClinicSubscription $active): SubscriptionService { - $subscriptionRepo = $this->createMock(ClinicSubscriptionRepository::class); + $subscriptionRepo = $this->createStub(ClinicSubscriptionRepository::class); $subscriptionRepo->method('findActive')->willReturn($active); $subscriptionRepo->method('save')->willReturnCallback(function (ClinicSubscription $s): void { $this->saved = $s; }); - $periodRepo = $this->createMock(SubscriptionPeriodRepository::class); + $periodRepo = $this->createStub(SubscriptionPeriodRepository::class); $periodRepo->method('findByUuid')->willReturn($period); return new SubscriptionService( $subscriptionRepo, - $this->createMock(SubscriptionPlanRepository::class), + $this->createStub(SubscriptionPlanRepository::class), $periodRepo, - $this->createMock(SiteConfigRepository::class), + $this->createStub(SiteConfigRepository::class), ); } private function period(int $durationMonths): SubscriptionPeriod { - $period = $this->createMock(SubscriptionPeriod::class); - $period->method('getPlan')->willReturn($this->createMock(SubscriptionPlan::class)); + $period = $this->createStub(SubscriptionPeriod::class); + $period->method('getPlan')->willReturn($this->createStub(SubscriptionPlan::class)); $period->method('getDurationMonths')->willReturn($durationMonths); return $period; @@ -54,7 +54,7 @@ class GrantSubscriptionTest extends TestCase public function testGrantCreatesSubscriptionWithoutPaymentAndRecordsTheAdmin(): void { - $admin = $this->createMock(User::class); + $admin = $this->createStub(User::class); $service = $this->service($this->period(1), null); $subscription = $service->grant('doctor', 7, 'period-uuid', $admin); @@ -73,7 +73,7 @@ class GrantSubscriptionTest extends TestCase { $service = $this->service($this->period(1), null); - $subscription = $service->grant('clinic', 3, 'period-uuid', $this->createMock(User::class)); + $subscription = $service->grant('clinic', 3, 'period-uuid', $this->createStub(User::class)); $this->assertFalse($subscription->isTrial()); $this->assertTrue($subscription->toArray()['is_granted']); @@ -84,7 +84,7 @@ class GrantSubscriptionTest extends TestCase $service = $this->service(null, null); try { - $service->grant('doctor', 1, 'missing-uuid', $this->createMock(User::class)); + $service->grant('doctor', 1, 'missing-uuid', $this->createStub(User::class)); $this->fail('expected AppException'); } catch (AppException $e) { $this->assertSame(ErrorCodes::ERR_NOT_FOUND_001, $e->getErrorCode()); @@ -97,12 +97,12 @@ class GrantSubscriptionTest extends TestCase { $currentExpiry = time() + 20 * 86400; - $active = $this->createMock(ClinicSubscription::class); + $active = $this->createStub(ClinicSubscription::class); $active->method('getExpiresAt')->willReturn($currentExpiry); $service = $this->service($this->period(1), $active); - $subscription = $service->grant('doctor', 7, 'period-uuid', $this->createMock(User::class)); + $subscription = $service->grant('doctor', 7, 'period-uuid', $this->createStub(User::class)); $this->assertEqualsWithDelta($currentExpiry + 30 * 86400, $subscription->getExpiresAt(), 5); }