repozo: exit with non-zero code in case of verification failure

When verification failed, repozo prints a message on standard error, but the program always exits with a return code indicating a success. In case of error it's more natural to exit with an error return code.
zopefoundation · Feb 1, 2024 · 802fd5f · 802fd5f
1 parent 891a8a6
commit 802fd5f
Show file tree

Hide file tree

Showing 3 changed files with 67 additions and 48 deletions.
diff --git a/CHANGES.rst b/CHANGES.rst
@@ -11,6 +11,8 @@
 
 - Fix sorting issue in ``scripts/space.py``.
 
+- Fix exit code of ``repozo`` script in case of verification error.
+  For details see `#396 <https://github.com/zopefoundation/ZODB/pull/396>`_.
 
 5.8.1 (2023-07-18)
 ==================

diff --git a/src/ZODB/scripts/repozo.py b/src/ZODB/scripts/repozo.py
@@ -780,15 +780,16 @@ def do_verify(options):
                         filename, options.quick)
                     when_uncompressed = ''
             except OSError:
-                error("%s is missing", filename)
-                continue
+                raise VerificationFail("%s is missing" % filename)
             if size != expected_size:
-                error("%s is %d bytes%s, should be %d bytes", filename,
-                      size, when_uncompressed, expected_size)
+                raise VerificationFail(
+                    "%s is %d bytes%s, should be %d bytes" % (
+                        filename, size, when_uncompressed, expected_size))
             elif not options.quick:
                 if actual_sum != sum:
-                    error("%s has checksum %s%s instead of %s", filename,
-                          actual_sum, when_uncompressed, sum)
+                    raise VerificationFail(
+                        "%s has checksum %s%s instead of %s" % (
+                            filename, actual_sum, when_uncompressed, sum))
 
 
 def get_checksum_and_size_of_gzipped_file(filename, quick):

diff --git a/src/ZODB/scripts/tests/test_repozo.py b/src/ZODB/scripts/tests/test_repozo.py
@@ -998,18 +998,8 @@ def test_w_incr_backup_with_verify_size_inconsistent(self):
 class Test_do_verify(OptionsTestBase, unittest.TestCase):
 
     def _callFUT(self, options):
-        from ZODB.scripts import repozo
-        errors = []
-        orig_error = repozo.error
-
-        def _error(msg, *args):
-            errors.append(msg % args)
-        repozo.error = _error
-        try:
-            repozo.do_verify(options)
-            return errors
-        finally:
-            repozo.error = orig_error
+        from ZODB.scripts.repozo import do_verify
+        do_verify(options)
 
     def _makeFile(self, hour, min, sec, ext, text=None):
         from ZODB.scripts.repozo import _GzipCloser
@@ -1040,7 +1030,7 @@ def test_all_is_fine(self):
             2, 3, 4, '.dat',
             '/backup/2010-05-14-02-03-04.fs 0 3 e1faffb3e614e6c2fba74296962386b7\n'  # noqa: E501 line too long
             '/backup/2010-05-14-04-05-06.deltafs 3 7 f50881ced34c7d9e6bce100bf33dec60\n')  # noqa: E501 line too long
-        self.assertEqual(self._callFUT(options), [])
+        self._callFUT(options)
 
     def test_all_is_fine_gzip(self):
         options = self._makeOptions(quick=False)
@@ -1050,85 +1040,111 @@ def test_all_is_fine_gzip(self):
             2, 3, 4, '.dat',
             '/backup/2010-05-14-02-03-04.fsz 0 3 e1faffb3e614e6c2fba74296962386b7\n'  # noqa: E501 line too long
             '/backup/2010-05-14-04-05-06.deltafsz 3 7 f50881ced34c7d9e6bce100bf33dec60\n')  # noqa: E501 line too long
-        self.assertEqual(self._callFUT(options), [])
+        self._callFUT(options)
 
     def test_missing_file(self):
