Conflict with dama/doctrine-test-bundle

[soullivaneuh]

Since the update to 2.0.0-alpha4, I have the following error on each integration test:

1) Tests\AppBundle\Controller\DefaultControllerTest::testIndex
Doctrine\DBAL\Driver\PDOException: SQLSTATE[42000]: Syntax error or access violation: 1305 SAVEPOINT DOCTRINE2_SAVEPOINT_2 does not exist

/code/vendor/doctrine/dbal/lib/Doctrine/DBAL/Driver/PDOConnection.php:62
/code/vendor/dama/doctrine-test-bundle/src/DAMA/DoctrineTestBundle/Doctrine/DBAL/StaticConnection.php:59
/code/vendor/doctrine/dbal/lib/Doctrine/DBAL/Connection.php:1449
/code/vendor/doctrine/dbal/lib/Doctrine/DBAL/Connection.php:1385
/code/vendor/doctrine/orm/lib/Doctrine/ORM/EntityManager.php:242
/code/var/cache/test/Container1xxocvm/EntityManager_9a5be93.php:62
/code/vendor/doctrine/data-fixtures/lib/Doctrine/Common/DataFixtures/Executor/ORMExecutor.php:73
/code/vendor/liip/functional-test-bundle/src/Services/DatabaseTools/ORMDatabaseTool.php:141
/code/vendor/liip/functional-test-bundle/src/Services/DatabaseTools/ORMDatabaseTool.php:80
/code/vendor/liip/functional-test-bundle/src/Services/DatabaseTools/AbstractDatabaseTool.php:135
/code/vendor/liip/functional-test-bundle/src/Test/WebTestCase.php:281
/code/tests/AppBundle/Controller/WebTestCase.php:35

First, I was thinking about a DoctrineTestBundle issue, so I gave more reports in dmaicher/doctrine-test-bundle#58.

But I didn't update this bundle and the issue does not exist anymore if I rollback to 2.0.0-alpha3 version of your bundle.

I suspect the backup/restore services introduced in #398 to cause side effects to doctrine-test-bundle, but I didn't get into investigation yet.

What do those services? Probably same things? There is a way to deactivate them?

Regards.

[Jean85]

I use both those bundles, but I do not use the backup system for the fixtures. I still (sometimes) have this issue. I'll try to investigate this further.

[Jean85]

Additions, in my case:

• the issue is triggered when running tests in parallel, and leveraging the transaction given by the DoctrineTestBundle to achieve DB isolation;
• rolling back to 2.0.0-alpha3 (neither updating to 2.x-dev) did NOT solve the issue; maybe it's just the trigger that disappears?

[soullivaneuh]

@Jean85 Do you have exactly the same issue?

It's kinda weird, I don't use parallel testing and the rollback did "fix" the issue. 🤔

[soullivaneuh]

I did a git bisect of this package and it seems this commit brake my tests: 5923bed.

I don't know how yet, but I can confirm it's related to #398 now.

@alekseytupichenkov @alexislefebvre do you have any idea of how to provide a quickfix? Otherwise, I would suggest to revert it for now.

[Jean85]

I think that parallel testing just triggers the underlying issue, as well as having the alpha4 version.

The diff between alpha3 and alpha4 is this one: 3bca1f2...321db83

The main difference is the introduction of the cache_db feature, and the refactoring of the DB tools.

Thanks for the bisect @soullivaneuh ! Looking at the diff, I think that the issue is in the TRUNCATE, I think is not transaction-safe; this will create issues with the persistent static connection of the DAMA Bundle, which does a transaction rollback after each test.

Which fixture functionality are you using from the Liip bundle?

[soullivaneuh]

I think that the issue is in the TRUNCATE

Certainly because the table related operation are not managed for rollback.

Plus, as I can see, the proposed feature is more or less the same as dama proposed. But I need dama for other non-functionnal tests.

Which fixture functionality are you using from the Liip bundle?

What do you mean by that?

I use Alice v3, is that you wanted to know?

[Jean85]

I use Alice v3, is that you wanted to know?

Yes, it's what I wanted to know; specifically, I would like to know which fixture features do you trigger, because I use none of those.

In my case, I'm probably creating some race condition in the transactions, and since the connection stays the same in consequent tests, one of the transaction fails. I fear that the definitive fix should be done in the DAMA bundle...

[soullivaneuh]

Well, I don't use extra configuration. Here is my WebTestCase override:

namespace Tests\AppBundle\Controller;

