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 does not rollback when a client makes multiple requests in a WebTestCase #208

Closed patrick-vandy closed 2 years ago

patrick-vandy commented 2 years ago

After running a composer update (see below), some of my tests started failing. I've narrowed the problem down to a WebTestCase creating a client and then making multiple requests (typically when a test is submitting a form). If only a single request is made, the transaction rolls back as expected. To reproduce:

<?php

namespace App\Tests;

use Symfony\Bundle\FrameworkBundle\Test\WebTestCase;

class DoctrineTestBundleBug extends WebTestCase
{
    public function testFoo(): void
    {
        $client = static::createClient();
        static::getContainer()->get('doctrine')->getConnection()->executeQuery("INSERT INTO test (test) VALUES ('foo')");
        $client->request('GET', '/');
        // this second request is the culprit. comment this out and the transaction will rollback as expected
        $client->request('GET', '/');
        $this->assertTrue(true);
    }
}

The above test will pass but the inserted row remains in the test table. If I comment out the second $client->request('GET', '/') then I don't see any data in the test table after it runs.

Platform:

Here are the exact versions of doctrine and symfony packages, and the versions that I upgraded from => to.

Updating dependencies
Lock file operations: 1 install, 90 updates, 0 removals
  - Upgrading dama/doctrine-test-bundle (v6.7.5 => v7.0.0)
  - Upgrading doctrine/common (3.2.2 => 3.3.0)
  - Upgrading doctrine/dbal (3.3.2 => 3.3.5)
  - Upgrading doctrine/doctrine-bundle (2.5.5 => 2.6.2)
  - Upgrading doctrine/instantiator (1.4.0 => 1.4.1)
  - Upgrading doctrine/lexer (1.2.2 => 1.2.3)
  - Upgrading doctrine/migrations (3.4.1 => 3.4.2)
  - Upgrading doctrine/orm (2.11.1 => 2.12.0)
  - Upgrading doctrine/persistence (2.3.0 => 2.5.1)
  - Upgrading symfony/amqp-messenger (v5.4.3 => v5.4.5)
  - Upgrading symfony/asset (v5.4.3 => v5.4.7)
  - Upgrading symfony/cache (v5.4.3 => v5.4.7)
  - Upgrading symfony/cache-contracts (v2.5.0 => v2.5.1)
  - Upgrading symfony/config (v5.4.3 => v5.4.7)
  - Upgrading symfony/console (v5.4.3 => v5.4.7)
  - Upgrading symfony/dependency-injection (v5.4.3 => v5.4.7)
  - Upgrading symfony/deprecation-contracts (v2.5.0 => v2.5.1)
  - Upgrading symfony/doctrine-bridge (v5.4.3 => v5.4.7)
  - Upgrading symfony/doctrine-messenger (v5.4.3 => v5.4.7)
  - Upgrading symfony/dom-crawler (v5.4.3 => v5.4.6)
  - Upgrading symfony/dotenv (v5.4.3 => v5.4.5)
  - Upgrading symfony/error-handler (v5.4.3 => v5.4.7)
  - Upgrading symfony/event-dispatcher-contracts (v2.5.0 => v2.5.1)
  - Upgrading symfony/expression-language (v5.4.3 => v5.4.7)
  - Upgrading symfony/filesystem (v5.4.3 => v5.4.7)
  - Upgrading symfony/flex (v1.18.3 => v1.18.6)
  - Upgrading symfony/form (v5.4.3 => v5.4.7)
  - Upgrading symfony/framework-bundle (v5.4.4 => v5.4.7)
  - Upgrading symfony/http-client (v5.4.3 => v5.4.7)
  - Upgrading symfony/http-client-contracts (v2.5.0 => v2.5.1)
  - Upgrading symfony/http-foundation (v5.4.3 => v5.4.6)
  - Upgrading symfony/http-kernel (v5.4.4 => v5.4.7)
  - Upgrading symfony/intl (v5.4.3 => v5.4.5)
  - Upgrading symfony/ldap (v5.4.3 => v5.4.5)
  - Upgrading symfony/mailer (v5.4.3 => v5.4.7)
  - Upgrading symfony/maker-bundle (v1.36.4 => v1.38.0)
  - Upgrading symfony/mime (v5.4.3 => v5.4.7)
  - Upgrading symfony/notifier (v5.4.3 => v5.4.6)
  - Upgrading symfony/phpunit-bridge (v5.4.3 => v5.4.7)
  - Upgrading symfony/polyfill-ctype (v1.24.0 => v1.25.0)
  - Upgrading symfony/polyfill-intl-grapheme (v1.24.0 => v1.25.0)
  - Upgrading symfony/polyfill-intl-icu (v1.24.0 => v1.25.0)
  - Upgrading symfony/polyfill-intl-idn (v1.24.0 => v1.25.0)
  - Upgrading symfony/polyfill-intl-normalizer (v1.24.0 => v1.25.0)
  - Upgrading symfony/polyfill-mbstring (v1.24.0 => v1.25.0)
  - Upgrading symfony/polyfill-php72 (v1.24.0 => v1.25.0)
  - Upgrading symfony/polyfill-php73 (v1.24.0 => v1.25.0)
  - Upgrading symfony/polyfill-php80 (v1.24.0 => v1.25.0)
  - Upgrading symfony/polyfill-php81 (v1.24.0 => v1.25.0)
  - Upgrading symfony/process (v5.4.3 => v5.4.7)
  - Upgrading symfony/property-access (v5.4.3 => v5.4.7)
  - Upgrading symfony/property-info (v5.4.3 => v5.4.7)
  - Upgrading symfony/proxy-manager-bridge (v5.4.3 => v5.4.6)
  - Upgrading symfony/redis-messenger (v5.4.3 => v5.4.6)
  - Upgrading symfony/runtime (v5.4.3 => v5.4.7)
  - Upgrading symfony/security-bundle (v5.4.3 => v5.4.5)
  - Upgrading symfony/security-core (v5.4.3 => v5.4.7)
  - Upgrading symfony/security-http (v5.4.3 => v5.4.5)
  - Upgrading symfony/serializer (v5.4.3 => v5.4.7)
  - Upgrading symfony/service-contracts (v2.5.0 => v2.5.1)
  - Upgrading symfony/stopwatch (v5.4.3 => v5.4.5)
  - Upgrading symfony/translation (v5.4.3 => v5.4.7)
  - Upgrading symfony/translation-contracts (v2.5.0 => v2.5.1)
  - Upgrading symfony/twig-bridge (v5.4.3 => v5.4.7)
  - Upgrading symfony/validator (v5.4.3 => v5.4.7)
  - Upgrading symfony/var-dumper (v5.4.3 => v5.4.6)
  - Upgrading symfony/var-exporter (v5.4.3 => v5.4.7)
  - Upgrading symfony/web-profiler-bundle (v5.4.3 => v5.4.6)
