From 1444a685e842bcc31a3af4aa6f8068a419337382 Mon Sep 17 00:00:00 2001 From: Philipp Scheit Date: Wed, 20 Mar 2024 10:01:13 +0100 Subject: [PATCH] introduce a ControllerTestCase with getEm(), assertFormError() and use this in all controllers (#1441) --- tests/Controller/AboutControllerTest.php | 12 +--- tests/Controller/ApiControllerTest.php | 32 ++--------- .../ChangePasswordControllerTest.php | 39 +------------ tests/Controller/ControllerTestCase.php | 57 +++++++++++++++++++ tests/Controller/FeedControllerTest.php | 14 ++--- tests/Controller/ProfileControllerTest.php | 26 +-------- .../Controller/RegistrationControllerTest.php | 39 +------------ .../ResetPasswordControllerTest.php | 30 ++-------- tests/Controller/UserControllerTest.php | 26 +-------- tests/Controller/WebControllerTest.php | 38 +++---------- 10 files changed, 93 insertions(+), 220 deletions(-) create mode 100644 tests/Controller/ControllerTestCase.php diff --git a/tests/Controller/AboutControllerTest.php b/tests/Controller/AboutControllerTest.php index 0de0290d7..a9bd0eff3 100644 --- a/tests/Controller/AboutControllerTest.php +++ b/tests/Controller/AboutControllerTest.php @@ -12,24 +12,18 @@ namespace App\Tests\Controller; -use Symfony\Bundle\FrameworkBundle\Test\WebTestCase; - -class AboutControllerTest extends WebTestCase +class AboutControllerTest extends ControllerTestCase { public function testPackagist(): void { - $client = self::createClient(); - - $crawler = $client->request('GET', '/about'); + $crawler = $this->client->request('GET', '/about'); static::assertResponseIsSuccessful(); static::assertEquals('What is Packagist?', $crawler->filter('h2.title')->first()->text()); } public function testComposerRedirect(): void { - $client = self::createClient(); - - $client->request('GET', '/about-composer'); + $this->client->request('GET', '/about-composer'); static::assertResponseRedirects('https://getcomposer.org/', 301); } } diff --git a/tests/Controller/ApiControllerTest.php b/tests/Controller/ApiControllerTest.php index 6dec80155..e79fd4501 100644 --- a/tests/Controller/ApiControllerTest.php +++ b/tests/Controller/ApiControllerTest.php @@ -12,39 +12,17 @@ namespace App\Tests\Controller; +use App\Entity\Package; use App\Entity\SecurityAdvisory; +use App\Entity\User; use App\SecurityAdvisory\GitHubSecurityAdvisoriesSource; use App\SecurityAdvisory\RemoteSecurityAdvisory; use App\SecurityAdvisory\Severity; -use Doctrine\DBAL\Connection; -use Doctrine\Persistence\ManagerRegistry; -use Exception; -use App\Entity\Package; -use App\Entity\User; use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\Attributes\Depends; -use Symfony\Bundle\FrameworkBundle\KernelBrowser; -use Symfony\Bundle\FrameworkBundle\Test\WebTestCase; -class ApiControllerTest extends WebTestCase +class ApiControllerTest extends ControllerTestCase { - private KernelBrowser $client; - - public function setUp(): void - { - $this->client = self::createClient(); - static::getContainer()->get(Connection::class)->beginTransaction(); - - parent::setUp(); - } - - public function tearDown(): void - { - static::getContainer()->get(Connection::class)->rollBack(); - - parent::tearDown(); - } - public function testGithubFailsOnGet(): void { $this->client->request('GET', '/api/github'); @@ -73,7 +51,7 @@ public function testGithubApi($url): void $user->setPassword('testtest'); $user->setApiToken('token'); - $em = static::getContainer()->get(ManagerRegistry::class)->getManager(); + $em = self::getEM(); $em->persist($package); $em->persist($user); $em->flush(); @@ -180,7 +158,7 @@ public function testSecurityAdvisories(): void GitHubSecurityAdvisoriesSource::SOURCE_NAME, Severity::MEDIUM, ), GitHubSecurityAdvisoriesSource::SOURCE_NAME); - $em = static::getContainer()->get(ManagerRegistry::class)->getManager(); + $em = self::getEM(); $em->persist($advisory); $em->flush(); diff --git a/tests/Controller/ChangePasswordControllerTest.php b/tests/Controller/ChangePasswordControllerTest.php index 7e1203011..4d1e0b569 100644 --- a/tests/Controller/ChangePasswordControllerTest.php +++ b/tests/Controller/ChangePasswordControllerTest.php @@ -14,34 +14,11 @@ use App\Entity\User; use App\Validator\NotProhibitedPassword; -use Doctrine\DBAL\Connection; -use Doctrine\Persistence\ManagerRegistry; use PHPUnit\Framework\Attributes\TestWith; -use Symfony\Bundle\FrameworkBundle\KernelBrowser; -use Symfony\Bundle\FrameworkBundle\Test\WebTestCase; -use Symfony\Component\DomCrawler\Crawler; use Symfony\Component\PasswordHasher\Hasher\UserPasswordHasherInterface; -class ChangePasswordControllerTest extends WebTestCase +class ChangePasswordControllerTest extends ControllerTestCase { - private KernelBrowser $client; - - public function setUp(): void - { - $this->client = self::createClient(); - $this->client->disableReboot(); // Prevent reboot between requests - static::getContainer()->get(Connection::class)->beginTransaction(); - - parent::setUp(); - } - - public function tearDown(): void - { - static::getContainer()->get(Connection::class)->rollBack(); - - parent::tearDown(); - } - #[TestWith(['SuperSecret123', 'ok'])] #[TestWith(['test@example.org', 'prohibited-password-error'])] public function testChangePassword(string $newPassword, string $expectedResult): void @@ -58,7 +35,7 @@ public function testChangePassword(string $newPassword, string $expectedResult): $currentPasswordHash = self::getContainer()->get(UserPasswordHasherInterface::class)->hashPassword($user, $currentPassword); $user->setPassword($currentPasswordHash); - $em = static::getContainer()->get(ManagerRegistry::class)->getManager(); + $em = self::getEM(); $em->persist($user); $em->flush(); @@ -83,17 +60,7 @@ public function testChangePassword(string $newPassword, string $expectedResult): if ($expectedResult === 'prohibited-password-error') { $this->assertResponseStatusCodeSame(422); - $this->assertFormError((new NotProhibitedPassword)->message, $crawler); + $this->assertFormError((new NotProhibitedPassword)->message, 'change_password_form', $crawler); } } - - private function assertFormError(string $message, Crawler $crawler): void - { - $formCrawler = $crawler->filter('[name="change_password_form"]'); - $this->assertCount( - 1, - $formCrawler->filter('.alert-danger:contains("' . $message . '")'), - $formCrawler->html()."\nShould find an .alert-danger within the form with the message: '$message'", - ); - } } diff --git a/tests/Controller/ControllerTestCase.php b/tests/Controller/ControllerTestCase.php new file mode 100644 index 000000000..d2a6a990f --- /dev/null +++ b/tests/Controller/ControllerTestCase.php @@ -0,0 +1,57 @@ + + * Nils Adermann + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace App\Tests\Controller; + +use Doctrine\DBAL\Connection; +use Doctrine\ORM\EntityManagerInterface; +use Doctrine\Persistence\ManagerRegistry; +use Symfony\Bundle\FrameworkBundle\KernelBrowser; +use Symfony\Bundle\FrameworkBundle\Test\WebTestCase; +use Symfony\Component\DomCrawler\Crawler; + +class ControllerTestCase extends WebTestCase +{ + protected KernelBrowser $client; + + public function setUp(): void + { + $this->client = self::createClient(); + $this->client->disableReboot(); // prevent reboot to keep the transaction + + static::getContainer()->get(Connection::class)->beginTransaction(); + + parent::setUp(); + } + + public function tearDown(): void + { + static::getContainer()->get(Connection::class)->rollBack(); + + parent::tearDown(); + } + + public function getEM(): EntityManagerInterface + { + return static::getContainer()->get(ManagerRegistry::class)->getManager(); + } + + protected function assertFormError(string $message, string $formName, Crawler $crawler): void + { + $formCrawler = $crawler->filter(sprintf('[name="%s"]', $formName)); + $this->assertCount( + 1, + $formCrawler->filter('.alert-danger:contains("' . $message . '")'), + $formCrawler->html()."\nShould find an .alert-danger within the form with the message: '$message'", + ); + } +} diff --git a/tests/Controller/FeedControllerTest.php b/tests/Controller/FeedControllerTest.php index 062f288ed..d4ad6cbd3 100644 --- a/tests/Controller/FeedControllerTest.php +++ b/tests/Controller/FeedControllerTest.php @@ -13,25 +13,23 @@ namespace App\Tests\Controller; use PHPUnit\Framework\Attributes\DataProvider; -use Symfony\Bundle\FrameworkBundle\Test\WebTestCase; use Symfony\Component\Routing\Generator\UrlGeneratorInterface; -class FeedControllerTest extends WebTestCase +class FeedControllerTest extends ControllerTestCase { #[DataProvider('provideForFeed')] public function testFeedAction(string $feed, string $format, ?string $vendor = null): void { - $client = static::createClient(); - $url = static::getContainer()->get(UrlGeneratorInterface::class)->generate($feed, ['_format' => $format, 'vendor' => $vendor]); - $crawler = $client->request('GET', $url); + $this->client->request('GET', $url); - $this->assertEquals(200, $client->getResponse()->getStatusCode(), $client->getResponse()->getContent()); - $this->assertStringContainsString($format, $client->getResponse()->getContent()); + $response = $this->client->getResponse(); + $this->assertEquals(200, $response->getStatusCode(), $response->getContent()); + $this->assertStringContainsString($format, $response->getContent()); if ($vendor !== null) { - $this->assertStringContainsString($vendor, $client->getResponse()->getContent()); + $this->assertStringContainsString($vendor, $response->getContent()); } } diff --git a/tests/Controller/ProfileControllerTest.php b/tests/Controller/ProfileControllerTest.php index 1d9a98333..f8e2de25d 100644 --- a/tests/Controller/ProfileControllerTest.php +++ b/tests/Controller/ProfileControllerTest.php @@ -13,31 +13,9 @@ namespace App\Tests\Controller; use App\Entity\User; -use Doctrine\DBAL\Connection; -use Doctrine\Persistence\ManagerRegistry; -use Symfony\Bundle\FrameworkBundle\KernelBrowser; -use Symfony\Bundle\FrameworkBundle\Test\WebTestCase; -class ProfileControllerTest extends WebTestCase +class ProfileControllerTest extends ControllerTestCase { - private KernelBrowser $client; - - public function setUp(): void - { - $this->client = self::createClient(); - $this->client->disableReboot(); // Prevent reboot between requests - static::getContainer()->get(Connection::class)->beginTransaction(); - - parent::setUp(); - } - - public function tearDown(): void - { - static::getContainer()->get(Connection::class)->rollBack(); - - parent::tearDown(); - } - public function testEditProfile(): void { $user = new User; @@ -51,7 +29,7 @@ public function testEditProfile(): void $user->initializeConfirmationToken(); $user->setPasswordRequestedAt(new \DateTime()); - $em = static::getContainer()->get(ManagerRegistry::class)->getManager(); + $em = self::getEM(); $em->persist($user); $em->flush(); diff --git a/tests/Controller/RegistrationControllerTest.php b/tests/Controller/RegistrationControllerTest.php index 341f5c932..68a58cfe0 100644 --- a/tests/Controller/RegistrationControllerTest.php +++ b/tests/Controller/RegistrationControllerTest.php @@ -14,33 +14,10 @@ use App\Entity\User; use App\Validator\NotProhibitedPassword; -use Doctrine\DBAL\Connection; -use Doctrine\Persistence\ManagerRegistry; use PHPUnit\Framework\Attributes\TestWith; -use Symfony\Bundle\FrameworkBundle\KernelBrowser; -use Symfony\Bundle\FrameworkBundle\Test\WebTestCase; -use Symfony\Component\DomCrawler\Crawler; -class RegistrationControllerTest extends WebTestCase +class RegistrationControllerTest extends ControllerTestCase { - private KernelBrowser $client; - - public function setUp(): void - { - $this->client = self::createClient(); - $this->client->disableReboot(); // Prevent reboot between requests - static::getContainer()->get(Connection::class)->beginTransaction(); - - parent::setUp(); - } - - public function tearDown(): void - { - static::getContainer()->get(Connection::class)->rollBack(); - - parent::tearDown(); - } - public function testRegisterWithoutOAuth(): void { $crawler = $this->client->request('GET', '/register/'); @@ -56,7 +33,7 @@ public function testRegisterWithoutOAuth(): void $this->client->submit($form); $this->assertResponseStatusCodeSame(302); - $em = static::getContainer()->get(ManagerRegistry::class)->getManager(); + $em = self::getEM(); $user = $em->getRepository(User::class)->findOneBy(['username' => 'max.example']); $this->assertInstanceOf(User::class, $user); $this->assertSame('max@example.com', $user->getEmailCanonical(), "user email should have been canonicalized"); @@ -80,16 +57,6 @@ public function testRegisterWithTooSimplePasswords(string $password): void $crawler = $this->client->submit($form); $this->assertResponseStatusCodeSame(422, 'Should be invalid because password is the same as email or username'); - $this->assertFormError((new NotProhibitedPassword)->message, $crawler); - } - - private function assertFormError(string $message, Crawler $crawler): void - { - $formCrawler = $crawler->filter('[name="registration_form"]'); - $this->assertCount( - 1, - $formCrawler->filter('.alert-danger:contains("' . $message . '")'), - $formCrawler->html()."\nShould find an .alert-danger within the form with the message: '$message'", - ); + $this->assertFormError((new NotProhibitedPassword)->message, 'registration_form', $crawler); } } diff --git a/tests/Controller/ResetPasswordControllerTest.php b/tests/Controller/ResetPasswordControllerTest.php index c8f2ca3ed..bda4c11ce 100644 --- a/tests/Controller/ResetPasswordControllerTest.php +++ b/tests/Controller/ResetPasswordControllerTest.php @@ -14,34 +14,12 @@ use App\Entity\User; use App\Tests\Mock\TotpAuthenticatorStub; -use Doctrine\DBAL\Connection; -use Doctrine\Persistence\ManagerRegistry; use Scheb\TwoFactorBundle\Security\Http\Authenticator\TwoFactorAuthenticator; -use Symfony\Bundle\FrameworkBundle\KernelBrowser; -use Symfony\Bundle\FrameworkBundle\Test\WebTestCase; use Symfony\Component\DomCrawler\Crawler; use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorageInterface; -class ResetPasswordControllerTest extends WebTestCase +class ResetPasswordControllerTest extends ControllerTestCase { - private KernelBrowser $client; - - public function setUp(): void - { - $this->client = self::createClient(); - $this->client->disableReboot(); // Prevent reboot between requests - static::getContainer()->get(Connection::class)->beginTransaction(); - - parent::setUp(); - } - - public function tearDown(): void - { - static::getContainer()->get(Connection::class)->rollBack(); - - parent::tearDown(); - } - public function testResetPassword(): void { $user = $this->setupUserWithPasswordResetRequest(false); @@ -96,7 +74,7 @@ private function setupUserWithPasswordResetRequest(bool $withTwoFactor): User $user->setTotpSecret('secret'); } - $em = static::getContainer()->get(ManagerRegistry::class)->getManager(); + $em = self::getEM(); $em->persist($user); $em->flush(); @@ -117,7 +95,7 @@ private function submitPasswordResetFormAndAsserStatusCode(Crawler $crawler, str private function assertUserHasNewPassword(User $user, ?string $oldPassword): void { - $em = static::getContainer()->get(ManagerRegistry::class)->getManager(); + $em = self::getEM(); $em->clear(); $user = $em->getRepository(User::class)->find($user->getId()); @@ -127,7 +105,7 @@ private function assertUserHasNewPassword(User $user, ?string $oldPassword): voi private function assertUserHasUnchangedPassword(User $user, ?string $oldPassword): void { - $em = static::getContainer()->get(ManagerRegistry::class)->getManager(); + $em = self::getEM(); $em->clear(); $user = $em->getRepository(User::class)->find($user->getId()); diff --git a/tests/Controller/UserControllerTest.php b/tests/Controller/UserControllerTest.php index b85396cd4..3a6a93167 100644 --- a/tests/Controller/UserControllerTest.php +++ b/tests/Controller/UserControllerTest.php @@ -14,31 +14,9 @@ use App\Entity\User; use App\Tests\Mock\TotpAuthenticatorStub; -use Doctrine\DBAL\Connection; -use Doctrine\Persistence\ManagerRegistry; -use Symfony\Bundle\FrameworkBundle\KernelBrowser; -use Symfony\Bundle\FrameworkBundle\Test\WebTestCase; -class UserControllerTest extends WebTestCase +class UserControllerTest extends ControllerTestCase { - private KernelBrowser $client; - - public function setUp(): void - { - $this->client = self::createClient(); - $this->client->disableReboot(); // Prevent reboot between requests - static::getContainer()->get(Connection::class)->beginTransaction(); - - parent::setUp(); - } - - public function tearDown(): void - { - static::getContainer()->get(Connection::class)->rollBack(); - - parent::tearDown(); - } - public function testEnableTwoFactoCode(): void { $user = new User; @@ -48,7 +26,7 @@ public function testEnableTwoFactoCode(): void $user->setPassword('testtest'); $user->setApiToken('token'); - $em = static::getContainer()->get(ManagerRegistry::class)->getManager(); + $em = self::getEM(); $em->persist($user); $em->flush(); diff --git a/tests/Controller/WebControllerTest.php b/tests/Controller/WebControllerTest.php index e55024bbc..8b366219d 100644 --- a/tests/Controller/WebControllerTest.php +++ b/tests/Controller/WebControllerTest.php @@ -12,34 +12,12 @@ namespace App\Tests\Controller; +use App\Entity\Package; use App\Search\Query; use App\Tests\Search\AlgoliaMock; -use Doctrine\DBAL\Connection; -use Exception; -use App\Entity\Package; -use Symfony\Bundle\FrameworkBundle\KernelBrowser; -use Symfony\Bundle\FrameworkBundle\Test\WebTestCase; -use Symfony\Component\DependencyInjection\ContainerInterface; -class WebControllerTest extends WebTestCase +class WebControllerTest extends ControllerTestCase { - private KernelBrowser $client; - - public function setUp(): void - { - $this->client = self::createClient(); - static::getContainer()->get(Connection::class)->beginTransaction(); - - parent::setUp(); - } - - public function tearDown(): void - { - static::getContainer()->get(Connection::class)->rollBack(); - - parent::tearDown(); - } - public function testHomepage(): void { $crawler = $this->client->request('GET', '/'); @@ -48,7 +26,7 @@ public function testHomepage(): void public function testRedirectsOnMatch(): void { - $this->initializePackages($this->client->getContainer()); + $this->initializePackages(); $this->client->request('GET', '/', ['query' => 'twig/twig']); static::assertResponseRedirects('/packages/twig/twig', 302); @@ -63,7 +41,7 @@ public function testHomepageDoesntRedirectsOnNoMatch(): void public function testSearchRedirectsOnMatch(): void { - $this->initializePackages($this->client->getContainer()); + $this->initializePackages(); $this->client->request('GET', '/search/', ['query' => 'twig/twig']); static::assertResponseRedirects('/packages/twig/twig', 302); @@ -112,7 +90,7 @@ public function testSearchJsonWithQueryAndTagsAndTypes(): void public function testPackages(): void { - $this->initializePackages($this->client->getContainer()); + $this->initializePackages(); //we expect at least one package $crawler = $this->client->request('GET', '/explore/'); @@ -121,7 +99,7 @@ public function testPackages(): void public function testPackage(): void { - $this->initializePackages($this->client->getContainer()); + $this->initializePackages(); //we expect package to be clickable and showing at least 'package' div $crawler = $this->client->request('GET', '/packages/symfony/symfony'); @@ -131,9 +109,9 @@ public function testPackage(): void /** * @return Package[] */ - protected function initializePackages(ContainerInterface $container): array + protected function initializePackages(): array { - $em = $container->get('doctrine')->getManager(); + $em = $this->getEM(); $twigPackage = new Package();