Opened 13 years ago

Last modified 3 days ago

#20551 assigned Cleanup/optimization

Duplicate fixture labels

Reported by: Anssi Kääriäinen Owned by: Dmitri
Component: Core (Management commands) Version: dev
Severity: Normal Keywords:
Cc: Ülgen Sarıkavak Triage Stage: Accepted
Has patch: yes 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 (5)

comment:1 by Aymeric Augustin, 13 years ago

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 by Anssi Kääriäinen, 13 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 Ülgen Sarıkavak, 3 years ago

Cc: Ülgen Sarıkavak added

comment:4 by Dmitri, 3 days ago

The duplicate mentioned in the description (admin-views-users.xml) no longer exists on main, but the same problem remains in gis_tests.

distapp, geoapp, geogapp and relatedapp each shipped a fixture named "initial". loaddata loads every match across fixture directories, so each test case using fixtures = initial loaded all four files when the full gis_tests suite ran.

Resolving the label with all four apps installed:

before: 'initial'         -> 4 files (distapp, geoapp, geogapp, relatedapp)
after:  '<app>_initial'   -> 1 file each

Renamed them to <app>_initial. gis_tests on PostGIS: 678 tests, OK (skipped=49), same as main.

PR: ​https://github.com/django/django/pull/22081

Last edited 3 days ago by Dmitri (previous) (diff)

comment:5 by Dmitri, 3 days ago

Has patch: set
Owner: changed from nobody to Dmitri
Status: new → assigned
Note: See TracTickets for help on using tickets.
Back to Top