dmaicher commented 2 years ago

Can you create a minimal reproducer repository? I cannot reproduce that issue.

patrick-vandy commented 2 years ago

Can you create a minimal reproducer repository? I cannot reproduce that issue.

@dmaicher Sure, I'll see if I can recreate it on a minimal set up when I get some time. It's possible it's something with a 3rd party package or other dependency, as I haven't tried to reproduce outside of the application yet. I'll let you know what I find out.

patrick-vandy commented 2 years ago

@dmaicher I wasn't able to reproduce on a minimal setup with symfony, doctrine, etc. This is happening on a very large project with many dependencies, so I'm suspecting there's an edge case with a specific package being pulled in and I need to isolate which one is the culprit. I'm going to go through all of our packages one by one until I figure out which one it is and I'll get back with you.

dmaicher commented 2 years ago

Closing for now. Let me know if you find a way of reproducing this.

aless673 commented 2 years ago

My scenario

If the PHP test file do DB changes it will be rolledback correctly (ex: static::getContainer()->get('doctrine')->getConnection()->executeQuery(...) If the PHP test file do navigatation (WebTestCase) and while navigating the app do changes in the DB, those changes won't be rolledback

@dmaicher can you confirm that navigation DB changes should be rolledback too ?

dmaicher commented 2 years ago

@aless673 I can confirm that whatever kind of test (KernelTestCase, WebTestCase) and no matter how many requests or queries you perform: they should be rolled back.

If this is not the case for you: please try to create a reproducer as it works fine for me in several projects.

aless673 commented 2 years ago

@aless673 I can confirm that whatever kind of test (KernelTestCase, WebTestCase) and no matter how many requests or queries you perform: they should be rolled back.

If this is not the case for you: please try to create a reproducer as it works fine for me in several projects.

Here the repo updated with instructions in the README

https://github.com/aless673/test_rollback_doctrine

patrick-vandy commented 2 years ago

@aless673 Thanks for getting this reproduced on a minimal repository (something I never got around to).

I'm curious, was there a specific config setting and/or dependency you added that causes this? Or is it simply failing to rollback based on how you write the tests?

When I created a new, minimal Symfony project I was unable to reproduce, but the issue still exists for us and we have had to lock doctrine-test-bundle at a specific version to continue using it.

aless673 commented 2 years ago

It is failing on a test i could not make easier Also the repo is a minimal SF project, with no special dependency

Which version of doctrine-test-bundle are you using ? I would like to test, and maybe try to find the breaking point

patrick-vandy commented 2 years ago

@aless673 We are using 6.7 but I misspoke on my last comment. The issue started occurring when I ran a composer update on the entire project and I believe it was upgrading one of the core doctrine packages that made the issue start occurring. Looking at our composer file, we have locked doctrine/doctrine-bundle at 2.5.5, I believe going to 2.6.2 is when it started breaking.

aless673 commented 2 years ago

@aless673 We are using 6.7 but I misspoke on my last comment. The issue started occurring when I ran a composer update on the entire project and I believe it was upgrading one of the core doctrine packages that made the issue start occurring. Looking at our composer file, we have locked doctrine/doctrine-bundle at 2.5.5, I believe going to 2.6.2 is when it started breaking.

i tried with many different downgraded versions (doctrine-bundle, doctrine orm and doctrine-test-bundle) and i cannot make it works...

dmaicher commented 2 years ago

@aless673 your reproducer is using Symfony Panther. This will actually run some browser (Chrome?) and interact via the WebDriver protocol, or? Those cases are not supported. It only works for "normal" WebTestCases that run within one php process.

patrick-vandy commented 1 month ago

I wasn't able to reproduce on a minimal setup with symfony, doctrine, etc. This is happening on a very large project with many dependencies, so I'm suspecting there's an edge case with a specific package being pulled in and I need to isolate which one is the culprit. I'm going to go through all of our packages one by one until I figure out which one it is and I'll get back with you.

@dmaicher It's been a couple of years, but I wanted to follow up for any future readers who stumble on to this. I finally identified the issue thanks to version 8 introducing connection_keys, I realized the problem is due to multiple connections. After upgrading, I was able to easily fix following this example on the README

I have assigned the same connection key to both of my connections, and everything now works as expected. Thanks for your patience and help

patrick-vandy commented 1 month ago

@dmaicher A little more context and for future readers: The exact cause was very specific to a large project with a mix of legacy, non-Symfony code. The multiple requests via the client in a WebTestCase was a red herring. The real issue was when connection_1 had been opened and performed writes, then connection_2 was opened and closed prior to connection_1 closing (and rolling back). This was happening for us when we had multiple calls to the client, because the second call was typically to a legacy page that used the other connection.