@mrsombre It seems that currently our test project https://github.com/Codeception/symfony-module-tests (which is used for CI and test execution) is not ready for this change.

In order to merge this I would need you to also adapt the test project and (somehow) make the tests pass.

As soon as I can verify that your change works and that we are able to test it through the test project, I will gladly merge this.

Be sure to follow the contribution guidelines if you have any doubts and you can report any questions here, I (or someone else) will be happy to help you.

@TavoNiievez i see. Root of the problem in fact, that now connection is real and doctrine commits transaction, so rollback hack doesn't work anymore.
I'm looking into this problem. Trying to find a good solution to prevent transaction closing before module can revert it.

The root cause in vendor/doctrine/doctrine-bundle/DoctrineBundle.php:138:

• Every request (like form submit) cause to reboot kernel
• This cause doctrine bundle to close active connections
• Transaction opened by Doctrine module caused to commit / close too
• Then new connection opened for data insertion in the controller, but the Doctrine transaction hack hasn't work anymore and data are actually written to DB

I prevent this with a bit ugly solution using reflections, otherwise I can't anyhow override service param to prevent doctrine to close connections on reboot.

Hi, any updates on this, anything to fix/ improve? @TavoNiievez

@mrsombre I'm fine with changes, even using Reflections.
Still I think we should have a test for this.

Unfortunately, compiled container cannot be modified anyhow, so reflections are only option.
Any advices how implement test before change merged into master?

The following is an oversimplification, but it is a way to check for these changes. Currently this test should fail... and with your changes it should pass.

    /**
     * @see https://github.com/Codeception/module-symfony/pull/129
     */
    public function aDescriptiveTestName(FunctionalTester $I)
    {
        $containerDbalConnection = $I->grabService('doctrine.dbal.default_connection');
        $emDbalConnection = $I->getEm()->getConnection();
        $I->assertSame($emDbalConnection, $containerDbalConnection);
    }

Regarding 'where' to put this, I think symfony-module-tests/tests/Functional/IssuesCest.php would be a good place.

Yeah, I see how to write a test. The question was about how I'll add a test if it can't pass. Btw it's like I add test as PR, then we merge this PR and then test, right?

Yes, the idea is that:

1. You present a PR in symfony-module-tests (tests should fail).
2. I apply these changes locally and test it works. Then I merge this PR with master.
3. When re-executing the build in the PR of symfony-module-tests the tests should pass. That PR is merged.
4. Finally, a tag is created and the change is released.

I know it is a somewhat complex workflow, but we are open to suggestions.

Bundle tests with the project? xD I got an idea, shall add tests soon, thanks for your help, this change is important for our workflow (mixing orm with dbal).

@mrsombre Please take a look at: https://github.com/pimcore/pimcore/pull/10331/checks?check_run_id=3619279351#step:11:1078

these changes appear to be causing problems in Pimcore when trying to update.