From d7f9eae966d30554982c737efd72aeb4f3231cda Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9rome=20Perrin?= Date: Tue, 6 Feb 2024 13:08:57 +0900 Subject: [PATCH] repozo: exit with non-zero code in case of verification failure (#396) 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. --- CHANGES.rst | 3 + src/ZODB/scripts/repozo.py | 13 ++-- src/ZODB/scripts/tests/test_repozo.py | 100 +++++++++++++++----------- 3 files changed, 68 insertions(+), 48 deletions(-) diff --git a/CHANGES.rst b/CHANGES.rst index 3c8689abe..5928eaf32 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -11,6 +11,9 @@ - Fix sorting issue in ``scripts/space.py``. +- Fix exit code of ``repozo`` script in case of verification error. + For details see `#396 `_. + 5.8.1 (2023-07-18) ================== diff --git a/src/ZODB/scripts/repozo.py b/src/ZODB/scripts/repozo.py index 946dc9e03..aa7b82490 100755 --- 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 index 6120e9181..f19c81703 100644 --- 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,31 +1040,40 @@ 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') @@ -1082,12 +1081,15 @@ def test_bad_size(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), - [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') @@ -1095,13 +1097,18 @@ def test_bad_size_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), - [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') @@ -1109,13 +1116,19 @@ def test_bad_checksum(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), - [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') @@ -1123,12 +1136,15 @@ def test_bad_checksum_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), - [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):