dmaicher/doctrine-test-bundle

Doctrine\DBAL\Driver\ExceptionConverterDriver is deprecated

garak opened this issue ยท 18 comments

garak commented

After upgrading to doctrine/dbal 2.11:

The "DAMA\DoctrineTestBundle\Doctrine\DBAL\StaticDriver" class implements "Doctrine\DBAL\Driver\ExceptionConverterDriver" that is deprecated in PHPUnitExtension::executeBeforeFirstTest

I don't see a way to fix this in a forward compatible way ๐Ÿ˜• We can fix it here for doctrine/dbal 3 and not implement that interface anymore.

Checked doctrine/dbal#4118 and https://github.com/doctrine/dbal/pull/4129/files

Or do you see a way to get rid of the deprecation already?

garak commented

Can't we just stop implementing ExceptionConverterDriver?

Can't we just stop implementing ExceptionConverterDriver?

Well that will probably have side effects? See https://github.com/doctrine/dbal/blob/2.11.x/lib/Doctrine/DBAL/DBALException.php#L181

garak commented

Do you think? I see that exception is converted with the old interface, otherwise is re-thrown as generic Exception. Is that so different?

I think it would be bad as the test environment would behave differently than dev/prod environments if the underlying driver implements ExceptionConverterDriver.

For example I'm mostly using Doctrine\DBAL\Driver\PDOMySql\Driver which will throw specific exceptions. So in my app I also handle those specific exceptions in some cases. For example UniqueConstraintViolationException.

If now in the tests the StaticDriver of this bundle will not throw any specific exceptions anymore it could certainly break things.

I quickly confirmed it with a functional test.

This works without any changes:

    public function testWillThrowSpecificException(): void
    {
        $this->expectException(TableNotFoundException::class);
        $this->connection->insert('does_not_exist', ['foo' => 'bar']);
    }

and it fails when StaticDriver does not implement ExceptionConverterDriver anymore:

1) Tests\Functional\FunctionalTest::testWillThrowSpecificException
Failed asserting that exception of type "Doctrine\DBAL\Exception" matches expected exception "Doctrine\DBAL\Exception\TableNotFoundException". Message was: "An exception occurred while executing 'INSERT INTO does_not_exist (foo) VALUES (?)' with params ["bar"]:
garak commented

I see.
I guess we should keep it as is for now.
I would keep this issue open as well, just as a reminder.

Why not create a different PHPUnitExtension class for latest DBAL versions?

One would use a StaticDriver that implementst the interface, and the new one would implement another StaticDriver without the interface.

Makes sure that using the interface is opt-out, it's backwards-compatible, and when Doctrine DBAL removes the class, migration path is quicker, since the legacy interface might then have a runtime check on the interface.

WDYT?

@Pierstoval yes for DBAL 3 we will need a StaticDriver that does not implement the interface anymore.

But for dbal 2.11+, < 3 we still need to implement the deprecated interface ๐Ÿ˜•

Let's wait until DBAL 3 release then ๐Ÿ‘

Hello,
I'm having the same deprecation error and I cannot have my data reset on each test.

Does it make transactions rollback fail ?

Does it make transactions rollback fail ?

No. Its just a deprecation. You can safely ignore it.

Compatibility with dbal 3 is merged & released. As I don't see any way of fixing this for dbal 2.x (unless there is some forward compatibility layer on dbal 2.x for it) I will close here.

@dmaicher Would creating a new release that's only compatible with dbal 3.x be an option? I'll understand if you're not willing to maintain 2 release streams, but this would be the cleanest option IMO.

@BenMorel are you still seeing this deprecation when using dbal 3?

@dmaicher Sorry, I think I misunderstood the problem. We're using doctrine/dbal 2.13.

I guess this boils down to releasing a version that would be compatible with 2.11+ only? The aim being to have a version that's clear of deprecation warnings.

I guess this boils down to releasing a version that would be compatible with 2.11+ only? The aim being to have a version that's clear of deprecation warnings.

This is impossible. The driver interface is deprecated on dbal 2.x, but there is no replacement as far as I see. So we need to use it on 2.x as otherwise the functionality is broken.

Oh right, I see now. They've deprecated the ExceptionConverterDriver because the convertException() method has been moved to the Driver interface. It's really a pity to see no way to remove the deprecation warning that clutters the tests...

Anyway, sorry for the noise!