use Liip\FunctionalTestBundle\Test\WebTestCase as BaseTestCase;
use Symfony\Bundle\FrameworkBundle\Client;
use Symfony\Component\BrowserKit\Cookie;
use Symfony\Component\Security\Core\Authentication\Token\PreAuthenticatedToken;
use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
use Symfony\Component\Security\Core\User\UserInterface;

abstract class WebTestCase extends BaseTestCase
{
    /**
     * @var mixed[]
     */
    private $fixtures;

    /**
     * {@inheritdoc}
     */
    protected function setUp(): void
    {
        parent::setUp();

        $this->fixtures = $this->loadFixtureFiles(\array_merge([
            __DIR__.'/../../fixtures/orm/country.yml',
            __DIR__.'/../../fixtures/orm/group.yml',
            __DIR__.'/../../fixtures/orm/user.yml',
            __DIR__.'/../../fixtures/orm/user_browser.yml',
            __DIR__.'/../../fixtures/orm/organization.yml',
            __DIR__.'/../../fixtures/orm/api_key.yml',
        ], $this->getFixturesFiles()));
    }

    /**
     * @return string[]
     */
    protected function getFixturesFiles(): array
    {
        return [];
    }

   // Not related stuff.
}

As you can see, just providing yaml fixtures files.

And here is the configurations of this bundle, nelmio and dama:

liip_functional_test:
    authentication:
        username: user_default
        password: test
    command_verbosity: normal
    command_decoration: false

nelmio_alice:
    locale: fr_FR
    seed: '%seed%'

dama_doctrine_test:
    enable_static_connection: true
    enable_static_meta_data_cache: true
    enable_static_query_cache: true

For dama, I think it's the default configuration generated by Flex.

Here is also my phpunit.xml.dist file:

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

<phpunit
        backupGlobals               = "false"
        backupStaticAttributes      = "false"
        colors                      = "true"
        convertErrorsToExceptions   = "true"
        convertNoticesToExceptions  = "true"
        convertWarningsToExceptions = "true"
        processIsolation            = "false"
        stopOnFailure               = "false"
        syntaxCheck                 = "false"
        bootstrap                   = "tests/bootstrap.php"
>

    <php>
        <env name="RESET" value="0" />
        <env name="SYMFONY_DEPRECATIONS_HELPER" value="0" /> <!-- https://github.com/symfony/symfony/pull/24867 -->
        <env name="APP_ENV" value="test"/>
        <server name="KERNEL_CLASS" value="App\Kernel" />
    </php>

    <listeners>
        <listener class="\DAMA\DoctrineTestBundle\PHPUnit\PHPUnitListener" />
    </listeners>

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

    <filter>
        <whitelist>
            <directory>./src</directory>
        </whitelist>
    </filter>

</phpunit>

[Jean85]

Well, then the cause (at least for you) is the TRUNCATE for sure, since by default loadFixtureFiles has append = false. Can you try to replace the TRUNCATE with a DELETE to see if that is enough?

[soullivaneuh]

I'm on other issue right now, but I'm also pretty sure it's related to the TRUNCATE queries. It's clearly explained on the dama readme.

This is why I'm asking for a way to disable it. ;-)

[dmaicher]

In my case, I'm probably creating some race condition in the transactions, and since the connection stays the same in consequent tests, one of the transaction fails. I fear that the definitive fix should be done in the DAMA bundle...

@Jean85 interesting issue... feel free to create an issue on my bundle and I would be happy to look into it 😉

@soullivaneuh I'm also pretty sure its the TRUNCATE/ALTER queries. See dmaicher/doctrine-test-bundle#58 (comment)

[ghost]

Hi all, so, is this actual issue? Or all fixed in dmaicher/doctrine-test-bundle#58 (comment)

[Jean85]

@Jean85 interesting issue... feel free to create an issue on my bundle and I would be happy to look into it wink

Thanks @dmaicher! What kind of info do you need to debug it? I'm trying to restrict the reproducing case but it seems hard...

[soullivaneuh]

@powerpd doctrine-test-bundle is not a problem. This, is the real issue.

[alekseytupichenkov]

Hi, sorry for long response
It is not because I use TRUNCATE/ALTER queries because in your case you don't use database cache
Problem in lines https://github.com/liip/LiipFunctionalTestBundle/blob/2.x/src/Services/DatabaseTools/ORMDatabaseTool.php#L127

            $schemaTool->dropDatabase();
            if (!empty($this->getMetadatas())) {
                $schemaTool->createSchema($this->getMetadatas());
            }

I think, in this case, better way is add some settings for enable/disable database schema changes? Because using SAVEPOINT with changing database schema is not good idea

