Fixes zopefoundation/persistent#18 : Make the .raw() and repr() of the Python...

Fixes zopefoundation/persistent#18 : Make the .raw() and repr() of the Python TimeStamp match the C version in more cases. Also, make the behaviour of comparisons and hashing match as well; test all this.

CHANGES.rst

@@ -11,6 +11,11 @@
 
 - 100% branch coverage.
 
+- Make the C and Python `TimeStamp` objects behave more alike. The
+  Python version now produces the same `repr` and `.raw()` output as
+  the C version, and has the same hashcode. In addition, the Python
+  version is now supports ordering and equality like the C version.
+
 4.0.8 (2014-03-20)
 ------------------
 
@@ -18,7 +23,7 @@
 
 - In pure-Python ``Persistent``, avoid loading state in ``_p_activate``
   for non-ghost objects (which could corrupt their state).  (PR #9)
-    
+
 - In pure-Python, and don't throw ``POSKeyError`` if ``_p_activate`` is
   called on an object that has never been committed.  (PR #9)
 

persistent/tests/test_timestamp.py

@@ -158,10 +158,132 @@ class TimeStampTests(pyTimeStampTests):
         from persistent.timestamp import TimeStamp
         return TimeStamp
 
+class PyAndCComparisonTests(unittest.TestCase):
+    """
+    Compares C and Python implementations.
+    """
+
+    # A particular instant in time
+    now = 1229959248.3
+    # That instant in time split as the result of this expression:
+    #    (time.gmtime(now)[:5] + (now % 60,))
+    now_ts_args = (2008, 12, 22, 15, 20, 48.299999952316284)
+
+    def _make_many_instants(self):
+        # Given the above data, return many slight variations on
+        # it to test matching
+        yield self.now_ts_args
+        for i in range(2000):
+            yield self.now_ts_args[:-1] + (self.now_ts_args[-1] + (i % 60.0)/100.0 , )
+
+    def _makeC(self, *args, **kwargs):
+        from persistent.timestamp import TimeStamp
+        return TimeStamp(*args, **kwargs)
+
+    def _makePy(self, *args, **kwargs):
+        from persistent.timestamp import pyTimeStamp
+        return pyTimeStamp(*args, **kwargs)
+
+    def _make_C_and_Py(self, *args, **kwargs):
+        return self._makeC(*args, **kwargs), self._makePy(*args, **kwargs)
+
+    def test_reprs_equal(self):
+        for args in self._make_many_instants():
+            c, py = self._make_C_and_Py(*args)
+            self.assertEqual(repr(c), repr(py))
+
+    def test_raw_equal(self):
+        c, py = self._make_C_and_Py(*self.now_ts_args)
+        self.assertEqual(c.raw(), py.raw())
+
+    def test_equal(self):
+        c, py = self._make_C_and_Py(*self.now_ts_args)
+
+        self.assertEqual(c, py)
+
+    def test_hash_equal(self):
+        c, py = self._make_C_and_Py(*self.now_ts_args)
+        self.assertEqual(hash(c), hash(py))
+
+    def test_hash_equal_constants(self):
+        # The simple constants make it easier to diagnose
+        # a difference in algorithms
+        c, py = self._make_C_and_Py(b'\x00\x00\x00\x00\x00\x00\x00\x00')
+        self.assertEqual(hash(c), 8)
+        self.assertEqual(hash(c), hash(py))
+
+        c, py = self._make_C_and_Py(b'\x00\x00\x00\x00\x00\x00\x00\x01')
+        self.assertEqual(hash(c), 9)
+        self.assertEqual(hash(c), hash(py))
+
+        c, py = self._make_C_and_Py(b'\x00\x00\x00\x00\x00\x00\x01\x00')
+        self.assertEqual(hash(c), 1000011)
+        self.assertEqual(hash(c), hash(py))
+
+        c, py = self._make_C_and_Py(b'\x00\x00\x00\x00\x00\x01\x00\x00')
+        self.assertEqual(hash(c), 1000006000001)
+        self.assertEqual(hash(c), hash(py))
+
+        c, py = self._make_C_and_Py(b'\x00\x00\x00\x00\x01\x00\x00\x00')
+        self.assertEqual(hash(c), 1000009000027000019)
+        self.assertEqual(hash(c), hash(py))
+
+        # Overflow kicks in at this point
+        c, py = self._make_C_and_Py(b'\x00\x00\x00\x01\x00\x00\x00\x00')
+        self.assertEqual(hash(c), -4442925868394654887)
+        self.assertEqual(hash(c), hash(py))
+
+        c, py = self._make_C_and_Py(b'\x00\x00\x01\x00\x00\x00\x00\x00')
+        self.assertEqual(hash(c), -3993531167153147845)
+        self.assertEqual(hash(c), hash(py))
+
+        c, py = self._make_C_and_Py(b'\x01\x00\x00\x00\x00\x00\x00\x00')
+        self.assertEqual(hash(c), -3099646879006235965)
+        self.assertEqual(hash(c), hash(py))
+
+    def test_ordering(self):
+        small_c  = self._makeC(b'\x00\x00\x00\x00\x00\x00\x00\x01')
+        big_c    = self._makeC(b'\x01\x00\x00\x00\x00\x00\x00\x00')
+
+        small_py = self._makePy(b'\x00\x00\x00\x00\x00\x00\x00\x01')
+        big_py = self._makePy(b'\x01\x00\x00\x00\x00\x00\x00\x00')
+
+        self.assertTrue(small_py < big_py)
+        self.assertTrue(small_py <= big_py)
+
+        self.assertTrue(small_py < big_c)
+        self.assertTrue(small_py <= big_c)
+        self.assertTrue(small_py <= small_c)
+
+        self.assertTrue(small_c < big_c)
+        self.assertTrue(small_c <= big_c)
+
+        self.assertTrue(small_c <= big_py)
+        self.assertTrue(big_c > small_py)
+        self.assertTrue(big_c >= big_py)
+
+        self.assertFalse(big_c == small_py)
+        self.assertFalse(small_py == big_c)
+
+        self.assertTrue(big_c != small_py)
+        self.assertTrue(small_py != big_c)
+
 
 def test_suite():
-    return unittest.TestSuite((
+    suite = [
         unittest.makeSuite(Test__UTC),
         unittest.makeSuite(pyTimeStampTests),
         unittest.makeSuite(TimeStampTests),
-    ))
+    ]
+
+    try:
+        from persistent.timestamp import pyTimeStamp
+        from persistent.timestamp import TimeStamp
+    except ImportError:
+        pass
+    else:
+        if pyTimeStamp != TimeStamp:
+            # We have both implementations available
+            suite.append(PyAndCComparisonTests)
+
+    return unittest.TestSuite(suite)

persistent/timestamp.py

@@ -13,6 +13,7 @@
 ##############################################################################
 __all__ = ('TimeStamp',)
 
+from ctypes import c_int64
 import datetime
 import math
 import struct
@@ -51,7 +52,7 @@ _SCONV = 60.0 / (1<<16) / (1<<16)
 def _makeRaw(year, month, day, hour, minute, second):
     a = (((year - 1900) * 12 + month - 1) * 31 + day - 1)
     a = (a * 24 + hour) * 60 + minute
-    b = int(round(second / _SCONV))
+    b = int(second / _SCONV) # Don't round() this; the C version does simple truncation
     return struct.pack('>II', a, b)
 
 def _parseRaw(octets):
@@ -129,6 +130,64 @@ class pyTimeStamp(object):
         later = struct.pack('>II', a, b + 1)
         return self.__class__(later)
 
+    def __eq__(self, other):
+        try:
+            return self.raw() == other.raw()
+        except AttributeError: #pragma: no cover
+            return NotImplemented
+
+    def __ne__(self, other):
+        try:
+            return self.raw() != other.raw()
+        except AttributeError: #pragma: no cover
+            return NotImplemented
+
+    def __hash__(self):
+        # Match the C implementation
+        a = bytearray(self._raw)
+        x = a[0] << 7
+        for i in a:
+            x = (1000003 * x) ^ i
+        x ^= 8
+
+        # Make sure to overflow and wraparound just
+        # like the C code does.
+        x = c_int64(x).value
+        if x == -1: #pragma: no cover
+            # The C version has this condition, but it's not clear
+            # why; it's also not immediately obvious what bytestring
+            # would generate this---hence the no-cover
+            x = -2
+        return x
+
+    # Now the rest of the comparison operators
+    # Sigh. Python 2.6 doesn't have functools.total_ordering
+    # so we have to do it by hand
+    def __lt__(self, other):
+        try:
+            return self.raw() < other.raw()
+        except AttributeError: #pragma: no cover
+            return NotImplemented
+
+    def __gt__(self, other):
+        try:
+            return self.raw() > other.raw()
+        except AttributeError: #pragma: no cover
+            return NotImplemented
+
+    def __le__(self, other):
+        try:
+            return self.raw() <= other.raw()
+        except AttributeError: #pragma: no cover
+            return NotImplemented
+
+    def __ge__(self, other):
+        try:
+            return self.raw() >= other.raw()
+        except AttributeError: #pragma: no cover
+            return NotImplemented
+
+
 try:
     from persistent._timestamp import TimeStamp
 except ImportError: #pragma NO COVER