+        from ZODB.scripts.repozo import VerificationFail
         options = self._makeOptions(quick=True)
         self._makeFile(2, 3, 4, '.fs', 'AAA')
         self._makeFile(
             2, 3, 4, '.dat',
             '/backup/2010-05-14-02-03-04.fs 0 3 e1faffb3e614e6c2fba74296962386b7\n'  # noqa: E501 line too long
             '/backup/2010-05-14-04-05-06.deltafs 3 7 f50881ced34c7d9e6bce100bf33dec60\n')  # noqa: E501 line too long
-        self.assertEqual(self._callFUT(options),
-                         [options.repository + os.path.sep +
-                          '2010-05-14-04-05-06.deltafs is missing'])
+        self.assertRaisesRegex(
+            VerificationFail,
+            '2010-05-14-04-05-06.deltafs is missing',
+            self._callFUT,
+            options,
+        )
 
     def test_missing_file_gzip(self):
+        from ZODB.scripts.repozo import VerificationFail
         options = self._makeOptions(quick=True)
         self._makeFile(2, 3, 4, '.fsz', 'AAA')
         self._makeFile(
             2, 3, 4, '.dat',
             '/backup/2010-05-14-02-03-04.fsz 0 3 e1faffb3e614e6c2fba74296962386b7\n'  # noqa: E501 line too long
             '/backup/2010-05-14-04-05-06.deltafsz 3 7 f50881ced34c7d9e6bce100bf33dec60\n')  # noqa: E501 line too long
-        self.assertEqual(self._callFUT(options),
-                         [options.repository + os.path.sep +
-                          '2010-05-14-04-05-06.deltafsz is missing'])
+        self.assertRaisesRegex(
+            VerificationFail,
+            '2010-05-14-04-05-06.deltafsz is missing',
+            self._callFUT,
+            options,
+        )
 
     def test_bad_size(self):
+        from ZODB.scripts.repozo import VerificationFail
         options = self._makeOptions(quick=False)
         self._makeFile(2, 3, 4, '.fs', 'AAA')
         self._makeFile(4, 5, 6, '.deltafs', 'BBB')
         self._makeFile(
             2, 3, 4, '.dat',
             '/backup/2010-05-14-02-03-04.fs 0 3 e1faffb3e614e6c2fba74296962386b7\n'  # noqa: E501 line too long
             '/backup/2010-05-14-04-05-06.deltafs 3 7 f50881ced34c7d9e6bce100bf33dec60\n')  # noqa: E501 line too long
-        self.assertEqual(self._callFUT(options),
-                         [options.repository + os.path.sep +
-                          '2010-05-14-04-05-06.deltafs is 3 bytes,'
-                          ' should be 4 bytes'])
+        self.assertRaisesRegex(
+            VerificationFail,
+            '2010-05-14-04-05-06.deltafs is 3 bytes, should be 4 bytes',
+            self._callFUT,
+            options,
+        )
 
     def test_bad_size_gzip(self):
+        from ZODB.scripts.repozo import VerificationFail
         options = self._makeOptions(quick=False)
         self._makeFile(2, 3, 4, '.fsz', 'AAA')
         self._makeFile(4, 5, 6, '.deltafsz', 'BBB')
         self._makeFile(
             2, 3, 4, '.dat',
             '/backup/2010-05-14-02-03-04.fsz 0 3 e1faffb3e614e6c2fba74296962386b7\n'   # noqa: E501 line too long
             '/backup/2010-05-14-04-05-06.deltafsz 3 7 f50881ced34c7d9e6bce100bf33dec60\n')  # noqa: E501 line too long
-        self.assertEqual(
-            self._callFUT(options),
-            [options.repository + os.path.sep +
-             '2010-05-14-04-05-06.deltafsz is 3 bytes (when uncompressed),'
-             ' should be 4 bytes'])
+        self.assertRaisesRegex(
+            VerificationFail,
+            (
+                '2010-05-14-04-05-06.deltafsz is 3 bytes'
+                r' \(when uncompressed\), should be 4 bytes'
+            ),
+            self._callFUT,
+            options,
+        )
 
     def test_bad_checksum(self):
