api-platform / docs

API Platform documentation
https://api-platform.com/docs/
164 stars 1.05k forks source link

Removed ResetDatabase trait #1930

Closed SebSept closed 3 months ago

SebSept commented 3 months ago

Because the tutorial make use of DAMADoctrineTestBundle to reset the database automatically before each test, the ResetDatabase trait is not needed (it may even cause trouble (I guess)).

dunglas commented 3 months ago

Shouldn't we remove DMADoctrineTestBundle instead for the sake of simplicity?

SebSept commented 3 months ago

I have no real experience using that bundle and no idea of the speed improvements provided.

It's just 2 actions, a composer req command and adding a line in phpunit.xml. So, not complicated. No more than a minute.

As mentioned on the Symfony documentation, installing DamaDoctrineTestBundle improve the speed and should be encouraged.

So my humble opinion of new comer is to keep DamaDoctrineTestBundle.

dunglas commented 3 months ago

Sounds reasonable! @kbond do you have an opinion about this?

kbond commented 3 months ago

We've made sure this trait works seamlessly with dama. In fact, it's required for the following scenarios:

This isn't to say it's required but if there is a compatibility issue, I'd like to fix.

SebSept commented 3 months ago

So DamaDoctrineTestBundle and Foundry ResetDatabase can look like a useless redundancy but they are not. Just to mention, Dama ensure database remains untouched after a test runs, while Foundry's DatabaseReset() maker sur that the database is in a certain state.

So the trait should remain in the code snippet.

Maybe we could add a word in the doc to clarify things...

dunglas commented 3 months ago

We can close then. Right?

SebSept commented 3 months ago

Yes. Do you think it worth adding a word in these page ?

kbond commented 3 months ago

Just to mention, Dama ensure database remains untouched after a test runs, while Foundry's DatabaseReset() maker sur that the database is in a certain state.

Technically, dama does the same.