Opened 10 years ago

Closed 10 years ago

Last modified 10 years ago

#22487 closed Bug (fixed)

Moving from initial_data to data migrations stops test data persisting

Reported by: Andrew Godwin Owned by: Andrew Godwin
Component: Testing framework Version: 1.7-beta-1
Severity: Release blocker Keywords:
Cc: jakerothenbuhler@…, merb, cmawebsite@… Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Before migrations came along, tests would happily flush the database before each test so they could restore the data that was previously in there - initial data could be easily loaded from the initial_data fixture.

Now we're encouraging data migrations to load initial data in - which are superior in almost every way - there's no easy way to replay that data back at the moment and so tests just don't see this data. Something needs to be done to allow initial data to persist across tests.

Change History (34)

comment:1 by Andrew Godwin, 10 years ago

This is being discussed at https://groups.google.com/forum/#!topic/django-developers/wR6hTy9Nu2k

My initial proposal, and what I'll look at first, is dumping the data in the database into a temporary (possibly in-memory) fixture at the start of the test run, and then having flush restore from that.

comment:2 by Andrew Godwin, 10 years ago

Status: newassigned
Type: UncategorizedBug

comment:3 by Tim Graham, 10 years ago

Triage Stage: UnreviewedAccepted

comment:4 by Jake Rothenbuhler, 10 years ago

Cc: jakerothenbuhler@… added

comment:5 by merb, 10 years ago

Cc: merb added

comment:6 by Andrew Godwin, 10 years ago

I have done some exploratory work on this, and implemented the dump-into-memory-and-restore-after-each-test solution here: https://github.com/django/django/pull/2643

It seems to not break - I haven't tested if it fully works yet - but it does slow the runtime of TransactionTestCases down by a factor of 3 (the schema/migrations test suite goes from 6 seconds to 18 seconds under PostgreSQL).

Tests can opt out of the new behaviour by setting serialized_rollback = False on the TestCase, which speeds things up again, so I think overall it's not _too_ bad. Would appreciate some comments, though. There's the possibility we could default that to False (nothing in Django's main tests needs it turned on), and then just document that users who are loading data in migrations probably want it on for their TransactionTestCases and/or their main tests if they're on MyISAM.

comment:7 by merb, 10 years ago

Currently this won't work on my side.
I always get something like that:
Sqlite3 http://pastebin.com/mb89mVqf
Postgresql http://pastebin.com/WZu9JGF9

This is the application I tested: http://gccc.de/unittest_persistance.tar.gz
Just a really simple application, so that I could run it with:

PYTHONPATH=../django python manage.py test

comment:8 by anonymous, 10 years ago

..nvm this should work, but i think we should add some tests to the pull request

comment:9 by Andrew Godwin <andrew@…>, 10 years ago

Resolution: fixed
Status: assignedclosed

In 8c12d51ea27479555e226894c50c83043211d71d:

Fixed #22487: Optional rollback emulation for migrated apps

comment:10 by Tim Graham, 10 years ago

Resolution: fixed
Status: closednew

This seems to be causing some (non-deterministic?) failures in proxy_model_inheritance.tests.ProxyModelInheritanceTests.test_table_exists, when running the full test suite:

Traceback (most recent call last):
  File "/mnt/jenkinsdata/workspace/django-1.7/database/sqlite3/python/python3.4/django/db/backends/utils.py", line 65, in execute
    return self.cursor.execute(sql, params)
  File "/mnt/jenkinsdata/workspace/django-1.7/database/sqlite3/python/python3.4/django/db/backends/sqlite3/base.py", line 485, in execute
    return Database.Cursor.execute(self, query, params)
sqlite3.OperationalError: table "migration_test_data_persistence_book" already exists

See:

I can reproduce it locally; not sure why ci.djangoproject.com doesn't appear to be affected.

comment:11 by Tim Graham <timograham@…>, 10 years ago

comment:12 by Tim Graham <timograham@…>, 10 years ago

In 417da8a78277a14bd2348aec12d79b43f3dac6ee:

[1.7.x] Doc edits for refs #22487.

Backport of c17cd151d8 from master

comment:13 by Andrew Godwin, 10 years ago

What looks to be happening in those failures is that somehow the 0001 migration for migration_test_data_persistence is being applied but not being marked as such, and so the call to migrate in proxy_model_inheritance tests is trying to re-apply it. Not sure why.

comment:14 by Andrew Godwin, 10 years ago

Resolution: fixed
Status: newclosed

OK, I've pushed a fix for the test failures in f146e70cb1cfe34a9993a9a1d95faf2bb8b15ab1; re-closing optimistically.

comment:15 by Tim Graham <timograham@…>, 10 years ago

In 6b97ae352170d7be47176787056239cfbacb4803:

Added rollback emulation to spatialite backend; refs #22487.

comment:16 by Tim Graham <timograham@…>, 10 years ago

In 11f0bdc00757e5646337633f6a045b085f0653b2:

[1.7.x] Added rollback emulation to spatialite backend; refs #22487.

Backport of 6b97ae3521 from master

comment:17 by Tim Graham, 10 years ago

Resolution: fixed
Status: closednew

As discussed on IRC, this change broke the tests on Oracle:

  Applying migration_test_data_persistence.0001_initial...

Traceback (most recent call last):
...
  File "/var/lib/jenkins/workspace/DjangOra-1.8/database/oracle11/python/python2.7/django/db/backends/oracle/base.py", line 894, in execute
    return self.cursor.execute(query, self._param_generator(params))
django.db.utils.IntegrityError: ORA-01400: cannot insert NULL into ("DJANGODJANGORA18_27"."MIGRATION_TEST_DATA_PERSISFB95"."ID")

comment:18 by Shai Berger <shai@…>, 10 years ago

Resolution: fixed
Status: newclosed

In fa42cf77b26bc1f09f9d62bc14338b295d700d87:

[1.7.x] Fixed #22487: Separated schema migration from data migration in test

The data migration failed on Oracle, killing the entire test suite.

Thanks timo for reporting the Oracle breakage,
and andrewgodwin for suggesting the solution.

Backport of 64d94cf from master

comment:19 by Shai Berger <shai@…>, 10 years ago

Resolution: fixed
Status: newclosed

In 64d94cffc73fc4de1389e16d98142d253155b325:

Fixed #22487: Separated schema migration from data migration in test

The data migration failed on Oracle, killing the entire test suite.

Thanks timo for reporting the Oracle breakage,
and andrewgodwin for suggesting the solution.

comment:20 by RafalP, 10 years ago

Running commit db9cb83d2fec781f6479f19b63d50b44a35ae632, I can't say this is fixed:

======================================================================
ERROR: test_root_category (misago.forums.tests.test_forum_model.ForumManagerTests)
root_category returns forums tree root


Traceback (most recent call last):

File "/Users/rafapiton/Documents/misago/misago/forums/tests/test_forum_model.py", line 15, in test_root_category

forum = Forum.objects.root_category()

File "/Users/rafapiton/Documents/misago/misago/forums/models.py", line 19, in root_category

return self.get(special_role='root_category')

File "/Users/rafapiton/venvs/misago06/lib/python2.7/site-packages/django/db/models/manager.py", line 92, in manager_method

return getattr(self.get_queryset(), name)(*args, kwargs)

File "/Users/rafapiton/venvs/misago06/lib/python2.7/site-packages/django/db/models/query.py", line 355, in get

self.model._meta.object_name)