@soullivaneuh @Jean85 what is yours opinion?

[alekseytupichenkov]

I reproduced it locally and in case when I remove this lines all's work fine

[Jean85]

I don't think there's a way to disable them at low level. Only way would be to inspect every query for those actions, but I don't even know if it's feasible.

[alekseytupichenkov]

I think we can't check every query, and maybe it's not necessary
But I can refactor working with databases and split queries and methods to make working more predictable
I can create methods like createDatabase, createSchema, migrateMigrations, etc and move logic to work with database from loadFixtures to that methods
In tests it will be more predictable and flexible, for example:

• in case when you want prepare database before run tests you can use just createSchema and loadFixtures without createDatabase
• in case when you use static connection (dama/doctrine-test-bundle) you must use just loadFixtures, without methods like createDatabase or dropSchema
• in case when you want use all functionals, you can use all methods like createDatabase, loadFixtures, backupDatabase, etc

[alekseytupichenkov]

OR I can stay just one method loadFixtures but add parameters to config like prepare_database, prepare_schema, etc

[alekseytupichenkov]

I started writing PR, so this is example how it will be:

• Init DB tool https://github.com/liip/LiipFunctionalTestBundle/pull/462/files#diff-3216cb8554ab63f3b7b8531ebcee5156R308
• Basic usage https://github.com/liip/LiipFunctionalTestBundle/pull/462/files#diff-3216cb8554ab63f3b7b8531ebcee5156R264

[alekseytupichenkov]

@Jean85 please, tell your opinion about changes in PR #462 and my previous message?

[soullivaneuh]

@alekseytupichenkov I'll try your PR to see if it fix my issue.

@alexislefebvre Bunch of release was done and this issue is still actual.

Could we consider a revert of #398 or a impler way to disable it waiting a proper fix?

[alekseytupichenkov]

Hi @soullivaneuh
I want to clarify, I think the problem is not in my PR, the problem is in the conflict between your library and LiipFunctionalTestBundle, you are trying to create a mysql savepoint, LiipFunctionalTestBundle try re-creates the database if necessary, I already wrote about it above.
In fact, my changes only standardize work with the database, now the re-creation of the database is not only for the sqlite, but also for the other databases

In case of it is important, as fast fix I can commenting this lines
https://github.com/liip/LiipFunctionalTestBundle/blob/2.x/src/Services/DatabaseTools/ORMDatabaseTool.php#L127-L130
and use only updateSchema
It will fix problem and later I'll finish my PR :)
@alexislefebvre what do you think about it?

[soullivaneuh]

I partially agree with you @alekseytupichenkov.

Indeed, the conflict with other library should not be your problem.

This is why I'm also suggesting on option to disable the feature. So it will be working for everyone. 👍

[alexislefebvre]

@soullivaneuh

Could we consider a revert of #398 or a impler way to disable it waiting a proper fix?

I would prefer a way to disable the feature instead of reverting the numerous changes from #398.

@alekseytupichenkov started to work in #462 and I would prefer to use a simple solution a release beta and stable versions before the end of this year.

[alexislefebvre]

It looks like DAMADoctrineTestBundle and LiipFunctionalTestBundle do the same thing, which leads to a conflict ultimately.

I'm wondering if we could rely on dama/doctrine-test-bundle for database and schema creation? We may remove the code from LiipFunctionalTestBundle that perform the same operations than DAMADoctrineTestBundle.

But it would force users to install DAMADoctrineTestBundle, it means that it would:

1. impose the installation and use of a new dependency
2. impose some drawbacks from DAMADoctrineTestBundle (I don't know this bundle so I'm not aware of limitations, etc.)

[ghost]

Hi @alexislefebvre, I want clarify DAMADoctrineTestBundle and LiipFunctionalTestBundle don't do the same thing.
DAMADoctrineTestBundle - create static connections and cache metadata, work only with Mysql
LiipFunctionalTestBundle - create db, update schema and make database backup (not only metadata), working with all databases

[dmaicher]

DAMADoctrineTestBundle - create static connections and cache metadata, work only with Mysql

technically not correct 😉 DAMADoctrineTestBundle works also for SQLite, PostgreSQL etc

But apart from that 👍

[soullivaneuh]

Exactly, DAMADoctrineTestBundle is just an easy way to rollback your database statement after each test.

[alexislefebvre]

I added a test environment in #471 where DAMADoctrineTestBundle is installed and tests are supposed to break but they didn't. Did I forgot something?

