dmaicher / doctrine-test-bundle

Symfony bundle to isolate your app's doctrine database tests and improve the test performance
MIT License
1.08k stars 60 forks source link

Transaction is started before connections are created #168

Closed rubenrubiob closed 3 years ago

rubenrubiob commented 3 years ago

Hi!

I am trying this bundle within a brand new Symfony project, in its 5.3 version and some customizations that I think are not relevant for this case. However, I am not able to make the bundle work.

I installed the bundle using Symfony Flex and I set up the extension in the phpunit.xml.dist file. I use PHPUnit standalone (phpunit/phpunit):

<?xml version="1.0" encoding="UTF-8"?>

<!-- https://phpunit.readthedocs.io/en/latest/configuration.html -->
<phpunit xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
         xsi:noNamespaceSchemaLocation="vendor/phpunit/phpunit/phpunit.xsd"
         backupGlobals="false"
         colors="true"
         bootstrap="tests/bootstrap.php"
         convertDeprecationsToExceptions="false"
         executionOrder="defects"
>
    <php>
        <ini name="display_errors" value="1" />
        <ini name="error_reporting" value="-1" />
        <server name="APP_ENV" value="test" force="true" />
        <server name="SHELL_VERBOSITY" value="-1" />
        <server name="SYMFONY_PHPUNIT_REMOVE" value="" />
        <server name="SYMFONY_PHPUNIT_VERSION" value="9.5" />
    </php>

    <testsuites>
        <testsuite name="Project Test Suite">
            <directory>tests</directory>
        </testsuite>
    </testsuites>

    <coverage processUncoveredFiles="true">
        <include>
            <directory suffix=".php">src</directory>
        </include>
    </coverage>

    <extensions>
        <extension class="DAMA\DoctrineTestBundle\PHPUnit\PHPUnitExtension" />
    </extensions>
</phpunit>

Hooks are actually called, but I think I am missing something, because I do not think they are called in the right order. My (simplified) test case is this one:

use Doctrine\ORM\EntityManagerInterface;
use Domain\Entity\Platform;
use Infrastructure\Persistence\Doctrine\Repository\DoctrinePlatformWriteRepository;
use Symfony\Bundle\FrameworkBundle\Test\KernelTestCase;

class DoctrinePlatformWriteRepositoryTest extends KernelTestCase
{
    public function testThatCreateAPlatformAndPersistItWorksProperly(): void
    {
        self::bootKernel();

        $container = self::getContainer();

        $doctrinePlatformWriteRepository = new DoctrinePlatformWriteRepository(
            $container->get(EntityManagerInterface::class)
        );

        $platform = Platform::create('foo');

        $doctrinePlatformWriteRepository->add($platform);

        \DAMA\DoctrineTestBundle\Doctrine\DBAL\StaticDriver::commit(); // For debugging
    }
}

The bootstrap file is the one that comes with Symfony:

<?php

use Symfony\Component\Dotenv\Dotenv;

require dirname(__DIR__).'/vendor/autoload.php';

if (file_exists(dirname(__DIR__).'/config/bootstrap.php')) {
    require dirname(__DIR__).'/config/bootstrap.php';
} elseif (method_exists(Dotenv::class, 'bootEnv')) {
    (new Dotenv())->bootEnv(dirname(__DIR__).'/.env');
}

When I run my test and I inspect the calls using xDebug, I see this execution order:

  1. \DAMA\DoctrineTestBundle\PHPUnit\PHPUnitExtension::executeBeforeFirstTest is called, which is correct.
  2. \DAMA\DoctrineTestBundle\PHPUnit\PHPUnitExtension::executeBeforeTest is called. This method calls \DAMA\DoctrineTestBundle\Doctrine\DBAL\AbstractStaticDriver::beginTransaction, that starts the transaction on each connection defined. However, when I inspect the connections using xDebug, I see that there is no connection defined:
image
  1. The test case is executed. It is at this moment, when I try to get the ConnectionInterface from the container, that \DAMA\DoctrineTestBundle\Doctrine\DBAL\AbstractStaticDriver::__construct is called, creating the connection.
  2. When I call \DAMA\DoctrineTestBundle\Doctrine\DBAL\StaticDriver::commit(), I see there is a connection defined, that it is correct. However, as no transaction has been started, I get an error saying There is no active transaction.
image

The problem, as you can see, is that the transaction is started before the connection is created. Am I missing something?

I saw that in https://github.com/dmaicher/doctrine-test-bundle/blob/master/tests/bootstrap.php you boot the kernel and create a Connection to create the database. Maybe that is what I am missing, creating a Connection before any test execution, so the StaticDriver has the connection defined (even though you then shutdown the kernel)?

I have checked the library internally, and searched through the issues of this repository, but I am not able to see the problem.

Just in case you need it, these are the require sections of my composer.json:

