django.test.TestCase should flush constraints

[Glenn Maynard]

In Postgresql, constraints are set DEFERRABLE INITIALLY DEFERRED. This causes it to only check those constraints when the transaction is committed. (This may affect the Oracle backend, too, based on a quick search.)

This causes a problem with TestCase: as they're never committed, many constraint violations can never be triggered.

It's an easy fix: cause SET CONSTRAINTS ALL IMMEDIATE to be executed in TestCase before rolling back the transaction, so any deferred constraints are flushed, and treat an exception from that as a test failure.

I'll work on a patch in a week or so when I have some time. I'm not sure how to test this, since it means testing the testing system itself.

[Glenn Maynard]

This isn't a silent failure; the test runner fails later on when it tries to flush the database--the TRUNCATE command gives "cannot TRUNCATE "table" because it has pending trigger events", where the trigger events are actually the constraints failing. SET CONSTRAINTS ALL IMMEDIATE will give it a correct error message, showing which constraint is actually being violated.

[pcardune]

Any idea when a patch will become available? I just encountered this same problem.

[Russell Keith-Magee]

@pcardune: The patch will become available about 30 seconds after somebody writes it.

The solution here isn't to change the constraint definition. Constraints need to be DEFERRABLE, not IMMEDIATE - there are a number of tests that fail if constraint checks aren't deferred. To see which ones, run the test suite using MySQL with InnoDB. InnoDB only has IMMEDIATE mode for transactions, and fails a lot of tests as a result.

The interim workaround - if you actually need transactions, use TransactionTestCase rather than TestCase. This is slower, but uses a database truncate rather than transaction rollback to flush the database.

I don't know what the final solution will be, but I suspect it will involve reliable cleanup of the transaction on rollback, rather than changing the database so that the triggers aren't hit.

[Glenn Maynard]

Nobody's suggested changing the constraints; I suspect you misunderstand what SET CONSTRAINTS ALL IMMEDIATE does. See
​http://www.postgresql.org/docs/current/static/sql-set-constraints.html. Execute this at the end of a test case, just before it rolls back, so it executes the constraint checks as if the transaction was committed.

I've been busy, but I'll try to get to this eventually if nobody gets to it first.

[Russell Keith-Magee]

Apologies - I misunderstood your proposal. Chalk that one up to my morning coffee not kicking in yet. Now that I'm actually reading what you wrote, your proposal looks to be just what is required.

[olaechea]

Fixing this would involve changing the testcase class and the database drivers for each of the supported databases ?

[James Bennett]

1.2 is feature-frozen, moving this feature request off the milestone.

[Ramiro Morales]

Initial patch - It causes six ERRORs in the multiple_database regression tests

[Ramiro Morales]

I've attached my intepretation of how this could be implemented.

There is still some issue (either in the multiple_database tests or the ORM functionality they exercise) because applying this patch make six errors to appear in the full test suite, all of them in these tests. All of this when running against Postgres, of course, it shoudn't affect other backends.

Also, IMHOwe can make an exception regarding tests for this particular issue.

[Ramiro Morales]

Forgot to paste the errors:

======================================================================
ERROR: test_generic_key_reverse_operations (regressiontests.multiple_database.tests.QueryTestCase)
Generic reverse manipulations are all constrained to a single DB
----------------------------------------------------------------------
Traceback (most recent call last):
  File "django/test/testcases.py", line 296, in __call__
    self._post_teardown()
  File "django/test/testcases.py", line 312, in _post_teardown
    self._fixture_teardown()
  File "django/test/testcases.py", line 604, in _fixture_teardown
    connections[db].check_constraints()
  File "django/db/backends/__init__.py", line 261, in check_constraints
    self.cursor().execute(s)
  File "django/db/backends/postgresql_psycopg2/base.py", line 44, in execute
    return self.cursor.execute(query, args)
IntegrityError: insert or update on table "multiple_database_review" violates foreign key constraint "multiple_database_review_content_type_id_fkey"
DETAIL:  Key (content_type_id)=(1) is not present in table "django_content_type".


