Opened 7 years ago

Closed 7 years ago

Last modified 5 years ago

#13171 closed (fixed)

field_subclassing test failing, needs to use get_prep_lookup

Reported by: Gabriel Hurley Owned by: Gabriel Hurley
Component: Database layer (models, ORM) Version: master
Severity: Keywords: get_prep_lookup, get_db_prep_lookup
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

I ran the full unit test suite this morning (Windows, SQLite3), and found that the field_subclassing tests were failing because the expected error for an invalid lookup_type wasn't being raised.

It turns out that the test had not been updated to reflect get_prep_lookup() being factored out of get_db_prep_lookup().

This patch corrects that, and raises a TypeError instead of a FieldError, since the docs say "Your method must be prepared to handle all of these lookup_type values and should raise [...] a TypeError if your field does not support that type of lookup."

Patch is attached, and the test now passes.

Attachments (1)

field_subclassing_tests_patch.diff (1.3 KB) - added by Gabriel Hurley 7 years ago.
Patches broken test for field_subclassing

Download all attachments as: .zip

Change History (10)

Changed 7 years ago by Gabriel Hurley

Patches broken test for field_subclassing

comment:1 Changed 7 years ago by Karen Tracey

Needs documentation: unset
Needs tests: unset
Patch needs improvement: unset

I am confused. I cannot get this test to fail on current trunk (Windows or Ubuntu, both sqlite), and it is passing on the buildbots (http://buildbot.djangoproject.com/waterfall)...?

comment:2 Changed 7 years ago by Gabriel Hurley

Very strange... let me see if I can track down anything more specific.

comment:3 Changed 7 years ago by Gabriel Hurley

So, as noted on #13172, the test failure was due to a version-specific bug in Python 2.6.1 and upgrading to version 2.6.5 made the test start passing again. Sorry for that.

However, since the docs now recommend doing db-agnostic checks like validating lookup_type in get_prep_lookup() and that a TypeError should be raised, the patch is still technically valid. It's just not very important.

For docs reference, see here:

http://docs.djangoproject.com/en/dev/howto/custom-model-fields/#django.db.models.get_prep_lookup

We might as well eat our own dogfood and comply with the recommendation in the docs, right?

comment:4 in reply to:  2 Changed 7 years ago by jkocherhans

Replying to gabrielhurley:

Very strange... let me see if I can track down anything more specific.

Just for the record, this error shows up with the stock python on Snow Leopard which is indeed 2.6.1.

comment:5 Changed 7 years ago by Gabriel Hurley

Ha! Good to know. Glad I'm not crazy :-)

Sadly that's all the more reason to apply the patch... even if its a Python problem at its core there are still gonna be a number of people out there running 2.6.1 if it's the default version included in SL.

comment:6 Changed 7 years ago by Russell Keith-Magee

Triage Stage: UnreviewedReady for checkin

Looks like the test case is wrong anyway in this particular case. If fixing the test case also corrects an obscure test case, I say RFC.

comment:7 Changed 7 years ago by Russell Keith-Magee

Resolution: fixed
Status: newclosed

(In [12838]) Fixed #13171 -- Corrected the field_subclassing unit test. Thanks to Gabriel Hurley for the report and patch.

comment:8 Changed 7 years ago by Russell Keith-Magee

(In [12839]) [1.1.X] Fixed #13171 -- Corrected the field_subclassing unit test. Thanks to Gabriel Hurley for the report and patch.

Backport of r12838 from trunk.

comment:9 Changed 5 years ago by Jacob

milestone: 1.2

Milestone 1.2 deleted

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