Improve fsck test coverage to 100%

nilmdb/fsck/fsck.py

@@ -31,9 +31,7 @@ class FsckError(Exception):
 
 
 class FixableFsckError(FsckError):
-    def __init__(self, msg="", *args):
-        if args:
-            msg = sprintf(msg, *args)
+    def __init__(self, msg=""):
         FsckError.__init__(self, f'{msg}\nThis may be fixable with "--fix".')
 
 
@@ -59,7 +57,8 @@ def retry_if_raised(exc, message=None, max_retries=100):
                 except exc:
                     if message:
                         log("%s\n\n", message)
-            raise Exception("Max number of retries (%d) exceeded; giving up")
+            raise Exception("Max number of retries (%d) exceeded; giving up" %
+                            max_retries)
         return f2
     return f1
 
@@ -73,8 +72,7 @@ class Progress(object):
             widgets=[progressbar.Percentage(), ' ',
                      progressbar.Bar(), ' ',
                      progressbar.ETA()])
-        if self.bar.term_width == 0:
-            self.bar.term_width = 75
+        self.bar.term_width = self.bar.term_width or 75
 
     def __enter__(self):
         self.bar.start()
@@ -117,7 +115,9 @@ class Fsck(object):
         finally:
             if self.bulk:
                 self.bulk.close()
-            if self.sql:
+            if self.sql:  # pragma: no cover
+                # (coverage doesn't handle finally clauses correctly;
+                # both branches here are tested)
                 self.sql.commit()
                 self.sql.close()
         log("ok\n")
@@ -243,15 +243,14 @@ class Fsck(object):
 
                 # Check that we can open bulkdata
                 try:
-                    tab = None
-                    try:
-                        tab = nilmdb.server.bulkdata.Table(bulk)
-                    except Exception as e:
-                        raise FsckError("%s: can't open bulkdata: %s",
-                                        path, str(e))
-                finally:
-                    if tab:
-                        tab.close()
+                    tab = nilmdb.server.bulkdata.Table(bulk)
+                except Exception as e:  # pragma: no cover --
+                    # No coverage here because, in the current code,
+                    # everything that would cause the bulkdata to fail
+                    # has been already checked.
+                    raise FsckError("%s: can't open bulkdata: %s",
+                                    path, str(e))
+                tab.close()
 
     ### Check that bulkdata is good enough to be opened
 
@@ -262,9 +261,9 @@ class Fsck(object):
         if fmt["version"] != 3:
             raise FsckError("%s: bad or unsupported bulkdata version %d",
                             path, fmt["version"])
-        row_per_file = int(fmt["rows_per_file"])
-        if row_per_file < 1:
-            raise FsckError(f"{path}: bad row_per_file {row_per_file}")
+        rows_per_file = int(fmt["rows_per_file"])
+        if rows_per_file < 1:
+            raise FsckError(f"{path}: bad rows_per_file {rows_per_file}")
         files_per_dir = int(fmt["files_per_dir"])
         if files_per_dir < 1:
             raise FsckError(f"{path}: bad files_per_dir {files_per_dir}")
@@ -288,7 +287,7 @@ class Fsck(object):
             files = list(filter(regex.search, os.listdir(subpath)))
             if not files:
                 self.fix_empty_subdir(subpath)
-                raise RetryFsck
+                raise RetryFsck  # pragma: no cover; raised by fix_empty_subdir
             # Verify that their size is a multiple of the row size
             for filename in files:
                 filepath = os.path.join(subpath, filename)
@@ -304,10 +303,11 @@ class Fsck(object):
         # as long as it's only ".removed" files.
         err("\n%s\n", msg)
         for fn in os.listdir(subpath):
-            if not fn.endswith(".removed"):
+            if not fn.endswith(b".removed"):
                 raise FsckError("can't fix automatically: please manually "
-                                "remove the file %s and try again",
-                                os.path.join(subpath, fn))
+                                "remove the file '%s' and try again",
+                                os.path.join(subpath, fn).decode(
+                                    'utf-8', errors='backslashreplace'))
         # Remove the whole thing
         err("Removing empty subpath\n")
         shutil.rmtree(subpath)
@@ -364,7 +364,7 @@ class Fsck(object):
                 erow = tab[epos-1]  # noqa: F841 unused
             except Exception as e:
                 self.fix_bad_interval(sid, intv, tab, str(e))
-                raise RetryFsck
+                raise RetryFsck  # pragma: no cover; raised by fix_bad_interval
         return len(ints)
 
     def fix_bad_interval(self, sid, intv, tab, msg):