{
    // ...
    "require": {
        "php": ">=8.0.8",
        "ext-ctype": "*",
        "ext-iconv": "*",
        "composer/package-versions-deprecated": "1.11.99.2",
        "doctrine/annotations": "^1.0",
        "doctrine/dbal": "^2.13",
        "doctrine/doctrine-bundle": "^2.4",
        "doctrine/orm": "^2.9",
        "google/protobuf": "^3.17",
        "league/openapi-psr7-validator": "^0.16.1",
        "league/tactician": "^1.1",
        "league/tactician-bundle": "^1.3",
        "league/tactician-doctrine": "^1.1",
        "phpdocumentor/reflection-docblock": "^5.2",
        "symfony/console": "5.3.*",
        "symfony/dependency-injection": "5.3.*",
        "symfony/dotenv": "5.3.*",
        "symfony/flex": "^1.3.1",
        "symfony/framework-bundle": "5.3.*",
        "symfony/http-foundation": "5.3.*",
        "symfony/http-kernel": "5.3.*",
        "symfony/property-access": "5.3.*",
        "symfony/property-info": "5.3.*",
        "symfony/proxy-manager-bridge": "5.3.*",
        "symfony/routing": "5.3.*",
        "symfony/runtime": "5.3.*",
        "symfony/serializer": "5.3.*",
        "symfony/serializer-pack": "^1.0",
        "symfony/validator": "5.3.*",
        "symfony/yaml": "5.3.*",
        "thecodingmachine/safe": "^1.3"
    },
    "replace": {
        "symfony/polyfill-ctype": "*",
        "symfony/polyfill-iconv": "*",
        "symfony/polyfill-php72": "*"
    },
    "conflict": {
        "symfony/symfony": "*"
    },
    "require-dev": {
        "brianium/paratest": "^6.3",
        "dama/doctrine-test-bundle": "^6.6",
        "doctrine/coding-standard": "^9.0",
        "marartner/psalm-no-empty": "^1.1",
        "marartner/psalm-strict-equality": "^1.0",
        "phpstan/extension-installer": "^1.1",
        "phpstan/phpstan": "^0.12.92",
        "phpstan/phpstan-doctrine": "^0.12.42",
        "phpstan/phpstan-phpunit": "^0.12.21",
        "phpstan/phpstan-strict-rules": "^0.12.10",
        "phpstan/phpstan-symfony": "^0.12.38",
        "phpunit/phpunit": "^9.5",
        "psalm/plugin-phpunit": "^0.16.1",
        "psalm/plugin-symfony": "^2.4",
        "roave/no-floaters": "^1.4",
        "roave/no-leaks": "^1.3",
        "roave/security-advisories": "dev-latest",
        "spaze/phpstan-disallowed-calls": "^1.5",
        "squizlabs/php_codesniffer": "^3.6",
        "symfony/stopwatch": "^5.3",
        "symfony/twig-bundle": "^5.3",
        "symfony/web-profiler-bundle": "^5.3",
        "thecodingmachine/phpstan-safe-rule": "^1.0",
        "vimeo/psalm": "^4.8",
        "weirdan/doctrine-psalm-plugin": "^1.0"
    // ...
}

Thank you!

Regards!

dmaicher commented 3 years ago

Are you able to provide a minimal reproducer that shows a failing test case?

rubenrubiob commented 3 years ago

I am trying to reproduce in a brand-new repository, but I am not able to get it to fail, it works well. Therefore, I think it is a problem with the set-up I had in my project. I will look into that to see if I find the issue. I will let you know if I find something weird, but I think the bundle works well 👍

I will close the issue, as it seems it works well.

Sorry for bothering you, and thank you for your time!

Jean85 commented 2 years ago

I stumbled on this, or something similar. I've found that the issue was that the container was inadvertently triggered very early, nested in a data provider, trying to get the EntityManager, which would spawn the connections too early, before the transaction started. This would result in stuff being flushed to DB.

easy-death commented 2 years ago

I got this error when accidentally added both listener and extension to phpunit.xml.dist. This caused rollback to call twice.

JoniJnm commented 1 year ago

If the connection doesn't exists in AbstractStaticDriver::beginTransaction, the transaction will be started here:

https://github.com/dmaicher/doctrine-test-bundle/blob/b60538abb4ad2410b03c9b150e1a806079c3d0ac/src/DAMA/DoctrineTestBundle/Doctrine/DBAL/PostConnectEventListener.php#L19

dmaicher commented 1 year ago

In case someone wants to try https://github.com/dmaicher/doctrine-test-bundle/pull/232

This changes the whole flow a bit to be compatible with the upcoming Doctrine DBAL v4. Maybe this issue is then also resolved :thinking:

leongersen commented 10 months ago

I just ran into this (in v7.2.1) and debugged extensively.

The issue is that self::$connections is empty in \DAMA\DoctrineTestBundle\Doctrine\DBAL\StaticDriver::beginTransaction.

This happens because self::$keepStaticConnections is false in \DAMA\DoctrineTestBundle\Doctrine\DBAL\StaticDriver::connect.

This is unexpected because it should be set to true in \DAMA\DoctrineTestBundle\PHPUnit\PHPUnitExtension::executeBeforeFirstTest.

This leads me to the conclusion that a database connection is made before executeBeforeFirstTest can be called.

Dumping debug_backtrace() in \StaticDriver::connect finds the culprit:

I have tests that fetch entities from the database in a dataProvider. These dataproviders are called before executeBeforeFirstTest.

TLDR Don't open database connections in a dataProvider.

Jean85 commented 10 months ago

FYI this is the reason why PHPUnit 10 deprecates having non-static data providers: to discourage having complex logic that reaches into the app under test and its services, since data providers are called very early in PHPUnit lifecycle.