Opened 13 years ago

Closed 13 years ago

#10381 closed (fixed)

Test failure caused by r9921

Reported by: Malcolm Tredinnick Owned by: Russell Keith-Magee
Component: Uncategorized Version: 1.0
Severity: Keywords:
Cc: Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no


This looks minor and I don't want to interrupt my flow to dive deep now. I'm seeing on test failure (both in the full suite and in isolation):

.-(malcolm@lancre 15:10:35) ~/BleedingEdge/Django/django.git/tests
`--> ./ --settings=settings fixtures
FAIL: Doctest: modeltests.fixtures.models.__test__.API_TESTS
Traceback (most recent call last):
  File "/usr/lib/python2.5/site-packages/django/test/", line 2180, in runTest
    raise self.failureException(self.format_failure(new.getvalue()))
AssertionError: Failed doctest test for modeltests.fixtures.models.__test__.API_TESTS
  File "/home/malcolm/BleedingEdge/Django/django.git/tests/modeltests/fixtures/", line unknown line number, in API_TESTS

File "/home/malcolm/BleedingEdge/Django/django.git/tests/modeltests/fixtures/", line ?, in modeltests.fixtures.models.__test__.API_TESTS
Failed example:
    management.call_command('dumpdata', 'fixtures.Category', 'sites', format='json')
    [{"pk": 1, "model": "fixtures.category", "fields": {"description": "Latest news stories", "title": "News Stories"}}, {"pk": 1, "model": "", "fields": {"domain": "", "name": ""}}]
    [{"pk": 1, "model": "", "fields": {"domain": "", "name": ""}}, {"pk": 1, "model": "fixtures.category", "fields": {"description": "Latest news stories", "title": "News Stories"}}]

Ran 2 tests in 2.773s

FAILED (failures=1)

Git-bisect narrows it down to r9921 (ticket #5610), which seems quite believable.

Attachments (1)

10381-r9921.diff (1.0 KB) - added by Russell Keith-Magee 13 years ago.
Patch to enforce predictable ordering in dumpdata output

Download all attachments as: .zip

Change History (5)

Changed 13 years ago by Russell Keith-Magee

Attachment: 10381-r9921.diff added

Patch to enforce predictable ordering in dumpdata output

comment:1 Changed 13 years ago by Russell Keith-Magee

Triage Stage: UnreviewedAccepted

This appears to be a dictionary ordering problem. I can't reproduce it myself, but I suspect that the attached patch should fix the problem.

comment:2 Changed 13 years ago by Malcolm Tredinnick

Oh, sorry ... I was too blind to notice it was just a permuted order. :-(

Is there some reason the order needs to be preserved. What I mean is: SortedDict isn't free to use. Should we just compare the output in the test using == instead of doing a repr comparison to avoid having to insert something in the core code?

The patch does make the problem go away.

comment:3 Changed 13 years ago by Russell Keith-Magee

Granted, SortedDict isn't free, but the overhead isn't particularly expensive in this particular case. Worst case, you have a dictionary with as many keys as you have apps. It's also transient storage - it only exists for the duration of the dumpdata command call. This isn't an area where we have to be especially paranoid about efficiency.

There is also a minor advantage to maintaining the order (aside from making the tests work) - the apps will be serialized in the order that they are specified on the command line.

Just replacing the comparison with a == isn't a simple solution here - the output is a serialized JSON string, not just a list of dicts; also, the outer structure is a list, so the order is significant on comparison (not in a particularly meaningful way in this case, but it would still defeat any simple parsed JSON comparison)

One alternate approach would be to serialize apps in the same order the occur in the INSTALLED_APPS setting. This would yield a predictable iteration order at the cost of a marginally more CPU intensive approach to developing the iteration list. Personally, I'm comfortable with the cost of the SortedDict approach in this case, but I'm happy to use INSTALLED_APPS order too. Let me know which one you would prefer and I'll commit.

comment:4 Changed 13 years ago by Malcolm Tredinnick

Resolution: fixed
Status: newclosed

(In [9964]) Fixed #10381 -- Fixed some a machine-dependent test failure after r9921.

The patch is from Russell, but I'm applying it because I want my tests to pass
again (and he doesn't see the failure).

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