From 8f889fb30cf00596f2b56dd10ee1b0cd638de352 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Joachim=20L=C3=B8vgaard?= Date: Mon, 26 Oct 2026 10:14:52 +0100 Subject: [PATCH 1/9] Conflict with DoctrineBundle 2.11.0-2.11.2 DoctrineOrmMappingsPass::createXmlMappingDriver(), which SetonoClientBundle::build() uses to register the Metadata mapping, passes null as the file extension to ORM's XmlDriver in these versions. ORM 3 types that argument as a string, so with ORM 3 the entity manager can't be built and the application doesn't boot: XmlDriver::__construct(): Argument #2 ($fileExtension) must be of type string, null given DoctrineBundle fixed it in 2.11.3. It went unnoticed because no test booted the bundle with DoctrineBundle; the CI job with the lowest dependencies on Symfony 7.0 resolves exactly this combination (DoctrineBundle 2.11.0, ORM 3.1). --- composer.json | 3 +++ 1 file changed, 3 insertions(+) diff --git a/composer.json b/composer.json index 6f881b6..f0acc6d 100644 --- a/composer.json +++ b/composer.json @@ -46,6 +46,9 @@ "shipmonk/composer-dependency-analyser": "^1.8.4", "sylius-labs/coding-standard": "^4.5" }, + "conflict": { + "doctrine/doctrine-bundle": ">=2.11.0 <2.11.3" + }, "prefer-stable": true, "autoload": { "psr-4": { From bf56c78448bfcf1c62b82e18d666c8872eef54dc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Joachim=20L=C3=B8vgaard?= Date: Mon, 26 Oct 2026 10:15:14 +0100 Subject: [PATCH 2/9] Add functional tests that run the bundle in a Symfony application with a database The unit tests replace Doctrine and the other services with doubles, so nothing verified that the services are wired together correctly, or that metadata actually ends up in a database and comes back. The new Functional suite boots a small application (FrameworkBundle, DoctrineBundle and this bundle on SQLite) and drives it over HTTP with KernelBrowser: - ClientCookieTest: a new visitor gets a client id, a returning visitor keeps it, the first seen timestamp is kept, version 1 and invalid cookies are handled, and a PreStoreCookieEvent listener can veto the cookie - MetadataTest: metadata is stored, loaded, updated and removed per visitor, the expiry of keys survives between requests and expired keys are removed, nothing is queried when the metadata isn't used, and a concurrent insert of the same client doesn't break the request - DoctrineMappingTest: Metadata is mapped as an entity, and the mapping is valid and matches the schema Each test starts from a freshly created schema. The kernel keeps its cache, log and database in the system temp dir, separated per Infection process. DAMA DoctrineTestBundle isn't used, because its requirements would raise the lowest tested versions to ORM 3 and DBAL 3.8. --- CLAUDE.md | 5 +- composer.json | 5 +- phpunit.xml.dist | 3 + .../App/Controller/ClientController.php | 99 ++++++++++ .../EventSubscriber/SkipCookieSubscriber.php | 29 +++ tests/Functional/App/Kernel.php | 98 ++++++++++ tests/Functional/ClientCookieTest.php | 131 +++++++++++++ tests/Functional/DoctrineMappingTest.php | 37 ++++ tests/Functional/FunctionalTestCase.php | 132 +++++++++++++ tests/Functional/MetadataTest.php | 175 ++++++++++++++++++ 10 files changed, 712 insertions(+), 2 deletions(-) create mode 100644 tests/Functional/App/Controller/ClientController.php create mode 100644 tests/Functional/App/EventSubscriber/SkipCookieSubscriber.php create mode 100644 tests/Functional/App/Kernel.php create mode 100644 tests/Functional/ClientCookieTest.php create mode 100644 tests/Functional/DoctrineMappingTest.php create mode 100644 tests/Functional/FunctionalTestCase.php create mode 100644 tests/Functional/MetadataTest.php diff --git a/CLAUDE.md b/CLAUDE.md index 3ecdf7f..1cb395f 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -12,7 +12,10 @@ This is a library/bundle — there is no runnable application here. It is exerci PHP 8.1+ is required (CI runs 8.1 and 8.2). Use the `8.1`/`8.2` shell switchers if needed. The dev tooling is the individually-pinned set from `setono/code-quality-pack` (inlined into `require-dev` rather than depending on the meta-package); PHPUnit is held at `^10.5` so PHP 8.1 stays supported (PHPUnit 11 needs 8.2). -- Tests: `composer phpunit` (or `vendor/bin/phpunit`). Unit tests live in `tests/Unit` (the `unit` suite: `vendor/bin/phpunit --testsuite unit`) +- Tests: `composer phpunit` (or `vendor/bin/phpunit`) runs both suites; pick one with `--testsuite unit` or `--testsuite functional` + - `tests/Unit`: isolated classes with Prophecy doubles. + - `tests/Functional`: the bundle inside a real Symfony app (`tests/Functional/App/Kernel.php`: FrameworkBundle + DoctrineBundle + this bundle), driven over HTTP with `KernelBrowser` against a SQLite database. Extend `FunctionalTestCase`, which recreates the schema before each test and has helpers to read the stored metadata. Every request gets a new container, like PHP-FPM. The kernel's cache, log and database live in `sys_get_temp_dir()/setono-client-bundle/`, so nothing is generated in the repo and parallel Infection processes don't share a database. Set `DATABASE_URL` to run against another database. + - No DAMA DoctrineTestBundle on purpose: its `doctrine/doctrine-bundle ^2.11` / `doctrine/dbal ^3.3` requirements would lift the CI "lowest" jobs from ORM 2/DBAL 2.13 to ORM 3/DBAL 3.8, so ORM 2 would no longer be tested. With a single table, recreating the schema per test is fast enough. - Single test: `vendor/bin/phpunit --filter after_loading_the_correct_parameter_has_been_set` or by path `vendor/bin/phpunit tests/Unit/DependencyInjection/SetonoClientExtensionTest.php` - Static analysis: `composer analyse` (PHPStan at `level: max`, config in `phpstan.neon.dist`; auto-loads the doctrine/symfony/phpunit/strict-rules extensions via `phpstan/extension-installer`) - Check style: `composer check-style` (ECS, Sylius Labs ruleset) diff --git a/composer.json b/composer.json index f0acc6d..0c8a9da 100644 --- a/composer.json +++ b/composer.json @@ -44,7 +44,10 @@ "phpunit/phpunit": "^10.5", "rector/rector": "^2.4.1", "shipmonk/composer-dependency-analyser": "^1.8.4", - "sylius-labs/coding-standard": "^4.5" + "sylius-labs/coding-standard": "^4.5", + "symfony/browser-kit": "^6.4 || ^7.0", + "symfony/framework-bundle": "^6.4 || ^7.0", + "symfony/routing": "^6.4 || ^7.0" }, "conflict": { "doctrine/doctrine-bundle": ">=2.11.0 <2.11.3" diff --git a/phpunit.xml.dist b/phpunit.xml.dist index 14e4ada..cc758c9 100644 --- a/phpunit.xml.dist +++ b/phpunit.xml.dist @@ -6,6 +6,9 @@ tests/Unit + + tests/Functional + diff --git a/tests/Functional/App/Controller/ClientController.php b/tests/Functional/App/Controller/ClientController.php new file mode 100644 index 0000000..b4648fe --- /dev/null +++ b/tests/Functional/App/Controller/ClientController.php @@ -0,0 +1,99 @@ + $metadataClass + */ + public function __construct( + private readonly ClientContextInterface $clientContext, + private readonly ManagerRegistry $managerRegistry, + #[Autowire('%setono_client.metadata_class%')] + private readonly string $metadataClass, + ) { + } + + /** + * Uses the client, but not its metadata + */ + public function id(): Response + { + return new Response($this->clientContext->getClient()->id); + } + + public function showMetadata(Client $client): JsonResponse + { + return new JsonResponse(iterator_to_array($client->metadata)); + } + + public function setMetadata(Request $request, Client $client): Response + { + $ttl = $request->query->get('ttl'); + + $client->metadata->set( + $request->query->getString('key'), + $request->query->getString('value'), + null === $ttl ? null : (int) $ttl, + ); + + return new Response(); + } + + public function removeMetadata(Request $request, Client $client): Response + { + $client->metadata->remove($request->query->getString('key')); + + return new Response(); + } + + /** + * Sets the metadata like setMetadata() does, but simulates a concurrent request from the same client + * that inserts the client's row right before this request does + */ + public function setMetadataConcurrently(Request $request, Client $client): Response + { + $client->metadata->set($request->query->getString('key'), $request->query->getString('value')); + + $manager = $this->managerRegistry->getManagerForClass($this->metadataClass); + \assert($manager instanceof EntityManagerInterface); + + $manager->getEventManager()->addEventListener(Events::onFlush, new class($manager->getConnection(), $manager->getClassMetadata($this->metadataClass), $client->id) { + /** + * @param ClassMetadata $classMetadata + */ + public function __construct( + private readonly Connection $connection, + private readonly ClassMetadata $classMetadata, + private readonly string $clientId, + ) { + } + + public function onFlush(): void + { + $this->connection->insert($this->classMetadata->getTableName(), [ + $this->classMetadata->getColumnName('clientId') => $this->clientId, + $this->classMetadata->getColumnName('metadata') => json_encode(['set_by' => 'concurrent request']), + ]); + } + }); + + return new Response(); + } +} diff --git a/tests/Functional/App/EventSubscriber/SkipCookieSubscriber.php b/tests/Functional/App/EventSubscriber/SkipCookieSubscriber.php new file mode 100644 index 0000000..db51257 --- /dev/null +++ b/tests/Functional/App/EventSubscriber/SkipCookieSubscriber.php @@ -0,0 +1,29 @@ + 'skipCookie', + ]; + } + + public function skipCookie(PreStoreCookieEvent $event): void + { + if ($event->request->query->has('skip_cookie')) { + $event->store = false; + } + } +} diff --git a/tests/Functional/App/Kernel.php b/tests/Functional/App/Kernel.php new file mode 100644 index 0000000..ae4b855 --- /dev/null +++ b/tests/Functional/App/Kernel.php @@ -0,0 +1,98 @@ +getVarDir() . '/cache/' . $this->environment; + } + + public function getLogDir(): string + { + return $this->getVarDir() . '/log'; + } + + /** + * The application is configured in this class, so the config dir only receives the config reference + * that Symfony 7.4+ dumps in debug mode + */ + protected function getConfigDir(): string + { + return $this->getVarDir() . '/config'; + } + + /** + * Infection runs the tests in parallel processes and gives each a token, so each process gets its own database + */ + private function getVarDir(): string + { + return sprintf('%s/setono-client-bundle%s', sys_get_temp_dir(), (string) getenv('TEST_TOKEN')); + } + + protected function configureContainer(ContainerConfigurator $container): void + { + $container->extension('framework', [ + 'secret' => 'secret', + 'test' => true, + 'http_method_override' => false, + 'handle_all_throwables' => true, + 'php_errors' => ['log' => true], + ]); + + // Set the DATABASE_URL environment variable to run the tests against another database + $container->parameters()->set('env(DATABASE_URL)', 'sqlite:///%kernel.cache_dir%/test.db'); + + $container->extension('doctrine', [ + 'dbal' => ['url' => '%env(resolve:DATABASE_URL)%'], + 'orm' => [], + ]); + + $services = $container->services(); + + // Logs to a file instead of the test output + $services->set('logger', Logger::class)->args(['debug', '%kernel.cache_dir%/log.txt']); + + $services + ->defaults()->autowire()->autoconfigure() + ->load(__NAMESPACE__ . '\\Controller\\', __DIR__ . '/Controller')->tag('controller.service_arguments') + ->load(__NAMESPACE__ . '\\EventSubscriber\\', __DIR__ . '/EventSubscriber') + ; + } + + protected function configureRoutes(RoutingConfigurator $routes): void + { + $routes->add('client_id', '/')->controller([ClientController::class, 'id']); + $routes->add('metadata_show', '/metadata')->controller([ClientController::class, 'showMetadata']); + $routes->add('metadata_set', '/metadata/set')->controller([ClientController::class, 'setMetadata']); + $routes->add('metadata_remove', '/metadata/remove')->controller([ClientController::class, 'removeMetadata']); + $routes->add('metadata_set_concurrently', '/metadata/set-concurrently')->controller([ClientController::class, 'setMetadataConcurrently']); + } +} diff --git a/tests/Functional/ClientCookieTest.php b/tests/Functional/ClientCookieTest.php new file mode 100644 index 0000000..ba554b0 --- /dev/null +++ b/tests/Functional/ClientCookieTest.php @@ -0,0 +1,131 @@ +browser->request('GET', '/'); + + self::assertResponseIsSuccessful(); + self::assertResponseHasCookie(self::COOKIE_NAME); + + $clientId = $this->getClientId(); + self::assertMatchesRegularExpression(self::UUID_PATTERN, $clientId); + self::assertSame($clientId, $this->browser->getResponse()->getContent()); + } + + /** + * @test + */ + public function it_sets_a_cookie_that_javascript_can_read_and_that_expires_in_a_year(): void + { + $before = time(); + $this->browser->request('GET', '/'); + + $cookies = $this->browser->getResponse()->headers->getCookies(); + self::assertCount(1, $cookies); + self::assertFalse($cookies[0]->isHttpOnly()); + self::assertGreaterThanOrEqual(strtotime('+365 days', $before), $cookies[0]->getExpiresTime()); + self::assertLessThanOrEqual(strtotime('+365 days'), $cookies[0]->getExpiresTime()); + } + + /** + * @test + */ + public function it_recognizes_a_returning_visitor(): void + { + $this->browser->request('GET', '/'); + $clientId = $this->getClientId(); + + $this->browser->request('GET', '/'); + + self::assertSame($clientId, $this->browser->getResponse()->getContent()); + self::assertSame($clientId, $this->getClientId()); + } + + /** + * @test + */ + public function it_keeps_the_first_seen_timestamp_and_updates_the_last_seen_timestamp(): void + { + $this->setCookie((new Cookie('client-id', 2, 1_000, 2_000))->toString()); + + $before = time(); + $this->browser->request('GET', '/'); + + $cookie = $this->getCookie(); + self::assertSame('client-id', $cookie->clientId); + self::assertSame(1_000, $cookie->firstSeenAt); + self::assertGreaterThanOrEqual($before, $cookie->lastSeenAt); + } + + /** + * @test + */ + public function it_upgrades_a_version_1_cookie(): void + { + // Version 1 cookies only hold the client id + $this->setCookie('client-id'); + + $this->browser->request('GET', '/'); + + self::assertSame('client-id', $this->browser->getResponse()->getContent()); + + $cookie = $this->getCookie(); + self::assertSame('client-id', $cookie->clientId); + self::assertSame(2, $cookie->version); + } + + /** + * @test + */ + public function it_treats_a_visitor_with_an_invalid_cookie_as_a_new_visitor(): void + { + $this->setCookie('1.2.3'); + + $this->browser->request('GET', '/'); + + $clientId = $this->getClientId(); + self::assertMatchesRegularExpression(self::UUID_PATTERN, $clientId); + self::assertSame($clientId, $this->browser->getResponse()->getContent()); + } + + /** + * @test + */ + public function it_does_not_set_the_cookie_when_a_listener_vetoes_it(): void + { + $this->browser->request('GET', '/?skip_cookie=1'); + + self::assertResponseIsSuccessful(); + self::assertResponseNotHasCookie(self::COOKIE_NAME); + } + + /** + * Sets the cookie like the browser stores it after receiving it from the application + */ + private function setCookie(string $value): void + { + $this->browser->getCookieJar()->set(new BrowserCookie(self::COOKIE_NAME, $value, null, '/', 'localhost')); + } + + private function getCookie(): Cookie + { + $cookie = $this->browser->getCookieJar()->get(self::COOKIE_NAME); + self::assertNotNull($cookie); + + return Cookie::fromString($cookie->getValue()); + } +} diff --git a/tests/Functional/DoctrineMappingTest.php b/tests/Functional/DoctrineMappingTest.php new file mode 100644 index 0000000..ffade33 --- /dev/null +++ b/tests/Functional/DoctrineMappingTest.php @@ -0,0 +1,37 @@ +getClassMetadata(Metadata::class); + + self::assertFalse($classMetadata->isMappedSuperclass); + self::assertSame('setono_client__metadata', $classMetadata->getTableName()); + self::assertSame(['clientId'], $classMetadata->getIdentifierFieldNames()); + self::assertSame('string', $classMetadata->getTypeOfField('clientId')); + self::assertSame('json', $classMetadata->getTypeOfField('metadata')); + self::assertTrue($classMetadata->isNullable('metadata')); + } + + /** + * @test + */ + public function its_mapping_is_valid_and_matches_the_database_schema(): void + { + $validator = new SchemaValidator(self::getEntityManager()); + + self::assertSame([], $validator->validateMapping()); + self::assertTrue($validator->schemaInSyncWithMetadata()); + } +} diff --git a/tests/Functional/FunctionalTestCase.php b/tests/Functional/FunctionalTestCase.php new file mode 100644 index 0000000..a26a6c6 --- /dev/null +++ b/tests/Functional/FunctionalTestCase.php @@ -0,0 +1,132 @@ +browser = static::createClient(); + + $manager = self::getEntityManager(); + $schemaTool = new SchemaTool($manager); + $schemaTool->dropDatabase(); + $schemaTool->createSchema($manager->getMetadataFactory()->getAllMetadata()); + } + + protected static function getKernelClass(): string + { + return Kernel::class; + } + + /** + * Returns the client id stored in the browser's cookie + */ + protected function getClientId(): string + { + $cookie = $this->browser->getCookieJar()->get(self::COOKIE_NAME); + self::assertNotNull($cookie, 'The browser does not have the client id cookie'); + + return Cookie::fromString($cookie->getValue())->clientId; + } + + /** + * Returns the metadata stored in the database for the given client, or null if the client has no row + * + * @return array|null + */ + protected static function fetchStoredMetadata(string $clientId): ?array + { + $classMetadata = self::getMetadataClassMetadata(); + + $metadata = self::getConnection()->fetchOne(sprintf( + 'SELECT %s FROM %s WHERE %s = ?', + $classMetadata->getColumnName('metadata'), + $classMetadata->getTableName(), + $classMetadata->getColumnName('clientId'), + ), [$clientId]); + + if (false === $metadata) { + return null; + } + + if (null === $metadata) { + return []; + } + + self::assertIsString($metadata); + $metadata = json_decode($metadata, true, 512, \JSON_THROW_ON_ERROR); + self::assertIsArray($metadata); + + return $metadata; + } + + protected static function countStoredMetadata(): int + { + $count = self::getConnection()->fetchOne(sprintf('SELECT COUNT(*) FROM %s', self::getMetadataClassMetadata()->getTableName())); + self::assertIsNumeric($count); + + return (int) $count; + } + + /** + * @param array $metadata + */ + protected static function storeMetadata(string $clientId, array $metadata): void + { + $classMetadata = self::getMetadataClassMetadata(); + + self::getConnection()->update($classMetadata->getTableName(), [ + $classMetadata->getColumnName('metadata') => json_encode($metadata, \JSON_THROW_ON_ERROR), + ], [ + $classMetadata->getColumnName('clientId') => $clientId, + ]); + } + + protected static function getConnection(): Connection + { + return self::getEntityManager()->getConnection(); + } + + protected static function getEntityManager(): EntityManagerInterface + { + $managerRegistry = self::getContainer()->get('doctrine'); + self::assertInstanceOf(ManagerRegistry::class, $managerRegistry); + + $manager = $managerRegistry->getManager(); + self::assertInstanceOf(EntityManagerInterface::class, $manager); + + return $manager; + } + + /** + * @return ClassMetadata + */ + protected static function getMetadataClassMetadata(): ClassMetadata + { + $metadataClass = self::getContainer()->getParameter('setono_client.metadata_class'); + self::assertIsString($metadataClass); + self::assertTrue(class_exists($metadataClass)); + + return self::getEntityManager()->getClassMetadata($metadataClass); + } +} diff --git a/tests/Functional/MetadataTest.php b/tests/Functional/MetadataTest.php new file mode 100644 index 0000000..7737e37 --- /dev/null +++ b/tests/Functional/MetadataTest.php @@ -0,0 +1,175 @@ +browser->request('GET', '/'); + + self::assertResponseIsSuccessful(); + self::assertSame(0, self::countStoredMetadata()); + } + + /** + * @test + */ + public function it_stores_the_metadata_of_a_new_visitor(): void + { + $this->browser->request('GET', '/metadata/set?key=foo&value=bar'); + + self::assertResponseIsSuccessful(); + self::assertSame(['foo' => 'bar'], self::fetchStoredMetadata($this->getClientId())); + } + + /** + * @test + */ + public function it_loads_the_stored_metadata_of_a_returning_visitor(): void + { + $this->browser->request('GET', '/metadata/set?key=foo&value=bar'); + + $this->browser->request('GET', '/metadata'); + + self::assertResponseIsSuccessful(); + self::assertSame(['foo' => 'bar'], $this->getResponseData()); + } + + /** + * @test + */ + public function it_updates_the_stored_metadata_of_a_returning_visitor(): void + { + $this->browser->request('GET', '/metadata/set?key=foo&value=bar'); + $this->browser->request('GET', '/metadata/set?key=baz&value=qux'); + $this->browser->request('GET', '/metadata/set?key=foo&value=changed'); + + self::assertSame(['foo' => 'changed', 'baz' => 'qux'], self::fetchStoredMetadata($this->getClientId())); + self::assertSame(1, self::countStoredMetadata()); + } + + /** + * @test + */ + public function it_removes_metadata(): void + { + $this->browser->request('GET', '/metadata/set?key=foo&value=bar'); + $this->browser->request('GET', '/metadata/set?key=baz&value=qux'); + + $this->browser->request('GET', '/metadata/remove?key=foo'); + self::assertSame(['baz' => 'qux'], self::fetchStoredMetadata($this->getClientId())); + + $this->browser->request('GET', '/metadata/remove?key=baz'); + self::assertSame([], self::fetchStoredMetadata($this->getClientId())); + } + + /** + * @test + */ + public function it_keeps_the_metadata_of_each_visitor_apart(): void + { + $this->browser->request('GET', '/metadata/set?key=foo&value=first'); + $firstClientId = $this->getClientId(); + + $this->browser->getCookieJar()->clear(); + $this->browser->request('GET', '/metadata/set?key=foo&value=second'); + $secondClientId = $this->getClientId(); + + self::assertNotSame($firstClientId, $secondClientId); + self::assertSame(['foo' => 'first'], self::fetchStoredMetadata($firstClientId)); + self::assertSame(['foo' => 'second'], self::fetchStoredMetadata($secondClientId)); + } + + /** + * @test + */ + public function it_does_not_query_the_database_when_the_metadata_is_not_used(): void + { + $this->browser->request('GET', '/metadata/set?key=foo&value=bar'); + + // From now on, every query on the metadata fails + self::getConnection()->executeStatement(sprintf('DROP TABLE %s', self::getMetadataClassMetadata()->getTableName())); + + $this->browser->request('GET', '/'); + self::assertResponseIsSuccessful(); + + // Proves that the query would have failed + $this->browser->catchExceptions(false); + $this->expectException(TableNotFoundException::class); + $this->browser->request('GET', '/metadata'); + } + + /** + * @test + */ + public function it_keeps_the_expiry_of_metadata_between_requests(): void + { + $before = time(); + $this->browser->request('GET', '/metadata/set?key=foo&value=bar&ttl=3600'); + + $stored = self::fetchStoredMetadata($this->getClientId()); + self::assertIsArray($stored); + self::assertSame('bar', $stored['foo']); + + $expires = $stored[Metadata::EXPIRES_KEY]; + self::assertIsArray($expires); + self::assertIsInt($expires['foo']); + self::assertGreaterThanOrEqual($before + 3600, $expires['foo']); + self::assertLessThanOrEqual(time() + 3600, $expires['foo']); + + $this->browser->request('GET', '/metadata'); + self::assertSame(['foo' => 'bar'], $this->getResponseData()); + } + + /** + * @test + */ + public function it_removes_expired_metadata(): void + { + $this->browser->request('GET', '/metadata/set?key=foo&value=bar'); + $clientId = $this->getClientId(); + + self::storeMetadata($clientId, [ + 'foo' => 'bar', + 'baz' => 'qux', + Metadata::EXPIRES_KEY => ['foo' => time() - 1], + ]); + + $this->browser->request('GET', '/metadata'); + + self::assertSame(['baz' => 'qux'], $this->getResponseData()); + self::assertSame(['baz' => 'qux'], self::fetchStoredMetadata($clientId)); + } + + /** + * @test + */ + public function it_keeps_the_row_of_a_concurrent_request_that_stored_it_first(): void + { + $this->browser->request('GET', '/metadata/set-concurrently?key=foo&value=bar'); + + self::assertResponseIsSuccessful(); + self::assertResponseHasCookie(self::COOKIE_NAME); + self::assertSame(['set_by' => 'concurrent request'], self::fetchStoredMetadata($this->getClientId())); + } + + /** + * @return array + */ + private function getResponseData(): array + { + $data = json_decode((string) $this->browser->getResponse()->getContent(), true, 512, \JSON_THROW_ON_ERROR); + self::assertIsArray($data); + + return $data; + } +} From 4923efeaa241e99e0a706d0b9866fca90cabe723 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Joachim=20L=C3=B8vgaard?= Date: Mon, 26 Oct 2026 10:24:09 +0100 Subject: [PATCH 3/9] Use Kernel::getEnvironment() in the test kernel Kernel::$environment is only typed as a string from Symfony 7.4, so PHPStan rejected concatenating it on Symfony 6.4 and 7.0. --- tests/Functional/App/Kernel.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Functional/App/Kernel.php b/tests/Functional/App/Kernel.php index ae4b855..de29a3b 100644 --- a/tests/Functional/App/Kernel.php +++ b/tests/Functional/App/Kernel.php @@ -32,7 +32,7 @@ public function getProjectDir(): string public function getCacheDir(): string { - return $this->getVarDir() . '/cache/' . $this->environment; + return $this->getVarDir() . '/cache/' . $this->getEnvironment(); } public function getLogDir(): string From c75c3f530526e3ad0c3228336e0dd1402d523592 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Joachim=20L=C3=B8vgaard?= Date: Mon, 26 Oct 2026 10:30:36 +0100 Subject: [PATCH 4/9] Use Webmozart Assert in the test controller --- composer.json | 3 ++- tests/Functional/App/Controller/ClientController.php | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/composer.json b/composer.json index 0c8a9da..0a1fba3 100644 --- a/composer.json +++ b/composer.json @@ -47,7 +47,8 @@ "sylius-labs/coding-standard": "^4.5", "symfony/browser-kit": "^6.4 || ^7.0", "symfony/framework-bundle": "^6.4 || ^7.0", - "symfony/routing": "^6.4 || ^7.0" + "symfony/routing": "^6.4 || ^7.0", + "webmozart/assert": "^1.11 || ^2.0" }, "conflict": { "doctrine/doctrine-bundle": ">=2.11.0 <2.11.3" diff --git a/tests/Functional/App/Controller/ClientController.php b/tests/Functional/App/Controller/ClientController.php index b4648fe..f817e25 100644 --- a/tests/Functional/App/Controller/ClientController.php +++ b/tests/Functional/App/Controller/ClientController.php @@ -16,6 +16,7 @@ use Symfony\Component\HttpFoundation\JsonResponse; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\Response; +use Webmozart\Assert\Assert; final class ClientController { @@ -72,7 +73,7 @@ public function setMetadataConcurrently(Request $request, Client $client): Respo $client->metadata->set($request->query->getString('key'), $request->query->getString('value')); $manager = $this->managerRegistry->getManagerForClass($this->metadataClass); - \assert($manager instanceof EntityManagerInterface); + Assert::isInstanceOf($manager, EntityManagerInterface::class); $manager->getEventManager()->addEventListener(Events::onFlush, new class($manager->getConnection(), $manager->getClassMetadata($this->metadataClass), $client->id) { /** From a05affa9f0ddb76b8a661b56a2050169fd55c40a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Joachim=20L=C3=B8vgaard?= Date: Mon, 26 Oct 2026 10:30:36 +0100 Subject: [PATCH 5/9] Validate the client id with the Uid component in the cookie tests symfony/uid is a production dependency that only the tests reference directly (setono/client uses it to generate the ids), so the dependency analyser now ignores it being used only in dev paths instead of being unused. --- composer-dependency-analyser.php | 3 ++- tests/Functional/ClientCookieTest.php | 7 +++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/composer-dependency-analyser.php b/composer-dependency-analyser.php index 1974ae0..7037bea 100644 --- a/composer-dependency-analyser.php +++ b/composer-dependency-analyser.php @@ -4,5 +4,6 @@ use ShipMonk\ComposerDependencyAnalyser\Config\ErrorType; return (new Configuration()) - ->ignoreErrorsOnPackage('symfony/uid', [ErrorType::UNUSED_DEPENDENCY]) // When initializing the default client context, the Uuid class from this component is used + // setono/client generates the client ids with symfony/uid when the default client context creates a client, so src/ doesn't reference it, only the tests + ->ignoreErrorsOnPackage('symfony/uid', [ErrorType::PROD_DEPENDENCY_ONLY_IN_DEV]) ; diff --git a/tests/Functional/ClientCookieTest.php b/tests/Functional/ClientCookieTest.php index ba554b0..eac3e3c 100644 --- a/tests/Functional/ClientCookieTest.php +++ b/tests/Functional/ClientCookieTest.php @@ -6,11 +6,10 @@ use Setono\Client\Cookie; use Symfony\Component\BrowserKit\Cookie as BrowserCookie; +use Symfony\Component\Uid\Uuid; final class ClientCookieTest extends FunctionalTestCase { - private const UUID_PATTERN = '/^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/'; - /** * @test */ @@ -22,7 +21,7 @@ public function it_gives_a_new_visitor_a_client_id(): void self::assertResponseHasCookie(self::COOKIE_NAME); $clientId = $this->getClientId(); - self::assertMatchesRegularExpression(self::UUID_PATTERN, $clientId); + self::assertTrue(Uuid::isValid($clientId)); self::assertSame($clientId, $this->browser->getResponse()->getContent()); } @@ -98,7 +97,7 @@ public function it_treats_a_visitor_with_an_invalid_cookie_as_a_new_visitor(): v $this->browser->request('GET', '/'); $clientId = $this->getClientId(); - self::assertMatchesRegularExpression(self::UUID_PATTERN, $clientId); + self::assertTrue(Uuid::isValid($clientId)); self::assertSame($clientId, $this->browser->getResponse()->getContent()); } From 63907df0aff1a1a39519e5ead0154b744de56e2d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Joachim=20L=C3=B8vgaard?= Date: Mon, 26 Oct 2026 10:34:08 +0100 Subject: [PATCH 6/9] Don't depend on the order of JSON keys in the functional tests MySQL stores the metadata in a native JSON column, which doesn't keep the order of the keys. The suite now also passes with DATABASE_URL pointing at MySQL 8.4 and PostgreSQL 16. --- tests/Functional/MetadataTest.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/Functional/MetadataTest.php b/tests/Functional/MetadataTest.php index 7737e37..dcfc6f5 100644 --- a/tests/Functional/MetadataTest.php +++ b/tests/Functional/MetadataTest.php @@ -53,7 +53,8 @@ public function it_updates_the_stored_metadata_of_a_returning_visitor(): void $this->browser->request('GET', '/metadata/set?key=baz&value=qux'); $this->browser->request('GET', '/metadata/set?key=foo&value=changed'); - self::assertSame(['foo' => 'changed', 'baz' => 'qux'], self::fetchStoredMetadata($this->getClientId())); + // assertEquals() because MySQL doesn't keep the order of the keys in a JSON column + self::assertEquals(['foo' => 'changed', 'baz' => 'qux'], self::fetchStoredMetadata($this->getClientId())); self::assertSame(1, self::countStoredMetadata()); } From bbb195f33a62d5a15c1d1142c6d2062f112d4530 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Joachim=20L=C3=B8vgaard?= Date: Mon, 26 Oct 2026 11:31:47 +0100 Subject: [PATCH 7/9] Harden the functional tests - Read the database from SETONO_CLIENT_BUNDLE_DATABASE_URL instead of the commonly exported DATABASE_URL, and only drop the bundle's tables - Give each checkout its own temporary directory - Remove the cached container once per process, because Infection swaps in mutated code without changing file times - Always boot the test environment with debug on, whatever APP_ENV and APP_DEBUG say - Rethrow exceptions instead of rendering error pages - Use the entity manager of the metadata class in the helpers, like the bundle does - Only log warnings and errors - Check the raw value of the upgraded version 1 cookie, because the version 1 value parses as version 2 too --- CLAUDE.md | 2 +- composer.json | 1 + tests/Functional/App/Kernel.php | 24 ++++++----- tests/Functional/ClientCookieTest.php | 14 ++----- tests/Functional/FunctionalTestCase.php | 56 +++++++++++++++++++++---- tests/Functional/MetadataTest.php | 1 - 6 files changed, 66 insertions(+), 32 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 1cb395f..74c89bf 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -14,7 +14,7 @@ PHP 8.1+ is required (CI runs 8.1 and 8.2). Use the `8.1`/`8.2` shell switchers - Tests: `composer phpunit` (or `vendor/bin/phpunit`) runs both suites; pick one with `--testsuite unit` or `--testsuite functional` - `tests/Unit`: isolated classes with Prophecy doubles. - - `tests/Functional`: the bundle inside a real Symfony app (`tests/Functional/App/Kernel.php`: FrameworkBundle + DoctrineBundle + this bundle), driven over HTTP with `KernelBrowser` against a SQLite database. Extend `FunctionalTestCase`, which recreates the schema before each test and has helpers to read the stored metadata. Every request gets a new container, like PHP-FPM. The kernel's cache, log and database live in `sys_get_temp_dir()/setono-client-bundle/`, so nothing is generated in the repo and parallel Infection processes don't share a database. Set `DATABASE_URL` to run against another database. + - `tests/Functional`: the bundle inside a real Symfony app (`tests/Functional/App/Kernel.php`: FrameworkBundle + DoctrineBundle + this bundle), driven over HTTP with `KernelBrowser` against a SQLite database. Extend `FunctionalTestCase`, which recreates the schema before each test, rethrows exceptions instead of rendering error pages, and has helpers to read the stored metadata. Every request gets a new container, like PHP-FPM. The kernel always runs in the `test` environment with debug on, whatever `APP_ENV`/`APP_DEBUG` say (the Symfony CLI's `php` sets `APP_ENV=dev`). Its cache, log and database live in `sys_get_temp_dir()/setono-client-bundle-/`, so nothing is generated in the repo, and neither two checkouts nor parallel Infection processes share a container or a database. Each PHPUnit process removes the cached container once, because Infection swaps in mutated code without changing file times. Set `SETONO_CLIENT_BUNDLE_DATABASE_URL` (not `DATABASE_URL`, on purpose) to run against another database: the tests drop the bundle's tables in it, and all Infection processes share it, so run Infection with `--threads=1` then. - No DAMA DoctrineTestBundle on purpose: its `doctrine/doctrine-bundle ^2.11` / `doctrine/dbal ^3.3` requirements would lift the CI "lowest" jobs from ORM 2/DBAL 2.13 to ORM 3/DBAL 3.8, so ORM 2 would no longer be tested. With a single table, recreating the schema per test is fast enough. - Single test: `vendor/bin/phpunit --filter after_loading_the_correct_parameter_has_been_set` or by path `vendor/bin/phpunit tests/Unit/DependencyInjection/SetonoClientExtensionTest.php` - Static analysis: `composer analyse` (PHPStan at `level: max`, config in `phpstan.neon.dist`; auto-loads the doctrine/symfony/phpunit/strict-rules extensions via `phpstan/extension-installer`) diff --git a/composer.json b/composer.json index 0a1fba3..0cfebe2 100644 --- a/composer.json +++ b/composer.json @@ -46,6 +46,7 @@ "shipmonk/composer-dependency-analyser": "^1.8.4", "sylius-labs/coding-standard": "^4.5", "symfony/browser-kit": "^6.4 || ^7.0", + "symfony/filesystem": "^6.4 || ^7.0", "symfony/framework-bundle": "^6.4 || ^7.0", "symfony/routing": "^6.4 || ^7.0", "webmozart/assert": "^1.11 || ^2.0" diff --git a/tests/Functional/App/Kernel.php b/tests/Functional/App/Kernel.php index de29a3b..d1a341b 100644 --- a/tests/Functional/App/Kernel.php +++ b/tests/Functional/App/Kernel.php @@ -32,12 +32,12 @@ public function getProjectDir(): string public function getCacheDir(): string { - return $this->getVarDir() . '/cache/' . $this->getEnvironment(); + return self::getVarDir() . '/cache/' . $this->getEnvironment(); } public function getLogDir(): string { - return $this->getVarDir() . '/log'; + return self::getVarDir() . '/log'; } /** @@ -46,15 +46,16 @@ public function getLogDir(): string */ protected function getConfigDir(): string { - return $this->getVarDir() . '/config'; + return self::getVarDir() . '/config'; } /** - * Infection runs the tests in parallel processes and gives each a token, so each process gets its own database + * Each checkout gets its own directory, and so does each of the parallel processes that Infection runs and gives a token. + * That way, no two test runs share a container or a database */ - private function getVarDir(): string + public static function getVarDir(): string { - return sprintf('%s/setono-client-bundle%s', sys_get_temp_dir(), (string) getenv('TEST_TOKEN')); + return sprintf('%s/setono-client-bundle-%s%s', sys_get_temp_dir(), hash('xxh3', __DIR__), (string) getenv('TEST_TOKEN')); } protected function configureContainer(ContainerConfigurator $container): void @@ -67,18 +68,19 @@ protected function configureContainer(ContainerConfigurator $container): void 'php_errors' => ['log' => true], ]); - // Set the DATABASE_URL environment variable to run the tests against another database - $container->parameters()->set('env(DATABASE_URL)', 'sqlite:///%kernel.cache_dir%/test.db'); + // Set the SETONO_CLIENT_BUNDLE_DATABASE_URL environment variable to run the tests against another database. The tests drop + // the bundle's tables in it. The variable isn't DATABASE_URL, so the tests can't pick up the database of another application + $container->parameters()->set('env(SETONO_CLIENT_BUNDLE_DATABASE_URL)', 'sqlite:///%kernel.cache_dir%/test.db'); $container->extension('doctrine', [ - 'dbal' => ['url' => '%env(resolve:DATABASE_URL)%'], + 'dbal' => ['url' => '%env(resolve:SETONO_CLIENT_BUNDLE_DATABASE_URL)%'], 'orm' => [], ]); $services = $container->services(); - // Logs to a file instead of the test output - $services->set('logger', Logger::class)->args(['debug', '%kernel.cache_dir%/log.txt']); + // Logs warnings and errors to a file instead of the test output. It's in the cache dir, so it's removed with the cache + $services->set('logger', Logger::class)->args(['warning', '%kernel.cache_dir%/log.txt']); $services ->defaults()->autowire()->autoconfigure() diff --git a/tests/Functional/ClientCookieTest.php b/tests/Functional/ClientCookieTest.php index eac3e3c..53967df 100644 --- a/tests/Functional/ClientCookieTest.php +++ b/tests/Functional/ClientCookieTest.php @@ -82,9 +82,9 @@ public function it_upgrades_a_version_1_cookie(): void self::assertSame('client-id', $this->browser->getResponse()->getContent()); - $cookie = $this->getCookie(); - self::assertSame('client-id', $cookie->clientId); - self::assertSame(2, $cookie->version); + // Checks the raw value, because the version 1 cookie that was sent parses as a version 2 cookie too + self::assertResponseHasCookie(self::COOKIE_NAME); + self::assertMatchesRegularExpression('/^2\.\d+\.\d+\.client-id$/', (string) $this->browser->getCookieJar()->get(self::COOKIE_NAME)?->getValue()); } /** @@ -119,12 +119,4 @@ private function setCookie(string $value): void { $this->browser->getCookieJar()->set(new BrowserCookie(self::COOKIE_NAME, $value, null, '/', 'localhost')); } - - private function getCookie(): Cookie - { - $cookie = $this->browser->getCookieJar()->get(self::COOKIE_NAME); - self::assertNotNull($cookie); - - return Cookie::fromString($cookie->getValue()); - } } diff --git a/tests/Functional/FunctionalTestCase.php b/tests/Functional/FunctionalTestCase.php index a26a6c6..4679615 100644 --- a/tests/Functional/FunctionalTestCase.php +++ b/tests/Functional/FunctionalTestCase.php @@ -13,6 +13,7 @@ use Setono\ClientBundle\Tests\Functional\App\Kernel; use Symfony\Bundle\FrameworkBundle\KernelBrowser; use Symfony\Bundle\FrameworkBundle\Test\WebTestCase; +use Symfony\Component\Filesystem\Filesystem; /** * Boots the test application with a fresh database. Like PHP-FPM, every request is handled by a new container @@ -23,14 +24,36 @@ abstract class FunctionalTestCase extends WebTestCase protected KernelBrowser $browser; + private static bool $cacheCleared = false; + + /** + * Infection runs every mutant in a new process, but swaps in the mutated code without changing the modification time of the file. + * A container compiled by an earlier process would look fresh, and could lack the mutant or contain the mutant of that process. + * So every process compiles the container again + */ + public static function setUpBeforeClass(): void + { + parent::setUpBeforeClass(); + + if (!self::$cacheCleared) { + (new Filesystem())->remove(Kernel::getVarDir() . '/cache'); + self::$cacheCleared = true; + } + } + protected function setUp(): void { - $this->browser = static::createClient(); + // Fixed, so APP_ENV and APP_DEBUG in the environment, e.g. from the Symfony CLI, don't change the application + $this->browser = static::createClient(['environment' => 'test', 'debug' => true]); + + // An exception fails the test instead of becoming an error response + $this->browser->catchExceptions(false); $manager = self::getEntityManager(); + $classes = $manager->getMetadataFactory()->getAllMetadata(); $schemaTool = new SchemaTool($manager); - $schemaTool->dropDatabase(); - $schemaTool->createSchema($manager->getMetadataFactory()->getAllMetadata()); + $schemaTool->dropSchema($classes); + $schemaTool->createSchema($classes); } protected static function getKernelClass(): string @@ -39,14 +62,22 @@ protected static function getKernelClass(): string } /** - * Returns the client id stored in the browser's cookie + * Returns the cookie stored in the browser */ - protected function getClientId(): string + protected function getCookie(): Cookie { $cookie = $this->browser->getCookieJar()->get(self::COOKIE_NAME); self::assertNotNull($cookie, 'The browser does not have the client id cookie'); - return Cookie::fromString($cookie->getValue())->clientId; + return Cookie::fromString($cookie->getValue()); + } + + /** + * Returns the client id stored in the browser's cookie + */ + protected function getClientId(): string + { + return $this->getCookie()->clientId; } /** @@ -112,7 +143,8 @@ protected static function getEntityManager(): EntityManagerInterface $managerRegistry = self::getContainer()->get('doctrine'); self::assertInstanceOf(ManagerRegistry::class, $managerRegistry); - $manager = $managerRegistry->getManager(); + // The bundle uses the manager of the metadata class too + $manager = $managerRegistry->getManagerForClass(self::getMetadataClass()); self::assertInstanceOf(EntityManagerInterface::class, $manager); return $manager; @@ -122,11 +154,19 @@ protected static function getEntityManager(): EntityManagerInterface * @return ClassMetadata */ protected static function getMetadataClassMetadata(): ClassMetadata + { + return self::getEntityManager()->getClassMetadata(self::getMetadataClass()); + } + + /** + * @return class-string + */ + private static function getMetadataClass(): string { $metadataClass = self::getContainer()->getParameter('setono_client.metadata_class'); self::assertIsString($metadataClass); self::assertTrue(class_exists($metadataClass)); - return self::getEntityManager()->getClassMetadata($metadataClass); + return $metadataClass; } } diff --git a/tests/Functional/MetadataTest.php b/tests/Functional/MetadataTest.php index dcfc6f5..d6ba3b4 100644 --- a/tests/Functional/MetadataTest.php +++ b/tests/Functional/MetadataTest.php @@ -104,7 +104,6 @@ public function it_does_not_query_the_database_when_the_metadata_is_not_used(): self::assertResponseIsSuccessful(); // Proves that the query would have failed - $this->browser->catchExceptions(false); $this->expectException(TableNotFoundException::class); $this->browser->request('GET', '/metadata'); } From 217e83fc7212684166efc4c39b39e0d5387aac01 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Joachim=20L=C3=B8vgaard?= Date: Mon, 26 Oct 2026 12:04:17 +0100 Subject: [PATCH 8/9] Require DoctrineBundle 2.11.3 instead of conflicting with 2.11.0 to 2.11.2 DoctrineBundle 2.11 requires DBAL 3.7 and conflicts with ORM below 2.17, so the DBAL and ORM requirements now say so too. --- README.md | 4 +++- composer.json | 9 +++------ 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/README.md b/README.md index 92cd20c..fb188f2 100644 --- a/README.md +++ b/README.md @@ -36,7 +36,9 @@ Typical use cases: first‑party analytics, marketing attribution, returning‑v | --- | --- | | PHP | `>= 8.1` | | Symfony | `^6.4` or `^7.0` | -| Doctrine ORM | `^2.0` or `^3.0` | +| Doctrine ORM | `^2.17` or `^3.0` | +| Doctrine DBAL | `^3.7` or `^4.0` | +| DoctrineBundle | `^2.11.3` | ## Installation diff --git a/composer.json b/composer.json index 0cfebe2..b8cfa06 100644 --- a/composer.json +++ b/composer.json @@ -11,9 +11,9 @@ ], "require": { "php": ">=8.1", - "doctrine/dbal": "^2.13 || ^3.0 || ^4.0", - "doctrine/doctrine-bundle": "^2.7", - "doctrine/orm": "^2.0 || ^3.0", + "doctrine/dbal": "^3.7 || ^4.0", + "doctrine/doctrine-bundle": "^2.11.3", + "doctrine/orm": "^2.17 || ^3.0", "doctrine/persistence": "^2.5 || ^3.0", "psr/event-dispatcher": "^1.0", "setono/client": "^1.1.1", @@ -51,9 +51,6 @@ "symfony/routing": "^6.4 || ^7.0", "webmozart/assert": "^1.11 || ^2.0" }, - "conflict": { - "doctrine/doctrine-bundle": ">=2.11.0 <2.11.3" - }, "prefer-stable": true, "autoload": { "psr-4": { From 9691b112737a69668c8a051c80248dad572a5a5d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Joachim=20L=C3=B8vgaard?= Date: Mon, 26 Oct 2026 12:04:17 +0100 Subject: [PATCH 9/9] Isolate the functional tests with DAMA DoctrineTestBundle --- CLAUDE.md | 3 +- composer.json | 2 ++ phpunit.xml.dist | 5 ++- tests/Functional/App/Kernel.php | 16 +++++++++- tests/Functional/FunctionalTestCase.php | 28 ++--------------- tests/Functional/MetadataTest.php | 35 +++++++++++++++++---- tests/bootstrap.php | 42 +++++++++++++++++++++++++ 7 files changed, 95 insertions(+), 36 deletions(-) create mode 100644 tests/bootstrap.php diff --git a/CLAUDE.md b/CLAUDE.md index 74c89bf..8ee652d 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -14,8 +14,7 @@ PHP 8.1+ is required (CI runs 8.1 and 8.2). Use the `8.1`/`8.2` shell switchers - Tests: `composer phpunit` (or `vendor/bin/phpunit`) runs both suites; pick one with `--testsuite unit` or `--testsuite functional` - `tests/Unit`: isolated classes with Prophecy doubles. - - `tests/Functional`: the bundle inside a real Symfony app (`tests/Functional/App/Kernel.php`: FrameworkBundle + DoctrineBundle + this bundle), driven over HTTP with `KernelBrowser` against a SQLite database. Extend `FunctionalTestCase`, which recreates the schema before each test, rethrows exceptions instead of rendering error pages, and has helpers to read the stored metadata. Every request gets a new container, like PHP-FPM. The kernel always runs in the `test` environment with debug on, whatever `APP_ENV`/`APP_DEBUG` say (the Symfony CLI's `php` sets `APP_ENV=dev`). Its cache, log and database live in `sys_get_temp_dir()/setono-client-bundle-/`, so nothing is generated in the repo, and neither two checkouts nor parallel Infection processes share a container or a database. Each PHPUnit process removes the cached container once, because Infection swaps in mutated code without changing file times. Set `SETONO_CLIENT_BUNDLE_DATABASE_URL` (not `DATABASE_URL`, on purpose) to run against another database: the tests drop the bundle's tables in it, and all Infection processes share it, so run Infection with `--threads=1` then. - - No DAMA DoctrineTestBundle on purpose: its `doctrine/doctrine-bundle ^2.11` / `doctrine/dbal ^3.3` requirements would lift the CI "lowest" jobs from ORM 2/DBAL 2.13 to ORM 3/DBAL 3.8, so ORM 2 would no longer be tested. With a single table, recreating the schema per test is fast enough. + - `tests/Functional`: the bundle inside a real Symfony app (`tests/Functional/App/Kernel.php`: FrameworkBundle + DoctrineBundle + DAMA DoctrineTestBundle + this bundle), driven over HTTP with `KernelBrowser` against a SQLite database. `tests/bootstrap.php` creates the schema once per PHPUnit process, and DAMA's PHPUnit extension (registered in `phpunit.xml.dist`) runs every test in a transaction that it rolls back. So tests must not run DDL: MySQL commits a transaction on DDL. To check which queries a request ran, read DoctrineBundle's debug data holder (see `MetadataTest::getExecutedQueries()`). Extend `FunctionalTestCase`, which rethrows exceptions instead of rendering error pages and has helpers to read the stored metadata. Every request gets a new container, like PHP-FPM. The kernel always runs in the `test` environment with debug on, whatever `APP_ENV`/`APP_DEBUG` say (the Symfony CLI's `php` sets `APP_ENV=dev`). Its cache, log and database live in `sys_get_temp_dir()/setono-client-bundle-/`, so nothing is generated in the repo, and neither two checkouts nor parallel Infection processes share a container or a database. The bootstrap also removes the cached container, because Infection swaps in mutated code without changing file times. Set `SETONO_CLIENT_BUNDLE_DATABASE_URL` (not `DATABASE_URL`, on purpose) to run against another database: the bootstrap drops the bundle's tables in it, and all Infection processes share it, so run Infection with `--threads=1` then. - Single test: `vendor/bin/phpunit --filter after_loading_the_correct_parameter_has_been_set` or by path `vendor/bin/phpunit tests/Unit/DependencyInjection/SetonoClientExtensionTest.php` - Static analysis: `composer analyse` (PHPStan at `level: max`, config in `phpstan.neon.dist`; auto-loads the doctrine/symfony/phpunit/strict-rules extensions via `phpstan/extension-installer`) - Check style: `composer check-style` (ECS, Sylius Labs ruleset) diff --git a/composer.json b/composer.json index b8cfa06..3e837a9 100644 --- a/composer.json +++ b/composer.json @@ -27,6 +27,7 @@ "symfony/var-exporter": "^6.4 || ^7.0" }, "require-dev": { + "dama/doctrine-test-bundle": "^8.2", "ergebnis/composer-normalize": "^2.50", "infection/infection": "^0.27 || ^0.28", "jangregor/phpstan-prophecy": "^2.3", @@ -46,6 +47,7 @@ "shipmonk/composer-dependency-analyser": "^1.8.4", "sylius-labs/coding-standard": "^4.5", "symfony/browser-kit": "^6.4 || ^7.0", + "symfony/doctrine-bridge": "^6.4 || ^7.0", "symfony/filesystem": "^6.4 || ^7.0", "symfony/framework-bundle": "^6.4 || ^7.0", "symfony/routing": "^6.4 || ^7.0", diff --git a/phpunit.xml.dist b/phpunit.xml.dist index cc758c9..2019b86 100644 --- a/phpunit.xml.dist +++ b/phpunit.xml.dist @@ -1,6 +1,6 @@ @@ -10,6 +10,9 @@ tests/Functional + + + diff --git a/tests/Functional/App/Kernel.php b/tests/Functional/App/Kernel.php index d1a341b..1be4160 100644 --- a/tests/Functional/App/Kernel.php +++ b/tests/Functional/App/Kernel.php @@ -4,7 +4,9 @@ namespace Setono\ClientBundle\Tests\Functional\App; +use DAMA\DoctrineTestBundle\DAMADoctrineTestBundle; use Doctrine\Bundle\DoctrineBundle\DoctrineBundle; +use Doctrine\DBAL\Connection; use Setono\ClientBundle\SetonoClientBundle; use Setono\ClientBundle\Tests\Functional\App\Controller\ClientController; use Symfony\Bundle\FrameworkBundle\FrameworkBundle; @@ -22,6 +24,7 @@ public function registerBundles(): iterable { yield new FrameworkBundle(); yield new DoctrineBundle(); + yield new DAMADoctrineTestBundle(); yield new SetonoClientBundle(); } @@ -72,8 +75,16 @@ protected function configureContainer(ContainerConfigurator $container): void // the bundle's tables in it. The variable isn't DATABASE_URL, so the tests can't pick up the database of another application $container->parameters()->set('env(SETONO_CLIENT_BUNDLE_DATABASE_URL)', 'sqlite:///%kernel.cache_dir%/test.db'); + $dbal = ['url' => '%env(resolve:SETONO_CLIENT_BUNDLE_DATABASE_URL)%']; + + // DAMA DoctrineTestBundle runs each test in a transaction, so the transactions of the bundle need savepoints. + // DBAL 4 always uses them and deprecates the option. Like DoctrineBundle, this tells DBAL 3 by its getEventManager() + if (method_exists(Connection::class, 'getEventManager')) { + $dbal['use_savepoints'] = true; + } + $container->extension('doctrine', [ - 'dbal' => ['url' => '%env(resolve:SETONO_CLIENT_BUNDLE_DATABASE_URL)%'], + 'dbal' => $dbal, 'orm' => [], ]); @@ -82,6 +93,9 @@ protected function configureContainer(ContainerConfigurator $container): void // Logs warnings and errors to a file instead of the test output. It's in the cache dir, so it's removed with the cache $services->set('logger', Logger::class)->args(['warning', '%kernel.cache_dir%/log.txt']); + // Lets the tests read the queries that a request executed + $services->alias('test.doctrine.debug_data_holder', 'doctrine.debug_data_holder')->public(); + $services ->defaults()->autowire()->autoconfigure() ->load(__NAMESPACE__ . '\\Controller\\', __DIR__ . '/Controller')->tag('controller.service_arguments') diff --git a/tests/Functional/FunctionalTestCase.php b/tests/Functional/FunctionalTestCase.php index 4679615..e652d16 100644 --- a/tests/Functional/FunctionalTestCase.php +++ b/tests/Functional/FunctionalTestCase.php @@ -7,16 +7,15 @@ use Doctrine\DBAL\Connection; use Doctrine\ORM\EntityManagerInterface; use Doctrine\ORM\Mapping\ClassMetadata; -use Doctrine\ORM\Tools\SchemaTool; use Doctrine\Persistence\ManagerRegistry; use Setono\Client\Cookie; use Setono\ClientBundle\Tests\Functional\App\Kernel; use Symfony\Bundle\FrameworkBundle\KernelBrowser; use Symfony\Bundle\FrameworkBundle\Test\WebTestCase; -use Symfony\Component\Filesystem\Filesystem; /** - * Boots the test application with a fresh database. Like PHP-FPM, every request is handled by a new container + * Boots the test application. tests/bootstrap.php creates the schema once, and DAMA DoctrineTestBundle rolls back + * what each test writes. Like PHP-FPM, every request is handled by a new container */ abstract class FunctionalTestCase extends WebTestCase { @@ -24,23 +23,6 @@ abstract class FunctionalTestCase extends WebTestCase protected KernelBrowser $browser; - private static bool $cacheCleared = false; - - /** - * Infection runs every mutant in a new process, but swaps in the mutated code without changing the modification time of the file. - * A container compiled by an earlier process would look fresh, and could lack the mutant or contain the mutant of that process. - * So every process compiles the container again - */ - public static function setUpBeforeClass(): void - { - parent::setUpBeforeClass(); - - if (!self::$cacheCleared) { - (new Filesystem())->remove(Kernel::getVarDir() . '/cache'); - self::$cacheCleared = true; - } - } - protected function setUp(): void { // Fixed, so APP_ENV and APP_DEBUG in the environment, e.g. from the Symfony CLI, don't change the application @@ -48,12 +30,6 @@ protected function setUp(): void // An exception fails the test instead of becoming an error response $this->browser->catchExceptions(false); - - $manager = self::getEntityManager(); - $classes = $manager->getMetadataFactory()->getAllMetadata(); - $schemaTool = new SchemaTool($manager); - $schemaTool->dropSchema($classes); - $schemaTool->createSchema($classes); } protected static function getKernelClass(): string diff --git a/tests/Functional/MetadataTest.php b/tests/Functional/MetadataTest.php index d6ba3b4..09fae06 100644 --- a/tests/Functional/MetadataTest.php +++ b/tests/Functional/MetadataTest.php @@ -4,8 +4,8 @@ namespace Setono\ClientBundle\Tests\Functional; -use Doctrine\DBAL\Exception\TableNotFoundException; use Setono\Client\Metadata; +use Symfony\Bridge\Doctrine\Middleware\Debug\DebugDataHolder; final class MetadataTest extends FunctionalTestCase { @@ -97,15 +97,13 @@ public function it_does_not_query_the_database_when_the_metadata_is_not_used(): { $this->browser->request('GET', '/metadata/set?key=foo&value=bar'); - // From now on, every query on the metadata fails - self::getConnection()->executeStatement(sprintf('DROP TABLE %s', self::getMetadataClassMetadata()->getTableName())); - $this->browser->request('GET', '/'); self::assertResponseIsSuccessful(); + self::assertSame([], self::getExecutedQueries()); - // Proves that the query would have failed - $this->expectException(TableNotFoundException::class); + // Proves that the queries of a request are recorded $this->browser->request('GET', '/metadata'); + self::assertNotSame([], self::getExecutedQueries()); } /** @@ -172,4 +170,29 @@ private function getResponseData(): array return $data; } + + /** + * Returns the SQL of the queries that the last request executed, as recorded by DoctrineBundle in debug mode + * + * @return list + */ + private static function getExecutedQueries(): array + { + $debugDataHolder = self::getContainer()->get('test.doctrine.debug_data_holder'); + self::assertInstanceOf(DebugDataHolder::class, $debugDataHolder); + + $queries = []; + foreach ($debugDataHolder->getData() as $connectionQueries) { + self::assertIsArray($connectionQueries); + + foreach ($connectionQueries as $query) { + self::assertIsArray($query); + self::assertIsString($query['sql']); + + $queries[] = $query['sql']; + } + } + + return $queries; + } } diff --git a/tests/bootstrap.php b/tests/bootstrap.php new file mode 100644 index 0000000..343a11f --- /dev/null +++ b/tests/bootstrap.php @@ -0,0 +1,42 @@ +remove(Kernel::getVarDir() . '/cache'); + +// DAMA DoctrineTestBundle rolls back what each test writes, so the schema is created once, before it starts its transactions +foreach (['test'] as $environment) { + $kernel = new Kernel($environment, true); + $kernel->boot(); + + $container = $kernel->getContainer(); + + $metadataClass = $container->getParameter('setono_client.metadata_class'); + Assert::string($metadataClass); + Assert::classExists($metadataClass); + + $managerRegistry = $container->get('doctrine'); + Assert::isInstanceOf($managerRegistry, ManagerRegistry::class); + + $manager = $managerRegistry->getManagerForClass($metadataClass); + Assert::isInstanceOf($manager, EntityManagerInterface::class); + + $classes = $manager->getMetadataFactory()->getAllMetadata(); + $schemaTool = new SchemaTool($manager); + $schemaTool->dropSchema($classes); + $schemaTool->createSchema($classes); + + $kernel->shutdown(); +}