Opened 16 years ago
Closed 16 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 |
Description
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 `--> ./runtests.py --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/_doctest.py", 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/models.py", line unknown line number, in API_TESTS ---------------------------------------------------------------------- File "/home/malcolm/BleedingEdge/Django/django.git/tests/modeltests/fixtures/models.py", line ?, in modeltests.fixtures.models.__test__.API_TESTS Failed example: management.call_command('dumpdata', 'fixtures.Category', 'sites', format='json') Expected: [{"pk": 1, "model": "fixtures.category", "fields": {"description": "Latest news stories", "title": "News Stories"}}, {"pk": 1, "model": "sites.site", "fields": {"domain": "example.com", "name": "example.com"}}] Got: [{"pk": 1, "model": "sites.site", "fields": {"domain": "example.com", "name": "example.com"}}, {"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)
Change History (5)
by , 16 years ago
Attachment: | 10381-r9921.diff added |
---|
comment:1 by , 16 years ago
Triage Stage: | Unreviewed → Accepted |
---|
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 by , 16 years ago
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 by , 16 years ago
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 by , 16 years ago
Resolution: | → fixed |
---|---|
Status: | new → closed |
Patch to enforce predictable ordering in dumpdata output