[POC] add ResetDatabase trait for resetting database between tests

[kbond]

This is part of an initiative by myself, @wouterj, @Nyholm and @weaverryan to improve the testing experience in Symfony. A common requirement in Symfony tests is to reset the test database before running your test suite and reset your schema between tests. This solution allows you to add the ResetDatabase trait to test's that you'd like this to happen in.

Usage:

class MyTest extends KernelTestCase
{
    use ResetDatabase;

    public function testSomething(): void
    {
        // if first test in the suite using the trait, the test database is dropped (if exists) and created
        // before this test, the schema is dropped and created
    }
}

By default, only the default connection and manager is reset but this can be configured via the DOCTRINE_RESET_CONNECTIONS and DOCTRINE_RESET_OBJECT_MANAGERS env vars.

This code was extracted from zenstruck/foundry so I know it works but I'm sure needs some refinement (and of course tests).

[ostrolucky]

Approach is wrong, it spawns sub process which boots whole kernel unnecessarily again, breaks whatever transactions that are in place and other sideffects.

Not a fan of whole idea either, there are specialized bundles for this which are much better/more advanced than whatever we will do here, like dama/doctrine-test-bundle or theofidry/alice-data-fixtures
. I believe it's better to not attempt to do everything doctrine related in doctrine-bundle, but leave it separated in specialized packages.

[wouterj]

I believe it's better to not attempt to do everything doctrine related in doctrine-bundle, but leave it separated in specialized packages.

I would like to argue that it's better to add the most core functionality in the DoctrineBundle. Other bundles are easily missed by people using the DoctrineBundle, resulting in a much poorer test experience. E.g. I only recently learned about the DamaDoctrineTestBundle and many people I shared it with also didn't know about that bundle. However, I believe this bundle is crucial when testing Symfony apps using a database.

I don't know @dmaicher's opinion on this topic and I definitely don't want to destroy someone else's open source project, but it might be a good idea to consider whether the bundle should be merged in the DoctrineBundle.

---

The approach taken by @kbond is a bit different here. It provides a naive drop and create of the schema before each test. This is not optimal, but better than nothing. It's a "here is a simple solution, use DamaDoctrineTestBundle to get a better more advanced one" approach (I haven't looked at the real implementation details, I think it's good to help a bit more than saying "it's wrong").

For what it's worth, this PR is almost a direct copy of the approach taken by Laravel.

---

In any case, thank you @kbond for starting this PR and opening the discussion on this subject! Enjoy your holidays!

[kbond]

"Naive" is definitely the right way to describe this ;). What foundry does is: try to use DamaDoctrineTestBundle if installed/enabled, if not, fallback to this method. Perhaps a similar approach could be taken here?

What's nice is when using DamaDoctrineTestBundle and running into a failure that you need to debug in a real database, you can simply disable that bundle and rerun that test. The fallback does the same thing as DamaDoctrineTestBundle but without the transaction.

[ostrolucky]

I would like to argue that it's better to add the most core functionality in the DoctrineBundle. Other bundles are easily missed by people using the DoctrineBundle, resulting in a much poorer test experience. E.g. I only recently learned about the DamaDoctrineTestBundle and many people I shared it with also didn't know about that bundle. However, I believe this bundle is crucial when testing Symfony apps using a database.

That's a documentation problem. Even if we merged this, people wouldn't know about this, so issue you pointed out is completely unrelated to having functionality inside bundle. Next thing you know, we would be merging alice, because creating fixtures is also a common problem? We don't have resources for that. We should fix documentation instead and point people to these libraries. Btw, you as symfony maintainer are in position to just change flex aliases and install these libraries together with orm-pack or whatever if you feel this is a common enough problem (or as you put it, "crucial") that people should have this whenever they install doctrine-bundle.

I think it's good to help a bit more than saying "it's wrong").

I did. Don't use commands. They are designed as facades for humans. We have SchemaTool for doing changes programmatically. I didn't give more tips on purpose, becase there is no point to work on this PR if there is no decision made what to do about this request and my time is limited. For now, I vote with 👎.

[wouterj]

Thanks for you detailed response!

Just for reference (I've no reason to think that my opinion is extremely relevant here, given my limited Doctrine contributions so I'll happily accept whatever the conclusion of the Doctrine team is on this matter), this is my observation on which I based my opinion that this feature is "crucial" and lacking an "official solution": There are quite some packages implementing database resetting for tests in the Symfony ecosystem, all with their own approach. The most popular ones that I'm aware of:

• https://github.com/liip/LiipTestFixturesBundle/blob/master/doc/database.md#tips-for-fixture-loading-tests
• https://github.com/dmaicher/doctrine-test-bundle
• https://github.com/hautelook/AliceBundle#database-testing
• https://github.com/zenstruck/foundry#enable-foundry-in-your-testcase