======================================================================
ERROR: test_generic_key_separation (regressiontests.multiple_database.tests.QueryTestCase)
Generic fields are constrained to a single database
----------------------------------------------------------------------
Traceback (most recent call last):
  File "django/test/testcases.py", line 296, in __call__
    self._post_teardown()
  File "django/test/testcases.py", line 312, in _post_teardown
    self._fixture_teardown()
  File "django/test/testcases.py", line 604, in _fixture_teardown
    connections[db].check_constraints()
  File "django/db/backends/__init__.py", line 261, in check_constraints
    self.cursor().execute(s)
  File "django/db/backends/postgresql_psycopg2/base.py", line 44, in execute
    return self.cursor.execute(query, args)
IntegrityError: insert or update on table "multiple_database_review" violates foreign key constraint "multiple_database_review_content_type_id_fkey"
DETAIL:  Key (content_type_id)=(1) is not present in table "django_content_type".


======================================================================
ERROR: test_foreign_key_cross_database_protection (regressiontests.multiple_database.tests.RouterTestCase)
Foreign keys can cross databases if they two databases have a common source
----------------------------------------------------------------------
Traceback (most recent call last):
  File "django/test/testcases.py", line 296, in __call__
    self._post_teardown()
  File "django/test/testcases.py", line 312, in _post_teardown
    self._fixture_teardown()
  File "django/test/testcases.py", line 604, in _fixture_teardown
    connections[db].check_constraints()
  File "django/db/backends/__init__.py", line 261, in check_constraints
    self.cursor().execute(s)
  File "django/db/backends/postgresql_psycopg2/base.py", line 44, in execute
    return self.cursor.execute(query, args)
IntegrityError: insert or update on table "multiple_database_book" violates foreign key constraint "multiple_database_book_editor_id_fkey"
DETAIL:  Key (editor_id)=(20) is not present in table "multiple_database_person".


======================================================================
ERROR: test_generic_key_cross_database_protection (regressiontests.multiple_database.tests.RouterTestCase)
Generic Key operations can span databases if they share a source
----------------------------------------------------------------------
Traceback (most recent call last):
  File "django/test/testcases.py", line 296, in __call__
    self._post_teardown()
  File "django/test/testcases.py", line 312, in _post_teardown
    self._fixture_teardown()
  File "django/test/testcases.py", line 604, in _fixture_teardown
    connections[db].check_constraints()
  File "django/db/backends/__init__.py", line 261, in check_constraints
    self.cursor().execute(s)
  File "django/db/backends/postgresql_psycopg2/base.py", line 44, in execute
    return self.cursor.execute(query, args)
IntegrityError: insert or update on table "multiple_database_review" violates foreign key constraint "multiple_database_review_content_type_id_fkey"
DETAIL:  Key (content_type_id)=(1) is not present in table "django_content_type".


======================================================================
ERROR: test_generic_key_managers (regressiontests.multiple_database.tests.RouterTestCase)
Generic key relations are represented by managers, and can be controlled like managers
----------------------------------------------------------------------
Traceback (most recent call last):
  File "django/test/testcases.py", line 296, in __call__
    self._post_teardown()
  File "django/test/testcases.py", line 312, in _post_teardown
    self._fixture_teardown()
  File "django/test/testcases.py", line 604, in _fixture_teardown
    connections[db].check_constraints()
  File "django/db/backends/__init__.py", line 261, in check_constraints
    self.cursor().execute(s)
  File "django/db/backends/postgresql_psycopg2/base.py", line 44, in execute
    return self.cursor.execute(query, args)
IntegrityError: insert or update on table "multiple_database_review" violates foreign key constraint "multiple_database_review_content_type_id_fkey"
DETAIL:  Key (content_type_id)=(1) is not present in table "django_content_type".


