Opened 11 years ago
Last modified 8 months ago
#20551 new Cleanup/optimization
Duplicate fixture labels
Reported by: | Anssi Kääriäinen | Owned by: | nobody |
---|---|---|---|
Component: | Core (Management commands) | Version: | dev |
Severity: | Normal | Keywords: | |
Cc: | Ülgen Sarıkavak | Triage Stage: | Accepted |
Has patch: | no | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
There are a couple of instances where we load non-necessary fixture files:
- There are a couple of initial_data fixtures (in fixtures[_regress] tests IIRC). Any initial_data fixture is loaded for every TransactionTestCase, so this causes a bit of repetitive non-necessary work. If we add app-dependency to tests, then this isn't an issue any more.
- There are at least one duplicate fixture name. admin-views-users.xml can be found from admin_views and admin_inlines. So, if admin_views loads "admin-views-users.xml" fixture, it ends up loading both of the files. Again app-dependency patch will solve this issue.
I wonder if the current way loaddata works could be improved a bit. The idea is to make fixture labels more explicit. The possible changes would be:
- write the filename in full, that is the filename must include the format (.xml for example) and if the file is compressed, then also compression (.bz2 for example). so, one must write admin-views-users.xml.bz2 instead of just admin-views-users.
- no duplicates allowed (except for initial_data). So, if there are multiple admin-views-users fixtures, then one must explicitly write from which app the fixture is: "admin_views/admin-views-users.xml".
There could also be some explicit "allow duplicates" markers: "*/initial_data.*" for all initial_data files in any app (the first *), in any format or compression (the latter *).
These changes aren't necessary to fix the immediate issues in Django's test suite. App dependencies patch (#20483) will solve these issues, or just plain renaming of the problematic fixture files will suffice.
Change History (3)
comment:1 by , 11 years ago
comment:2 by , 11 years ago
Triage Stage: | Unreviewed → Accepted |
---|
I think just renaming the duplicate fixtures is enough for now. Do these still get double-loaded? Even if not having admin-views-users fixture in admin_inlines tests is pointless.
comment:3 by , 8 months ago
Cc: | added |
---|
Given the changes in fixture loading I committed yesterday, app-dependency won't suffice... We need a way to clear the
fixture_dirs
cached property in loaddata.Surprisingly, the previous code allows for duplicates in different apps but not in the same app. It could be an accident. I preserved that behavior but I don't understand it.