DoesNotExist: Forum matching query does not exist.

There is DB migration in place that creates forums tree. When I run "manage.py migrate", DB is created correctly. When I put debug code in my migration to raise Exception() with .count() for apps.get_model('misago_forums', 'Forum"), it returns valid number (5 forums), however as soon as test tries to reach to this table, its empty. I've included similiar code at beginning of test, this time exception returns 0.

I am running tests on Postgres 9.3:

comment:21 by Andrew Godwin, 10 years ago

Can you confirm you have set serialized_rollback = True on your TransactionTestCase?

comment:22 by RafalP, 10 years ago

Hello Andrew,

I've removed all test cases from my test suite, leaving only one. It's TransactionTestCase with serialized_rollback = True, and now I have errors on table repopulation:

======================================================================
ERROR: test_invalid_name (misago.users.tests.test_validators.ValidateUsernameLengthTests)
validate_username_length disallows invalid names
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/rafapiton/venvs/misago06/lib/python2.7/site-packages/django/test/testcases.py", line 182, in __call__
    self._pre_setup()
  File "/Users/rafapiton/venvs/misago06/lib/python2.7/site-packages/django/test/testcases.py", line 754, in _pre_setup
    self._fixture_setup()
  File "/Users/rafapiton/venvs/misago06/lib/python2.7/site-packages/django/test/testcases.py", line 797, in _fixture_setup
    connections[db_name]._test_serialized_contents
  File "/Users/rafapiton/venvs/misago06/lib/python2.7/site-packages/django/db/backends/creation.py", line 435, in deserialize_db_from_string
    obj.save()
  File "/Users/rafapiton/venvs/misago06/lib/python2.7/site-packages/django/core/serializers/base.py", line 176, in save
    setattr(self.object, accessor_name, object_list)
  File "/Users/rafapiton/venvs/misago06/lib/python2.7/site-packages/django/db/models/fields/related.py", line 1177, in __set__
    manager.add(*value)
  File "/Users/rafapiton/venvs/misago06/lib/python2.7/site-packages/django/db/models/fields/related.py", line 917, in add
    self._add_items(self.source_field_name, self.target_field_name, *objs)
  File "/Users/rafapiton/venvs/misago06/lib/python2.7/site-packages/django/db/models/fields/related.py", line 1026, in _add_items
    for obj_id in new_ids
  File "/Users/rafapiton/venvs/misago06/lib/python2.7/site-packages/django/db/models/query.py", line 407, in bulk_create
    self._batched_insert(objs_without_pk, fields, batch_size)
  File "/Users/rafapiton/venvs/misago06/lib/python2.7/site-packages/django/db/transaction.py", line 339, in __exit__
    connection.commit()
  File "/Users/rafapiton/venvs/misago06/lib/python2.7/site-packages/django/db/backends/__init__.py", line 176, in commit
    self._commit()
  File "/Users/rafapiton/venvs/misago06/lib/python2.7/site-packages/django/db/backends/__init__.py", line 145, in _commit
    return self.connection.commit()
  File "/Users/rafapiton/venvs/misago06/lib/python2.7/site-packages/django/db/utils.py", line 94, in __exit__
    six.reraise(dj_exc_type, dj_exc_value, traceback)
  File "/Users/rafapiton/venvs/misago06/lib/python2.7/site-packages/django/db/backends/__init__.py", line 145, in _commit
    return self.connection.commit()
IntegrityError: insert or update on table "misago_users_rank_roles" violates foreign key constraint "misago_users_ran_role_id_5cb6d82bf6031b71_fk_misago_acl_role_id"
DETAIL:  Key (role_id)=(3) is not present in table "misago_acl_role".

Also please correct me if I am wrong, but from reading documentation I get picture that if my app contains data migrations, and this data is important for my tests, I should make those tests inherit from TestCase and use database with transactions support (such as Postgres)?

Thanks for your time,
Ralph

comment:23 by Andrew Godwin, 10 years ago

Yes, you should not be using TransactionTestCase unless you're very sure you want to use it (i.e. you're testing transaction handling).

