Merge pull request #80 from zopefoundation/coverage

Reach 100% coverage

.coveragerc

 [run]
 source = persistent
+omit =
+     persistent/_ring_build.py
 
 [report]
 exclude_lines =
-    # pragma: no cover
+    pragma: no cover
     class I[A-Z]\w+\((Interface|I[A-Z].*)\):
     raise NotImplementedError
     raise AssertionError
+    if __name__ == '__main__':

.travis.yml

@@ -56,14 +56,18 @@ before_install:
     - if [[ "$TRAVIS_OS_NAME" == "osx" ]]; then source terryfy/travis_tools.sh; fi
     - if [[ "$TRAVIS_OS_NAME" == "osx" ]]; then get_python_environment $TERRYFY_PYTHON venv; fi
 install:
-    - pip install -U pip setuptools cffi wheel
+    - pip install -U pip setuptools cffi wheel coverage coveralls
     - pip install -U -e .[test]
 script:
     - python --version
-    - zope-testrunner --test-path=. --auto-color --auto-progress
+    # coverage makes PyPy run about 3x slower, but the tests only take
+    # .4s to begin with (the whole process takes about 1.5), so that's
+    # still only 4.5s, which is maneagable.
+    - coverage run -m zope.testrunner --test-path=. --auto-color --auto-progress
 notifications:
     email: false
 after_success:
+    - coveralls
     - echo [distutils]                                  > ~/.pypirc
     - echo index-servers = pypi                        >> ~/.pypirc
     - echo [pypi]                                      >> ~/.pypirc

CHANGES.rst

@@ -4,6 +4,11 @@
 4.3.1 (unreleased)
 ------------------
 
+- Reach and maintain 100% test coverage.
+
+- Simplify ``__init__.py``, including removal of an attempted legacy
+  import of ``persistent.TimeStamp``.
+
 - Add support for Python 3.7 and drop support for Python 3.3.
 
 - Build the CFFI modules (used on PyPy or when PURE_PYTHON is set) `at
@@ -16,6 +21,12 @@
   See `issue 75
   <https://github.com/zopefoundation/persistent/issues/75>`_.
 
+- Fix deleting the ``_p_oid`` of a pure-Python persistent object when
+  it is in a cache.
+
+- Fix deleting special (``_p``) attributes of a pure-Python persistent
+  object that overrides ``__delattr__`` and correctly calls ``_p_delattr``.
+
 
 4.3.0 (2018-07-30)
 ------------------

README.rst

@@ -4,16 +4,19 @@
 .. image:: https://travis-ci.org/zopefoundation/persistent.svg?branch=master
         :target: https://travis-ci.org/zopefoundation/persistent
 
+.. image:: https://coveralls.io/repos/github/zopefoundation/persistent/badge.svg?branch=master
+        :target: https://coveralls.io/github/zopefoundation/persistent?branch=master
+
 .. image:: https://readthedocs.org/projects/persistent/badge/?version=latest
         :target: http://persistent.readthedocs.org/en/latest/
         :alt: Documentation Status
 
 .. image:: https://img.shields.io/pypi/v/persistent.svg
-        :target: https://pypi.python.org/pypi/persistent
-        :alt: PyPI
+        :target: https://pypi.org/project/persistent
+        :alt: Latest release
 
 .. image:: https://img.shields.io/pypi/pyversions/persistent.svg
-        :target: https://pypi.python.org/pypi/persistent
+        :target: https://pypi.org/project/persistent
         :alt: Python versions
 
 This package contains a generic persistence implementation for Python. It
@@ -22,7 +25,7 @@ a database such as the ZODB.
 
 Please see the Sphinx documentation (``docs/index.rst``) for further
 information, or view the documentation at Read The Docs, for either
