Bug in tests/modeltests/basic doctests?

1 view
Skip to first unread message

ogghead

unread,
Dec 15, 2006, 4:48:23 PM12/15/06
to Django developers
Here's a snippet from the doctests in tests/modeltests/basic/models.py:

#####
# You can combine queries with & and |.
>>> s1 = Article.objects.filter(id__exact=1)
>>> s2 = Article.objects.filter(id__exact=2)
>>> s1 | s2
[<Article: Area woman programs in Python>, <Article: Second article>]
#####

Is this a fair test? Using the Oracle backend in the
boulder-oracle-sprint branch, I get this:
[<Article: Second article>, <Article: Area woman programs in Python>]
which fails because the doctest expected them in the opposite order.

As far as I can tell, the Django documentation doesn't imply that any
ordering is guaranteed when ORing queries together. The SQL generated
against MySQL and Oracle is this:
SELECT `basic_article`.`id`, `basic_article`.`headline`,
`basic_article`.`pub_date`
FROM `basic_article`
WHERE ((`basic_article`.`id` = %s) OR (`basic_article`.`id` = %s))
(Oracle's is identical except for uppercasing the columns.) There's no
ORDER BY, so we get whatever natural ordering the database yields,
which isn't behavior to rely upon.

Should I fix the doctest(s) to be more accomodating, or am I missing
something about the expected behavior here?

Russell Keith-Magee

unread,
Dec 15, 2006, 6:02:15 PM12/15/06
to django-d...@googlegroups.com
On 12/16/06, ogghead <ogg...@gmail.com> wrote:
>
> Is this a fair test? Using the Oracle backend in the
> boulder-oracle-sprint branch, I get this:
> [<Article: Second article>, <Article: Area woman programs in Python>]
> which fails because the doctest expected them in the opposite order.
...

> Should I fix the doctest(s) to be more accomodating, or am I missing
> something about the expected behavior here?

Yes. If you look at some of the other tests (modeltests.lookup, for
example), there is a Meta block that defines ordering - this is
specifically to handle problems like the one you describe.

For some reason, the basic test doesn't have this. Patches are welcome.

Yours,
Russ Magee %-)

ogghead

unread,
Dec 15, 2006, 6:23:38 PM12/15/06
to Django developers
Adding this to the lone model in tests/modeltests/basic/models.py fixes
it:

class Meta:
ordering = ('id',)

But it seems a bit strange to specify explicitly an ordering for a
column that was implicitly created. Do others think this is ok? If
so, I can create a proper patch if needed.

Russell Keith-Magee

unread,
Dec 15, 2006, 6:51:55 PM12/15/06
to django-d...@googlegroups.com
On 12/16/06, ogghead <ogg...@gmail.com> wrote:
>

I would suggest following the lead of the other packages - e.g.,
ordering on pub_date. This will cause follow on problems with the
tests, so the patch will be more than just two lines.

Yours,
Russ Magee %-)

ogghead

unread,
Dec 18, 2006, 1:46:13 PM12/18/06
to Django developers
Thanks for the guidance, Russ. Adding ordering = ('pub_date',) fixes
the issue and also seems to match the intent of the doctests. I
submitted a bug and a patch at
http://code.djangoproject.com/ticket/3164 .

Russell Keith-Magee

unread,
Dec 18, 2006, 10:45:18 PM12/18/06
to django-d...@googlegroups.com

Fixed in [4228]. There was a little more required than just adding the
ordering - the existing test results made the assumption that when
there were three results with the same date, they would be returned in
the order in which they were added. This wasn't the case on my
machine, so I added an explicit second ordering term, and fixed the
result order to suit.

Yours,
Russ Magee %-)

Reply all
Reply to author
Forward
0 new messages