As for your traceback, it looks like you have a custom user with some extra ForeignKeys that aren't being populated right. Does normal use of loaddata/dumpdata work with your app and this model?

comment:24 by RafalP, 10 years ago

Andrew,

Indeed I have custom User model with relations to other models.

I've did dumpdata to file, then ran flush and finally ran loaddata for file I've created ealier, and Django raised no errors.

To sum this up, if my test case is TransactionTestCase with serialized_rollback = True, test case fails on IntegrityError. This test case is dependant on data created in data migration, so when I make it inherit from TestCase instead, it fails because database tables are emptied before tests are ran. I am running my tests against PostgreSQL 9.3 database and I have no idea why this happens, because I was under impression that flushes should happen only on DB's that can't use transactions to reverse db changes made by tests?

Again, thanks for your time and energy!

comment:25 by Andrew Godwin, 10 years ago

It happens on DBs without transactions _and_ with TransactionTestCase, because the latter is not enclosed in a transaction and so we can't roll it back, unfortunately. This is why you should avoid TransactionTestCase unless you really need it :)

Is it possible that you could reduce the problem to a couple of models and a migration so I can replicate it? Failing that, if you're comfortable, a larger slice/the whole project would work (I'm happy to directly negotiate access so it's not in public if the project isn't open source).

comment:26 by Rafal, 10 years ago

Its open source, and branch that I am trying to make work with Dj 1.7 is available here:

https://github.com/rafalp/Misago/tree/future

I may have messed up the test runner when I've started experimenting just-in-case its entirely my idiocy that tests that passed suddenly started to fail badly, so if setup.py test blurps, give line 49 in runtests.py a look and adapt it accordingly.

comment:27 by RafalP, 10 years ago

Andrew,

I have created tiny project just to see if its not me doing something wrong. It contains one app. This app contains three things: model, migrations, and tests. First migration creates table in database and second migration populates it with some data. At end of migration there is count() to make sure bulk_create succeeded. Then tests in test suite see if Model.objects.count() == 6.

There are two tests cases, two tests in each. My intention was to see if data disappears after individual test, or tests case. Migration succeds meaning bulk_create did its job, but all tests fail on PostgreSQL 9.3:

https://github.com/rafalp/django-data-migrations-bug

So its either me being real idiot with using data migrations in tests, or bug 22487 is not fixed.

comment:28 by RafalP, 10 years ago

Resolution: fixed
Status: closednew

comment:29 by Collin Anderson, 10 years ago

I'm pretty new to postgres, tests, transactions, migrations, and initial data, but I can confirm on postgres 9.1 and latest 1.7.x that the migration does load in the initial data, and that it is gone by the time the TestCases are run.

Version 1, edited 10 years ago by Collin Anderson (previous) (next) (diff)

comment:30 by Collin Anderson, 10 years ago

Cc: cmawebsite@… added

comment:31 by Andrew Godwin <andrew@…>, 10 years ago

Resolution: fixed
Status: newclosed

In 0fba4c0ed7fa1d71a6a9d6777cc64933f8d0005a:

Fixed #22487: Don't flush out data from before normal TestCases

comment:32 by Andrew Godwin <andrew@…>, 10 years ago

In 0dd737a719cfbd3bc11ff6f2b90161883fcebea0:

[1.7.x] Fixed #22487: Don't flush out data from before normal TestCases

comment:33 by Tim Graham <timograham@…>, 10 years ago

In 088b30f49b0f5e14fc6876eb424af340d5479dac:

[1.7.x] refs #22487: Don't flush out data from before normal TestCases (spatialite)

comment:34 by RafalP, 10 years ago

I can confirm fix. All tests in demo project pass, my project test suite dropped form 76 fails to 6.

Great work, thanks a lot!

Note: See TracTickets for help on using tickets.
Back to Top