The test class inherits from https://github.com/liip/LiipFunctionalTestBundle/blob/2.x/tests/Test/WebTestCaseConfigMysqlTest.php so it launches the same tests.

[alexislefebvre]

Does anyone have any idea to replicate the issue with DAMADoctrineTestBundle? The test added in #471 should fail but it doesn't, and I don't understand why.

[Jean85]

Issuing commands like CREATE ALTER etc will not make it fail; the only issue is in the fact that those commands have implicit transaction commit, so you should check after the test if the data was rolled back or not.

[alexislefebvre]

Thanks for your answer @Jean85. I can add a check that the data is rolled back but I would like to replicate the error reported in this issue, so we could then try to fix it.

[magnetik]

I think the philosophy of fixtures loading of thoses two bundles (dama & liip) are incompatible and cannot really be mixed.

Indeed this bundle allows you to specify a different set of fixtures for every tests (at least I'm using
extensively loadFixtures([Foobar::class, Bar::class]) in one test and loadFixtures([Foo::class]); in another), whereas dama's doctrine bundle encourage you to load all fixtures once and never change it again because it will be rollbacked to the "initial" state with your fixtures after changes.

[alexislefebvre]

I found a simple workaround in another project: laravel/framework#18429 (comment)

If we call $this->loadFixtures([…], true);, with $append = true, there's no TRUNCATE so it avoids the conflict with DAMADoctrineTestBundle.

---

Update: it doesn't always work, on my local environment it works after I delete the Symdony cache. 🤔

[alexislefebvre]

I think the philosophy of fixtures loading of thoses two bundles (dama & liip) are incompatible and cannot really be mixed.

You're right, I'll stop searching for a solution and accept that we can't use the 2 bundles in the same time.

I created a PR to explain this: liip/LiipTestFixturesBundle#3

[soullivaneuh]

#423 (comment)

This is why I'm also suggesting on option to disable the feature.

@alexislefebvre May we consider this? With an note on the readme, it would allow to use both, accepting not using some part of the liip bundle.

In my case, I need both for different test case, but I can't really separate as dama listener is loaded from the PHPUnit configuration. Except if you have a magic workaround for me? :-)

Regards

[alexislefebvre]

Thanks for the reminder, I forgot this option. I'll try to find a way.

[alexislefebvre]

I can replicate the issue in a different project, the error disappears if I disable the static connection:

dama_doctrine_test:
    enable_static_connection: false

If this temporary solution works, we can merge liip/LiipTestFixturesBundle#11

[alexislefebvre]

I still don't understand why there's no error when I run internal tests with DAMADoctrineTestBundle as in liip/LiipTestFixturesBundle#10, but if I put the same configuration in a project, I have the same error than in the issue. 🤔

[soullivaneuh]

Disabling enable_static_connection "solve" the issue, but disable the state reset, so not very helpful. :-D

[alexislefebvre]

Well, that's still better than an exception 😉

[alexislefebvre]

@soullivaneuh Could you please try the change added on the PR liip/LiipTestFixturesBundle#15?

1. Add this in your composer.json file:

    "repositories": [
        {
            "type": "vcs",
            "url": "https://github.com/liip/LiipTestFixturesBundle.git"
        }
    ]

2. composer require --dev liip/test-fixtures-bundle:dev-fix-conflict-with-dama-doctrine-test-bundle
3. Set liip_test_fixtures.keep_database_and_schema: true in your configuration

[alexislefebvre]

liip/LiipTestFixturesBundle#15 has been merged, you can disable automatic creation of database in the last release of LiipTestFixturesBundle: https://github.com/liip/LiipTestFixturesBundle/releases/tag/1.1.0

[soullivaneuh]

Sorry @alexislefebvre I didn't get the time to test.

So if I well understand, I have to migrate to v3 with the new test-fixtures bundle to get the fix?

[alexislefebvre]

Sorry @alexislefebvre I didn't get the time to test.

No problem. :)

So if I well understand, I have to migrate to v3 with the new test-fixtures bundle to get the fix?

Exactly, then you'll be have a setting to disable changes related to database and schema: https://github.com/liip/LiipTestFixturesBundle/blob/836599c15fa6d7333aef1a60f68f3c55fe5ca7ae/doc/caveats.md

[soullivaneuh]

I'll give a try on my next project upgrade section, thanks!

[emancypage]

Hi, I also experienced this conflict with DAMA and Liip bundles with the same problem. I am using 1.9.1 version of the LiipFunctionalTestBundle.
I applied @alexislefebvre fix in my project and everything works correct.