======================================================================
ERROR: test_m2m_managers (regressiontests.multiple_database.tests.RouterTestCase)
M2M relations are represented by managers, and can be controlled like managers
----------------------------------------------------------------------
Traceback (most recent call last):
  File "django/test/testcases.py", line 296, in __call__
    self._post_teardown()
  File "django/test/testcases.py", line 312, in _post_teardown
    self._fixture_teardown()
  File "django/test/testcases.py", line 604, in _fixture_teardown
    connections[db].check_constraints()
  File "django/db/backends/__init__.py", line 261, in check_constraints
    self.cursor().execute(s)
  File "django/db/backends/postgresql_psycopg2/base.py", line 44, in execute
    return self.cursor.execute(query, args)
IntegrityError: insert or update on table "multiple_database_book_authors" violates foreign key constraint "multiple_database_book_authors_person_id_fkey"
DETAIL:  Key (person_id)=(1) is not present in table "multiple_database_person".


----------------------------------------------------------------------
Ran 3861 tests in 2135.680s

FAILED (errors=6, skipped=50, expected failures=3)

[Ramiro Morales]

Better patch (use addError() instead of reraising exceptions in _fixture_teardown()) + workaround for one of the failing multiple_database tests it introduces

[Jim Dalton]

I've been doing some work on #3615 recently that I believe touches upon (and hopefully resolves?) this issue. That ticket refers to a problem in MySQL under InnoDB, where fixtures with forward references fail to load. The solution is to disable foreign key checks in MySQL, which then creates a situation where you need to immediately check constraints after data has been loaded. This is done using a special SQL SELECT statement for MySQL and sqlite. The most recent patch (uploaded last night) is designed to work with all backends, i.e. it adds a framework any backend can implement to check constraints immediately.

The primary difference between the patch there and this one is that the check_constraints call happens in my patch right after the data is loaded, rather than at the end of the test run. It's actually implemented in the loaddata command, so even more importantly it will check data being loaded via that command or initial data loaded via syncdb.

Anyhow, it should be as simple as overriding BaseDatabaseWrapper.check_constraints() in postgresql with an implementation that executes that SET CONSTRAINTS ALL IMMEDIATE statement. Note that check_constraints takes a list of tables as a kwarg and will be called with this, but postgresql doesn't need to use it. This method will be called after the data is loaded for every fixture. If for some reason that presents a problem, the alternative would be to implement get_key_columns() on postgresql's Introspection class. The default check_constraints() will check all of the key columns in a table for foreign key violations via ordinary SQL that should be platform independent. Doubt you guys would need to do this, but if for some reason it's an issue to run the set constraints immediate on every load that's an alternative to explore.

[Jim Dalton]

I think the correct solution here is to enable constraint checks at the *beginning* of the test transaction, for all backends. They can then be temporarily deferred as needed, e.g. during fixture loading or object deletion.

I have done this as part of an experimental branch I was working on for #3615, diff here: ​http://paste.pocoo.org/show/437952/ and not only does it work fine, it revealed a whole bunch of errors that have silently accumulated in the test suite. In fact, the reason the MySQL InnoDB test suite has been failing so long is because InnoDB's execution model during tests is closer to production, and MySQL was in fact revealing these real errors.

In working through these, I have already managed to eliminate them and get them down to just a handful, for all backends. All that's required now is working through the handful of remaining bugs and perhaps bolstering things with a few additional tests.

