Opened 17 months ago

Last modified 14 months ago

#22431 new Cleanup/optimization

TestCase swallows IntegrityError when creating object with invalid foreign key

Reported by: cjerdonek Owned by: nobody
Component: Documentation Version: 1.6
Severity: Normal Keywords: TestCase, IntegrityError, TransactionTestCase, ForeignKey
Cc: charettes 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 (6)

comment:1 Changed 17 months ago by aaugustin

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

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 17 months ago 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.

comment:3 Changed 17 months ago by cjerdonek

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 16 months ago by timo

  • Component changed from Database layer (models, ORM) to Documentation
  • Triage Stage changed from Unreviewed to Accepted
  • Type changed from Bug to Cleanup/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 14 months ago by timo

#23081 is a duplicate.

comment:6 Changed 14 months ago by charettes

  • Cc charettes 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 14 months ago by charettes (previous) (diff)
Note: See TracTickets for help on using tickets.
Back to Top