Opened 15 months ago

Closed 12 months ago

Last modified 12 months ago

#22487 closed Bug (fixed)

Moving from initial_data to data migrations stops test data persisting

Reported by: andrewgodwin Owned by: andrewgodwin
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 Changed 15 months ago by andrewgodwin

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

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 Changed 15 months ago by andrewgodwin

  • Status changed from new to assigned
  • Type changed from Uncategorized to Bug

comment:3 Changed 15 months ago by timo

  • Triage Stage changed from Unreviewed to Accepted

comment:4 Changed 14 months ago by jrothenbuhler

  • Cc jakerothenbuhler@… added

comment:5 Changed 14 months ago by merb

  • Cc merb added

comment:6 Changed 14 months ago by andrewgodwin

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 Changed 14 months ago by merb

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 Changed 13 months ago by anonymous

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

comment:9 Changed 13 months ago by Andrew Godwin <andrew@…>

  • Resolution set to fixed
  • Status changed from assigned to closed

In 8c12d51ea27479555e226894c50c83043211d71d:

Fixed #22487: Optional rollback emulation for migrated apps

comment:10 Changed 13 months ago by timo

  • Resolution fixed deleted
  • Status changed from closed to new

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 Changed 13 months ago by Tim Graham <timograham@…>

comment:12 Changed 13 months ago by Tim Graham <timograham@…>

In 417da8a78277a14bd2348aec12d79b43f3dac6ee:

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

Backport of c17cd151d8 from master

comment:13 Changed 13 months ago by andrewgodwin

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 Changed 13 months ago by andrewgodwin

  • Resolution set to fixed
  • Status changed from new to closed

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

comment:15 Changed 13 months ago by Tim Graham <timograham@…>

In 6b97ae352170d7be47176787056239cfbacb4803:

Added rollback emulation to spatialite backend; refs #22487.

comment:16 Changed 13 months ago by Tim Graham <timograham@…>

In 11f0bdc00757e5646337633f6a045b085f0653b2:

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

Backport of 6b97ae3521 from master

comment:17 Changed 13 months ago by timo

  • Resolution fixed deleted
  • Status changed from closed to new

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 Changed 13 months ago by Shai Berger <shai@…>

  • Resolution set to fixed
  • Status changed from new to closed

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 Changed 13 months ago by Shai Berger <shai@…>

  • Resolution set to fixed
  • Status changed from new to closed

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 Changed 13 months ago by RafalP

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 Changed 13 months ago by andrewgodwin

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

comment:22 Changed 13 months ago by RafalP

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 Changed 13 months ago by andrewgodwin

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 Changed 13 months ago by RafalP

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 Changed 13 months ago by andrewgodwin

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 Changed 13 months ago by Rafal

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 Changed 12 months ago by RafalP

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 Changed 12 months ago by RafalP

  • Resolution fixed deleted
  • Status changed from closed to new

comment:29 Changed 12 months ago by CollinAnderson

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 12 months ago by CollinAnderson (previous) (next) (diff)

comment:30 Changed 12 months ago by CollinAnderson

  • Cc cmawebsite@… added

comment:31 Changed 12 months ago by Andrew Godwin <andrew@…>

  • Resolution set to fixed
  • Status changed from new to closed

In 0fba4c0ed7fa1d71a6a9d6777cc64933f8d0005a:

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

comment:32 Changed 12 months ago by Andrew Godwin <andrew@…>

In 0dd737a719cfbd3bc11ff6f2b90161883fcebea0:

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

comment:33 Changed 12 months ago by Tim Graham <timograham@…>

In 088b30f49b0f5e14fc6876eb424af340d5479dac:

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

comment:34 Changed 12 months ago by RafalP

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