Skip to content
New issue

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

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

Already on GitHub? # to your account

Force ordering of settings and transactional_db fixtures #870 #871

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

bdauvergne
Copy link

@bdauvergne bdauvergne commented Sep 24, 2020

Fixes #870.

Copy link
Member

@bluetech bluetech left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR, I understand the issue now, and the change LGTM.

I suggested a comment, and this also needs a rebase against the latest master. Afterwards, if the CI is green, we can merge.

"""A Django settings object which restores changes after the testrun"""
skip_if_no_django()

if 'transactional_db' in request.fixturenames:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest adding a comment explaining the rationale:

Suggested change
if 'transactional_db' in request.fixturenames:
# Order the `settings` fixture after DB.
# We don't want overridden settings to be in effect during
# DB setup/teardown/post_migrate.
if 'transactional_db' in request.fixturenames:

@bdauvergne
Copy link
Author

Tests failures seem to be the same than on master, with djmain.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Order of settings and transactional_db fixtures
2 participants