Break long, confusing test cases into isolated, better named versions.

src/persistent/tests/testPersistent.py

@@ -17,14 +17,7 @@ Picklable = None # avoid global import of Persistent;  updated later
 
 class PersistenceTest(unittest.TestCase):
 
-    def setUp(self):
-        from persistent.tests.utils import ResettingJar
-        self.jar = ResettingJar()
-
-    def tearDown(self):
-        self.jar.close()
-
-    def _make_P(self):
+    def _makeOne(self):
         from persistent import Persistent
 
         class P(Persistent):
@@ -32,153 +25,220 @@ class PersistenceTest(unittest.TestCase):
 
         return P()
 
-    def testOidAndJarAttrs(self):
-        obj = self._make_P()
+    def _makeJar(self):
+        from persistent.tests.utils import ResettingJar
+        return ResettingJar()
+
+    def test_oid_initial_value(self):
+        obj = self._makeOne()
         self.assertEqual(obj._p_oid, None)
+
+    def test_oid_mutable_and_deletable_when_no_jar(self):
+        obj = self._makeOne()
         obj._p_oid = 12
         self.assertEqual(obj._p_oid, 12)
         del obj._p_oid
 
-        self.jar.add(obj)
+    def test_oid_immutable_when_in_jar(self):
+        obj = self._makeOne()
+        jar = self._makeJar()
+        jar.add(obj)
 
         # Can't change oid of cache object.
         def deloid():
             del obj._p_oid
         self.assertRaises(ValueError, deloid)
+
         def setoid():
             obj._p_oid = 12
         self.assertRaises(ValueError, setoid)
 
-        def deloid():
-            del obj._p_jar
-        self.assertRaises(ValueError, deloid)
-        def setoid():
-            obj._p_jar = 12
-        self.assertRaises(ValueError, setoid)
+    # The value returned for _p_changed can be one of:
+    # 0 -- it is not changed
+    # 1 -- it is changed
+    # None -- it is a ghost
 
-    def testChangedAndState(self):
+    def test_change_via_setattr(self):
         from persistent import CHANGED
-        from persistent import GHOST
-        from persistent import UPTODATE
-        obj = self._make_P()
-        self.jar.add(obj)
-
-        # The value returned for _p_changed can be one of:
-        # 0 -- it is not changed
-        # 1 -- it is changed
-        # None -- it is a ghost
+        obj = self._makeOne()
+        jar = self._makeJar()
+        jar.add(obj)
 
         obj.x = 1
+
         self.assertEqual(obj._p_changed, 1)
         self.assertEqual(obj._p_state, CHANGED)
-        self.assert_(obj in self.jar.registered)
+        self.assert_(obj in jar.registered)
+
+    def test_setattr_then_mark_uptodate(self):
+        from persistent import UPTODATE
+        obj = self._makeOne()
+        jar = self._makeJar()
+        jar.add(obj)
 
+        obj.x = 1
         obj._p_changed = 0
+
         self.assertEqual(obj._p_changed, 0)
         self.assertEqual(obj._p_state, UPTODATE)
-        self.jar.registered.clear()
+
+    def test_set_changed_directly(self):
+        from persistent import CHANGED
+        obj = self._makeOne()
+        jar = self._makeJar()
+        jar.add(obj)
 
         obj._p_changed = 1
+
         self.assertEqual(obj._p_changed, 1)
         self.assertEqual(obj._p_state, CHANGED)
-        self.assert_(obj in self.jar.registered)
+        self.assert_(obj in jar.registered)
+
+    def test_cant_ghostify_if_changed(self):
+        from persistent import CHANGED
+        obj = self._makeOne()
+        jar = self._makeJar()
+        jar.add(obj)
 
         # setting obj._p_changed to None ghostifies if the
         # object is in the up-to-date state, but not otherwise.
+        obj.x = 1
         obj._p_changed = None
+
         self.assertEqual(obj._p_changed, 1)
         self.assertEqual(obj._p_state, CHANGED)
+
+    def test_can_ghostify_if_uptodate(self):
+        from persistent import GHOST
+        obj = self._makeOne()
+        jar = self._makeJar()
+        jar.add(obj)
+
+        obj.x = 1
         obj._p_changed = 0
-        # Now it's a ghost.
         obj._p_changed = None
+
         self.assertEqual(obj._p_changed, None)
         self.assertEqual(obj._p_state, GHOST)
 
-        obj = self._make_P()
-        self.jar.add(obj)
-        obj._p_changed = 1
+    def test_can_ghostify_if_changed_but_del__p_changed(self):
+        from persistent import GHOST
+        obj = self._makeOne()
+        jar = self._makeJar()
+        jar.add(obj)
+
         # You can transition directly from modified to ghost if
         # you delete the _p_changed attribute.
+        obj.x = 1
         del obj._p_changed
+
         self.assertEqual(obj._p_changed, None)
         self.assertEqual(obj._p_state, GHOST)
 
-    def testStateReadonly(self):
+    def test__p_state_immutable(self):
         from persistent import CHANGED
         from persistent import GHOST
         from persistent import STICKY
         from persistent import UPTODATE
         # make sure we can't write to _p_state; we don't want yet
         # another way to change state!
