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

Fix clobbering of real database by tests #182

Conversation

qris
Copy link

@qris qris commented Oct 17, 2014

Force disconnection if a database is already connected when patching for DB reuse.

DjangoCMS opens database connections very early, to set up URL routing, before
pytest has time to patch the database name to use the test database. Thus the
connection is already open and changing the NAME has no effect, so the real
database gets clobbered instead of the test one.

This isn't easy to trigger - I've only just noticed it and I've been using
DjangoCMS together with pytest-django for several projects. I don't yet know
what causes it. But because it's so catastrophic, I think we ought to put some
protection against it in place, hence this patch.

…for DB reuse.

DjangoCMS opens database connections very early, to set up URL routing, before
pytest has time to patch the database name to use the test database. Thus the
connection is already open and changing the NAME has no effect, so the real
database gets clobbered instead of the test one.

This isn't easy to trigger - I've only just noticed it and I've been using
DjangoCMS together with pytest-django for several projects. I don't yet know
what causes it. But because it's so catastrophic, I think we ought to put some
protection against it in place, hence this patch.
@qris
Copy link
Author

qris commented Oct 17, 2014

I just found two examples of things that cause this to happen. They both seem to occur because pytest-django initialises Django before running the reuse_db plugin. I'm not sure if that's a good idea, because it will cause some things to happen on the production database. Presumably manage.py test has a better way around this, or manages to get in earlier in the Django setup.

Static initialisation of class members from the database:

class CurriculumDict(models.Model):
    choices=((lang.code, lang.name) for lang in Language.objects.all()))

Resolving a URL name statically, which causes URL setup to happen early, and if DjangoCMS is enabled this causes it to hit the database:

class UsersAppTest(AptivateEnhancedTestCase):
    REGISTER_URI = reverse('registration_register')

@blueyed
Copy link
Contributor

blueyed commented Oct 17, 2014

Thanks for raising this issue and investigating into it!

It would be good to have some test for this.

If i understand it correctly, the "Static initialisation" example would trigger it?

@pelme
Copy link
Member

pelme commented Jun 25, 2016

From what I understand, this PR/discussion deals with errors that is caused by doing database queries, url lookups and other stuff at import time. These problems has been fixed by Django and its app refactor.

We have also made the database checking stricter in the latest version.

I'm therefore going to close this PR, feel free to open new issues or a new PR against the latest master if these problems still persists.

@pelme pelme closed this Jun 25, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants