From 8fe980c94e911d55631df949d33fa6b49f051cc6 Mon Sep 17 00:00:00 2001 From: Thomas Waldmann Date: Sat, 18 Feb 2023 13:57:57 +0100 Subject: [PATCH 1/2] "auto" compressor tests: don't assume a specific size, fixes #7363 The tests assumed a specific compressed results size, which is bad, because it might vary depending on the zlib implementation. Now the "auto" compressor tests just check if it is the same size as when unconditionally using the zlib compressor. --- src/borg/testsuite/archiver.py | 27 +++++++++++++++++---------- 1 file changed, 17 insertions(+), 10 deletions(-) diff --git a/src/borg/testsuite/archiver.py b/src/borg/testsuite/archiver.py index ee5b849bb..261163a11 100644 --- a/src/borg/testsuite/archiver.py +++ b/src/borg/testsuite/archiver.py @@ -2521,21 +2521,11 @@ class ArchiverTestCase(ArchiverTestCaseBase): def test_compression_zlib_compressible(self): size, csize = self._get_sizes('zlib', compressible=True) assert csize < size * 0.1 - assert csize == 35 def test_compression_zlib_uncompressible(self): size, csize = self._get_sizes('zlib', compressible=False) assert csize >= size - def test_compression_auto_compressible(self): - size, csize = self._get_sizes('auto,zlib', compressible=True) - assert csize < size * 0.1 - assert csize == 35 # same as compression 'zlib' - - def test_compression_auto_uncompressible(self): - size, csize = self._get_sizes('auto,zlib', compressible=False) - assert csize == size + 3 # same as compression 'none' - def test_compression_lz4_compressible(self): size, csize = self._get_sizes('lz4', compressible=True) assert csize < size * 0.1 @@ -2560,6 +2550,23 @@ class ArchiverTestCase(ArchiverTestCaseBase): size, csize = self._get_sizes('zstd', compressible=False) assert csize == size + 3 # same as compression 'none' + def test_compression_auto_compressible(self): + # this is testing whether the "auto" meta-compressor chooses the "expensive" zlib compression + # if it detects that the data is very compressible (and not "lz4" nor "none" compression). + auto_size, auto_csize = self._get_sizes('auto,zlib', compressible=True) + self.cmd('delete', self.repository_location) + zlib_size, zlib_csize = self._get_sizes('zlib', compressible=True) + assert auto_size == zlib_size + assert auto_csize < auto_size * 0.1 # it did compress! + assert auto_csize == zlib_csize # looking at the result size, it seems to be zlib compressed + + def test_compression_auto_uncompressible(self): + # this is testing whether the "auto" meta-compressor chooses the "none" compression (storing the + # data "as is" with just the 3 bytes header) if all else would result in something bigger. + size, csize = self._get_sizes('auto,zlib', compressible=False) + assert csize >= size + assert csize == size + 3 # same as compression 'none' + def test_change_passphrase(self): self.cmd('init', '--encryption=repokey', self.repository_location) os.environ['BORG_NEW_PASSPHRASE'] = 'newpassphrase' From 8cf313900a7b6ccd44a0b7ab43ad94bc1893e139 Mon Sep 17 00:00:00 2001 From: Thomas Waldmann Date: Thu, 23 Feb 2023 02:30:52 +0100 Subject: [PATCH 2/2] "auto" compressor tests: do not assume zlib is better than lz4, fixes #7363 while that might be true for many cases, we can not assume it is always true, as proven by the failing test on S390. --- src/borg/testsuite/archiver.py | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/src/borg/testsuite/archiver.py b/src/borg/testsuite/archiver.py index 261163a11..ee968acb8 100644 --- a/src/borg/testsuite/archiver.py +++ b/src/borg/testsuite/archiver.py @@ -2551,14 +2551,19 @@ class ArchiverTestCase(ArchiverTestCaseBase): assert csize == size + 3 # same as compression 'none' def test_compression_auto_compressible(self): - # this is testing whether the "auto" meta-compressor chooses the "expensive" zlib compression - # if it detects that the data is very compressible (and not "lz4" nor "none" compression). + # this is testing whether the "auto" meta-compressor behaves as expected: + # - it checks whether the data is compressible (detector is the lz4 compressor) + # - as the data is compressible, it runs the "expensive" zlib compression on it + # - it returns whatever is shortest, either the lz4 compressed data or the zlib compressed data. auto_size, auto_csize = self._get_sizes('auto,zlib', compressible=True) self.cmd('delete', self.repository_location) zlib_size, zlib_csize = self._get_sizes('zlib', compressible=True) - assert auto_size == zlib_size + self.cmd('delete', self.repository_location) + lz4_size, lz4_csize = self._get_sizes('lz4', compressible=True) + assert auto_size == zlib_size == lz4_size assert auto_csize < auto_size * 0.1 # it did compress! - assert auto_csize == zlib_csize # looking at the result size, it seems to be zlib compressed + smallest_csize = min(zlib_csize, lz4_csize) + assert auto_csize == smallest_csize def test_compression_auto_uncompressible(self): # this is testing whether the "auto" meta-compressor chooses the "none" compression (storing the