+        from ZODB.scripts.repozo import VerificationFail
         options = self._makeOptions(quick=False)
         self._makeFile(2, 3, 4, '.fs', 'AAA')
         self._makeFile(4, 5, 6, '.deltafs', 'BbBB')
         self._makeFile(
             2, 3, 4, '.dat',
             '/backup/2010-05-14-02-03-04.fs 0 3 e1faffb3e614e6c2fba74296962386b7\n'  # noqa: E501 line too long
             '/backup/2010-05-14-04-05-06.deltafs 3 7 f50881ced34c7d9e6bce100bf33dec60\n')  # noqa: E501 line too long
-        self.assertEqual(self._callFUT(options),
-                         [options.repository + os.path.sep +
-                          '2010-05-14-04-05-06.deltafs has checksum'
-                          ' 36486440db255f0ee6ab109d5d231406 instead of'
-                          ' f50881ced34c7d9e6bce100bf33dec60'])
+        self.assertRaisesRegex(
+            VerificationFail,
+            (
+                '2010-05-14-04-05-06.deltafs has checksum'
+                + ' 36486440db255f0ee6ab109d5d231406 instead of'
+                + ' f50881ced34c7d9e6bce100bf33dec60'
+            ),
+            self._callFUT,
+            options,
+        )
 
     def test_bad_checksum_gzip(self):
+        from ZODB.scripts.repozo import VerificationFail
         options = self._makeOptions(quick=False)
         self._makeFile(2, 3, 4, '.fsz', 'AAA')
         self._makeFile(4, 5, 6, '.deltafsz', 'BbBB')
         self._makeFile(
             2, 3, 4, '.dat',
             '/backup/2010-05-14-02-03-04.fsz 0 3 e1faffb3e614e6c2fba74296962386b7\n'  # noqa: E501 line too long
             '/backup/2010-05-14-04-05-06.deltafsz 3 7 f50881ced34c7d9e6bce100bf33dec60\n')  # noqa: E501 line too long
-        self.assertEqual(
-            self._callFUT(options),
-            [options.repository + os.path.sep +
-             '2010-05-14-04-05-06.deltafsz has checksum'
-             ' 36486440db255f0ee6ab109d5d231406 (when uncompressed) instead of'
-             ' f50881ced34c7d9e6bce100bf33dec60'])
+        self.assertRaisesRegex(
+            VerificationFail,
+            (
+                '2010-05-14-04-05-06.deltafsz has checksum'
+                + r' 36486440db255f0ee6ab109d5d231406 \(when uncompressed\)'
+                + ' instead of f50881ced34c7d9e6bce100bf33dec60'),
+            self._callFUT,
+            options,
+        )
 
     def test_quick_ignores_checksums(self):
         options = self._makeOptions(quick=True)
@@ -1138,7 +1154,7 @@ def test_quick_ignores_checksums(self):
                 2, 3, 4, '.dat',
                 '/backup/2010-05-14-02-03-04.fs 0 3 aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\n'  # noqa: E501 line too long
                 '/backup/2010-05-14-04-05-06.deltafs 3 7 bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb\n')  # noqa: E501 line too long
-        self.assertEqual(self._callFUT(options), [])
+        self._callFUT(options)
 
     def test_quick_ignores_checksums_gzip(self):
         options = self._makeOptions(quick=True)
@@ -1148,7 +1164,7 @@ def test_quick_ignores_checksums_gzip(self):
             2, 3, 4, '.dat',
             '/backup/2010-05-14-02-03-04.fsz 0 3 aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\n'  # noqa: E501 line too long
             '/backup/2010-05-14-04-05-06.deltafsz 3 7 bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb\n')  # noqa: E501 line too long
-        self.assertEqual(self._callFUT(options), [])
+        self._callFUT(options)
 
 
 class MonteCarloTests(unittest.TestCase):