[Django] #22431: TestCase swallows IntegrityError when creating object with invalid foreign key

66 views
Skip to first unread message

Django

unread,
Apr 13, 2014, 9:13:14 PM4/13/14
to django-...@googlegroups.com
#22431: TestCase swallows IntegrityError when creating object with invalid foreign
key
-------------------------------------+-------------------------------------
Reporter: cjerdonek | Owner: nobody
Type: Bug | Status: new
Component: Database layer | Version: 1.6
(models, ORM) | Keywords:
Severity: Normal | TestCase,IntegrityError,TransactionTestCase,ForeignKey
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Django's TestCase class seems to swallow `IntegrityErrors` when creating
and saving an object when a foreign key is manually specified that is
invalid.

When calling `save()` in this case, no exception is raised, and the object
returned by `save()` is given a valid `id`. However, the object is not
actually saved to the database. When switching from `TestCase` to
`TransactionTestCase`, the error is raised as expected when saving, and
the error looks something like the following:

{{{
IntegrityError: insert or update on table "app_table1" violates foreign
key constraint "app_table1_table2_id_fkey"
DETAIL: Key (table2_id)=(X) is not present in table "app_table2".
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/22431>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

Django

unread,
Apr 14, 2014, 2:17:03 AM4/14/14
to django-...@googlegroups.com
#22431: TestCase swallows IntegrityError when creating object with invalid foreign
key
-------------------------------------+-------------------------------------
Reporter: cjerdonek | Owner: nobody
Type: Bug | Status: new
Component: Database layer | Version: 1.6
(models, ORM) | Resolution:
Severity: Normal | Triage Stage:
Keywords: | Unreviewed
TestCase,IntegrityError,TransactionTestCase,ForeignKey| Needs documentation: 0
Has patch: 0 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Changes (by aaugustin):

* needs_better_patch: => 0
* needs_tests: => 0
* needs_docs: => 0


Comment:

Databases defer foreign key checks until the end of the current
transaction. Since `TestCase` runs its tests in a transaction that is
eventually rolled back, indeed, the foreign key check is skipped. I don't
think we can fix that limitation, but we can document it.

--
Ticket URL: <https://code.djangoproject.com/ticket/22431#comment:1>

Django

unread,
Apr 14, 2014, 3:08:34 AM4/14/14
to django-...@googlegroups.com
#22431: TestCase swallows IntegrityError when creating object with invalid foreign
key
-------------------------------------+-------------------------------------
Reporter: cjerdonek | Owner: nobody
Type: Bug | Status: new
Component: Database layer | Version: 1.6
(models, ORM) | Resolution:
Severity: Normal | Triage Stage:
Keywords: | Unreviewed
TestCase,IntegrityError,TransactionTestCase,ForeignKey| Needs documentation: 0
Has patch: 0 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------

Comment (by cjerdonek):

I wonder if there is a way to force or simulate such a check to be done
earlier in the process. For example, after such a call to `save()`, if I
were to access the property corresponding to such a field within that same
test, I would get a `DoesNotExist` exception. So the information is
there.

If there is no practical fix, it would be good if the documentation also
gave a work-around or two. I don't have enough experience with the issue
though to know what would be useful beyond avoiding `TestCase` altogether
in those situations.

--
Ticket URL: <https://code.djangoproject.com/ticket/22431#comment:2>

Django

unread,
Apr 14, 2014, 3:21:56 AM4/14/14
to django-...@googlegroups.com
#22431: TestCase swallows IntegrityError when creating object with invalid foreign
key
-------------------------------------+-------------------------------------
Reporter: cjerdonek | Owner: nobody
Type: Bug | Status: new
Component: Database layer | Version: 1.6
(models, ORM) | Resolution:
Severity: Normal | Triage Stage:
Keywords: | Unreviewed
TestCase,IntegrityError,TransactionTestCase,ForeignKey| Needs documentation: 0
Has patch: 0 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------

Comment (by cjerdonek):

It looks like PostgreSQL at least
[http://www.postgresql.org/docs/9.1/static/sql-set-constraints.html
provides the ability] to check constraints immediately (at least for
"UNIQUE, PRIMARY KEY, REFERENCES (foreign key), and EXCLUDE constraints").
The option can be enabled on a per-transaction basis by calling `SET
CONSTRAINTS` within the transaction. So maybe `TestCase` can be
configured to call this during test initialization after starting the
transaction.

--
Ticket URL: <https://code.djangoproject.com/ticket/22431#comment:3>

Django

unread,
Apr 29, 2014, 2:17:58 PM4/29/14
to django-...@googlegroups.com
#22431: TestCase swallows IntegrityError when creating object with invalid foreign
key
-------------------------------------+-------------------------------------
Reporter: cjerdonek | Owner: nobody
Type: | Status: new
Cleanup/optimization | Version: 1.6
Component: Documentation | Resolution:
Severity: Normal | Triage Stage: Accepted
Keywords: | Needs documentation: 0
TestCase,IntegrityError,TransactionTestCase,ForeignKey| Patch needs improvement: 0
Has patch: 0 | UI/UX: 0
Needs tests: 0 |
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Changes (by timo):

* component: Database layer (models, ORM) => Documentation
* type: Bug => Cleanup/optimization
* stage: Unreviewed => Accepted


Comment:

I would guess switching to immediate constraint checking would have a
performance penalty. It doesn't seem worth it for the majority of cases so
I'd be in favor of simply documenting this. If you want to try to work up
a patch otherwise, we can of course take a look.

--
Ticket URL: <https://code.djangoproject.com/ticket/22431#comment:4>

Django

unread,
Jul 22, 2014, 6:25:38 PM7/22/14
to django-...@googlegroups.com
#22431: TestCase swallows IntegrityError when creating object with invalid foreign
key
-------------------------------------+-------------------------------------
Reporter: cjerdonek | Owner: nobody
Type: | Status: new
Cleanup/optimization | Version: 1.6
Component: Documentation | Resolution:
Severity: Normal | Triage Stage: Accepted
Keywords: | Needs documentation: 0
TestCase,IntegrityError,TransactionTestCase,ForeignKey| Patch needs improvement: 0
Has patch: 0 | UI/UX: 0
Needs tests: 0 |
Easy pickings: 0 |
-------------------------------------+-------------------------------------

Comment (by timo):

#23081 is a duplicate.

--
Ticket URL: <https://code.djangoproject.com/ticket/22431#comment:5>

Django

unread,
Jul 23, 2014, 3:04:26 AM7/23/14
to django-...@googlegroups.com
#22431: TestCase swallows IntegrityError when creating object with invalid foreign
key
-------------------------------------+-------------------------------------
Reporter: cjerdonek | Owner: nobody
Type: | Status: new
Cleanup/optimization | Version: 1.6
Component: Documentation | Resolution:
Severity: Normal | Triage Stage: Accepted
Keywords: | Needs documentation: 0
TestCase,IntegrityError,TransactionTestCase,ForeignKey| Patch needs improvement: 0
Has patch: 0 | UI/UX: 0
Needs tests: 0 |
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Changes (by charettes):

* cc: charettes (added)


Comment:

Following the discussion on #23081 I gave the `SET CONSTRAINTS` approach
[https://github.com/django/django/pull/2945 a try].

I ended up calling `connection.check_constraints` before rolling back the
transaction which should catch most of the integrity errors (good news,
the Django test suite has none).

The main drawback of this approach is that the source of the query which
caused the integrity error is lost and one has to figure that out from the
traceback.

While we could add a more explicit message pointing out to the possible
source I think the correct solution to mimic the production behavior would
be the following:

Given the connection is in `autocommit` mode before creating its `Atomic`
instances in `TestCase._fixture_setup`:

1. Issue a `SET CONSTRAINTS ALL IMMEDIATE` to mimic `autocommit`
constraint checking.
2. For the duration of the test we should make sure `__enter__`ing the
outer most (excluding the one used in `TestCase._fixture_setup`) `Atomic`
instance issues a `SET CONSTRAINTS ALL DEFERRED` statement.
3. ... and that `__exit__`ing it issues a `SET CONSTRAINTS ALL IMMEDIATE`.

''Note that I completely overlooked the feasibility of this approach on
other backends. The fact that ''

Given the connection '''is not''' in `autocommit` mode before creating its
`Atomic` instances in `TestCase._fixture_setup`:

1. Issue a `connection.check_constraints` every time `transaction.commit`
is called.
2. Issue a `connection.check_constraints` before rolling back the test
transaction.

Given the complexity of the task I must side with @timo and suggest that
we start by documenting this issue.

--
Ticket URL: <https://code.djangoproject.com/ticket/22431#comment:6>

Django

unread,
Apr 22, 2016, 10:52:34 AM4/22/16
to django-...@googlegroups.com
#22431: TestCase swallows IntegrityError when creating object with invalid foreign
key
-------------------------------------+-------------------------------------
Reporter: cjerdonek | Owner: nobody
Type: | Status: new
Cleanup/optimization |
Component: Documentation | Version: 1.6
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
TestCase,IntegrityError,TransactionTestCase,ForeignKey|
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by timgraham):

Does #11665 address this (constraints are checked at the end of each
test)?

--
Ticket URL: <https://code.djangoproject.com/ticket/22431#comment:7>

Django

unread,
Apr 22, 2016, 11:12:29 AM4/22/16
to django-...@googlegroups.com
#22431: TestCase swallows IntegrityError when creating object with invalid foreign
key
-------------------------------------+-------------------------------------
Reporter: cjerdonek | Owner: nobody
Type: | Status: closed

Cleanup/optimization |
Component: Documentation | Version: 1.6
Severity: Normal | Resolution: duplicate

Keywords: | Triage Stage: Accepted
TestCase,IntegrityError,TransactionTestCase,ForeignKey|
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by charettes):

* status: new => closed
* resolution: => duplicate


Comment:

It does.

--
Ticket URL: <https://code.djangoproject.com/ticket/22431#comment:8>

Reply all
Reply to author
Forward
0 new messages