Make sure that UndoableRecoveryStorage tests don't leak connections. …

…See newtdb/db#2

relstorage/tests/RecoveryStorage.py

@@ -166,8 +166,9 @@ def checkSimpleRecovery(self):
 
     def checkPackWithGCOnDestinationAfterRestore(self):
         raises = self.assertRaises
-        db = DB(self._storage)
-        conn = db.open()
+        closing = self._closing
+        db = closing(DB(self._storage))
+        conn = closing(db.open())
         root = conn.root()
         root.obj = obj1 = MinPO(1)
         txn = transaction.get()
@@ -188,8 +189,8 @@ def checkPackWithGCOnDestinationAfterRestore(self):
         # destination storage.  To trigger a pack, we have to
         # add another transaction to the destination that is
         # not packed.
-        db2 = DB(self._dst)
-        conn2 = db2.open()
+        db2 = closing(DB(self._dst))
+        conn2 = closing(db2.open())
         conn2.root().extra = 0
         txn = transaction.get()
         txn.note(u'root.extra = 0')
@@ -230,8 +231,9 @@ class UndoableRecoveryStorage(BasicRecoveryStorage):
     """These tests require the source storage to be undoable"""
 
     def checkRestoreAcrossPack(self):
-        db = DB(self._storage)
-        c = db.open()
+        closing = self._closing
+        db = closing(DB(self._storage))
+        c = closing(db.open())
         r = c.root()
         r["obj1"] = MinPO(1)
         transaction.commit()
@@ -245,12 +247,12 @@ def checkRestoreAcrossPack(self):
 
         # copy the final transaction manually.  even though there
         # was a pack, the restore() ought to succeed.
-        it = self._storage.iterator()
+        it = closing(self._storage.iterator())
         # Get the last transaction and its record iterator. Record iterators
         # can't be accessed out-of-order, so we need to do this in a bit
         # complicated way:
         final = None
-        for final  in it:
+        for final in it:
             records = list(final)
 
         self._dst.tpc_begin(final, final.tid, final.status)
@@ -260,6 +262,7 @@ def checkRestoreAcrossPack(self):
         self._dst.tpc_vote(final)
         self._dst.tpc_finish(final)
 
+
     def checkRestoreWithMultipleObjectsInUndoRedo(self):
         # pylint:disable=too-many-statements
         from ZODB.FileStorage import FileStorage
@@ -284,9 +287,9 @@ def checkRestoreWithMultipleObjectsInUndoRedo(self):
         # and this test does some horrid white-box abuse to test it.
 
         is_filestorage = isinstance(self._storage, FileStorage)
-
-        db = DB(self._storage)
-        c = db.open()
+        closing = self._closing
+        db = closing(DB(self._storage))
+        c = closing(db.open())
         r = c.root()
 
         # Create some objects.
@@ -299,7 +302,8 @@ def checkRestoreWithMultipleObjectsInUndoRedo(self):
         r["obj2"].x = 'x2'
         transaction.commit()
 
-        c2 = db.open()
+        c2 = closing(db.open())
+
         r = c2.root()
         self.assertEqual(r["obj1"].x, 'x1')
         self.assertEqual(r["obj2"].x, 'x2')
@@ -321,7 +325,7 @@ def checkRestoreWithMultipleObjectsInUndoRedo(self):
         self._storage.tpc_vote(t)
         self._storage.tpc_finish(t)
 
-        r = db.open().root()
+        r = closing(db.open()).root()
         self.assertRaises(AttributeError, getattr, r["obj1"], 'x')
         self.assertRaises(AttributeError, getattr, r["obj2"], 'x')
 
@@ -345,7 +349,7 @@ def checkRestoreWithMultipleObjectsInUndoRedo(self):
         self._storage.tpc_vote(t)
         self._storage.tpc_finish(t)
 
