Skip to content

Support Serialized Rollbacks for Databases #919

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

Closed
wants to merge 3 commits into from

Conversation

jpulec
Copy link

@jpulec jpulec commented Mar 27, 2021

Description

This PR adds support for serialized rollbacks for databases. It's mostly taken directly from #721 and has been updated to mergable with the current HEAD.

Opening this as draft initially to run CI.

Fixes #329

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, it mostly looks good to me, I left some comments.

One thing that is not so nice is that (IIUC) django_db_serialized_rollback and django_db_reset_sequences do not compose properly. Making it so will require refactoring the code a bit. If you can make it so that'd be great, if not, I can look at it after the PR is merged.

@@ -243,6 +253,8 @@ def transactional_db(request, django_db_setup, django_db_blocker):
"""
if "django_db_reset_sequences" in request.fixturenames:
request.getfixturevalue("django_db_reset_sequences")
if "django_db_serialized_rollback" 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.

Need to add django_db_serialized_rollback to the docstring above.

Comment on lines +281 to +286
"""Enable serialized rollback after transaction test cases

This fixture only has an effect when the ``transactional_db``
fixture is active, which happen as a side-effect of requesting
``live_server``.

Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
"""Enable serialized rollback after transaction test cases
This fixture only has an effect when the ``transactional_db``
fixture is active, which happen as a side-effect of requesting
``live_server``.
"""Enable serialized rollback after transactional tests.
This fixture only has an effect when the :fixture:`transactional_db`
fixture is active, which happen as a side-effect of requesting
:fixture:`live_server`.

@@ -218,13 +225,16 @@ def db(request, django_db_setup, django_db_blocker):
"""
if "django_db_reset_sequences" in request.fixturenames:
request.getfixturevalue("django_db_reset_sequences")
if "django_db_serialized_rollback" 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.

Need to add it to the docstring above.

class ResetSequenceTestCase(django_case):
reset_sequences = True

django_case = ResetSequenceTestCase

if serialized_rollback:
class SerializedRollbackTestCase(django_case):
Copy link
Member

Choose a reason for hiding this comment

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

Creating another subclass seems unnecessary. I will combine with the above (can call it PytestDjangoTestCase).

:type serialized_rollback: bool
:param serialized_rollback:
The ``serialized_rollback`` argument enables `rollback emulation`_.
After a `django.test.TransactionTestCase`_ runs, the database is
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
After a `django.test.TransactionTestCase`_ runs, the database is
After a :class:`django.test.TransactionTestCase` runs, the database is

After a `django.test.TransactionTestCase`_ runs, the database is
flushed, destroying data created in data migrations. This is the
default behavior of Django. Setting ``serialized_rollback=True``
tells Django to restore that data.
Copy link
Member

Choose a reason for hiding this comment

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

Need to mention django_db_serialized_rollback in the paragraph below.

marker applied in order to access the database.

.. _rollback emulation: https://docs.djangoproject.com/en/stable/topics/testing/overview/#rollback-emulation
.. _django.test.TestCase: https://docs.djangoproject.com/en/dev/topics/testing/overview/#testcase
Copy link
Member

Choose a reason for hiding this comment

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

intersphinx references should be used when possible (see my example suggestion above,

:class:`django.test.TransactionTestCase`

bluetech added a commit to bluetech/pytest-django that referenced this pull request Nov 29, 2021
Thanks to Aymeric Augustin, Daniel Hahler and Fábio C. Barrionuevo da
Luz for previous work on this feature.

Fix pytest-dev#329.
Closes pytest-dev#353.
Closes pytest-dev#721.
Closes pytest-dev#919.
Closes pytest-dev#956.
bluetech added a commit to bluetech/pytest-django that referenced this pull request Nov 29, 2021
Thanks to Aymeric Augustin, Daniel Hahler and Fábio C. Barrionuevo da
Luz for previous work on this feature.

Fix pytest-dev#329.
Closes pytest-dev#353.
Closes pytest-dev#721.
Closes pytest-dev#919.
Closes pytest-dev#956.
bluetech added a commit to bluetech/pytest-django that referenced this pull request Nov 29, 2021
Thanks to Aymeric Augustin, Daniel Hahler and Fábio C. Barrionuevo da
Luz for previous work on this feature.

Fix pytest-dev#329.
Closes pytest-dev#353.
Closes pytest-dev#721.
Closes pytest-dev#919.
Closes pytest-dev#956.
bluetech added a commit to bluetech/pytest-django that referenced this pull request Nov 29, 2021
Thanks to Aymeric Augustin, Daniel Hahler and Fábio C. Barrionuevo da
Luz for previous work on this feature.

Fix pytest-dev#329.
Closes pytest-dev#353.
Closes pytest-dev#721.
Closes pytest-dev#919.
Closes pytest-dev#956.
bluetech added a commit to bluetech/pytest-django that referenced this pull request Nov 29, 2021
Thanks to Aymeric Augustin, Daniel Hahler and Fábio C. Barrionuevo da
Luz for previous work on this feature.

Fix pytest-dev#329.
Closes pytest-dev#353.
Closes pytest-dev#721.
Closes pytest-dev#919.
Closes pytest-dev#956.
bluetech added a commit to bluetech/pytest-django that referenced this pull request Nov 29, 2021
Thanks to Aymeric Augustin, Daniel Hahler and Fábio C. Barrionuevo da
Luz for previous work on this feature.

Fix pytest-dev#329.
Closes pytest-dev#353.
Closes pytest-dev#721.
Closes pytest-dev#919.
Closes pytest-dev#956.
@bluetech
Copy link
Member

A PR which will replace this PR is in #970. I intend to merge and release it in a few days. Comments and tests are welcome.

bluetech added a commit to bluetech/pytest-django that referenced this pull request Dec 1, 2021
Thanks to Aymeric Augustin, Daniel Hahler and Fábio C. Barrionuevo da
Luz for previous work on this feature.

Fix pytest-dev#329.
Closes pytest-dev#353.
Closes pytest-dev#721.
Closes pytest-dev#919.
Closes pytest-dev#956.
@bluetech bluetech closed this in #970 Dec 1, 2021
bluetech added a commit that referenced this pull request Dec 1, 2021
Thanks to Aymeric Augustin, Daniel Hahler and Fábio C. Barrionuevo da
Luz for previous work on this feature.

Fix #329.
Closes #353.
Closes #721.
Closes #919.
Closes #956.
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.

Tests using live_server fixture removing data from data migrations
3 participants