@@ -393,10 +393,10 @@ class Fsck(object):
                                 "end_time=? AND start_pos=? AND end_pos=?",
                                 (new_etime, new_epos, sid, stime, etime,
                                  spos, epos))
-                    if cur.rowcount != 1:
+                    if cur.rowcount != 1:  # pragma: no cover (shouldn't fail)
                         raise FsckError("failed to fix SQL database")
                 raise RetryFsck
-            err("actually it can't be truncated; times are bad too")
+            err("actually it can't be truncated; times are bad too\n")
 
         # Otherwise, the only hope is to delete the interval entirely.
         err("*** Deleting the entire interval from SQL.\n")
@@ -409,7 +409,7 @@ class Fsck(object):
                         "stream_id=? AND start_time=? AND "
                         "end_time=? AND start_pos=? AND end_pos=?",
                         (sid, stime, etime, spos, epos))
-            if cur.rowcount != 1:
+            if cur.rowcount != 1:  # pragma: no cover (shouldn't fail)
                 raise FsckError("failed to remove interval")
         raise RetryFsck
 
@@ -438,7 +438,7 @@ class Fsck(object):
     def check_table_data(self, sid, ints, tab, update):
         # Pull out all of the interval's data and verify that it's
         # monotonic.
-        maxrows = 100000
+        maxrows = getattr(self, 'maxrows_override', 100000)
         path = self.stream_path[sid]
         layout = self.stream_layout[sid]
         dtype = nilmdb.client.numpyclient.layout_to_dtype(layout)
@@ -459,8 +459,11 @@ class Fsck(object):
                 # Get raw data, convert to NumPy arary
                 try:
                     raw = tab.get_data(start, stop, binary=True)
-                    data = numpy.fromstring(raw, dtype)
-                except Exception as e:
+                    data = numpy.frombuffer(raw, dtype)
+                except Exception as e:  # pragma: no cover
+                    # No coverage because it's hard to trigger this -- earlier
+                    # checks check the ranges, so this would probably be a real
+                    # disk error, malloc failure, etc.
                     raise FsckError(
                         "%s: failed to grab rows %d through %d: %s",
                         path, start, stop, repr(e))
@@ -470,12 +473,14 @@ class Fsck(object):
                     raise FsckError("%s: non-monotonic timestamp(s) in rows "
                                     "%d through %d", path, start, stop)
                 first_ts = data['timestamp'][0]
+                print("first_ts", first_ts, "last_ts", last_ts)
                 if last_ts is not None and first_ts <= last_ts:
                     raise FsckError("%s: first interval timestamp %d is not "
                                     "greater than the previous last interval "
                                     "timestamp %d, at row %d",
                                     path, first_ts, last_ts, start)
                 last_ts = data['timestamp'][-1]
+                print("last_ts", last_ts)
 
                 # These are probably fixable, by removing the offending
                 # intervals.  But I'm not going to bother implementing

tests/test_fsck.py

@@ -122,6 +122,7 @@ class TestFsck(object):
         self.failmsg("test2i", "bad bulkdata table")
         self.failmsg("test2j", "overlap in intervals")
         self.failmsg("test2k", "overlap in file offsets")
+        self.ok("test2k1")
         self.failmsg("test2l", "unsupported bulkdata version")
         self.failmsg("test2m", "bad rows_per_file")
         self.failmsg("test2n", "bad files_per_dir")
@@ -134,4 +135,26 @@ class TestFsck(object):
         self.failmsg("test2q", "extra bytes present", fix=False)
         self.okmsg("test2q", "Truncating file")
 
+        self.failmsg("test2r", "error accessing rows", fix=False)
+        self.okmsg("test2r", "end position is past endrows")
 
+        self.okmsg("test2r1", "actually it can't be truncated")
+        self.contain("Deleting the entire interval")
+        self.contain("restarting fsck")
+        self.ok("test2r2")
+
+        self.failmsg("test2s", "non-monotonic timestamp(s)")
+
+        def check_small_maxrows(f):
+            f.maxrows_override = 1
+            f.check()
+        self.fail("test2t", checker=check_small_maxrows)
+        self.contain("first interval timestamp 0 is not greater")
+
+        self.ok("test2t", skip=True)
+
+        @nilmdb.fsck.fsck.retry_if_raised(Exception, max_retries=3)
+        def foo():
+            raise Exception("hi")
+        with assert_raises(Exception):
+            foo()