Make test code style more consistent with the rest of the codebase (#1116).

borgmatic/hooks/data_source/config.py

@@ -26,6 +26,7 @@ def resolve_database_option(option, data_source, connection_params=None, restore
         return value
     if restore and f'restore_{option}' in data_source:
         return data_source[f'restore_{option}']
+
     return data_source.get(option)
 
 
@@ -51,6 +52,7 @@ def get_hostname_from_config(data_source, connection_params=None, restore=False)
     # ... and finally fall back to the normal options
     if 'container' in data_source:
         return get_ip_from_container(data_source['container'])
+
     return data_source.get('hostname')
 
 

tests/integration/hooks/data_source/test_config.py

@@ -0,0 +1,54 @@
+from flexmock import flexmock
+
+from borgmatic.hooks.data_source import config as module
+
+
+def test_resolve_database_option_gets_hostname_from_container_ip():
+    data_source = {
+        'container': 'original_container',
+        'hostname': 'original_hostname',
+        'restore_container': 'restore_container',
+        'restore_hostname': 'restore_hostname',
+    }
+
+    flexmock(module).should_receive('get_ip_from_container').with_args(
+        'original_container'
+    ).and_return('container_ip_1')
+
+    assert module.resolve_database_option('hostname', data_source) == 'container_ip_1'
+
+
+def test_resolve_database_option_gets_hostname_from_connection_params_container_ip():
+    data_source = {
+        'container': 'original_container',
+        'hostname': 'original_hostname',
+        'restore_container': 'restore_container',
+        'restore_hostname': 'restore_hostname',
+    }
+    connection_params = {'container': 'connection_container', 'hostname': 'connection_hostname'}
+
+    flexmock(module).should_receive('get_ip_from_container').with_args('original_container').never()
+    flexmock(module).should_receive('get_ip_from_container').with_args(
+        'connection_container'
+    ).and_return('container_ip_2')
+
+    assert (
+        module.resolve_database_option('hostname', data_source, connection_params)
+        == 'container_ip_2'
+    )
+
+
+def test_resolve_database_option_gets_hostname_from_restore_container_ip():
+    data_source = {
+        'container': 'original_container',
+        'hostname': 'original_hostname',
+        'restore_container': 'restore_container',
+        'restore_hostname': 'restore_hostname',
+    }
+
+    flexmock(module).should_receive('get_ip_from_container').with_args('original_container').never()
+    flexmock(module).should_receive('get_ip_from_container').with_args(
+        'restore_container'
+    ).and_return('container_ip_3')
+
+    assert module.resolve_database_option('hostname', data_source, restore=True) == 'container_ip_3'

tests/unit/hooks/data_source/test_config.py

@@ -3,26 +3,69 @@ from subprocess import CalledProcessError
 import pytest
 from flexmock import flexmock
 
-from borgmatic.hooks.data_source import config
+from borgmatic.hooks.data_source import config as module
 
 
-def test_get_database_option():
+def test_resolve_database_option_uses_config_value():
+    data_source = {'option': 'original_value', 'restore_option': 'restore_value'}
+
+    assert module.resolve_database_option('option', data_source) == 'original_value'
+
+
+def test_resolve_database_option_with_restore_uses_restore_value():
+    data_source = {'option': 'original_value', 'restore_option': 'restore_value'}
+
+    assert module.resolve_database_option('option', data_source, restore=True) == 'restore_value'
+
+
+def test_resolve_database_option_with_connection_params_uses_connection_params_value():
     data_source = {'option': 'original_value', 'restore_option': 'restore_value'}
     connection_params = {'option': 'connection_value'}
 
-    assert config.resolve_database_option('option', data_source) == 'original_value'
-    assert config.resolve_database_option('option', data_source, restore=True) == 'restore_value'
     assert (
-        config.resolve_database_option('option', data_source, connection_params)
-        == 'connection_value'
-    )
-    assert (
-        config.resolve_database_option('option', data_source, connection_params, restore=True)
+        module.resolve_database_option('option', data_source, connection_params)
         == 'connection_value'
     )
 
 
-def test_get_hostname_option_via_container():
+def test_resolve_database_option_with_restore_and_connection_params_uses_connection_params_value():
+    data_source = {'option': 'original_value', 'restore_option': 'restore_value'}
+    connection_params = {'option': 'connection_value'}
+
+    assert (
+        module.resolve_database_option('option', data_source, connection_params, restore=True)
+        == 'connection_value'
+    )
+
+
+def test_resolve_database_option_with_hostname_uses_hostname_specific_function():
+    data_source = {'hostname': 'original_value'}
+    connection_params = {'hostname': 'connection_value'}
+
+    flexmock(module).should_receive('get_hostname_from_config').and_return('special_value').once()
+
+    assert (
+        module.resolve_database_option('hostname', data_source, connection_params, restore=True)
+        == 'special_value'
+    )
+
+
+def test_get_hostname_from_config_gets_container_ip():
+    data_source = {
+        'container': 'original_container',
+        'hostname': 'original_hostname',
+        'restore_container': 'restore_container',
+        'restore_hostname': 'restore_hostname',
+    }
+
+    flexmock(module).should_receive('get_ip_from_container').with_args(
+        'original_container'
+    ).and_return('container_ip_1')
+
+    assert module.get_hostname_from_config(data_source) == 'container_ip_1'
+
+
+def test_get_hostname_from_config_gets_connection_params_container_ip():
     data_source = {
         'container': 'original_container',
         'hostname': 'original_hostname',
@@ -31,73 +74,84 @@ def test_get_hostname_option_via_container():
     }
     connection_params = {'container': 'connection_container', 'hostname': 'connection_hostname'}
 
-    flexmock(config).should_receive('get_ip_from_container').with_args(
-        'original_container'
-    ).and_return('container_ip_1')
-    flexmock(config).should_receive('get_ip_from_container').with_args(
+    flexmock(module).should_receive('get_ip_from_container').with_args('original_container').never()
+    flexmock(module).should_receive('get_ip_from_container').with_args(
         'connection_container'
     ).and_return('container_ip_2')
-    flexmock(config).should_receive('get_ip_from_container').with_args(
+
+    assert module.get_hostname_from_config(data_source, connection_params) == 'container_ip_2'
+
+
+def test_get_hostname_from_config_gets_restore_container_ip():
+    data_source = {
+        'container': 'original_container',
+        'hostname': 'original_hostname',
+        'restore_container': 'restore_container',
+        'restore_hostname': 'restore_hostname',
+    }
+
+    flexmock(module).should_receive('get_ip_from_container').with_args('original_container').never()
+    flexmock(module).should_receive('get_ip_from_container').with_args(
         'restore_container'
     ).and_return('container_ip_3')
 
-    assert config.resolve_database_option('hostname', data_source) == 'container_ip_1'
-    assert (
-        config.resolve_database_option('hostname', data_source, connection_params)
-        == 'container_ip_2'
-    )
-    assert config.resolve_database_option('hostname', data_source, restore=True) == 'container_ip_3'
+    assert module.get_hostname_from_config(data_source, restore=True) == 'container_ip_3'
 
 
-def test_get_container_ip_without_engines():
-    flexmock(config.shutil).should_receive('which').and_return(None).and_return(None)
+def test_get_ip_from_container_without_engines_errors():
+    flexmock(module.shutil).should_receive('which').and_return(None).and_return(None)
 
     with pytest.raises(ValueError):
-        config.get_ip_from_container('yolo')
+        module.get_ip_from_container('yolo')
 
 
-def test_get_container_ip_success():
-    flexmock(config.shutil).should_receive('which').and_return(None).and_return('/usr/bin/podman')
+def test_get_ip_from_container_parses_top_level_ip_address():
+    flexmock(module.shutil).should_receive('which').and_return(None).and_return('/usr/bin/podman')
 
-    flexmock(config).should_receive('execute_command_and_capture_output').and_return(
+    flexmock(module).should_receive('execute_command_and_capture_output').and_return(
         '{"IPAddress": "1.2.3.4"}'
     )
 
-    addr = config.get_ip_from_container('yolo')
-    assert addr == '1.2.3.4'
+    assert module.get_ip_from_container('yolo') == '1.2.3.4'
 
-    flexmock(config).should_receive('execute_command_and_capture_output').and_return(
+
+def test_get_ip_from_container_parses_network_ip_address():
+    flexmock(module.shutil).should_receive('which').and_return(None).and_return('/usr/bin/podman')
+
+    flexmock(module).should_receive('execute_command_and_capture_output').and_return(
         '{"Networks": {"my_network": {"IPAddress": "5.6.7.8"}}}'
     )
 
-    assert config.get_ip_from_container('yolo') == '5.6.7.8'
+    assert module.get_ip_from_container('yolo') == '5.6.7.8'
 
 
-def test_get_container_ip_container_not_found():
-    flexmock(config.shutil).should_receive('which').and_return('/usr/bin/podman')
-    flexmock(config).should_receive('execute_command_and_capture_output').and_raise(
+def test_get_ip_from_container_without_container_errors():
+    flexmock(module.shutil).should_receive('which').and_return('/usr/bin/podman')
+    flexmock(module).should_receive('execute_command_and_capture_output').and_raise(
         CalledProcessError, 1, ['/usr/bin/podman', 'inspect', 'yolo'], None, 'No such object'
     )
 
     with pytest.raises(CalledProcessError):
-        config.get_ip_from_container('does not exist')
+        module.get_ip_from_container('does not exist')
 
 
-def test_get_container_ip_container_no_network():
-    flexmock(config.shutil).should_receive('which').and_return(None).and_return('/usr/bin/podman')
+def test_get_ip_from_container_without_network_errors():
+    flexmock(module.shutil).should_receive('which').and_return(None).and_return('/usr/bin/podman')
 
-    flexmock(config).should_receive('execute_command_and_capture_output').and_return('{}')
+    flexmock(module).should_receive('execute_command_and_capture_output').and_return('{}')
 
     with pytest.raises(ValueError) as exc_info:
-        config.get_ip_from_container('yolo')
+        module.get_ip_from_container('yolo')
+
     assert 'Could not determine ip address for container' in str(exc_info.value)
 
 
-def test_get_container_ip_broken_output():
-    flexmock(config.shutil).should_receive('which').and_return(None).and_return('/usr/bin/podman')
+def test_get_ip_from_container_with_broken_output_errors():
+    flexmock(module.shutil).should_receive('which').and_return(None).and_return('/usr/bin/podman')
 
-    flexmock(config).should_receive('execute_command_and_capture_output').and_return('abc')
+    flexmock(module).should_receive('execute_command_and_capture_output').and_return('abc')
 
     with pytest.raises(ValueError) as exc_info:
-        config.get_ip_from_container('yolo')
+        module.get_ip_from_container('yolo')
+
     assert 'Could not decode JSON output' in str(exc_info.value)