Opened 22 months ago

Last modified 22 months ago

#20551 new Cleanup/optimization

Duplicate fixture labels

Reported by: akaariai Owned by: nobody
Component: Core (Management commands) Version: master
Severity: Normal Keywords:
Cc: 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:

  1. 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.
  2. 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 (2)

comment:1 Changed 22 months ago by aaugustin

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

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.

comment:2 Changed 22 months ago by akaariai

  • Triage Stage changed from Unreviewed to 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.

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