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

Trait or annotation to disable DAMA in some files #182

Open bastien70 opened 2 years ago

bastien70 commented 2 years ago

Hello, I am using DAMA for my tests.

This is useful for some tests, but not for End to End tests, since it does not support it.

But it gives me bugs because DAMA still manages to rollback certain transactions in the middle of my EndToEnd tests (those requiring that I initialize a record in the database. DAMA rollback directly)

It would be nice to have a trait or something that allows us to say in some files to disable DAMA support.

It forces me to create all the entities I need upstream, that's not cool

I found the previous issue : #171 but it doesn't seem to suit what i want

dmaicher commented 2 years ago

Feel free to look into it :+1:

A workaround might be something like this (not tested):


use DAMA\DoctrineTestBundle\Doctrine\DBAL\StaticDriver;
use PHPUnit\Framework\TestCase;

class MyTestWithoutDAMATransactionManagement extends TestCase
{
    public static function setUpBeforeClass(): void
    {
        StaticDriver::setKeepStaticConnections(false);
    }

    public static function tearDownAfterClass(): void
    {
        StaticDriver::setKeepStaticConnections(true);
    }

    // your tests
}
bastien70 commented 2 years ago

It seems to work ! Thank you very much !

dmaicher commented 2 years ago

I also recently had a use-case for this on one of my apps. I looked into it a bit and with the current PHPUnit Extensions this seems rather complicated to achieve as we don't have access to much in the Hooks.

I would wait for the new PHPUnit 10 event system to achieve this.

bastien70 commented 2 years ago

I also recently had a use-case for this on one of my apps. I looked into it a bit and with the current PHPUnit Extensions this seems rather complicated to achieve as we don't have access to much in the Hooks.

I would wait for the new PHPUnit 10 event system to achieve this.

Awesome ! I can not wait to see it. I was a bit disturbed because with your fallback method we get a warning on each of the tests, because the AbstractStaticDriver is supposed to be internal

DigitalTimK commented 2 years ago

That's a feature that would be also useful for me!!!!

But don't forget here to call parent::setUpBeforeClass()and parent::tearDownAfterClass() too.


Update

Solution works only in isolation of that class. Test-classes executed after MyTestWithoutDAMATransactionManagement-class fail, because the database is not reset anymore (Zenstruck\Foundry\Test\ResetDatabase). Unfortunately I have no idea how to solve this ...

chriskaya commented 2 years ago

My PR having been rejected by @dmaicher for reasons I totally understand, I'm now using a custom phpunit extension built on top of doctrine-test-bundle.

If you need this, you may do the same: https://github.com/chriskaya/custom-dama-extension/blob/main/CustomDamaExtension.php

BTW, thx @dmaicher for your work :)

MalikOvaiz commented 2 years ago

Hi,

I am having problem in using this bundle. Data entry in table does not reset after test suit.

I am using symfony 5.4 and phpunit 9.5(under require-dev). I have also enabled enable_static_connection, enable_static_meta_data_cache, enable_static_query_cache in dama_doctrine_test_bundle.yml

And also enabled extension in phpunit.xml.dist

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

I have verified, entry in config/bundles also exists:

DAMA\DoctrineTestBundle\DAMADoctrineTestBundle::class => ['test' => true],

In test case I am inserting following data:

        $sql = "INSERT INTO `data` VALUES ('HELLO', 'Hello')";

        $em = $this->entityManager;
        $stmt = $em->getConnection()->prepare($sql);
        $stmt->execute();

And it still exists there after test execution.

Am I missing something?

kAlvaro commented 1 year ago

My team is using the bundle in Symfony/5.4 and PDO driver with PostgreSQL/14.4 and trying to disable it for specific tests is causing connection leaks.

When enabled it's all good. \DAMA\DoctrineTestBundle\Doctrine\DBAL\StaticDriver::connect() keeps an array of cached connections and picks connections from there. But when you do StaticDriver::setKeepStaticConnections(false) then \Doctrine\DBAL\Driver\PDO\PgSQL\Driver::connect() gets called every time and, for reasons still unknown to me, all new PDO instances remain in memory and pg_stat_activity table shows a growing list of connections with the same exact parameters, in idle status and SET NAMES ... being the last query. Calling connect() every time is literally how regular application calls work, so it must be a problem caused by the bundle driver that sits in between when trying to abuse it for something is wasn't designed to do.

dmaicher commented 1 year ago

My team is using the bundle in Symfony/5.4 and PDO driver with PostgreSQL/14.4 and trying to disable it for specific tests is causing connection leaks.

When enabled it's all good. \DAMA\DoctrineTestBundle\Doctrine\DBAL\StaticDriver::connect() keeps an array of cached connections and picks connections from there. But when you do StaticDriver::setKeepStaticConnections(false) then \Doctrine\DBAL\Driver\PDO\PgSQL\Driver::connect() gets called every time and, for reasons still unknown to me, all new PDO instances remain in memory and pg_stat_activity table shows a growing list of connections with the same exact parameters, in idle status and SET NAMES ... being the last query. Calling connect() every time is literally how regular application calls work, so it must be a problem caused by the bundle driver that sits in between when trying to abuse it for something is wasn't designed to do.

@kAlvaro hm hard to say whats happening there :confused: would you be able to provide a small reproducer?

I mean in case of StaticDriver::setKeepStaticConnections(false) the driver literally just delegates to the native driver :man_shrugging:

https://github.com/dmaicher/doctrine-test-bundle/blob/74a6078bf0c60e971333e7d238fdc076aa268d6a/src/DAMA/DoctrineTestBundle/Doctrine/DBAL/StaticDriver.php#L27

kAlvaro commented 1 year ago

@dmaicher Upon further investigation (it's a complex frustratingly hard to diagnose setup) I'm suspecting that DAMA bundle actually works around a prior leak. If I disable the bundle entirely all my tests start leaking. I apologise for any time lost.

osteel commented 8 months ago

A workaround might be something like this (not tested):

Just stumbled upon this by googling around – this should be mentioned in the doc IMO :)