Opened 3 years ago

Closed 8 months ago

#22431 closed Cleanup/optimization (duplicate)

TestCase swallows IntegrityError when creating object with invalid foreign key

Reported by: Chris Jerdonek Owned by: nobody
Component: Documentation Version: 1.6
Severity: Normal Keywords: TestCase, IntegrityError, TransactionTestCase, ForeignKey
Cc: Simon Charette Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

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".

Change History (8)

comment:1 Changed 3 years ago by Aymeric Augustin

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.

comment:2 Changed 3 years ago by Chris Jerdonek

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.

comment:3 Changed 3 years ago by Chris Jerdonek

It looks like PostgreSQL at least 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.

comment:4 Changed 3 years ago by Tim Graham

Component: Database layer (models, ORM)Documentation
Triage Stage: UnreviewedAccepted
Type: BugCleanup/optimization

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.

comment:5 Changed 2 years ago by Tim Graham

#23081 is a duplicate.

comment:6 Changed 2 years ago by Simon Charette

Cc: Simon Charette added

Following the discussion on #23081 I gave the SET CONSTRAINTS approach 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 MySQL InnoDB always checks foreign key constraints immediately is a good example of SQL standards deviation to take into account.

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.

Last edited 2 years ago by Simon Charette (previous) (diff)

comment:7 Changed 8 months ago by Tim Graham

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

comment:8 Changed 8 months ago by Simon Charette

Resolution: duplicate
Status: newclosed

It does.

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