Test for #123 #124
Test for #123 #124
Conversation
|
Great, thanks. I will review as soon as possible. |
|
I've amended the commit according to the previous discussion (split the test into 2, added missing I've tried to fix this by just calling get_prep_value in I think that to make django-hstore's behavior consistent with Django it should just work with Python values until save - when the values would be run through get_db_prep_value of field in schema. (I just tested, and with Django I can assign anything to a field and I'll get back the same object. It will only fail at clean/save). |
| def test_serializing_custom_type(self): | ||
| d = SchemaDataBag() | ||
| d.custom = CustomType('some value') | ||
| self.assertEqual(dict.get(d.data, 'custom'), '[some value]') |
There was a problem hiding this comment.
I got the previous test to pass but here I have some doubts.
Isn't dict.get(d.data, 'custom') the same as d.data.get('custom')?
If that is correct it should return the CustomType instance, not the serialized value "[some value]".
If I'm not wrong the serialized value must be used only before storing the value into the database, which would mean this is not the right way to test for the desired behaviour.
What do you think?
PS: in django-hstore's VirtualField implementation, calling d.data.get('custom') is equivalent to calling d.custom
There was a problem hiding this comment.
If the first test passes even after saving/reloading the instance, then this one is not really needed. dict.get(d.data, 'custom') is pretty much what d.data.get('custom') calls when calling super().get().
When writing this test I assumed the serialization would be done when assigning values, and only later did I realize that's not how native Django fields work.
|
I haven't been able to go ahead unfortunately, but I'll try to commit my unfinished work in a separate branch ASAP so hopefully more people can audit and send suggestions. |
|
Quick update: when I started working on this it required more time than I had available and I could not finish it. Disappointing.. |
nemesifier
force-pushed
the
master
branch
2 times, most recently
from
d1c11c8
to
b0c59b3
Compare
See #123