Opened 5 years ago

Last modified 5 years ago

#31437 closed Bug

Invalid model structure used in tests. — at Initial Version

Reported by: Carlton Gibson Owned by: Carlton Gibson
Component: Database layer (models, ORM) Version: dev
Severity: Normal Keywords:
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Whilst researching the fix for #30427, #16176, it becomes apparent that a abstract multiple inheritance structure introduced for #24305 in 85ef98dc6ec565b1add417bd76808664e7318026 was invalid.

Checking out 85ef98dc6ec565b1add417bd76808664e7318026 and adding a model check call on the defined models, results in a failure:

(django) .../django/tests ((85ef98dc6e...))$ git diff
diff --git a/tests/model_inheritance/test_abstract_inheritance.py b/tests/model_inheritance/test_abstract_inheritance.py
index 71b8513a57..3964ff9858 100644
--- a/tests/model_inheritance/test_abstract_inheritance.py
+++ b/tests/model_inheritance/test_abstract_inheritance.py
@@ -60,6 +60,8 @@ class AbstractInheritanceTests(TestCase):
         class Derived(DescendantOne, DescendantTwo):
             pass
 
+
+        self.assertEqual(Derived.check(), [])
         self.assertEqual(DescendantOne._meta.get_field('name').max_length, 30)
         self.assertEqual(DescendantTwo._meta.get_field('name').max_length, 50)
         self.assertEqual(Derived._meta.get_field('name').max_length, 50)


Here's the failure:

FAIL: test_multiple_parents_mro (model_inheritance.test_abstract_inheritance.AbstractInheritanceTests)
         
First list contains 1 additional elements.
First extra element 0:
<Error: level=40, msg="The field 'name' clashes with the field 'name' from model 'model_inheritance.derived'.", hint=None, obj=<django.db.models.fields.CharField: name>, id='models.E006'>

- [<Error: level=40, msg="The field 'name' clashes with the field 'name' from model 'model_inheritance.derived'.", hint=None, obj=<django.db.models.fields.CharField: name>, id='models.E006'>]
+ []

This issue occurs because the doubly inherited fields are not present in
local_fields before iterating the bases in ModelBase.__new__(), the list of local_fields is not
updated whilst so iterating (which would not be a realistic option), and each
field adds itself to the model in contribute_to_class().

As a result overriding multiply inherited fields has never been supported. That
it's not is in line with the situation with MTI. (Thus no great hardship.)

The tests should be adjusted to communicate the actual situation.

What is more, the system check message is misleading:

"The field 'name' clashes with the field 'name' from model 'model_inheritance.derived'."

It's pointing to the child model, rather than the parent(s) that we hoped for.

Ideally the system check error message would be improved to point to the parent models in this case,
but it is not at all obvious how we can identify the relevant fields reliably. Yes, we can walk the bases
again, but all kinds of structures may be possible, such that the effort is probably not worth it.
(This has gone unnoticed for a number of years after all.)

Change History (0)

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