zenstruck / foundry

A model factory library for creating expressive, auto-completable, on-demand dev/test fixtures with Symfony and Doctrine.
https://symfony.com/bundles/ZenstruckFoundryBundle/current/index.html
MIT License
609 stars 63 forks source link

Error with DAMA + reset mode migrate + global story #538

Open Fabrn opened 6 months ago

Fabrn commented 6 months ago

Hello there !

Context

I've created a Story because I need it for several test cases. In order to ease the usage, I've chosen to load the Story using the global_state configuration like so :

global_state:
    - App\Story\MyStory

Issue

After installing DAMA, and switched reset_mode to migrate, I started having these errors :

PDOException: SQLSTATE[42000]: Syntax error or access violation: 1305 SAVEPOINT DOCTRINE2_SAVEPOINT_5 does not exist

Removing the global_state to replace it with manual story loading in test using MyStory::load() does work. Also, I want to add the fact that my Story does only produce creations. It doesn't explicitly produce any of the statements that generate implicit commits according to MySQL documentation.

Solution

Sadly I didn't send any time yet searching for clues on the topic. Is it a bad usage from me ?

If you have any question, I'm here to help you asap 👍

Thank you guys !

nikophil commented 6 months ago

Hi!

global state + dama + ResetDatabase trait is quite a complex problem sometimes.

I'm wondering if this is not the same problem than here: https://github.com/zenstruck/foundry/issues/534 (I'm running postgres, so it could be kinda the same problem, but with different errors) I need to work on this error.

Do you have the problem without using migrate as a reset_mode?

kbond commented 6 months ago

We need to update the CI to allow/test with DAMA 8.

Did you see this note? https://github.com/dmaicher/doctrine-test-bundle?tab=readme-ov-file#troubleshooting

Fabrn commented 6 months ago

Hi!

global state + dama + ResetDatabase trait is quite a complex problem sometimes.

I'm wondering if this is not the same problem than here: #534 (I'm running postgres, so it could be kinda the same problem, but with different errors) I need to work on this error.

Do you have the problem without using migrate as a reset_mode?

I have not tried to run tests without migrate because I don't think it is useful since Foundry uses DAMA if you are in migrate mode. It may work but I lose the point.

We need to update the CI to allow/test with DAMA 8.

Did you see this note? https://github.com/dmaicher/doctrine-test-bundle?tab=readme-ov-file#troubleshooting

Yeah I've checked all of it. Sadly it didn't help me much and the only viable solution for me was to manually execute stories (which is not a pain at all, it is fine, but could be prevented).

nikophil commented 6 months ago

I have not tried to run tests without migrate because I don't think it is useful since Foundry uses DAMA if you are in migrate mode. It may work but I lose the point.

I don't understand this statement :thinking: I'm using foundry + dama without migrate mode :sweat_smile:

Fabrn commented 6 months ago

I have not tried to run tests without migrate because I don't think it is useful since Foundry uses DAMA if you are in migrate mode. It may work but I lose the point.

I don't understand this statement 🤔 I'm using foundry + dama without migrate mode 😅

I mean yes you can, but according to this piece of documentation on Symfony's website, the main goal of using DAMA and Foundry together is to speed up the migrate mode (if I understand it correctly) :

Before the first test using the ResetDatabase trait, it drops (if exists) and creates the test database. Then, by default, before each test, it resets the schema using doctrine:schema:drop/doctrine:schema:create.

Alternatively, you can have it run your migrations instead by modifying the reset_mode option in configuration file. When using this mode, before each test, the database is dropped/created and your migrations run (via doctrine:migrations:migrate). This mode can really make your test suite slow (especially if you have a lot of migrations). It is highly recommended to use DamaDoctrineTestBundle to improve the speed. When this bundle is enabled, the database is dropped/created and migrated only once for the suite.

nikophil commented 6 months ago

Maybe this part of the docs needs a reword: dama improves performance in both reset modes, since you only have to reset your db once. But it might improve drastically for migration mode, because migration mode is really slower than schema mode. Nonetheless, I'd advise to use schema mode unless you really actually need your migrations to be ran before the tests.

Fabrn commented 6 months ago

Maybe this part of the docs needs a reword: dama improves performance in both reset modes, since you only have to reset your db once. But it might improve drastically for migration mode, because migration mode is really slower than schema mode. Nonetheless, I'd advise to use schema mode unless you really actually need your migrations to be ran before the tests.

Alright ! Thank you for the advice, just tested using schema mode, I confirm it is a bit faster ! Also, I confirm the issue is happening only using migrate mode. I guess this is caused by the fact that migrations do ALTER TABLE etc.

nikophil commented 6 months ago

ok then, we've found the culprit: we actually have a problem with dama + reset_mode=migrate + global story

@kbond I think we actually must add a full CI permutation with this case

moynzzz commented 1 month ago

I can confirm I have the same issue:

An exception occurred while executing a query: SQLSTATE[42000]: Syntax error or access violation: 1305 SAVEPOINT DOCTRINE_2 does not exist

In the migrations of course I have ALTER statements etc. So I kind of expect in the Foundry's database resetter to exists something that will disable DAMA and enable it after migrations take place.

I can provide a reproducer and an example and I want to work on a fix if possible but I dont know where to start.

I think I somehow need to disable the DAMA in the ORMDatabaseResetter but it seems thats its disabled already.

moynzzz commented 1 month ago

I found the solution the issue is not in foundry or the database resetter. This is because I forgot to add this to my migrations:

public function isTransactional(): bool
{
    return false;
}

I think this makes doctrine migrations to always commit a transaction and that breaks DAMA.

kbond commented 1 month ago

I found the solution the issue is not in foundry or the database resetter. This is because I forgot to add this to my migrations:

Ah, glad you found a solution - I've been burned by this before as well. We should perhaps make a note in the docs.