Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
Fabrn opened this issue Dec 12, 2023 · 11 comments
Open

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

Fabrn opened this issue Dec 12, 2023 · 11 comments
Labels
bug Something isn't working

Comments

@Fabrn
Copy link

Fabrn commented Dec 12, 2023

Hello there !

  • PHP version : 8.2.13
  • DAMA version : 8.0.1
  • Foundry version : 1.36.0

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
Copy link
Member

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?

@kbond
Copy link
Member

kbond commented Dec 13, 2023

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
Copy link
Author

Fabrn commented Dec 13, 2023

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
Copy link
Member

nikophil commented Dec 13, 2023

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 😅

@Fabrn
Copy link
Author

Fabrn commented Dec 13, 2023

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
Copy link
Member

nikophil commented Dec 13, 2023

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
Copy link
Author

Fabrn commented Dec 14, 2023

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
Copy link
Member

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

@nikophil nikophil added the bug Something isn't working label Dec 14, 2023
@nikophil nikophil changed the title Global story loading causes DAMA to crash because of implicit commits Error with DAMA + reset mode migrate + global story Dec 14, 2023
@moynzzz
Copy link

moynzzz commented May 13, 2024

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
Copy link

moynzzz commented May 13, 2024

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
Copy link
Member

kbond commented May 13, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Development

No branches or pull requests

4 participants