-the latest (``http://persistent.readthedocs.io/en/latest/) or stable 
+the latest (``http://persistent.readthedocs.io/en/latest/``) or stable
 release (``http://persistent.readthedocs.io/en/stable/``).
 
 .. note::

docs/api/attributes.rst

@@ -309,3 +309,21 @@ object as changed if the name starts with ``tmp_``:
    Traceback (most recent call last):
    ...
    AttributeError: tmp_z
+
+If we attempt to delete ``_p_oid``, we find that we can't, and the
+object is also not activated or changed:
+
+.. doctest::
+
+   >>> del o._p_oid
+   Traceback (most recent call last):
+   ...
+   ValueError: can't delete _p_oid of cached object
+   >>> o._p_changed
+   False
+
+We are allowed to delete ``_p_changed``, which sets it to ``None``:
+
+   >>> del o._p_changed
+   >>> o._p_changed is None
+   True

docs/api/cache.rst

@@ -47,13 +47,13 @@ Caches have a :meth:`new_ghost` method that:
    >>> jar = ResettingJar()
    >>> cache = persistent.PickleCache(jar, 10, 100)
    >>> ob = C.__new__(C)
-   >>> cache.new_ghost('1', ob)
+   >>> cache.new_ghost(b'1', ob)
 
    >>> ob._p_changed
    >>> ob._p_jar is jar
    True
-   >>> ob._p_oid
-   '1'
+   >>> ob._p_oid == b'1'
+   True
 
    >>> cache.cache_non_ghost_count
    0

docs/api/pickling.rst

@@ -18,8 +18,8 @@ machinery happy:
    >>> f, (c,), state = x.__reduce__()
    >>> f.__name__
    '__newobj__'
-   >>> f.__module__
-   'copy_reg'
+   >>> f.__module__.replace('_', '') # Normalize Python2/3
+   'copyreg'
    >>> c.__name__
    'Simple'
 
@@ -57,8 +57,8 @@ by overriding :meth:`__getnewargs__`, :meth:`__getstate__` and
    >>> (f, (c, ax, ay), a) = x.__reduce__()
    >>> f.__name__
    '__newobj__'
-   >>> f.__module__
-   'copy_reg'
+   >>> f.__module__.replace('_', '') # Normalize Python2/3
+   'copyreg'
    >>> c.__name__
    'Custom'
    >>> ax, ay, a
@@ -79,7 +79,6 @@ ignores any slots which map onto the "persistent" namespace (prefixed with
 
 .. doctest::
 
-   >>> import copy_reg
    >>> from persistent.tests.cucumbers import SubSlotted
    >>> x = SubSlotted('x', 'y', 'z')
 

persistent/__init__.py

@@ -15,55 +15,49 @@
 
 Fall back to pure Python implementations.
 """
-import os
 
-PURE_PYTHON = os.environ.get('PURE_PYTHON')
-if not PURE_PYTHON:
-    try:
-        from persistent.cPersistence import Persistent
-        from persistent.cPersistence import GHOST
-        from persistent.cPersistence import UPTODATE
-        from persistent.cPersistence import CHANGED
-        from persistent.cPersistence import STICKY
-        from persistent.cPersistence import simple_new
-    except ImportError: #pragma NO COVER
-        from persistent.persistence import Persistent
-        from persistent.persistence import GHOST
-        from persistent.persistence import UPTODATE
-        from persistent.persistence import CHANGED
-        from persistent.persistence import STICKY
-    else:
-        from persistent._compat import copy_reg
-        copy_reg.constructor(simple_new)
-        # Make an interface declaration for Persistent, if zope.interface
-        # is available.  Note that the Python version already does this.
-        try:
+import sys
+
+__all__ = [
+    'IPersistent',
+    'Persistent',
+    'GHOST',
+    'UPTODATE',
+    'CHANGED',
+    'STICKY',
+    'PickleCache',
+    'TimeStamp',
+]
+from persistent._compat import PURE_PYTHON
+from persistent.interfaces import IPersistent
+
+import persistent.timestamp as TimeStamp
+
+from persistent import persistence as pyPersistence
+from persistent import picklecache as pyPickleCache
+
+try:
+    # Be careful not to shadow the modules
+    from persistent import cPersistence as _cPersistence
+    from persistent import cPickleCache as _cPickleCache
+except ImportError: # pragma: no cover
+    _cPersistence = None
+    _cPickleCache = None
+else:
+    # Make an interface declaration for Persistent
+    # Note that the Python version already does this.
     from zope.interface import classImplements
-        except ImportError: #pragma NO COVER
-            pass
-        else:
-            from persistent.interfaces import IPersistent
-            classImplements(Persistent, IPersistent)
+    classImplements(_cPersistence.Persistent, IPersistent)
+
+
+_persistence = pyPersistence if PURE_PYTHON or _cPersistence is None else _cPersistence
+_picklecache = pyPickleCache if PURE_PYTHON or _cPickleCache is None else _cPickleCache
 
-    try:
-        from persistent.cPickleCache import PickleCache
-    except ImportError: #pragma NO COVER
-        from persistent.picklecache import PickleCache
+Persistent = _persistence.Persistent
+GHOST = _persistence.GHOST
+UPTODATE = _persistence.UPTODATE
+CHANGED = _persistence.CHANGED
+STICKY = _persistence.STICKY
+PickleCache = _picklecache.PickleCache
 
-    try:
-        import persistent.TimeStamp
-    except ImportError: #pragma NO COVER
-        import persistent.timestamp as TimeStamp
-        import sys
-        sys.modules['persistent.TimeStamp'
-                   ] = sys.modules['persistent.timestamp']
-else: #pragma NO COVER
-    from persistent.persistence import Persistent
-    from persistent.persistence import GHOST
-    from persistent.persistence import UPTODATE
-    from persistent.persistence import CHANGED
-    from persistent.persistence import STICKY
-    from persistent.picklecache import PickleCache
-    import persistent.timestamp as TimeStamp
-    import sys
-    sys.modules['persistent.TimeStamp'] = sys.modules['persistent.timestamp']
+sys.modules['persistent.TimeStamp'] = sys.modules['persistent.timestamp']

persistent/_compat.py

@@ -13,8 +13,11 @@
 ##############################################################################
 
 import sys
+import os
 
-if sys.version_info[0] > 2: #pragma NO COVER
+PURE_PYTHON = os.environ.get('PURE_PYTHON')
+
+if sys.version_info[0] > 2: # pragma: no cover
     import copyreg as copy_reg
     from collections import UserDict as IterableUserDict
     from collections import UserList
@@ -36,7 +39,7 @@ if sys.version_info[0] > 2: #pragma NO COVER
     PYTHON3 = True
     PYTHON2 = False
 
-else: #pragma NO COVER
+else: # pragma: no cover
     import copy_reg
     from UserDict import IterableUserDict
     from UserList import UserList

persistent/interfaces.py

@@ -23,7 +23,7 @@ try:
     from persistent.cPersistence import UPTODATE
     from persistent.cPersistence import CHANGED
     from persistent.cPersistence import STICKY
-except ImportError: #pragma NO COVER
+except ImportError: # pragma: no cover
     GHOST = -1
     UPTODATE = 0
     CHANGED = 1

persistent/list.py

@@ -28,9 +28,6 @@ class PersistentList(UserList, persistent.Persistent):
     """
     __super_setitem = UserList.__setitem__
     __super_delitem = UserList.__delitem__
-    if PYTHON2:  # pragma: no cover
-        __super_setslice = UserList.__setslice__
-        __super_delslice = UserList.__delslice__
     __super_iadd = UserList.__iadd__
     __super_imul = UserList.__imul__
     __super_append = UserList.append
@@ -49,6 +46,10 @@ class PersistentList(UserList, persistent.Persistent):
         self.__super_delitem(i)
         self._p_changed = 1
 
+    if PYTHON2:  # pragma: no cover
+        __super_setslice = UserList.__setslice__
+        __super_delslice = UserList.__delslice__
+
         def __setslice__(self, i, j, other):
             self.__super_setslice(i, j, other)
             self._p_changed = 1
@@ -95,9 +96,3 @@ class PersistentList(UserList, persistent.Persistent):
     def extend(self, other):
         self.__super_extend(other)
         self._p_changed = 1
-
-    # This works around a bug in Python 2.1.x (up to 2.1.2 at least) where the
-    # __cmp__ bogusly raises a RuntimeError, and because this is an extension
-    # class, none of the rich comparison stuff works anyway.
-    def __cmp__(self, other):
-        return cmp(self.data, self._UserList__cast(other))

persistent/persistence.py

@@ -38,6 +38,7 @@ _STICKY = 0x0002
 
 _OGA = object.__getattribute__
 _OSA = object.__setattr__
+_ODA = object.__delattr__
 
 # These names can be used from a ghost without causing it to be
 # activated. These are standardized with the C implementation
@@ -301,7 +302,7 @@ class Persistent(object):
                 if (_OGA(self, '_Persistent__jar') is not None and
                     _OGA(self, '_Persistent__oid') is not None):
                     _OGA(self, '_p_register')()
-        object.__delattr__(self, name)
+        _ODA(self, name)
 
     def _slotnames(self, _v_exclude=True):
         slotnames = copy_reg._slotnames(type(self))
@@ -467,7 +468,7 @@ class Persistent(object):
         """ See IPersistent.
         """
         if name.startswith('_p_'):
-            setattr(self, name, value)
+            _OSA(self, name, value)
             return True
         self._p_activate()
         self._p_accessed()
@@ -477,7 +478,12 @@ class Persistent(object):
         """ See IPersistent.
         """
         if name.startswith('_p_'):
-            delattr(self, name)
+            if name == '_p_oid' and self._p_is_in_cache(_OGA(self, '_Persistent__jar')):
+                # The C implementation forbids deleting the oid
+                # if we're already in a cache. Match its error message
+                raise ValueError('can not change _p_jar of cached object')
+
+            _ODA(self, name)
             return True
         self._p_activate()
         self._p_accessed()

persistent/tests/attrhooks.py

@@ -35,8 +35,7 @@ class OverridesGetattr(Persistent):
         """
         # Don't pretend we have any special attributes.
         if name.startswith("__") and name.endswrith("__"):
-            raise AttributeError(name)
-        else:
+            raise AttributeError(name) # pragma: no cover
         return name.upper(), self._p_changed
 
 

persistent/tests/cucumbers.py

@@ -14,29 +14,22 @@
 # Example objects for pickling.
 
 from persistent import Persistent
-from persistent._compat import PYTHON2
-
 
 def print_dict(d):
-    d = d.items()
-    d.sort()
+    d = sorted(d.items())
     print('{%s}' % (', '.join(
         [('%r: %r' % (k, v)) for (k, v) in d]
         )))
 
 def cmpattrs(self, other, *attrs):
+    result = 0
     for attr in attrs:
         if attr[:3] in ('_v_', '_p_'):
-            continue
-        lhs, rhs = getattr(self, attr, None), getattr(other, attr, None)
-        if PYTHON2:
-           c = cmp(lhs, rhs)
-           if c:
-               return c
-        else:
-            if lhs != rhs:
-                return 1
-    return 0
+            raise AssertionError("_v_ and _p_ attrs not allowed")
+        lhs = getattr(self, attr, None)
+        rhs = getattr(other, attr, None)
+        result += lhs != rhs
+    return result
 
 class Simple(Persistent):
     def __init__(self, name, **kw):
@@ -84,7 +77,7 @@ class Slotted(Persistent):
 
     @property
     def _attrs(self):
-        return list(self.__dict__.keys())
+        raise NotImplementedError()
 
     def __eq__(self, other):
         return cmpattrs(self, other, '__class__', *self._attrs) == 0

persistent/tests/test_docs.py

+# -*- coding: utf-8 -*-
+##############################################################################
+#
+# Copyright (c) 2001, 2002 Zope Foundation and Contributors.
+# All Rights Reserved.
+#
+# This software is subject to the provisions of the Zope Public License,
+# Version 2.1 (ZPL).  A copy of the ZPL should accompany this distribution.
+# THIS SOFTWARE IS PROVIDED "AS IS" AND ANY AND ALL EXPRESS OR IMPLIED
+# WARRANTIES ARE DISCLAIMED, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
+# WARRANTIES OF TITLE, MERCHANTABILITY, AGAINST INFRINGEMENT, AND FITNESS
+# FOR A PARTICULAR PURPOSE.
+#
+##############################################################################
+"""
+Tests for the documentation.
+"""
+from __future__ import absolute_import
+from __future__ import division
+from __future__ import print_function
+
+
+# disable: accessing protected members, too many methods
+# pylint: disable=W0212,R0904
+
+import os.path
+import unittest
+import doctest
+
+import manuel.capture
+import manuel.codeblock
+import manuel.doctest
+import manuel.ignore
+import manuel.testing
+
+def test_suite():
+    here = os.path.dirname(__file__)
+    while not os.path.exists(os.path.join(here, 'setup.py')):
+        here = os.path.join(here, '..')
+
+    here = os.path.abspath(here)
+    docs = os.path.join(here, 'docs', 'api')
+
+    files_to_test = (
+        'cache.rst',
+        'attributes.rst',
+        'pickling.rst',
+    )
+    paths = [os.path.join(docs, f) for f in files_to_test]
+
+    m = manuel.ignore.Manuel()
+    m += manuel.doctest.Manuel(optionflags=(
+        doctest.NORMALIZE_WHITESPACE
+        | doctest.ELLIPSIS
+        | doctest.IGNORE_EXCEPTION_DETAIL
+    ))
+    m += manuel.codeblock.Manuel()
+    m += manuel.capture.Manuel()
+
+    suite = unittest.TestSuite()
+    suite.addTest(
+        manuel.testing.TestSuite(
+            m,
+            *paths
+        )
+    )
+
+    return suite

persistent/tests/test_list.py

@@ -20,6 +20,8 @@ l0 = []
 l1 = [0]
 l2 = [0, 1]
 
+# pylint:disable=protected-access
+
 class OtherList:
     def __init__(self, initlist):
         self.__data = initlist
@@ -34,6 +36,17 @@ class TestPList(unittest.TestCase):
         from persistent.list import PersistentList
         return PersistentList
 
+    def _makeJar(self):
+        class Jar(object):
+            def register(self, obj):
+                "no-op"
+        return Jar()
+
+    def _makeOne(self, *args):
+        inst = self._getTargetClass()(*args)
+        inst._p_jar = self._makeJar()
+        return inst
+
     def test_volatile_attributes_not_persisted(self):
         # http://www.zope.org/Collectors/Zope/2052
         m = self._getTargetClass()()
@@ -57,9 +70,9 @@ class TestPList(unittest.TestCase):
         uu1 = pl(u1)
         uu2 = pl(u2)
 
-        v = pl(tuple(u))
-        v0 = pl(OtherList(u0))
-        vv = pl("this is also a sequence")
+        pl(tuple(u))
+        pl(OtherList(u0))
+        pl("this is also a sequence")
 
         # Test __repr__
         eq = self.assertEqual
@@ -68,46 +81,48 @@ class TestPList(unittest.TestCase):
         eq(repr(u1), repr(l1), "repr(u1) == repr(l1)")
 
         # Test __cmp__ and __len__
+        try:
+            cmp
+        except NameError:
+            def cmp(a, b):
+                if a == b:
+                    return 0
+                if a < b:
+                    return -1
+                return 1
 
-        if PYTHON2:
         def mycmp(a, b):
             r = cmp(a, b)
-                if r < 0: return -1
-                if r > 0: return 1
+            if r < 0:
+                return -1
+            if r > 0:
+                return 1
             return r
 
-            all = [l0, l1, l2, u, u0, u1, u2, uu, uu0, uu1, uu2]
-            for a in all:
-                for b in all:
+        to_test = [l0, l1, l2, u, u0, u1, u2, uu, uu0, uu1, uu2]
+        for a in to_test:
+            for b in to_test:
                 eq(mycmp(a, b), mycmp(len(a), len(b)),
                    "mycmp(a, b) == mycmp(len(a), len(b))")
 
         # Test __getitem__
 
-        for i in range(len(u2)):
-            eq(u2[i], i, "u2[i] == i")
+        for i, val in enumerate(u2):
+            eq(val, i, "u2[i] == i")
 
         # Test __setitem__
 
         uu2[0] = 0
         uu2[1] = 100
-        try:
+        with self.assertRaises(IndexError):
             uu2[2] = 200
-        except IndexError:
-            pass
-        else:
-            raise TestFailed("uu2[2] shouldn't be assignable")
 
         # Test __delitem__
 
         del uu2[1]
         del uu2[0]
-        try:
+        with self.assertRaises(IndexError):
             del uu2[0]
-        except IndexError:
-            pass
-        else:
-            raise TestFailed("uu2[0] shouldn't be deletable")
 
         # Test __getslice__
 
@@ -198,12 +213,8 @@ class TestPList(unittest.TestCase):
 
         eq(u2.index(0), 0, "u2.index(0) == 0")
         eq(u2.index(1), 1, "u2.index(1) == 1")
-        try:
+        with self.assertRaises(ValueError):
             u2.index(2)
-        except ValueError:
-            pass
-        else:
-            raise TestFailed("expected ValueError")
 
         # Test reverse
 
@@ -220,23 +231,19 @@ class TestPList(unittest.TestCase):
         eq(u, u2, "u == u2")
 
         # Test keyword arguments to sort
-        if PYTHON2:
-            u.sort(cmp=lambda x,y: cmp(y, x))
+        if PYTHON2: # pragma: no cover
+            u.sort(cmp=lambda x, y: cmp(y, x))
             eq(u, [1, 0], "u == [1, 0]")
 
-        u.sort(key=lambda x:-x)
+        u.sort(key=lambda x: -x)
         eq(u, [1, 0], "u == [1, 0]")
 
         u.sort(reverse=True)
         eq(u, [1, 0], "u == [1, 0]")
 
         # Passing any other keyword arguments results in a TypeError
-        try:
+        with self.assertRaises(TypeError):
             u.sort(blah=True)
-        except TypeError:
-            pass
-        else:
-            raise TestFailed("expected TypeError")
 
         # Test extend
 
@@ -254,10 +261,72 @@ class TestPList(unittest.TestCase):
         u *= 3
         eq(u, u1 + u1 + u1, "u == u1 + u1 + u1")
 
+    def test_setslice(self):
+        inst = self._makeOne()
+        self.assertFalse(inst._p_changed)
+        inst[:] = [1, 2, 3]
+        self.assertEqual(inst, [1, 2, 3])
+        self.assertTrue(inst._p_changed)
+
+    def test_delslice(self):
+        inst = self._makeOne([1, 2, 3])
+        self.assertFalse(inst._p_changed)
+        self.assertEqual(inst, [1, 2, 3])
+        del inst[:]
+        self.assertTrue(inst._p_changed)
+
+    def test_iadd(self):
+        inst = self._makeOne()
+        self.assertFalse(inst._p_changed)
+        inst += [1, 2, 3]
+        self.assertEqual(inst, [1, 2, 3])
+        self.assertTrue(inst._p_changed)
+
+    def test_extend(self):
+        inst = self._makeOne()
+        self.assertFalse(inst._p_changed)
+        inst.extend([1, 2, 3])
+        self.assertEqual(inst, [1, 2, 3])
+        self.assertTrue(inst._p_changed)
+
+    def test_imul(self):
+        inst = self._makeOne([1])
+        self.assertFalse(inst._p_changed)
+        inst *= 2
+        self.assertEqual(inst, [1, 1])
+        self.assertTrue(inst._p_changed)
+
+    def test_append(self):
+        inst = self._makeOne()
+        self.assertFalse(inst._p_changed)
+        inst.append(1)
+        self.assertEqual(inst, [1])
+        self.assertTrue(inst._p_changed)
+
+    def test_insert(self):
+        inst = self._makeOne()
+        self.assertFalse(inst._p_changed)
+        inst.insert(0, 1)
+        self.assertEqual(inst, [1])
+        self.assertTrue(inst._p_changed)
+
+    def test_remove(self):
+        inst = self._makeOne([1])
+        self.assertFalse(inst._p_changed)
+        inst.remove(1)
+        self.assertEqual(inst, [])
+        self.assertTrue(inst._p_changed)
+
+    def test_reverse(self):
+        inst = self._makeOne([2, 1])
+        self.assertFalse(inst._p_changed)
+        inst.reverse()
+        self.assertEqual(inst, [1, 2])
+        self.assertTrue(inst._p_changed)
+
 
 def test_suite():
-    return unittest.makeSuite(TestPList)
+    return unittest.defaultTestLoader.loadTestsFromName(__name__)
 
-if __name__ == "__main__":
-    loader = unittest.TestLoader()
-    unittest.main(testLoader=loader)
+if __name__ == '__main__':
+    unittest.main()

persistent/tests/test_mapping.py

@@ -13,7 +13,7 @@
 ##############################################################################
 import unittest
 
-
+# pylint:disable=blacklisted-name, protected-access
 
 class Test_default(unittest.TestCase):
 
@@ -25,15 +25,14 @@ class Test_default(unittest.TestCase):
         return self._getTargetClass()(func)
 
     def test___get___from_class(self):
-        _called_with = []
         def _test(inst):
-            _called_with.append(inst)
-            return '_test'
+            raise AssertionError("Must not be caled")
+
         descr = self._makeOne(_test)
         class Foo(object):
             testing = descr
-        self.assertTrue(Foo.testing is descr)
-        self.assertEqual(_called_with, [])
+        self.assertIs(Foo.testing, descr)
+
 
     def test___get___from_instance(self):
         _called_with = []
@@ -86,9 +85,9 @@ class PersistentMappingTests(unittest.TestCase):
             def __init__(self, initmapping):
                 self.__data = initmapping
             def items(self):
-                return self.__data.items()
-        v0 = self._makeOne(OtherMapping(u0))
-        vv = self._makeOne([(0, 0), (1, 1)])
+                raise AssertionError("Not called")
+        self._makeOne(OtherMapping(u0))
+        self._makeOne([(0, 0), (1, 1)])
 
         # Test __repr__
         eq = self.assertEqual
@@ -97,24 +96,37 @@ class PersistentMappingTests(unittest.TestCase):
         eq(repr(u1), repr(l1), "repr(u1) == repr(l1)")
 
         # Test __cmp__ and __len__
+        try:
+            cmp
+        except NameError:
+            def cmp(a, b):
+                if a == b:
+                    return 0
+                if hasattr(a, 'items'):
+                    a = sorted(a.items())
+                    b = sorted(b.items())
+                if a < b:
+                    return -1
+                return 1
 
-        if PYTHON2:
         def mycmp(a, b):
             r = cmp(a, b)
-                if r < 0: return -1
-                if r > 0: return 1
+            if r < 0:
+                return -1
+            if r > 0:
+                return 1
             return r
 
-            all = [l0, l1, l2, u, u0, u1, u2, uu, uu0, uu1, uu2]
-            for a in all:
-                for b in all:
+        to_test = [l0, l1, l2, u, u0, u1, u2, uu, uu0, uu1, uu2]
+        for a in to_test:
+            for b in to_test:
                 eq(mycmp(a, b), mycmp(len(a), len(b)),
                    "mycmp(a, b) == mycmp(len(a), len(b))")
 
         # Test __getitem__
 
-        for i in range(len(u2)):
-            eq(u2[i], i, "u2[i] == i")
+        for i, val in enumerate(u2):
+            eq(val, i, "u2[i] == i")
 
         # Test get
 
@@ -136,12 +148,8 @@ class PersistentMappingTests(unittest.TestCase):
 
         del uu2[1]
         del uu2[0]
-        try:
+        with self.assertRaises(KeyError):
             del uu2[0]
-        except KeyError:
-            pass
-        else:
-            raise TestFailed("uu2[0] shouldn't be deletable")
 
         # Test __contains__
         for i in u2:
@@ -176,12 +184,8 @@ class PersistentMappingTests(unittest.TestCase):
         eq(x, 1, "u2.pop(1) == 1")
         self.assertTrue(1 not in u2, "1 not in u2")
 
-        try:
+        with self.assertRaises(KeyError):
             u2.pop(1)
-        except KeyError:
-            pass
-        else:
-            self.fail("1 should not be poppable from u2")
 
         x = u2.pop(1, 7)
         eq(x, 7, "u2.pop(1, 7) == 7")
@@ -230,8 +234,4 @@ class Test_legacy_PersistentDict(unittest.TestCase):
 
 
 def test_suite():
-    return unittest.TestSuite((
-        unittest.makeSuite(Test_default),
-        unittest.makeSuite(PersistentMappingTests),
-        unittest.makeSuite(Test_legacy_PersistentDict),
-    ))
+    return unittest.defaultTestLoader.loadTestsFromName(__name__)

persistent/tests/test_persistence.py

@@ -24,9 +24,18 @@ _is_pypy3 = py_impl() == 'PyPy' and sys.version_info[0] > 2
 _is_jython = py_impl() == 'Jython'
 
 #pylint: disable=R0904,W0212,E1101
+# pylint:disable=attribute-defined-outside-init,too-many-lines
+# pylint:disable=blacklisted-name
+# Hundreds of unused jar and OID vars make this useless
+# pylint:disable=unused-variable
 
 class _Persistent_Base(object):
 
+    # py2/3 compat
+    assertRaisesRegex = getattr(unittest.TestCase,
+                                'assertRaisesRegex',
+                                unittest.TestCase.assertRaisesRegexp)
+
     def _getTargetClass(self):
         # concrete testcase classes must override
         raise NotImplementedError()
@@ -79,10 +88,10 @@ class _Persistent_Base(object):
         class _BrokenJar(object):
             def __init__(self):
                 self.called = 0
-            def register(self,ob):
+            def register(self, ob):
                 self.called += 1
                 raise NotImplementedError()
-            def setstate(self,ob):
+            def setstate(self, ob):
                 raise NotImplementedError()
 
         jar = _BrokenJar()
@@ -90,8 +99,7 @@ class _Persistent_Base(object):
         return jar
 
     def _makeOneWithJar(self, klass=None, broken_jar=False):
-        from persistent.timestamp import _makeOctets
-        OID = _makeOctets('\x01' * 8)
+        OID = b'\x01' * 8
         if klass is not None:
             inst = klass()
         else:
@@ -162,12 +170,10 @@ class _Persistent_Base(object):
     def test_assign_p_jar_w_new_jar(self):
         inst, jar, OID = self._makeOneWithJar()
         new_jar = self._makeJar()
-        try:
+
+        with self.assertRaisesRegex(ValueError,
+                                    "can not change _p_jar of cached object"):
             inst._p_jar = new_jar
-        except ValueError as e:
-            self.assertEqual(str(e), "can not change _p_jar of cached object")
-        else:
-            self.fail("Should raise ValueError")
 
     def test_assign_p_jar_w_valid_jar(self):
         jar = self._makeJar()
@@ -189,43 +195,35 @@ class _Persistent_Base(object):
     def test_assign_p_oid_w_invalid_oid(self):
         inst, jar, OID = self._makeOneWithJar()
 
-        try:
+        with self.assertRaisesRegex(ValueError,
+                                    'can not change _p_oid of cached object'):
             inst._p_oid = object()
-        except ValueError as e:
-            self.assertEqual(str(e), 'can not change _p_oid of cached object')
-        else:
-            self.fail("Should raise value error")
-
 
     def test_assign_p_oid_w_valid_oid(self):
-        from persistent.timestamp import _makeOctets
-        OID = _makeOctets('\x01' * 8)
+        OID = b'\x01' * 8
         inst = self._makeOne()
         inst._p_oid = OID
         self.assertEqual(inst._p_oid, OID)
         inst._p_oid = OID  # reassign only same OID
 
     def test_assign_p_oid_w_new_oid_wo_jar(self):
-        from persistent.timestamp import _makeOctets
-        OID1 = _makeOctets('\x01' * 8)
-        OID2 = _makeOctets('\x02' * 8)
+        OID1 = b'\x01' * 8
+        OID2 = b'\x02' * 8
         inst = self._makeOne()
         inst._p_oid = OID1
         inst._p_oid = OID2
         self.assertEqual(inst._p_oid, OID2)
 
     def test_assign_p_oid_w_None_wo_jar(self):
-        from persistent.timestamp import _makeOctets
-        OID1 = _makeOctets('\x01' * 8)
+        OID1 = b'\x01' * 8
         inst = self._makeOne()
         inst._p_oid = OID1
         inst._p_oid = None
         self.assertEqual(inst._p_oid, None)
 
     def test_assign_p_oid_w_new_oid_w_jar(self):
-        from persistent.timestamp import _makeOctets
         inst, jar, OID = self._makeOneWithJar()
-        new_OID = _makeOctets('\x02' * 8)
+        new_OID = b'\x02' * 8
         def _test():
             inst._p_oid = new_OID
         self.assertRaises(ValueError, _test)
@@ -239,8 +237,7 @@ class _Persistent_Base(object):
         self.assertEqual(inst._p_oid, 42)
 
     def test_delete_p_oid_wo_jar(self):
-        from persistent.timestamp import _makeOctets
-        OID = _makeOctets('\x01' * 8)
+        OID = b'\x01' * 8
         inst = self._makeOne()
         inst._p_oid = OID
         del inst._p_oid
@@ -248,9 +245,18 @@ class _Persistent_Base(object):
 
     def test_delete_p_oid_w_jar(self):
         inst, jar, OID = self._makeOneWithJar()
-        def _test():
+        with self.assertRaises(ValueError):
+            del inst._p_oid
+
+    def test_delete_p_oid_of_subclass_calling_p_delattr(self):
+        class P(self._getTargetClass()):
+            def __delattr__(self, name):
+                super(P, self)._p_delattr(name)
+                raise AssertionError("Should not get here")
+
+        inst, _jar, _oid = self._makeOneWithJar(klass=P)
+        with self.assertRaises(ValueError):
             del inst._p_oid
-        self.assertRaises(ValueError, _test)
 
     def test_del_oid_like_ZODB_abort(self):
         # When a ZODB connection aborts, it removes registered objects from
@@ -276,30 +282,28 @@ class _Persistent_Base(object):
     def test_assign_p_serial_too_short(self):
         inst = self._makeOne()
         def _test():
-            inst._p_serial = '\x01\x02\x03'
+            inst._p_serial = b'\x01\x02\x03'
         self.assertRaises(ValueError, _test)
 
     def test_assign_p_serial_too_long(self):
         inst = self._makeOne()
         def _test():
-            inst._p_serial = '\x01\x02\x03' * 3
+            inst._p_serial = b'\x01\x02\x03' * 3
         self.assertRaises(ValueError, _test)
 
     def test_assign_p_serial_w_valid_serial(self):
-        from persistent.timestamp import _makeOctets
-        SERIAL = _makeOctets('\x01' * 8)
+        SERIAL = b'\x01' * 8
         inst = self._makeOne()
         inst._p_serial = SERIAL
         self.assertEqual(inst._p_serial, SERIAL)
 
     def test_delete_p_serial(self):
-        from persistent.timestamp import _makeOctets
         from persistent.persistence import _INITIAL_SERIAL
-        SERIAL = _makeOctets('\x01' * 8)
+        SERIAL = b'\x01' * 8
         inst = self._makeOne()
         inst._p_serial = SERIAL
         self.assertEqual(inst._p_serial, SERIAL)
-        del(inst._p_serial)
+        del inst._p_serial
         self.assertEqual(inst._p_serial, _INITIAL_SERIAL)
 
     def test_query_p_changed_unsaved(self):
@@ -623,15 +627,17 @@ class _Persistent_Base(object):
 
     def test_assign_p_estimated_size_wrong_type(self):
         inst = self._makeOne()
-        self.assertRaises(TypeError,
-                          lambda : setattr(inst, '_p_estimated_size', None))
+
+        with self.assertRaises(TypeError):
+            inst._p_estimated_size = None
+
         try:
-            long
+            constructor = long
         except NameError:
-            pass
-        else:
-            self.assertRaises(TypeError,
-                          lambda : setattr(inst, '_p_estimated_size', long(1)))
+            constructor = str
+
+        with self.assertRaises(TypeError):
+            inst._p_estimated_size = constructor(1)
 
     def test_assign_p_estimated_size_negative(self):
         inst = self._makeOne()
@@ -723,7 +729,7 @@ class _Persistent_Base(object):
             def __getattribute__(self, name):
                 if name == 'magic':
                     return 42
-                return super(Base,self).__getattribute__(name)
+                return super(Base, self).__getattribute__(name) # pragma: no cover
 
         self.assertEqual(getattr(Base(), 'magic'), 42)
 
@@ -733,8 +739,7 @@ class _Persistent_Base(object):
         self.assertRaises(AttributeError, getattr, Derived(), 'magic')
 
     def test___setattr___p__names(self):
-        from persistent.timestamp import _makeOctets
-        SERIAL = _makeOctets('\x01' * 8)
+        SERIAL = b'\x01' * 8
         inst, jar, OID = self._makeOneWithJar()
         inst._p_activate()
         NAMES = [('_p_jar', jar),
@@ -1030,7 +1035,6 @@ class _Persistent_Base(object):
         self.assertTrue(hasattr(inst1, 'foobar'))
 
     def test___reduce__(self):
-        from persistent._compat import copy_reg
         inst = self._makeOne()
         first, second, third = inst.__reduce__()
         self.assertTrue(first is copy_reg.__newobj__)
@@ -1038,7 +1042,6 @@ class _Persistent_Base(object):
         self.assertEqual(third, None)
 
     def test___reduce__w_subclass_having_getnewargs(self):
-        from persistent._compat import copy_reg
         class Derived(self._getTargetClass()):
             def __getnewargs__(self):
                 return ('a', 'b')
@@ -1049,7 +1052,6 @@ class _Persistent_Base(object):
         self.assertEqual(third, {})
 
     def test___reduce__w_subclass_having_getstate(self):
-        from persistent._compat import copy_reg
         class Derived(self._getTargetClass()):
             def __getstate__(self):
                 return {}
@@ -1060,7 +1062,6 @@ class _Persistent_Base(object):
         self.assertEqual(third, {})
 
     def test___reduce__w_subclass_having_getnewargs_and_getstate(self):
-        from persistent._compat import copy_reg
         class Derived(self._getTargetClass()):
             def __getnewargs__(self):
                 return ('a', 'b')
@@ -1487,8 +1488,7 @@ class _Persistent_Base(object):
         self._checkMRU(jar, [OID])
 
     def test__p_setattr_w__p__name(self):
-        from persistent.timestamp import _makeOctets
-        SERIAL = _makeOctets('\x01' * 8)
+        SERIAL = b'\x01' * 8
         inst, jar, OID = self._makeOneWithJar()
         inst._p_deactivate()
         self.assertTrue(inst._p_setattr('_p_serial', SERIAL))
@@ -1537,14 +1537,12 @@ class _Persistent_Base(object):
         # object stays in the up-to-date state.
         # It shouldn't change to the modified state, because it won't
         # be saved when the transaction commits.
-        from persistent._compat import _b
         class P(self._getTargetClass()):
             def __init__(self):
                 self.x = 0
-            def inc(self):
-                self.x += 1
+
         p = P()
-        p._p_oid = _b('1')
+        p._p_oid = b'1'
         p._p_jar = self._makeBrokenJar()
         self.assertEqual(p._p_state, 0)
         self.assertEqual(p._p_jar.called, 0)
@@ -1557,14 +1555,11 @@ class _Persistent_Base(object):
     def test__p_activate_w_broken_jar(self):
         # Make sure that exceptions that occur inside the data manager's
         # ``setstate()`` method propagate out to the caller.
-        from persistent._compat import _b
         class P(self._getTargetClass()):
             def __init__(self):
                 self.x = 0
-            def inc(self):
-                self.x += 1
         p = P()
-        p._p_oid = _b('1')
+        p._p_oid = b'1'
         p._p_jar = self._makeBrokenJar()
         p._p_deactivate()
         self.assertEqual(p._p_state, -1)
@@ -1618,23 +1613,21 @@ class _Persistent_Base(object):
         class subclass(self._getTargetClass()):
             _v_setattr_called = False
             def __setattr__(self, name, value):
-                object.__setattr__(self, '_v_setattr_called', True)
-                super(subclass,self).__setattr__(name, value)
+                raise AssertionError("Should not be called")
         inst = subclass()
         self.assertEqual(object.__getattribute__(inst,'_v_setattr_called'), False)
 
     def test_can_set__p_attrs_if_subclass_denies_setattr(self):
-        from persistent._compat import _b
         # ZODB defines a PersistentBroken subclass that only lets us
         # set things that start with _p, so make sure we can do that
         class Broken(self._getTargetClass()):
             def __setattr__(self, name, value):
                 if name.startswith('_p_'):
-                    super(Broken,self).__setattr__(name, value)
+                    super(Broken, self).__setattr__(name, value)
                 else:
-                    raise TypeError("Can't change broken objects")
+                    raise AssertionError("Can't change broken objects")
 
-        KEY = _b('123')
+        KEY = b'123'
         jar = self._makeJar()
 
         broken = Broken()
@@ -1743,17 +1736,17 @@ class PyPersistentTests(unittest.TestCase, _Persistent_Base):
         # pickle cache yet.
         # Nothing should blow up when this happens
         from persistent._compat import _b
-        KEY = _b('123')
+        KEY = b'123'
         jar = self._makeJar()
         c1 = self._makeOne()
         c1._p_oid = KEY
         c1._p_jar = jar
-        orig_mru = jar._cache.mru
+
         def mru(oid):
             # Mimic what the real cache does
             if oid not in jar._cache._mru:
                 raise KeyError(oid)
-            orig_mru(oid)
+            raise AssertionError("Should never get here")
         jar._cache.mru = mru
         c1._p_accessed()
         self._checkMRU(jar, [])
@@ -1815,7 +1808,7 @@ _add_to_suite = [PyPersistentTests]
 if not os.environ.get('PURE_PYTHON'):
     try:
         from persistent import cPersistence
-    except ImportError:
+    except ImportError: # pragma: no cover
         pass
     else:
         class CPersistentTests(unittest.TestCase, _Persistent_Base):
@@ -1846,12 +1839,7 @@ if not os.environ.get('PURE_PYTHON'):
                 self.assertRaises(TypeError, self._callFUT, '')
 
             def test_w_type(self):
-                import sys
-                TO_CREATE = [type, list, tuple, object]
-                # Python 3.3 segfaults when destroying a dict created via
-                # PyType_GenericNew.  See http://bugs.python.org/issue16676
-                if sys.version_info < (3, 3):
-                    TO_CREATE.append(dict)
+                TO_CREATE = [type, list, tuple, object, dict]
                 for typ in TO_CREATE:
                     self.assertTrue(isinstance(self._callFUT(typ), typ))
 

persistent/tests/test_picklecache.py

@@ -24,6 +24,12 @@ _marker = object()
 
 class PickleCacheTests(unittest.TestCase):
 
+    # py2/3 compat
+    assertRaisesRegex = getattr(unittest.TestCase,
+                                'assertRaisesRegex',
+                                unittest.TestCase.assertRaisesRegexp)
+
+
     def setUp(self):
         import persistent.picklecache
         self.orig_types = persistent.picklecache._CACHEABLE_TYPES
@@ -98,12 +104,8 @@ class PickleCacheTests(unittest.TestCase):
     def test___setitem___non_string_oid_raises_TypeError(self):
         cache = self._makeOne()
 
-        try:
+        with self.assertRaises(TypeError):
             cache[object()] = self._makePersist()
-        except TypeError:
-            pass
-        else:
-            self.fail("Didn't raise ValueError with non-string OID.")
 
     def test___setitem___duplicate_oid_same_obj(self):
         from persistent._compat import _b
@@ -121,12 +123,8 @@ class PickleCacheTests(unittest.TestCase):
         cache[KEY] = original
         duplicate = self._makePersist(oid=KEY)
 
-        try:
+        with self.assertRaises(ValueError):
             cache[KEY] = duplicate
-        except ValueError:
-            pass
-        else:
-            self.fail("Didn't raise KeyError with duplicate OID.")
 
     def test___setitem___ghost(self):
         from persistent.interfaces import GHOST
@@ -153,12 +151,8 @@ class PickleCacheTests(unittest.TestCase):
         cache = self._makeOne()
         uptodate = self._makePersist(state=UPTODATE)
 
-        try:
+        with self.assertRaises(ValueError):
             cache[KEY] = uptodate
-        except ValueError:
-            pass
-        else:
-            self.fail("Didn't raise ValueError when the key didn't match the OID")
 
 
     def test___setitem___non_ghost(self):
@@ -201,24 +195,16 @@ class PickleCacheTests(unittest.TestCase):
     def test___delitem___non_string_oid_raises_TypeError(self):
         cache = self._makeOne()
 
-        try:
+        with self.assertRaises(TypeError):
             del cache[object()]
-        except TypeError:
-            pass
-        else:
-            self.fail("Didn't raise ValueError with non-string OID.")
 
     def test___delitem___nonesuch_raises_KeyError(self):
         from persistent._compat import _b
         cache = self._makeOne()
         original = self._makePersist()
 
-        try:
+        with self.assertRaises(KeyError):
             del cache[_b('nonesuch')]
-        except KeyError:
-            pass
-        else:
-            self.fail("Didn't raise KeyError with nonesuch OID.")
 
     def test___delitem___w_persistent_class(self):
         from persistent._compat import _b
@@ -878,22 +864,16 @@ class PickleCacheTests(unittest.TestCase):
 
     def test_setting_non_persistent_item(self):
         cache = self._makeOne()
-        try:
+        with self.assertRaisesRegex(TypeError,
+                                    "Cache values must be persistent objects."):
             cache[None] = object()
-        except TypeError as e:
-            self.assertEqual(str(e), "Cache values must be persistent objects.")
-        else:
-            self.fail("Should raise TypeError")
 
     def test_setting_without_jar(self):
         cache = self._makeOne()
         p = self._makePersist(jar=None)
-        try:
+        with self.assertRaisesRegex(ValueError,
+                                    "Cached object jar missing"):
             cache[p._p_oid] = p
-        except ValueError as e:
-            self.assertEqual(str(e), "Cached object jar missing")
-        else:
-            self.fail("Should raise ValueError")
 
     def test_setting_already_cached(self):
         cache1 = self._makeOne()
@@ -902,12 +882,9 @@ class PickleCacheTests(unittest.TestCase):
         cache1[p._p_oid] = p
 
         cache2 = self._makeOne()
-        try:
+        with self.assertRaisesRegex(ValueError,
+                                    "Object already in another cache"):
             cache2[p._p_oid] = p
-        except ValueError as e:
-            self.assertEqual(str(e), "Object already in another cache")
-        else:
-            self.fail("Should raise value error")
 
     def test_cannot_update_mru_while_already_locked(self):
         cache = self._makeOne()
@@ -972,7 +949,7 @@ class PickleCacheTests(unittest.TestCase):
         del p._p_deactivate
         self.assertEqual(cache.full_sweep(), 1)
 
-    if _is_jython:
+    if _is_jython: # pragma: no cover
         def with_deterministic_gc(f):
             def test(self):
                 old_flags = gc.getMonitorGlobal()
@@ -1027,7 +1004,7 @@ class PickleCacheTests(unittest.TestCase):
 
         # It also shrank the measured size of the cache;
         # this would fail under PyPy if _SWEEP_NEEDS_GC was False
-        if force_collect:
+        if force_collect: # pragma: no cover
             gc.collect()
         self.assertEqual(len(cache), 1)
 
@@ -1053,12 +1030,11 @@ class PickleCacheTests(unittest.TestCase):
     def test_ring_impl(self):
         from .. import ring
 
-        if _is_pypy:
-            self.assertIs(ring.Ring, ring._CFFIRing)
-        elif ring._CFFIRing is not None or os.environ.get('USING_CFFI'):
-            self.assertIs(ring.Ring, ring._CFFIRing)
-        else:
-            self.assertIs(ring.Ring, ring._DequeRing)
+        expected = (ring._CFFIRing
+                    if _is_pypy or ring._CFFIRing is not None or os.environ.get('USING_CFFI')
+                    else ring._DequeRing)
+        self.assertIs(ring.Ring, expected)
+
 
 class DummyPersistent(object):
 

persistent/tests/test_ring.py

@@ -31,7 +31,7 @@ class DummyPersistent(object):
         if oid is None:
             self._p_oid = self._next_oid()
 
-    def __repr__(self):
+    def __repr__(self): # pragma: no cover
         return "<Dummy %r>" % self._p_oid
 
 class _Ring_Base(object):

persistent/tests/test_timestamp.py

@@ -63,9 +63,11 @@ class pyTimeStampTests(unittest.TestCase):
                     (1, 2, 3, 4, 5),
                     ('1', '2', '3', '4', '5', '6'),
                     (1, 2, 3, 4, 5, 6, 7),
+                    (b'123',),
                    ]
         for args in BAD_ARGS:
-            self.assertRaises((TypeError, ValueError), self._makeOne, *args)
+            with self.assertRaises((TypeError, ValueError)):
+                self._makeOne(*args)
 
     def test_ctor_from_invalid_strings(self):
         BAD_ARGS = [''
@@ -80,13 +82,12 @@ class pyTimeStampTests(unittest.TestCase):
             self.assertRaises((TypeError, ValueError), self._makeOne, *args)
 
     def test_ctor_from_string(self):
-        from persistent.timestamp import _makeOctets
         from persistent.timestamp import _makeUTC
         ZERO = _makeUTC(1900, 1, 1, 0, 0, 0)
         EPOCH = _makeUTC(1970, 1, 1, 0, 0, 0)
         DELTA = ZERO - EPOCH
         DELTA_SECS = DELTA.days * 86400 + DELTA.seconds
-        SERIAL = _makeOctets('\x00' * 8)
+        SERIAL = b'\x00' * 8
         ts = self._makeOne(SERIAL)
         self.assertEqual(ts.raw(), SERIAL)
         self.assertEqual(ts.year(), 1900)
@@ -104,13 +105,12 @@ class pyTimeStampTests(unittest.TestCase):
         self.assertEqual(before.timeTime(), 1297867042.80544)
 
     def test_ctor_from_elements(self):
-        from persistent.timestamp import _makeOctets
         from persistent.timestamp import _makeUTC
         ZERO = _makeUTC(1900, 1, 1, 0, 0, 0)
         EPOCH = _makeUTC(1970, 1, 1, 0, 0, 0)
         DELTA = ZERO - EPOCH
         DELTA_SECS = DELTA.days * 86400 + DELTA.seconds
-        SERIAL = _makeOctets('\x00' * 8)
+        SERIAL = b'\x00' * 8
         ts = self._makeOne(1900, 1, 1, 0, 0, 0.0)
         self.assertEqual(ts.raw(), SERIAL)
         self.assertEqual(ts.year(), 1900)
@@ -122,9 +122,8 @@ class pyTimeStampTests(unittest.TestCase):
         self.assertEqual(ts.timeTime(), DELTA_SECS)
 
     def test_laterThan_invalid(self):
-        from persistent.timestamp import _makeOctets
         ERRORS = (ValueError, TypeError)
-        SERIAL = _makeOctets('\x01' * 8)
+        SERIAL = b'\x01' * 8
         ts = self._makeOne(SERIAL)
         self.assertRaises(ERRORS, ts.laterThan, None)
         self.assertRaises(ERRORS, ts.laterThan, '')
@@ -134,26 +133,23 @@ class pyTimeStampTests(unittest.TestCase):
         self.assertRaises(ERRORS, ts.laterThan, object())
 
     def test_laterThan_self_is_earlier(self):
-        from persistent.timestamp import _makeOctets
-        SERIAL1 = _makeOctets('\x01' * 8)
-        SERIAL2 = _makeOctets('\x02' * 8)
+        SERIAL1 = b'\x01' * 8
+        SERIAL2 = b'\x02' * 8
         ts1 = self._makeOne(SERIAL1)
         ts2 = self._makeOne(SERIAL2)
         later = ts1.laterThan(ts2)
-        self.assertEqual(later.raw(), _makeOctets('\x02' * 7 + '\x03'))
+        self.assertEqual(later.raw(), b'\x02' * 7 + b'\x03')
 
     def test_laterThan_self_is_later(self):
-        from persistent.timestamp import _makeOctets
-        SERIAL1 = _makeOctets('\x01' * 8)
-        SERIAL2 = _makeOctets('\x02' * 8)
+        SERIAL1 = b'\x01' * 8
+        SERIAL2 = b'\x02' * 8
         ts1 = self._makeOne(SERIAL1)
         ts2 = self._makeOne(SERIAL2)
         later = ts2.laterThan(ts1)
         self.assertTrue(later is ts2)
 
     def test_repr(self):
-        from persistent.timestamp import _makeOctets
-        SERIAL = _makeOctets('\x01' * 8)
+        SERIAL = b'\x01' * 8
         ts = self._makeOne(SERIAL)
         self.assertEqual(repr(ts), repr(SERIAL))
 
@@ -163,14 +159,20 @@ class pyTimeStampTests(unittest.TestCase):
         # Check the corner cases when comparing non-comparable types
         ts = self._makeOne(2011, 2, 16, 14, 37, 22.0)
 
-        def check_py2(op, passes):
+        def check_common(op, passes):
             if passes == 'neither':
                 self.assertFalse(op(ts, None))
                 self.assertFalse(op(None, ts))
-            elif passes == 'both':
+                return True
+
+            if passes == 'both':
                 self.assertTrue(op(ts, None))
                 self.assertTrue(op(None, ts))
-            elif passes == 'first':
+                return True
+            return False
+
+        def check_py2(op, passes): # pragma: no cover
+            if passes == 'first':
                 self.assertTrue(op(ts, None))
                 self.assertFalse(op(None, ts))
             else:
@@ -178,13 +180,6 @@ class pyTimeStampTests(unittest.TestCase):
                 self.assertTrue(op(None, ts))
 
         def check_py3(op, passes):
-            if passes == 'neither':
-                self.assertFalse(op(ts, None))
-                self.assertFalse(op(None, ts))
-            elif passes == 'both':
-                self.assertTrue(op(ts, None))
-                self.assertTrue(op(None, ts))
-            else:
             self.assertRaises(TypeError, op, ts, None)
             self.assertRaises(TypeError, op, None, ts)
 
@@ -197,6 +192,7 @@ class pyTimeStampTests(unittest.TestCase):
                                 ('eq', 'neither'),
                                 ('ne', 'both')):
             op = getattr(operator, op_name)
+            if not check_common(op, passes):
                 check(op, passes)
 
 
@@ -223,7 +219,7 @@ class PyAndCComparisonTests(unittest.TestCase):
         # 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 , )
+            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
@@ -304,7 +300,7 @@ class PyAndCComparisonTests(unittest.TestCase):
             # in hash() on 32-bit platforms
             if not self._is_jython:
                 self.assertEqual(py.__hash__(), bit_64_hash)
-            else:
+            else: # pragma: no cover
                 # Jython 2.7's ctypes module doesn't properly
                 # implement the 'value' attribute by truncating.
                 # (It does for native calls, but not visibly to Python).
@@ -318,15 +314,13 @@ class PyAndCComparisonTests(unittest.TestCase):
             MUT._MAXINT = orig_maxint
             if orig_c_long is not None:
                 MUT.c_long = orig_c_long
-            else:
+            else: # pragma: no cover
                 del MUT.c_long
 
         # These are *usually* aliases, but aren't required
         # to be (and aren't under Jython 2.7).
-        if is_32_bit_hash:
-            self.assertEqual(py.__hash__(), bit_32_hash)
-        else:
-            self.assertEqual(py.__hash__(), bit_64_hash)
+        expected_hash = bit_32_hash if is_32_bit_hash else bit_64_hash
+        self.assertEqual(py.__hash__(), expected_hash)
 
     def test_hash_equal_constants(self):
         # The simple constants make it easier to diagnose
@@ -350,46 +344,36 @@ class PyAndCComparisonTests(unittest.TestCase):
 
         # overflow kicks in here on 32-bit platforms
         c, py = self._make_C_and_Py(b'\x00\x00\x00\x00\x00\x01\x00\x00')
-        if is_32_bit:
-            self.assertEqual(hash(c), -721379967)
-        else:
-            self.assertEqual(hash(c), 1000006000001)
+        expected = -721379967 if is_32_bit else 1000006000001
+        self.assertEqual(hash(c), expected)
         self.assertEqual(hash(c), hash(py))
 
         c, py = self._make_C_and_Py(b'\x00\x00\x00\x00\x01\x00\x00\x00')
-        if is_32_bit:
-            self.assertEqual(hash(c), 583896275)
-        else:
-            self.assertEqual(hash(c), 1000009000027000019)
+        expected = 583896275 if is_32_bit else 1000009000027000019
+        self.assertEqual(hash(c), expected)
         self.assertEqual(hash(c), hash(py))
 
         # Overflow kicks in at this point on 64-bit platforms
         c, py = self._make_C_and_Py(b'\x00\x00\x00\x01\x00\x00\x00\x00')
-        if is_32_bit:
-            self.assertEqual(hash(c), 1525764953)
-        else:
-            self.assertEqual(hash(c), -4442925868394654887)
+        expected = 1525764953 if is_32_bit else -4442925868394654887
+        self.assertEqual(hash(c), expected)
         self.assertEqual(hash(c), hash(py))
 
         c, py = self._make_C_and_Py(b'\x00\x00\x01\x00\x00\x00\x00\x00')
-        if is_32_bit:
-            self.assertEqual(hash(c), -429739973)
-        else:
-            self.assertEqual(hash(c), -3993531167153147845)
+        expected = -429739973 if is_32_bit else -3993531167153147845
+        self.assertEqual(hash(c), expected)
         self.assertEqual(hash(c), hash(py))
 
         c, py = self._make_C_and_Py(b'\x01\x00\x00\x00\x00\x00\x00\x00')
-        if is_32_bit:
-            self.assertEqual(hash(c), 263152323)
-        else:
-            self.assertEqual(hash(c), -3099646879006235965)
+        expected = 263152323 if is_32_bit else -3099646879006235965
+        self.assertEqual(hash(c), expected)
         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_c = self._makeC(b'\x01\x00\x00\x00\x00\x00\x00\x00')
         big_py = self._makePy(b'\x01\x00\x00\x00\x00\x00\x00\x00')
 
         self.assertTrue(small_py < big_py)
@@ -423,7 +407,7 @@ def test_suite():
     try:
         from persistent.timestamp import pyTimeStamp
         from persistent.timestamp import TimeStamp
-    except ImportError:
+    except ImportError: # pragma: no cover
         pass
     else:
         if pyTimeStamp != TimeStamp:

persistent/tests/test_wref.py

@@ -317,7 +317,7 @@ class PersistentWeakKeyDictionaryTests(unittest.TestCase):
         self.assertTrue(target[key] is value)
 
 
-def _makeTarget(oid='OID', **kw):
+def _makeTarget(oid='OID'):
     from persistent import Persistent
     from persistent._compat import _b
     class Derived(Persistent):
@@ -325,11 +325,9 @@ def _makeTarget(oid='OID', **kw):
             return hash(self._p_oid)
         def __eq__(self, other):
             return self._p_oid == other._p_oid
-        def __repr__(self):
+        def __repr__(self): # pragma: no cover
             return 'Derived: %s' % self._p_oid
     derived = Derived()
-    for k, v in kw.items():
-        setattr(derived, k, v)
     derived._p_oid = _b(oid)
     return derived
 
@@ -341,7 +339,4 @@ def _makeJar():
     return _Jar()
 
 def test_suite():
-    return unittest.TestSuite((
-        unittest.makeSuite(WeakRefTests),
-        unittest.makeSuite(PersistentWeakKeyDictionaryTests),
-    ))
+    return unittest.defaultTestLoader.loadTestsFromName(__name__)

persistent/tests/utils.py

@@ -18,19 +18,9 @@ class ResettingJar(object):
         obj._p_jar = self
         self.cache[obj._p_oid] = obj
 
-    def close(self):
-        pass
 
     # the following methods must be implemented to be a jar
 
-    def setklassstate(self):
-        # I don't know what this method does, but the pickle cache
-        # constructor calls it.
-        pass
-
-    def register(self, obj):
-        self.registered[obj] = 1
-
     def setstate(self, obj):
         # Trivial setstate() implementation that just re-initializes
         # the object.  This isn't what setstate() is supposed to do,
@@ -56,8 +46,6 @@ class RememberingJar(object):
         self.obj = obj
         self.remembered = obj.__getstate__()
 
-    def close(self):
-        pass
 
     def fake_commit(self):
         self.remembered = self.obj.__getstate__()
@@ -65,11 +53,6 @@ class RememberingJar(object):
 
     # the following methods must be implemented to be a jar
 
-    def setklassstate(self):
-        # I don't know what this method does, but the pickle cache
-        # constructor calls it.
-        pass
-
     def register(self, obj):
         self.registered[obj] = 1
 
@@ -79,4 +62,3 @@ class RememberingJar(object):
         # This isn't what setstate() is supposed to do,
         # but it suffices for the tests.
         obj.__setstate__(self.remembered)
-

persistent/timestamp.py

@@ -18,16 +18,12 @@ import math
 import struct
 import sys
 
+from persistent._compat import PURE_PYTHON
+
 _RAWTYPE = bytes
 _MAXINT = sys.maxsize
 
-def _makeOctets(s):
-    if sys.version_info < (3,):
-        return bytes(s)
-    return bytes(s, 'ascii') #pragma NO COVERAGE
-
-
-_ZERO = _makeOctets('\x00' * 8)
+_ZERO = b'\x00' * 8
 
 try:
     # Make sure to overflow and wraparound just
@@ -175,7 +171,7 @@ class pyTimeStamp(object):
 
         x = _wraparound(x)
 
-        if x == -1: #pragma: no cover
+        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
@@ -211,6 +207,8 @@ class pyTimeStamp(object):
 
 
 try:
-    from persistent._timestamp import TimeStamp
-except ImportError: #pragma NO COVER
-    TimeStamp = pyTimeStamp
+    from persistent._timestamp import TimeStamp as CTimeStamp
+except ImportError: # pragma: no cover
+    CTimeStamp = None
+
+TimeStamp = pyTimeStamp if PURE_PYTHON or CTimeStamp is None else CTimeStamp

persistent/wref.py

@@ -83,7 +83,7 @@ class PersistentWeakKeyDictionary(Persistent):
             self.update(adict)
         # XXX 'kwargs' is pointless, because keys must be strings, but we
         #     are going to try (and fail) to wrap a WeakRef around them.
-        if kwargs: #pragma NO COVER
+        if kwargs: # pragma: no cover
             self.update(kwargs)
 
     def __getstate__(self):

setup.py

@@ -116,6 +116,7 @@ setup(name='persistent',
           'test': [
               'zope.testrunner',
               "cffi ; platform_python_implementation == 'CPython'",
+              'manuel',
           ],
           'testing': (),
           'docs': [

tox.ini

@@ -36,7 +36,6 @@ deps =
     {[testenv]deps}
     coverage
 setenv =
-    PURE_PYTHON = 1
     USING_CFFI = 1
 
 [testenv:docs]