Code

Opened 4 years ago

Closed 4 years ago

Last modified 3 years ago

#13171 closed (fixed)

field_subclassing test failing, needs to use get_prep_lookup

Reported by: gabrielhurley Owned by: gabrielhurley
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 gabrielhurley 4 years ago.
Patches broken test for field_subclassing

Download all attachments as: .zip

Change History (10)

Changed 4 years ago by gabrielhurley

Patches broken test for field_subclassing

comment:1 Changed 4 years ago by kmtracey

  • 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 follow-up: Changed 4 years ago by gabrielhurley

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

comment:3 Changed 4 years ago by gabrielhurley

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 4 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 4 years ago by gabrielhurley

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 4 years ago by russellm

  • Triage Stage changed from Unreviewed to Ready 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 4 years ago by russellm

  • Resolution set to fixed
  • Status changed from new to closed

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

comment:8 Changed 4 years ago by russellm

(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 3 years ago by jacob

  • milestone 1.2 deleted

Milestone 1.2 deleted

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.