Opened 12 years ago
Closed 10 years 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 by , 12 years ago
comment:2 by , 12 years ago
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 by , 12 years ago
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 by , 12 years ago
| Component: | Database layer (models, ORM) → Documentation | 
|---|---|
| Triage Stage: | Unreviewed → Accepted | 
| Type: | Bug → 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:6 by , 11 years ago
| Cc: | 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:
- Issue a SET CONSTRAINTS ALL IMMEDIATEto mimicautocommitconstraint checking.
- For the duration of the test we should make sure __enter__ing the outer most (excluding the one used inTestCase._fixture_setup)Atomicinstance issues aSET CONSTRAINTS ALL DEFERREDstatement.
- ... and that __exit__ing it issues aSET 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:
- Issue a connection.check_constraintsevery timetransaction.commitis called.
- Issue a connection.check_constraintsbefore 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.
comment:7 by , 10 years ago
Does #11665 address this (constraints are checked at the end of each test)?
Databases defer foreign key checks until the end of the current transaction. Since
TestCaseruns 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.