Fill out code coverage for nilmdb/server/bulkdata.py

nilmdb/server/bulkdata.py

@@ -34,19 +34,19 @@ class BulkData(object):
         self.lockfile = None
 
         # Tuneables
-        if "file_size" in kwargs:
+        if "file_size" in kwargs and kwargs["file_size"] is not None:
             self.file_size = kwargs["file_size"]
         else:
             # Default to approximately 128 MiB per file
             self.file_size = 128 * 1024 * 1024
 
-        if "files_per_dir" in kwargs:
+        if "files_per_dir" in kwargs and kwargs["files_per_dir"] is not None:
             self.files_per_dir = kwargs["files_per_dir"]
         else:
             # 32768 files per dir should work even on FAT32
             self.files_per_dir = 32768
 
-        if "initial_nrows" in kwargs:
+        if "initial_nrows" in kwargs and kwargs["initial_nrows"] is not None:
             self.initial_nrows = kwargs["initial_nrows"]
         else:
             # First row is 0
@@ -70,7 +70,7 @@ class BulkData(object):
             self.lockfile.close()
             try:
                 os.unlink(self.lock)
-            except OSError: # pragma: no cover
+            except OSError:
                 pass
             self.lockfile = None
 
@@ -130,14 +130,10 @@ class BulkData(object):
                     os.mkdir(ospath)
                     made_dirs.append(ospath)
         except Exception as e:
-            # Try to remove paths that we created; ignore errors
-            exc_info = sys.exc_info()
-            for ospath in reversed(made_dirs): # pragma: no cover (hard to hit)
-                try:
-                    os.rmdir(ospath)
-                except OSError:
-                    pass
-            raise exc_info[1].with_traceback(exc_info[2])
+            # Remove paths that we created
+            for ospath in reversed(made_dirs):
+                os.rmdir(ospath)
+            raise
 
         return elements
 
@@ -310,7 +306,7 @@ class Table(object):
         with open(os.path.join(self.root, b"_format"), "rb") as f:
             fmt = pickle.load(f)
 
-        if fmt["version"] != 3: # pragma: no cover
+        if fmt["version"] != 3:
             # Old versions used floating point timestamps, which aren't
             # valid anymore.
             raise NotImplementedError("old version " + str(fmt["version"]) +
@@ -353,7 +349,7 @@ class Table(object):
             # Now find the last file in that dir
             path = os.path.join(self.root, subdir)
             files = list(filter(regex.search, os.listdir(path)))
-            if not files: # pragma: no cover (shouldn't occur)
+            if not files:
                 # Empty dir: try the next one
                 continue
 
@@ -393,7 +389,7 @@ class Table(object):
     def _row_from_offset(self, subdir, filename, offset):
         """Return the row number that corresponds to the given
         'subdir/filename' and byte-offset within that file."""
-        if (offset % self.row_size) != 0: # pragma: no cover
+        if (offset % self.row_size) != 0:
             # this shouldn't occur, unless there is some corruption somewhere
             raise ValueError("file offset is not a multiple of data size")
         filenum = int(subdir, 16) * self.files_per_dir + int(filename, 16)

tests/test_bulkdata.py

@@ -5,6 +5,8 @@ from nilmdb.utils.printf import *
 from nose.tools import *
 from nose.tools import assert_raises
 import itertools
+import errno
+import pickle
 
 from testutil.helpers import *
 
@@ -16,19 +18,26 @@ from nilmdb.server.bulkdata import BulkData
 class TestBulkData(object):
 
     def test_bulkdata(self):
-        for (size, files, db) in [ ( 0, 0, testdb ),
+        for (size, files, db) in [ ( None, None, testdb ),
                                        ( 25, 1000, testdb ),
                                        ( 1000, 3, testdb.decode("utf-8") ) ]:
             recursive_unlink(db)
             os.mkdir(db)
             self.do_basic(db, size, files)
 
+    def test_corruption(self):
+        db = testdb
+        recursive_unlink(db)
+        os.mkdir(db)
+
+        # Remove lock before close
+        data = BulkData(db)
+        os.unlink(data.lock)
+        data.close()
+
     def do_basic(self, db, size, files):
         """Do the basic test with variable file_size and files_per_dir"""
-        if not size or not files:
-            data = BulkData(db)
-        else:
-            data = BulkData(db, file_size = size, files_per_dir = files)
+        data = BulkData(db, file_size = size, files_per_dir = files)
 
         # Try opening it again (should result in locking error)
         with assert_raises(IOError) as e:
@@ -42,11 +51,18 @@ class TestBulkData(object):
             data.create("foo/bar", "uint16_8")
         data.create("/foo/bar", "uint16_8")
         data.create("/foo/baz/quux", "float64_16")
-        with assert_raises(ValueError):
+        with assert_raises(ValueError) as e:
             data.create("/foo/bar/baz", "uint16_8")
+        in_("path is subdir of existing node", str(e.exception))
         with assert_raises(ValueError):
             data.create("/foo/baz", "float64_16")
 
+        # filename too long (tests error paths in _create_parents)
+        with assert_raises(OSError) as e:
+            data.create("/test/long/" + "a"*10000 + "/foo", "int32_1")
+        eq_(e.exception.errno, errno.ENAMETOOLONG)
+
+
         # get node -- see if caching works
         nodes = []
         for i in range(5000):
@@ -95,14 +111,37 @@ class TestBulkData(object):
             node.remove(9999,9998)
 
         # close, reopen
-        # reopen
         data.close()
-        if not size or not files:
-            data = BulkData(db)
-        else:
-            data = BulkData(db, file_size = size, files_per_dir = files)
+        data = BulkData(db, file_size = size, files_per_dir = files)
         node = data.getnode("/foo/bar")
 
+        # make an empty dir that will get ignored by _get_nrows
+        data.close()
+        os.mkdir(os.path.join(testdb, b"data/foo/bar/0123"))
+        data = BulkData(db, file_size = size, files_per_dir = files)
+        node = data.getnode("/foo/bar")
+
+        # make a corrupted file that's the wrong size
+        data.close()
+        with open(os.path.join(testdb, b"data/foo/bar/0123/0123"), "wb") as f:
+            f.write(b"x"*17)
+        data = BulkData(db, file_size = size, files_per_dir = files)
+        with assert_raises(ValueError) as e:
+            node = data.getnode("/foo/bar")
+        in_("file offset is not a multiple of data size", str(e.exception))
+
+        # mess with format
+        data.close()
+        with open(os.path.join(testdb, b"data/foo/bar/_format"), "rb") as f:
+            fmt = pickle.load(f)
+        fmt["version"] = 2
+        with open(os.path.join(testdb, b"data/foo/bar/_format"), "wb") as f:
+            pickle.dump(fmt, f, 2)
+        data = BulkData(db, file_size = size, files_per_dir = files)
+        with assert_raises(NotImplementedError) as e:
+            node = data.getnode("/foo/bar")
+        in_("old version 2 bulk data store is not supported", str(e.exception))
+
         # Extract slices
         for s in misc_slices:
             eq_(get_node_slice(s), raw[s])