-        obj = self._make_P()
+        obj = self._makeOne()
         def setstate(value):
             obj._p_state = value
+
         self.assertRaises(Exception, setstate, GHOST)
         self.assertRaises(Exception, setstate, UPTODATE)
         self.assertRaises(Exception, setstate, CHANGED)
         self.assertRaises(Exception, setstate, STICKY)
 
-    def testInvalidate(self):
+    def test_invalidate(self):
         from persistent import GHOST
         from persistent import UPTODATE
-        obj = self._make_P()
-        self.jar.add(obj)
+        obj = self._makeOne()
+        jar = self._makeJar()
+        jar.add(obj)
 
         self.assertEqual(obj._p_changed, 0)
         self.assertEqual(obj._p_state, UPTODATE)
+
         obj._p_invalidate()
+
         self.assertEqual(obj._p_changed, None)
         self.assertEqual(obj._p_state, GHOST)
 
+    def test_invalidate_activate_invalidate(self):
+        from persistent import GHOST
+        obj = self._makeOne()
+        jar = self._makeJar()
+        jar.add(obj)
+
+        obj._p_invalidate()
         obj._p_activate()
         obj.x = 1
         obj._p_invalidate()
+
         self.assertEqual(obj._p_changed, None)
         self.assertEqual(obj._p_state, GHOST)
 
-    def testSerial(self):
-        noserial = "\000" * 8
-        obj = self._make_P()
-        self.assertEqual(obj._p_serial, noserial)
+    def test_initial_serial(self):
+        NOSERIAL = "\000" * 8
+        obj = self._makeOne()
+        self.assertEqual(obj._p_serial, NOSERIAL)
+
+    def test_setting_serial_w_invalid_types_raises(self):
+        # Serial must be an 8-digit string
+        obj = self._makeOne()
 
         def set(val):
             obj._p_serial = val
+
         self.assertRaises(ValueError, set, 1)
         self.assertRaises(ValueError, set, "0123")
         self.assertRaises(ValueError, set, "012345678")
         self.assertRaises(ValueError, set, u"01234567")
 
+    def test_del_serial_returns_to_initial(self):
+        NOSERIAL = "\000" * 8
+        obj = self._makeOne()
         obj._p_serial = "01234567"
         del obj._p_serial
-        self.assertEqual(obj._p_serial, noserial)
+        self.assertEqual(obj._p_serial, NOSERIAL)
 
-    def testMTime(self):
+    def test_initial_mtime(self):
+        obj = self._makeOne()
+        self.assertEqual(obj._p_mtime, None)
+
+    def test_setting_serial_sets_mtime_to_now(self):
         import time
         from persistent.TimeStamp import TimeStamp
-        obj = self._make_P()
-        self.assertEqual(obj._p_mtime, None)
 
+        obj = self._makeOne()
         t = int(time.time())
-        ts = TimeStamp(*time.gmtime(t)[:6])
-        obj._p_serial = repr(ts)
+        ts = TimeStamp(*time.gmtime(t)[:6]) # XXX: race?
+
+        obj._p_serial = repr(ts) # why repr it?
+
         self.assertEqual(obj._p_mtime, t)
         self.assert_(isinstance(obj._p_mtime, float))
 
-    def testPicklable(self):
+    def test_pickle_unpickle(self):
         import pickle
         from persistent import Persistent
 
+        # see above:  class must be at module scope to be pickled.
         global Picklable
         class Picklable(Persistent):
             pass
 
         obj = Picklable()
         obj.attr = "test"
+
         s = pickle.dumps(obj)
         obj2 = pickle.loads(s)
+
         self.assertEqual(obj.attr, obj2.attr)
 
-    def testGetattr(self):
+    def test___getattr__(self):
         from persistent import CHANGED
         from persistent import Persistent
 
@@ -196,11 +256,13 @@ class PersistenceTest(unittest.TestCase):
         self.assertEqual(obj.curly, 2)
         self.assertEqual(obj.moe, 3)
 
-        self.jar.add(obj)
+        jar = self._makeJar()
+        jar.add(obj)
         obj._p_deactivate()
 
         # The simple Jar used for testing re-initializes the object.
         self.assertEqual(obj.larry, 1)
+
         # The getattr hook modified the object, so it should now be
         # in the changed state.
         self.assertEqual(obj._p_changed, 1)
@@ -208,7 +270,7 @@ class PersistenceTest(unittest.TestCase):
         self.assertEqual(obj.curly, 2)
         self.assertEqual(obj.moe, 3)
 
-    def testGetattribute(self):
+    def test___getattribute__(self):
         from persistent import CHANGED
         from persistent import Persistent
 
@@ -231,11 +293,13 @@ class PersistenceTest(unittest.TestCase):
         self.assertEqual(obj.curly, 2)
         self.assertEqual(obj.moe, 3)
 
-        self.jar.add(obj)
+        jar = self._makeJar()
+        jar.add(obj)
         obj._p_deactivate()
 
         # The simple Jar used for testing re-initializes the object.
         self.assertEqual(obj.larry, 1)
+
         # The getattr hook modified the object, so it should now be
         # in the changed state.
         self.assertEqual(obj._p_changed, 1)