Merge pull request #267 from zopefoundation/issue266

Fix ResourceWarnings seen during testing, and a testrunner orphaned thread warning

.coveragerc

@@ -11,3 +11,4 @@ exclude_lines =
     pragma: no cover
     if __name__ == ['"]__main__['"]:
     assert False
+    self.fail

doc/reference/storages.rst

@@ -131,6 +131,7 @@ Demo storages are configured using the ``demostorage`` section::
    'base.fs'
    >>> storage.changes.getName()
    'Changes'
+   >>> storage.close()
 
 ``demostorage`` sections can contain up to 2 storage subsections,
 named ``base`` and ``changes``, specifying the demo storage's base and

src/ZODB/scripts/repozo.py

@@ -667,9 +667,11 @@ def do_recover(options):
     repofiles = find_files(options)
     if not repofiles:
         if options.date:
-            raise NoFiles('No files in repository before %s', options.date)
+            raise NoFiles('No files in repository before %s' % (options.date,))
         else:
             raise NoFiles('No files in repository')
+
+    files_to_close = ()
     if options.output is None:
         log('Recovering file to stdout')
         outfp = sys.stdout
@@ -682,51 +684,56 @@ def do_recover(options):
         log('Recovering file to %s', options.output)
         temporary_output_file = options.output + '.part'
         outfp = open(temporary_output_file, 'wb')
-    if options.withverify:
-        datfile = os.path.splitext(repofiles[0])[0] + '.dat'
-        with open(datfile) as fp:
-            truth_dict = {}
-            for line in fp:
-                fn, startpos, endpos, sum = line.split()
-                startpos = int(startpos)
-                endpos = int(endpos)
-                filename = os.path.join(options.repository,
-                                        os.path.basename(fn))
-                truth_dict[filename] = {
-                    'size': endpos - startpos,
-                    'sum': sum,
-                }
-        totalsz = 0
-        for repofile in repofiles:
-            reposz, reposum = concat([repofile], outfp)
-            expected_truth = truth_dict[repofile]
-            if reposz != expected_truth['size']:
-                raise VerificationFail(
-                    "%s is %d bytes, should be %d bytes" % (
-                        repofile, reposz, expected_truth['size']))
-            if reposum != expected_truth['sum']:
-                raise VerificationFail(
-                    "%s has checksum %s instead of %s" % (
-                        repofile, reposum, expected_truth['sum']))
-            totalsz += reposz
-            log("Recovered chunk %s : %s bytes, md5: %s", repofile, reposz, reposum)
-        log("Recovered a total of %s bytes", totalsz)
-    else:
-        reposz, reposum = concat(repofiles, outfp)
-        log('Recovered %s bytes, md5: %s', reposz, reposum)
+        files_to_close += (outfp,)
 
-    if options.output is not None:
-        last_base = os.path.splitext(repofiles[-1])[0]
-        source_index = '%s.index' % last_base
-        target_index = '%s.index' % options.output
-        if os.path.exists(source_index):
-            log('Restoring index file %s to %s', source_index, target_index)
-            shutil.copyfile(source_index, target_index)
+    try:
+        if options.withverify:
+            datfile = os.path.splitext(repofiles[0])[0] + '.dat'
+            with open(datfile) as fp:
+                truth_dict = {}
+                for line in fp:
+                    fn, startpos, endpos, sum = line.split()
+                    startpos = int(startpos)
+                    endpos = int(endpos)
+                    filename = os.path.join(options.repository,
+                                            os.path.basename(fn))
+                    truth_dict[filename] = {
+                        'size': endpos - startpos,
+                        'sum': sum,
+                    }
+            totalsz = 0
+            for repofile in repofiles:
+                reposz, reposum = concat([repofile], outfp)
+                expected_truth = truth_dict[repofile]
+                if reposz != expected_truth['size']:
+                    raise VerificationFail(
+                        "%s is %d bytes, should be %d bytes" % (
+                            repofile, reposz, expected_truth['size']))
+                if reposum != expected_truth['sum']:
+                    raise VerificationFail(
+                        "%s has checksum %s instead of %s" % (
+                            repofile, reposum, expected_truth['sum']))
+                totalsz += reposz
+                log("Recovered chunk %s : %s bytes, md5: %s", repofile, reposz, reposum)
+            log("Recovered a total of %s bytes", totalsz)
         else:
-            log('No index file to restore: %s', source_index)
+            reposz, reposum = concat(repofiles, outfp)
+            log('Recovered %s bytes, md5: %s', reposz, reposum)
 
-    if outfp != sys.stdout:
-        outfp.close()
+        if options.output is not None:
+            last_base = os.path.splitext(repofiles[-1])[0]
+            source_index = '%s.index' % last_base
+            target_index = '%s.index' % options.output
+            if os.path.exists(source_index):
+                log('Restoring index file %s to %s', source_index, target_index)
+                shutil.copyfile(source_index, target_index)
+            else:
+                log('No index file to restore: %s', source_index)
+    finally:
+        for f in files_to_close:
+            f.close()
+
+    if options.output is not None:
         try:
             os.rename(temporary_output_file, options.output)
         except OSError:

src/ZODB/tests/testThreadedShutdown.py

@@ -7,6 +7,8 @@ import ZODB
 
 class ZODBClientThread(threading.Thread):
 
+    sleep_time = 15
+
     def __init__(self, db, test):
         threading.Thread.__init__(self)
         self._exc_info = None
@@ -19,7 +21,7 @@ class ZODBClientThread(threading.Thread):
         conn = self.db.open()
         conn.sync()
         self.event.set()
-        time.sleep(15)
+        time.sleep(self.sleep_time)
 
         # conn.close calls self.transaction_manager.unregisterSynch(self)
         # and this succeeds.
@@ -37,6 +39,9 @@ class ShutdownTest(ZODB.tests.util.TestCase):
             'ZODBTests.fs', create=1)
         self._db = ZODB.DB(self._storage)
 
+    def tearDown(self):
+        ZODB.tests.util.TestCase.tearDown(self)
+
     def check_shutdown(self):
         client_thread = ZODBClientThread(self._db, self)
         client_thread.start()
@@ -47,8 +52,11 @@ class ShutdownTest(ZODB.tests.util.TestCase):
         # have different contents.
         self._db.close()
 
-    def tearDown(self):
-        ZODB.tests.util.TestCase.tearDown(self)
+        # Be sure not to 'leak' the running thread; if it hasn't
+        # finished after this, something went very wrong
+        client_thread.join(client_thread.sleep_time + 10)
+        if client_thread.is_alive():
+            self.fail("Client thread failed to close connection")
 
 
 def test_suite():