Opened 9 years ago
Last modified 2 months ago
#25251 assigned Bug
Inconsistent availability of data from migrations in TestCases when using --keepdb
Reported by: | David Szotten | Owned by: | David Sanders |
---|---|---|---|
Component: | Testing framework | Version: | 1.8 |
Severity: | Normal | Keywords: | |
Cc: | romain.garrigues.cs@…, rpkilby@…, jedie, Sarah Boyce, David Sanders, Sage Abdullah | Triage Stage: | Accepted |
Has patch: | yes | Needs documentation: | yes |
Needs tests: | yes | Patch needs improvement: | yes |
Easy pickings: | no | UI/UX: | no |
Description
I may have misunderstood something, but keepdb
doesn't work as i expect when used with TransactionTestCase.
# models.py class Model(models.Model): field = models.TextField() # migrations # 0001: default, as created by makemigrations # 0002: datamigration def create_data(apps, schema_editor): Model = apps.get_model("app", "Model") Model.objects.create(field='foo') class Migration(migrations.Migration): dependencies = [ ('app', '0001_initial'), ] operations = [ migrations.RunPython(create_data) ] # tests.py from .models import Model class Test(TransactionTestCase): def test_foo(self): self.assertEqual(Model.objects.count(), 1)
fails with --keepdb
(then second time so feature is being used)
needs a non-sqlite db so as to not use :memory:
Change History (45)
comment:1 by , 9 years ago
comment:2 by , 9 years ago
Summary: | TransactionTestCase and --keepdb → Inconsistent availability of data migrations in TransactionTestCase when using --keepdb |
---|---|
Triage Stage: | Unreviewed → Accepted |
comment:3 by , 9 years ago
I'm facing the same problem: my initial data are no more in the database at the end of my test suite, even with --keepdb
option.
As TransactionTestCase
is always flushing data on the tearDown
, why not reloading the serialized data after the flush, instead of on the setUp
step ?
I have tried a naive approach by moving the logic related to string serialized data loading on the tearDown
step, https://github.com/romgar/django/commit/7219308cf8f196463d03d1407b0ad0e9b918a3db and the data is then kept.
Not really sure about any other impact anywhere else.
I run the django test suite without any failure.
Any feedbacks on that ?
comment:4 by , 9 years ago
If you could submit a patch with that approach along with a regression test, I'll take a closer look.
comment:6 by , 9 years ago
I had a quick look at django test suite and didn't managed to easily get where the keepdb
option is tested.
I have seen admin_scripts.tests
to launch some commands during tests and migration_test_data_persistance.tests
related to TransactionTestCase
data persistence.
A relevant test in that situation would be:
- launch the test command with
--keepdb
option (or even directly theTestRunner
?) over an app that contains initial data migrations and aTransactionTestCase
, - check after the return of this command if the database is still containing initial data migrations,
- clean the database that has been created through the test command.
I would appreciate some guidance and/or code references to help me.
comment:7 by , 9 years ago
I think keepdb
isn't an important part of testing this (it doesn't really have any tests). Instead you could use an approach similar to what's done in tests/test_utils/test_transactiontestcase.py
where you invoke TransactionTestCase
's methods within a test and check the availability of the data.
comment:8 by , 9 years ago
Patch submitted https://github.com/django/django/pull/6137 with a regression test.
I will be glad to hear any comment about it.
I have created a new folder test_utils2
because I needed some migrations and didn't want to create the migrations related to already existing models in test_utils
.
comment:9 by , 9 years ago
Has patch: | set |
---|---|
Patch needs improvement: | set |
Please uncheck "Patch needs improvement" when tests are passing.
comment:10 by , 9 years ago
Patch needs improvement: | unset |
---|
comment:15 by , 9 years ago
Cc: | added |
---|
comment:16 by , 9 years ago
Left a few more comments. Please uncheck "Patch needs improvement" when the PR is updated.
comment:17 by , 9 years ago
Patch needs improvement: | unset |
---|
comment:18 by , 9 years ago
I have updated the rollback section and backward incompatible changes. Not really sure if if a good idea to add a "Changed in Django 1.9" and "Changed in Django 1.10" sections at the same time.
comment:19 by , 9 years ago
Patch needs improvement: | set |
---|
comment:20 by , 8 years ago
Patch needs improvement: | unset |
---|
comment:22 by , 8 years ago
Patch needs improvement: | unset |
---|
comment:23 by , 8 years ago
Patch needs improvement: | set |
---|
$ ./tests/runtests.py postgres_tests --settings=test_postgres -k
is crashing with the patch where it seems to work fine currently.
comment:24 by , 8 years ago
Patch needs improvement: | unset |
---|
comment:25 by , 8 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
I updated the MR according to my discussion with @aaugustin.
It impacted the DiscoverRunner
explicit test ordering. I didn't want to update the current tests to ensure backward-compatibility, I created new ones for TransactionTestCase
tests ordering.
comment:26 by , 8 years ago
Patch needs improvement: | set |
---|
Some cosmetic comments for improvement are on the PR.
comment:27 by , 8 years ago
Cc: | added |
---|
comment:29 by , 6 years ago
Cc: | added |
---|
comment:30 by , 6 years ago
I would be keen to continue and have a look again, but I previously had a hard time finding reviewers for it.
There currently are only cosmetic changes requested by Tim Graham, and I would prefer to update them only when the approach used to fix the issue is approved first.
comment:31 by , 6 years ago
Patch needs improvement: | unset |
---|
comment:32 by , 6 years ago
Patch needs improvement: | set |
---|
comment:35 by , 6 years ago
There's still an issue with this. Test methods that are decorated with skip doesn't have the __unittest_skip__
attribute set so the filtering of skipped tests doesn't work in that case (see PR comment). Since it seems a solution for that isn't forthcoming, I'm going to revert the patches to get the build that runs with --reverse
passing again. I'll also close #29948 (a regression after the initial fix) which should be addressed if another patch for this issue is provided. PR
comment:37 by , 6 years ago
Resolution: | fixed |
---|---|
Status: | closed → new |
comment:38 by , 4 years ago
Summary: | Inconsistent availability of data migrations in TransactionTestCase when using --keepdb → Inconsistent availability of data from migrations in TestCases when using --keepdb |
---|
A few more insights in what's happening:
- The issue occurs when
TransactionTestCase
s are involved. - Data will be unavailable even in regular
TestCase
s.
Let's assume an empty database. Additionally, the database must support transactions (e.g. PostgreSQL or MySQL (but not MyISAM
)). Let's further say you have two test classes (TestRegular(TestCase)
and TestTx(TransactionTestCase)
) with the test methods TestRegular.test_1
and TestTx.test_2
.
- When you run
manage.py test --keepdb
for the first time, Django will apply all migrations. That means, data created within those migrations will be present.
- Django runs tests inheriting from
TestCase
before those inheriting fromTransactionTestCase
(https://github.com/django/django/blob/9bf5e9418f425666726559c9f1981a516da30aab/django/test/runner.py#L632 and https://github.com/django/django/blob/9bf5e9418f425666726559c9f1981a516da30aab/django/test/runner.py#L807-L832). That means,test_1()
will be run beforetest_2()
.
- When data should be serialized for a database, Django will insert that data during the setup for a test function (https://github.com/django/django/blob/9bf5e9418f425666726559c9f1981a516da30aab/django/test/testcases.py#L1005-L1012). This can be enabled explicitly for
TransactionTestCase
usingserialized_rollback=True
(https://docs.djangoproject.com/en/3.1/topics/testing/overview/#rollback-emulation)
- When a test function in a
TransactionTestCase
is over, Django executes theflush
management command (https://github.com/django/django/blob/9bf5e9418f425666726559c9f1981a516da30aab/django/test/testcases.py#L1063-L1066). This empties the database.
- At the end of the test suite, however, Django does _not_ reinsert serialized test data, leaving the database empty after the first
--keepdb
run.
- Running
manage.py test --keepdb
again means, the database is empty (has no data that was previously created by the migrations). This means, tests in regularTestCase
s (e.g.test_1()
will now fail as well.
comment:39 by , 4 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
Here's a first draft at restoring data at the end of the test suite: https://github.com/django/django/pull/14147
comment:41 by , 3 years ago
Owner: | removed |
---|---|
Status: | assigned → new |
Feel free to take over https://github.com/django/django/pull/14147, Blaž. I'm not using TransactionTestCase
anymore at the moment.
comment:42 by , 16 months ago
Cc: | added |
---|
comment:43 by , 8 months ago
Cc: | added |
---|
comment:44 by , 8 months ago
Needs documentation: | set |
---|---|
Needs tests: | set |
Owner: | set to |
Status: | new → assigned |
comment:45 by , 2 months ago
Cc: | added |
---|
A
TransactionTestCase
resets the database after the test runs by truncating all tables. For this reason, the docs say, "Any initial data loaded in migrations will only be available inTestCase
tests and not inTransactionTestCase
tests." I think that technically the data is available in the firstTransactionTestCase
(since truncation hasn't happened yet), but not any others. Perhaps this should be fixed or the docs clarified.