Before I proceed I'd like to know if there's any consensus on the idea I'm presenting here, or whether anybody has any objections. I posted this to the dev list and didn't get a ton of feedback (​https://groups.google.com/d/topic/django-developers/J_5SuL0MoQU/discussion). Does anybody watching this ticket have any thoughts? If you think I'm wrong I'd like to hear it, so I can either reconsider my view or answer any objections you might have.

Thanks for listening.

[Aymeric Augustin]

#16457 was a duplicate.

[Jim Dalton]

Attached is an initial patch which encapsulates a slightly modified approach to the one outlined in my previous comment and originally proposed on the dev list.

In short, Simon Riggs pointed out in that discussion (​https://groups.google.com/d/topic/django-developers/J_5SuL0MoQU/discussion) that my conception of how transactions work was too naive and my proposed solution (to set constraint checks immediately at the start) did not in fact accurately reflect how things work in production.

Fortunately, the solution wasn't too hard. Emulating production behavior better is a simple matter of adjusting the way we monkey patch the transaction functions in the set up of TestCase. Rather than simply stubbing those out, I added the constraint checking feature from #3615 in its place. So even though the main transaction functions don't *do* what they do in production (i.e. actually commit or rollback the function), they now perform the constraint check they normally would at that point and raise the appropriate IntegrityError when they do.).

The patch also includes a test which demonstrates that an IntegrityError should be raised during a TestCase test if bad data is entered.

The patch is not complete for several reasons:

• Haven't addressed any of the errors that this raises in the rest of the suite yet. I have some solutions for many but I'd rather focus on getting a solution to the immediate issue down.
• I have one concern about the implementation of check_constraints() on postgresql (and Oracle perhaps). If the autocommit feature is on, then does it make sense to revert to deferred? I think those statements are all irrelevant if that mode is on, so I think the answer is probably no.
• I wonder if it makes sense to make use of a savepoint at the start of tests (and release it at the end), and then rollback to that savepoint if for any reason a constraint check fails? That would allow tests to continue even if a (planned) IntegrityError was hit. This seems like a rather obscure feature, but there is a theoretical possibility of tests that previously worked (when we never checked constraints) which might fail now, since once you hit an IntegrityError the test is for all intents and purposes over.

Note that this patch includes the entire latest patch from #3615, because it depends on that patch in order to work. Here's a diff between this patch and that patch which highlights the focus of this patch (​http://paste.pocoo.org/show/440770/).

[Jim Dalton]

Here's an improved patch that gets us closer. Basically this one starts to fix the errors that come up once you enable production-like constraint checking. You can see the commits here: ​https://github.com/jsdalton/django/commits/better_constraint_checks_during_testing but in a nutshell:

• I reworked the check_constraints() feature (from #3615) a bit to ensure we *don't* check constraints if the constraint_checking_disabled context manager is in effect. This prevented a regression that occurred in fixture loading when the new check_constraints() function I added to django.test.testcases got executed.

• Flush the ContentType cache during tests, which cleared up some issues in egression_tests.multiple_database. Also fixed an assertNumQueries count after the change, since a new contenttype select was being executed.

• Add @ignore_num_queries decorator. Basically, because the check_constraints() implementation can have a varying number of SQL statements depending on the backend, I implemented a decorator that prevents assertNumQueries from counting queries on any statements executed during a decorated function. For now just added it to django.test.testcases.check_constraints().

Any comments on the above? This work starts to deviate a bit from this ticket, but it's required to get tests passing.

This gets us to 3 tests away from passing all tests in Postgresql, with correct constraint checking. The only 3 tests we're missing now in multiple_database. I know ramiro had done some work on some of these. I guess I'll sit down with them at some point, but I'm kind of a bit stuck... If anyone has thoughts I'd love to hear them.

Next up for me is hitting the remaining failiures on MySQL. Almost there...

[Jim Dalton]

Okay, did a bit more work on this and I think we're there -- for this ticket at least.

Basically after running tests last night and analyzing the results, there are *only* two problems that exist:

• A few multiple_database tests throw errors (the ones ramiro already identified). In this patch, I have simply disabled constraint checking for those test cases (in setUp and tearDown). This puts the tests back where they are now -- since right now there are no constraint checks being run and that's why they are passing. I propose we open a new, separate ticket for the issue because frankly I think it's out of the scope of this ticket to make a decision about how it should be handled.

• A few select_for_update errors in MySQL. Basically, two of the select_for_update tests are failing for MySQL. They are failing presently (without the patch) and I don't have an easy solution for them either. Again, I propose we open a new ticket to handle these. It appears they are legitimate bugs from what I can tell -- the code being exercised is not behaving as expected, and this needs to be addressed.

Removing the patch needs improvement flag, because this patch should be ready for checkin -- assuming no one has any concerns with the approach I took to solve the various little issues. Also, do we need documentation?

Note again that that *includes* the patch from #3615 because that has not been committed yet, and the code there is required for this. For convenience, here again is a diff against that other patch which highlights more clearly the actual work being done for this ticket: ​http://paste.pocoo.org/show/442244/

[Jim Dalton]

Just opened #16490, a report on the select_for_update error I mentioned above. Once those two failures are fixed, all backends will pass all tests.

[Graham King]

The v3 patch applies cleanly on latest SVN (16549) with patch -p1, and, to my very happy surprise, all the tests pass with MySQL / InnoDB! I'm going to wipe my Django checkout and try again with everything clean just to be 100% sure.

On MySQL 5.1.54 / InnoDB I get: Ran 4030 tests in 9 hours, OK (skipped=58, expected failures=3).

For comparison in SQLite I got: Ran 4021 tests in 5 minutes, OK (skipped=64, expected failures=3).

Unless I've messed something up with my environment, I looks like this combined patch marks a historic moment in Django's history. Thanks!

[Russell Keith-Magee]

See #3615 for some comments on the constraint-checking part of this patch.

[Jim Dalton]

@graham_king - That's great to hear :) You did one better than me; I still have two failing tests in MySQL (unrelated to this ticket), which I filed a ticket for. Hopefully it goes well again when you do a clean wipe.

Also, I replied to Russell's concern on #3615. The short answer is we should be okay as is. I still might try to track down Simon Riggs, who was quite helpful with some other Postgresql questions on the developers list conversation, to be sure.

[Graham King]

With a clean checkout from SVN, and this patch, same result: all the tests pass on MySQL InnoDB. Fantastic!

[Jim Dalton]

Now that #3615 is committed this patch stopped applying cleanly. I've cleaned it up and it should apply to trunk just fine.

Bad news for me is that I'm back to getting failures again in MySQL. I get 3 failures and 1 error when I run the whole suite, but only 1 failure when I run each failing test in isolation. Postgres has no failures or errors though.

None of the MySQL errors are related to this patch or ticket, so it should be ready for checkin.

[Jim Dalton]

Quick clarification to my comment above. The errors I came across in MySQL are not related to this patch or ticket, and they all have tickets opened for them.

Here's the test output:

$ ./runtests.py --settings=testproject.settings
Creating test database for alias 'default'...
Creating test database for alias 'other'...
.......................................................................................................................................................................................................................................................................................................................................................................................................s...............s..................................................................s...s...............................................................s.....................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................s........s.sss................................................................................................................................................................x...........................................................................................................................s.........................................................................................................................................x.........................................................................................................................s.......................................................................................................................................................................................................................................................................................................s...........................................................................s.............x...........................................................................................................................................................................................................................................................s.......................................................................................................................................................................................................................................................................................................................................sEF.ss.............s.......s..............................................................................................................................................................................................................................................................................................................................ssss........F......................................................................................................................................................................................................................................ss......................s.....................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................F................................................................................................................................................................ss..s....
======================================================================
ERROR: test_table_exists (modeltests.proxy_model_inheritance.tests.ProxyModelInheritanceTests)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/jsdalton/webs/testproject/src/django/tests/modeltests/proxy_model_inheritance/tests.py", line 25, in setUp
    call_command('syncdb', verbosity=0)
  File "/Users/jsdalton/webs/testproject/src/django/django/core/management/__init__.py", line 166, in call_command
    return klass.execute(*args, **defaults)
  File "/Users/jsdalton/webs/testproject/src/django/django/core/management/base.py", line 220, in execute
    output = self.handle(*args, **options)
  File "/Users/jsdalton/webs/testproject/src/django/django/core/management/base.py", line 351, in handle
    return self.handle_noargs(**options)
  File "/Users/jsdalton/webs/testproject/src/django/django/core/management/commands/syncdb.py", line 121, in handle_noargs
    custom_sql = custom_sql_for_model(model, self.style, connection)
  File "/Users/jsdalton/webs/testproject/src/django/django/core/management/sql.py", line 150, in custom_sql_for_model
    app_dir = os.path.normpath(os.path.join(os.path.dirname(models.get_app(model._meta.app_label).__file__), 'sql'))
  File "/Users/jsdalton/webs/testproject/src/django/django/db/models/loading.py", line 151, in get_app
    raise ImproperlyConfigured("App with label %s could not be found" % app_label)
ImproperlyConfigured: App with label unmanaged_models could not be found

======================================================================
FAIL: test_block (modeltests.select_for_update.tests.SelectForUpdateTests)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/jsdalton/webs/testproject/src/django/django/test/testcases.py", line 645, in skip_wrapper
    return test_func(*args, **kwargs)
  File "/Users/jsdalton/webs/testproject/src/django/django/test/testcases.py", line 645, in skip_wrapper
    return test_func(*args, **kwargs)
  File "/Users/jsdalton/webs/testproject/src/django/tests/modeltests/select_for_update/tests.py", line 209, in test_block
    self.assertEqual('Fred', p.name)
AssertionError: 'Fred' != u'Reinhardt'

======================================================================
FAIL: test_bug_8245 (regressiontests.bug8245.tests.Bug8245Test)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/jsdalton/webs/testproject/src/django/tests/regressiontests/bug8245/tests.py", line 18, in test_bug_8245
    'autodiscover should have raised a "Bad admin module" error.')
AssertionError: autodiscover should have raised a "Bad admin module" error.

======================================================================
FAIL: test_load_error (regressiontests.templates.tests.TemplateTagLoading)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/jsdalton/webs/testproject/src/django/tests/regressiontests/templates/tests.py", line 1636, in test_load_error
    self.assertTrue('ImportError' in e.args[0])
AssertionError: False is not True

----------------------------------------------------------------------
Ran 4029 tests in 14788.690s

FAILED (failures=3, errors=1, skipped=30, expected failures=3)
Destroying test database for alias 'default'...
Destroying test database for alias 'other'...

And here are the tickets which refer to each of these:

• #16592 is behind the test_table_exists error
• #13906 is behind the test_block failure
• #16593 is behind the test_bug_8245 and test_load_error failures. These are just occurring because the error in test_table_exists is causing some stuff not to get restored properly.

[Jim Dalton]

Patch wasn't applying cleanly so I fixed it up a bit.

With this patch, I'm currently one test error (#16592) away from all tests passing on MySQL.

[Jim Dalton]

Patch wasn't applying cleanly again.

[Jim Dalton]

Pasting a conversation between myself and jacobkm on IRC:

[4:50pm] jacobkm: jsdalton: what's connections._ignore_num_queries for?

[4:50pm] • jacobkm smells a hack

[4:50pm] jsdalton: jacobkm: lol. the problem is that MySQL uses a different number of queries to perform certain operations, basically it's inconsistent

[4:51pm] jsdalton: mostly this had to do with fixture loading, if i recall correctly

[4:51pm] dgovinda left the chat room. (Ping timeout: 276 seconds)

[4:51pm] dgovinda joined the chat room.

[4:51pm] jsdalton: to be honest, expecting a consistent query count across multiple backends with differing implementations sort of struck me as problematic

[4:52pm] jacobkm: jsdalton: so it's a workaround to make assertNumQueriese consistent across backends?

[4:52pm] jacobkm: jsdalton: I'd rather see something more like "if backend == mysql: assertNumQuerys(6)" sort of thing.

[4:52pm] jsdalton: hm

[4:52pm] jacobkm: This seems... evil and undocumented.

[4:52pm] jsdalton: hm, thinking

[4:53pm] jacobkm: IOW I'd rather work around it in the tests than have it all up in django.db.

[4:54pm] jacobkm: jsdalton: other than that I think this looks really good. We'll need to put something in the release notes -- this has potential backwards-compatibility implications -- but if you've not got time I can take care of that.

[4:55pm] jsdalton: jacobkm: cool, sorry just opening up my code here and reuploading my thought process to my brain

[4:55pm] jacobkm: jsdalton: no worries!

[4:56pm] jacobkm: jsdalton: I'm headed for lucnch but I'll check in when I get back so leave anything here for me if you like.

[4:56pm] jsdalton: sounds good i'll see if i can recall my train of thought by then

[5:00pm] jsdalton: jacobkm: for when you return, here's a very brief summary of my reasoning behind ignore_num_queries for you to consider

[5:02pm] jsdalton: first, the reason for is the implementation of check_constraints() varies across backends (i believe MySQL and SQLite are different from the others)

[5:03pm] jsdalton: so yeah one solution would be to set up different assertions for each backend in those tests where they were inconsistent. that was probably what i wanted to do but i felt like that was cheating, since that could conceivably be prone to breakage down the road as implementation details changed

[5:03pm] jsdalton: be that as it may, i'd probably be fine with that

[5:05pm] jsdalton: i implemented ignore_num_queries because it seemed like a better more flexible long term solution. i did add tests for it as well, btw.

[5:05pm] jsdalton: if you look closely at the implementation, it doesn't *really* touch production code

[5:06pm] jsdalton: it adds a single private attribute to the connections variable in django.db

[5:07pm] jsdalton: otherwise, the only other place it lives is in CursorDebugWrapper, i.e. the same place where num queries does its thing

[5:09pm] jsdalton: and for now it's only used in the test suite around a piece of test code that emulates constraint checking

[5:10pm] jsdalton: anyhow, not sure this is a very strong case for it. to me it felt like less of a hack because it's just a way of telling assert num queries to ignore query counts for certain blocks of code, but if my argument is not particularly convincing i can take your suggestion of specifying different query counts per backend

If we can get a decision on @ignore_num_queries as a decorator vs. specifying different expected query counts per backend, I'd be happy to implement either and proceed. Jacob also mentioned documentation in the release notes, which I can try to tackle as well.

[Aymeric Augustin]

On IRC, Alex and I confirmed that it would be better to avoid the _ignore_num_queries hack and just make the test database-dependent instead.

[Jim Dalton]

There ended up being a lot of work in this ticket that related more to fixing errors in the MySQL test suite than the original issue here, which is whether and how constraint checking should be implemented during testing.

A few things:

1. I created a new ticket (#17055) to deal with the MySQL related issues separately.

2. While I"m still convinced of the merits of what I ultimately attempted to do (implement constraint checks at transaction commit points to better emulate production behavior and ensure the integrity of data in the test databases at all times during testing), the implementation is not in a good place for constraint checking in MySQL and SQLite. When running the test suites today I realized that triggering constraint checks at every transaction commit point creates unacceptable slowness in the test suite for those backends. It runs fine on PostgreSQL however. I do not have a good idea for how to work around it at this point in time. Also, the constraint checking creates a variable number of SQL queries making the suggestions for alternatives to my ignore_num_queries approach not quite as tenable. More thinking needs to go into that.

In short, it's hard for me to see a very clear path forward for this ticket. Checking for constraints via SQL statements is slow and there's not a great way to work around it.

The one idea I have is we could do a "supports_fast_constraint_checks" feature and only do the check constraints on commit if a backend had that. The benefit here is we would catch data integrity errors before they arise in PostgreSQL but wouldn't cause horrible slowdowns in MySQL and SQLite. We could then also avoid the different query counts issues, or at least apply one of those suggestions.

Anyhow, if anyone has any comments on this let me know. I'm having a hard time motivating myself to push through a patch like that unless there's interest.

Cheers.

[Jon Dufresne]

I've created a PR to verify constraints for databases that support deferrable constraints only.

Is there any reason to do so for other databases that don't support deferrable constraints?

​https://github.com/django/django/pull/6083

[Tim Graham]

Left some comments for improvement on the pull request.

[Jon Dufresne]

I have addressed all comments in the PR. Please let me know if you have any additional feedback. Thanks.

[Tim Graham <timograham@…>]

In fcd08c1:

Fixed #11665 -- Made TestCase check deferrable constraints after each test.

[Carlton Gibson <carlton.gibson@…>]

In 1939dd4:

Fixed #29928 -- Enabled deferred constraint checks on SQLite 3.20+.

Refs #11665, #14204.

Thanks Michel Samia for the report.