Tests with MigratorTestCase cause failures in other tests about django-test-migrations HOT 9 CLOSED

@sobolevn Thank you for the kind words. And you're most welcome, thanks for making a super useful package!

from django-test-migrations.

I've made #133 as a preview PR. Two tests fail because I slightly changed how signals behave. Happy to fix them if you think that's a good direction.

from django-test-migrations.

Looks like this is due to the content-type issue. We can possibly leave a database in a broken state after testing migrations.

@skarzi what do you think?

from django-test-migrations.

I just compared https://github.com/wemake-services/django-test-migrations/blob/master/django_test_migrations/contrib/unittest_case.py and https://github.com/skarzi/django-test-migrations/blob/master/test_migrations/mixins.py. A striking difference is that django-test-migrations disconnects signals. The Django issue makes me think the contenttypes are created in a receiver that listens to migrations. A theory I have is that the because that signal isn't emitted, the contenttypes aren't created after a MigratorTestCase tore down.
To verify this theory, I removed the _pre_setup and _post_teardown methods from MigratorTestCase. Two test runs with random order have passed so far. Maybe you can shed some light on why the signals are disconnected? Neither @skarzi nor the original blog post are doing that.

from django-test-migrations.

Original issue about signals #68 and #11

from django-test-migrations.

Probably we should disable signals only for Migrator.apply_initial_migration and Migrator.apply_tested_migration, but not for Migrator.reset to apply all migrations and emit post_migrate signal to setup one content types and all permissions for each model like django test case do, however, it's just a guess and I will validate it later on this week.

from django-test-migrations.

Probably we should disable signals only for Migrator.apply_initial_migration and Migrator.apply_tested_migration, but not for Migrator.reset to apply all migrations and emit post_migrate signal to setup one content types and all permissions for each model like django test case do

Sounds reasonble!

from django-test-migrations.

@skarzi I think you're onto something there. I just ran our test suite with a modified MigratorTestCase, and the issues went away. I also confirmed that the issues are still present without the modifications.

I modified tearDown to restore signals before calling reset. For that, I also removed the _post_teardown method. The complete class is here: https://gist.github.com/maikhoepfel/433fef487b0a8f0a4ba0e72b76dc945c#file-unittest_case-py-L33

I'm happy to provide a PR, but I'm unsure how to fix the same issue for the pytest fixture. My feeling is that handling signals is a core aspect of this library, and should move into the Migrator class. I think one could update apply_[initial|tested]_migration to silence signals before they start, and restore them before they return. That means a slight change in behavior, but should still fix the issue. It would also mean signals are handled correctly for people using Migrator directly.

Please let me know if you think that the signals handling code should move into Migrator. I'm happy to make a PR for it either way.

from django-test-migrations.

@maikhoepfel yes, looks like it is a good direction! Thanks a lot for your time and effort!

from django-test-migrations.