Please note that in the Symfony docs, we have been mentioning the Dama bundle for quite a while already: https://symfony.com/doc/current/testing/database.html#resetting-the-database-automatically-before-each-test

[dmaicher]

Related to dmaicher/doctrine-test-bundle#63

As @wouterj mentioned there are different approaches to having isolated tests that interact with a DB. Not sure my bundle is the right approach for every user of DoctrineBundle.

The bundle is mentioned in the Symfony docs since quite a while and @fabpot also mentioned it in his "Symfony 5: The fast track" book. So indeed as @ostrolucky mentioned I'm not sure just integrating the bundle here will fix the "not enough people know about it" issue?

[dunglas]

Creator of Alice's Database Testing Helpers here.

To me such helpers are a must-have when doing database testing on large projects, and a common pain point for Symfony developers. I would love to see an "official", unified, consistent and well maintained testing solution for Symfony (with the same Backward Compatibly promise as other Symfony Components).

We already have nice PHPUnit helpers directly in Symfony, and a browser-testing solution sharing the same API as BrowserKit (Panther).

What we miss are modern database testing helpers: fixtures, reload/refresh helpers etc. This is a long-standing issue regarding the testing experience with Symfony. I would love to see Zenstruck's packages merged directly into Symfony. If it happens, the last missing part will be refresh/reload helpers. (I would also love to see native parallel testing support with Paratest or something similar, but it's another story).

Both a transaction-based helper and a full reload helper must be supported: transaction-based helpers are faster, but when using E2E tools such as Panther or Cypress that can trigger on several HTTP requests, you cannot rely on transactions.

you as symfony maintainer are in position to just change flex aliases and install these libraries together with orm-pack or whatever if you feel this is a common enough problem

It's what we've done with Alice, but unfortunately it doesn't work. Alice has less resources for its maintenance than Symfony, and isn't able to follow the rhythm of the ecosystem (it doesn't support PHP 8 for instance), hurting many users. We have regular issues opened on API Platform's bug tracker because of Alice, and there is nothing we can do about that. Also, because they are different projects with different goals (which is totally fine), it's hard to provide a coherent and unified testing experience for Symfony. For instance, Alice's syntax is "weird" and not coherent with the syntax elsewhere in Symfony. Consequently, the testing experience with Symfony is less good than with other frameworks in other ecosystems.

[ostrolucky]

I believe this feature goes hand in hand with fixtures libraries, hence it would be better if this functionality stays in those libraries. That way all purging implementations follow same interface, are at same place and are easier for maintainers to change and add new features.

Reasoning why is it easier to discover the feature when it's integrated in here also never came. I think people expect to see this information whenever they look up how to write tests in Symfony, so information about purging should be in testing docs - and there it doesn't matter that it's not integrated in DoctrineBundle. Article already refers to use DoctrineFixturesBundle so why not also explain how to purge data there.

Hence I think it's time to close it, this PR was put on ice and we never came with uniform decision. If you think this is worth more discussion, open the issue and let's discuss there.

[kbond]

Understood. Thanks for considering!

[wouterj]

Regardless of implementation, may I ask for the specific reason not to include a feature like this in DoctrineBundle? Is it because this bundle does not provide anything for tests yet. I.e. would it make more sense if we contributed such test integration in the DoctrineFixturesBundle (e.g. by using the purger)?

I disagree that this is mostly a documentation/"marketing" issue (note we're reworking the testing docs and giving the Dama bundle an even more prominent place: symfony/symfony-docs#14731 ). While rewriting, I'm struggling with documenting the "if Dama bundle doesn't work for you" case. This case can be quite common (e.g. when using Panther, when requiring queries with implicit commits, when needing ALTER queries), so I think we have to document it (the group is too large to say "you're on your own"). What would you, as Doctrine maintainers, recommend us to document in this case?

[ostrolucky]

Regardless of implementation, may I ask for the specific reason not to include a feature like this in DoctrineBundle? Is it because this bundle does not provide anything for tests yet. I.e. would it make more sense if we contributed such test integration in the DoctrineFixturesBundle (e.g. by using the purger)?

Yes. Like I already mentioned, reasoning is this feature goes hand in hand with populating of the fixtures, so purging should also be included in these libraries. It would make more sense to include purging in bundle if we also had solution for fixtures integrated here.

What would you, as Doctrine maintainers, recommend us to document in this case?

Depends what fixture library you use. IIRC DoctrineFixturesBundle (which is the solution currently recommended in symfony docs) purges database automatically when fixtures are loaded. But you can always just call its purger manually. Should be as easy as doing

(new ORMPurger($em))->purge();

Then in case you use zenstruck/foundry, it has auto and manual refreshing features.