Skip to content

Commit

Permalink
Reword warning and refactor tests
Browse files Browse the repository at this point in the history
  • Loading branch information
didiermichel committed Oct 2, 2023
1 parent 5811ec6 commit 111431a
Show file tree
Hide file tree
Showing 3 changed files with 30 additions and 56 deletions.
34 changes: 17 additions & 17 deletions barman/compression.py
Original file line number Diff line number Diff line change
Expand Up @@ -603,7 +603,7 @@ def validate(self, pg_server_version, remote_status):
% (self.config.level, self.config.type)
)
if self.config.level == 0:
msg += "\nIf you need to create an uncompress archive, you should set `backup_compression = none`."
msg += "\nIf you need to create an archive not compressed, you should set `backup_compression = none`."
issues.append(msg)
if self.config.workers is not None:
issues.append(
Expand Down Expand Up @@ -810,6 +810,14 @@ def get_file_content(self, filename, archive):
:return: string content
"""

def validate_src_and_dst(self, src):
if src is None or src == "":
raise ValueError("Source path should be a string")

def validate_dst(self, dst):
if dst is None or dst == "":
raise ValueError("Destination path should be a string")


class GZipCompression(Compression):
name = "gzip"
Expand All @@ -831,10 +839,8 @@ def uncompress(self, src, dst, exclude=None, include_args=None):
:param include_args: list of filepath in the archive to extract.
:return:
"""
if src is None or src == "":
raise ValueError("Source path should be a string")
if dst is None or dst == "":
raise ValueError("Destination path should be a string")
self.validate_dst(src)
self.validate_dst(dst)
exclude = [] if exclude is None else exclude
exclude_args = []
for name in exclude:
Expand Down Expand Up @@ -898,10 +904,8 @@ def uncompress(self, src, dst, exclude=None, include_args=None):
:param include_args: list of filepath in the archive to extract.
:return:
"""
if src is None or src == "":
raise ValueError("Source path should be a string")
if dst is None or dst == "":
raise ValueError("Destination path should be a string")
self.validate_dst(src)
self.validate_dst(dst)
exclude = [] if exclude is None else exclude
exclude_args = []
for name in exclude:
Expand Down Expand Up @@ -973,10 +977,8 @@ def uncompress(self, src, dst, exclude=None, include_args=None):
:param include_args: list of filepath in the archive to extract.
:return:
"""
if src is None or src == "":
raise ValueError("Source path should be a string")
if dst is None or dst == "":
raise ValueError("Destination path should be a string")
self.validate_dst(src)
self.validate_dst(dst)
exclude = [] if exclude is None else exclude
exclude_args = []
for name in exclude:
Expand Down Expand Up @@ -1048,10 +1050,8 @@ def uncompress(self, src, dst, exclude=None, include_args=None):
:param include_args: list of filepath in the archive to extract.
:return:
"""
if src is None or src == "":
raise ValueError("Source path should be a string")
if dst is None or dst == "":
raise ValueError("Destination path should be a string")
self.validate_dst(src)
self.validate_dst(dst)
exclude = [] if exclude is None else exclude
exclude_args = []
for name in exclude:
Expand Down
43 changes: 5 additions & 38 deletions tests/test_command_wrappers.py
Original file line number Diff line number Diff line change
Expand Up @@ -1229,6 +1229,7 @@ def test_simple_invocation(self, popen, pipe_processor_loop, caplog):
@pytest.mark.parametrize(
("compression_config", "expected_args", "unexpected_args"),
[
## `gzip` compression before pg15
# If no compression is provided then no compression args are expected
(None, [], ["--gzip", "--compress", "--format"]),
# If gzip compression is provided with no level then we expect only
Expand All @@ -1250,43 +1251,7 @@ def test_simple_invocation(self, popen, pipe_processor_loop, caplog):
["--gzip", "--format=tar", "--compress=5"],
[],
),
],
)
def test_compression_gzip(self, compression_config, expected_args, unexpected_args):
"""
Verifies the expected pg_basebackup options are added for the specified
compression. Only cares whether the correct arguments are created and does
not verify the behaviour of the command itself for this is covered by other
tests.
"""
connection_mock = mock.MagicMock()
connection_mock.get_connection_string.return_value = "fake_connstring"
compression_mock = None
if compression_config is not None:
compression_mock = mock.MagicMock()
compression_mock.config = compression_config

# GIVEN a PgBaseBackup command initialised with the specified compression
# WHEN the wrapper is instantiated
cmd = command_wrappers.PgBaseBackup(
destination="/fake/target",
command=self.pg_basebackup_path,
connection=connection_mock,
version="14",
app_name="test_app_name",
compression=compression_mock,
)

# THEN all expected arguments are present
assert all(arg in cmd.args for arg in expected_args)

# AND no unexpected arguments are preset
for expected_arg in unexpected_args:
assert not any(expected_arg == arg.split("=")[0] for arg in cmd.args)

@pytest.mark.parametrize(
("compression_config", "expected_args", "unexpected_args"),
[
## `none` compression before pg15
# If none compression is provided with no level then we expect
# --format=tar and --compress=0 arguments
(
Expand All @@ -1303,7 +1268,9 @@ def test_compression_gzip(self, compression_config, expected_args, unexpected_ar
),
],
)
def test_compression_none(self, compression_config, expected_args, unexpected_args):
def test_compression_pre_15(
self, compression_config, expected_args, unexpected_args
):
"""
Verifies the expected pg_basebackup options are added for the specified
compression. Only cares whether the correct arguments are created and does
Expand Down
9 changes: 8 additions & 1 deletion tests/test_compressor.py
Original file line number Diff line number Diff line change
Expand Up @@ -428,6 +428,13 @@ class TestPgBaseBackupCompression(object):
ZSTDPgBaseBackupCompressionOption,
ZSTDCompression,
),
# Test no compression scenario
(
"none",
PgBaseBackupCompression,
NonePgBaseBackupCompressionOption,
NoneCompression,
),
],
)
def test_get_pg_basebackup_compression(
Expand Down Expand Up @@ -664,7 +671,7 @@ class TestGZipPgBaseBackupCompressionOption(object):
15000,
{"backup_compression": "gzip", "backup_compression_level": 0},
[
"backup_compression_level 0 unsupported by compression algorithm. gzip expects a compression level between 1 and 9 (-1 will use default compression level).\nIf you need to create an uncompress archive, you should set `backup_compression = none`."
"backup_compression_level 0 unsupported by compression algorithm. gzip expects a compression level between 1 and 9 (-1 will use default compression level).\nIf you need to create an archive not compressed, you should set `backup_compression = none`."
],
),
(
Expand Down

0 comments on commit 111431a

Please sign in to comment.