Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed #21523 - Added support for mock dates in Date/TimeField.to_python() #2027

Closed
wants to merge 1 commit into from

Conversation

Projects
None yet
5 participants
@hugorodgerbrown
Copy link

commented Dec 4, 2013

In cases where the value passed in to the DateField.to_python() method
is a real date, and datetime.date is being mocked out, the method would
raise an error attempting to call parse_date on the mock object (as the
parsing requires a string input). I’ve added in a duck-type check for
objects before the parse_date call - if the object has a ‘isoformat()’
method, then call this, and pass it into the parse_date function - which
should return an identical object.

@timgraham

View changes

tests/model_fields/tests.py Outdated
@@ -1,5 +1,6 @@
from __future__ import unicode_literals

import mock

This comment has been minimized.

Copy link
@timgraham

timgraham Aug 14, 2014

Member

None of our tests use mock currently so we'd have to add that as a dependency which I'm not sure is ideal. One idea would be to use unittest.mock if provides the same or similar functionality (available in Python 3.3+ and perhaps we could vendor it in django.utils for 3.2 and 2.7)

This comment has been minimized.

Copy link
@timgraham

timgraham Dec 24, 2014

Member

mock is now available via from django.test import mock if you want to update this patch.

Fixed #21523 - support for mock dates in DateField.to_python()
In cases where the value passed in to the DateField.to_python() method
is a real date, and datetime.date is being mocked out, the method would
raise an error attempting to call parse_date on the mock object (as the
parsing requires a string input). I’ve added in a duck-type check for
objects before the parse_date call - if the object has a ‘isoformat()’
method, then call this, and pass it into the parse_date function - which
should return an identical object.

@hugorodgerbrown hugorodgerbrown force-pushed the yunojuno:master branch to 9b2236f Dec 26, 2014

@hugorodgerbrown

This comment has been minimized.

Copy link
Author

commented Dec 26, 2014

Updated to use django.test.mock instead of external mock.

@timgraham

This comment has been minimized.

Copy link
Member

commented Dec 26, 2014

buildbot, test this please.

@SEJeff

This comment has been minimized.

Copy link

commented Feb 6, 2015

Any word on if this will get merged? I just ran into this exact same issue.

@timgraham

This comment has been minimized.

Copy link
Member

commented Feb 8, 2015

If someone (like yourself!) reviews the patch using the patch review checklist and marks the ticket "ready for checkin" that will move the issue forward.

@auvipy

This comment has been minimized.

Copy link
Contributor

commented Mar 7, 2015

@timgraham I have assign the ticket to myself. seems some release notes and doc is needed

@timgraham

This comment has been minimized.

Copy link
Member

commented Mar 7, 2015

@auvipy, not sure this really needs documentation but if you draft something I'll certainly take a look.

@@ -233,6 +233,36 @@ def test_datetimes_save_completely(self):
self.assertEqual(obj.dt, datetim)
self.assertEqual(obj.t, tim)

def test_datefield_to_python_issue_21523(self):
"""DateField.to_python should handle mock dates. (see #21523)"""

This comment has been minimized.

Copy link
@timgraham

timgraham Mar 30, 2015

Member
"""
DateField.to_python() should handle mock dates (#21523).
"""
# then this test will FAIL if value is a real date object, which
# is the case with issue #21523.
#
# NB use of assertTrue and isinstance rather than assertIsInstance

This comment has been minimized.

Copy link
@timgraham

timgraham Mar 30, 2015

Member

I don't understand your rationale -- assertIsInstance does the same check but throws a more helpful error message.

This comment has been minimized.

Copy link
@hugorodgerbrown

hugorodgerbrown Mar 30, 2015

Author

I understand, and I'm sure it's being over-cautious, but the intention of the patch isn't to assert that 'today is an instance of datetime.date', it's to assert that isinstance(today, datetime.date) is True. It's semantics, but it's an assumption like this (that assertIsInstance will never change its internal implementation) that caused the issue in the first place.

@@ -1200,6 +1200,9 @@ def to_python(self, value):
return value.date()
if isinstance(value, datetime.date):
return value
# duck-type check for objects that may be mocks - issue #21523

This comment has been minimized.

Copy link
@timgraham

timgraham Mar 30, 2015

Member
# duck type check for objects that may be mocks
value = datetime.datetime(value.year, value.month, value.day,
value.hour, value.minute, value.second, value.microsecond)
except AttributeError:
# looks like it is a date, and not a datetime.

This comment has been minimized.

Copy link
@timgraham

timgraham Mar 30, 2015

Member

no comma needed

@timgraham

This comment has been minimized.

Copy link
Member

commented Mar 30, 2015

Missing tests for the DateTimeField and TimeField changes.

@timgraham timgraham changed the title Fixed #21523 - support for mock dates in DateField.to_python() Fixed #21523 - Added support for mock dates in Date/TimeField.to_python() Mar 30, 2015

@timgraham

This comment has been minimized.

Copy link
Member

commented Apr 23, 2015

Closing due to inactivity. Please send a new PR if someone can update this per my comments.

@timgraham timgraham closed this Apr 23, 2015

@akki

This comment has been minimized.

Copy link
Contributor

commented Apr 16, 2016

Updated in #6468

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.