-        c3 = db.open()
+        c3 = closing(db.open())
         r = c3.root()
         self.assertEqual(r["obj1"].x, 'x1')
         self.assertEqual(r["obj2"].x, 'x2')
@@ -365,8 +369,3 @@ def checkRestoreWithMultipleObjectsInUndoRedo(self):
         # part of the test.
         self._dst.copyTransactionsFrom(self._storage)
         self.compare(self._storage, self._dst)
-
-        c.close()
-        c2.close()
-        c3.close()
-        db.close()

relstorage/tests/hftestbase.py

@@ -288,14 +288,14 @@ class HistoryFreeToFileStorage(RelStorageTestBase,
     keep_history = False
 
     def setUp(self):
+        super(HistoryFreeToFileStorage, self).setUp()
         self._storage = self.make_storage()
         self._dst = FileStorage("Dest.fs", create=True)
 
     def tearDown(self):
-        self._storage.close()
         self._dst.close()
-        self._storage.cleanup()
         self._dst.cleanup()
+        super(HistoryFreeToFileStorage, self).tearDown()
 
     def new_dest(self):
         return FileStorage('Dest.fs')
@@ -307,14 +307,14 @@ class HistoryFreeFromFileStorage(RelStorageTestBase,
     keep_history = False
 
     def setUp(self):
+        super(HistoryFreeFromFileStorage, self).setUp()
         self._dst = self._storage
         self._storage = FileStorage("Source.fs", create=True)
 
     def tearDown(self):
-        self._storage.close()
         self._dst.close()
-        self._storage.cleanup()
         self._dst.cleanup()
+        super(HistoryFreeFromFileStorage, self).tearDown()
 
     def new_dest(self):
         return self._dst

relstorage/tests/hptestbase.py

@@ -306,14 +306,14 @@ class HistoryPreservingToFileStorage(RelStorageTestBase,
     keep_history = True
 
     def setUp(self):
+        super(HistoryPreservingToFileStorage, self).setUp()
         self._storage = self.make_storage()
         self._dst = FileStorage("Dest.fs", create=True)
 
     def tearDown(self):
-        self._storage.close()
         self._dst.close()
-        self._storage.cleanup()
         self._dst.cleanup()
+        super(HistoryPreservingToFileStorage, self).tearDown()
 
     def new_dest(self):
         return FileStorage('Dest.fs')
@@ -325,14 +325,14 @@ class HistoryPreservingFromFileStorage(RelStorageTestBase,
     keep_history = True
 
     def setUp(self):
+        super(HistoryPreservingFromFileStorage, self).setUp()
         self._dst = self.make_storage()
         self._storage = FileStorage("Source.fs", create=True)
 
     def tearDown(self):
-        self._storage.close()
         self._dst.close()
-        self._storage.cleanup()
         self._dst.cleanup()
+        super(HistoryPreservingFromFileStorage, self).tearDown()
 
     def new_dest(self):
         return self._dst

relstorage/tests/reltestbase.py

@@ -77,15 +77,32 @@ class RelStorageTestBase(StorageCreatingMixin,
     keep_history = None  # Override
     _storage_created = None
 
+    __to_close = ()
+
     def setUp(self):
-        pass
+        self.__to_close = []
         # Note that we're deliberately NOT calling super's setup.
         # It does stuff on disk, etc, that's not necessary for us
         # and just slows us down by ~10%.
         #super(RelStorageTestBase, self).setUp()
+        # Also note that subclasses might not even call us! If they're going to
+        # use _closing, they have to.
+
+    def _closing(self, o):
+        """
+        Close the object before tearDown (opposite of addCleanup
+        so that exceptions will propagate).
+
+        Returns the given object.
+        """
+        self.__to_close.append(o)
+        return o
 
     def tearDown(self):
         transaction.abort()
+        for x in reversed(self.__to_close):
+            x.close()
+        self.__to_close = ()
         # XXX: This could create one! Do subclasses override self._storage?
         storage = self._storage
         if storage is not None: