From b8d349d0483a31af7f8a1a54473fe39494eae714 Mon Sep 17 00:00:00 2001 From: Dan Helfman Date: Tue, 1 Aug 2023 16:27:53 -0700 Subject: [PATCH] Additional test coverage (#732). --- borgmatic/config/load.py | 10 +++- tests/integration/config/test_load.py | 76 +++++++++++++++++++++++++++ tests/unit/config/test_normalize.py | 7 +++ 3 files changed, 91 insertions(+), 2 deletions(-) diff --git a/borgmatic/config/load.py b/borgmatic/config/load.py index 25b57df7..78e05551 100644 --- a/borgmatic/config/load.py +++ b/borgmatic/config/load.py @@ -56,14 +56,20 @@ def include_configuration(loader, filename_node, include_directory): if isinstance(filename_node.value, str): return probe_and_include_file(filename_node.value, include_directories) - if isinstance(filename_node.value, list): + if ( + isinstance(filename_node.value, list) + and len(filename_node.value) + and isinstance(filename_node.value[0], ruamel.yaml.nodes.ScalarNode) + ): + # Reversing the values ensures the correct ordering if these includes are subsequently + # merged together. return [ probe_and_include_file(node.value, include_directories) for node in reversed(filename_node.value) ] raise ValueError( - f'!include value type ({type(filename_node.value)}) is not supported; use a single filename or a list of filenames' + '!include value is not supported; use a single filename or a list of filenames' ) diff --git a/tests/integration/config/test_load.py b/tests/integration/config/test_load.py index 03875f69..069f6406 100644 --- a/tests/integration/config/test_load.py +++ b/tests/integration/config/test_load.py @@ -44,6 +44,14 @@ def test_load_configuration_replaces_complex_constants(): assert module.load_configuration('config.yaml') == {'key': {'subkey': 'value'}} +def test_load_configuration_with_only_integer_value_does_not_raise(): + builtins = flexmock(sys.modules['builtins']) + config_file = io.StringIO('33') + config_file.name = 'config.yaml' + builtins.should_receive('open').with_args('config.yaml').and_return(config_file) + assert module.load_configuration('config.yaml') == 33 + + def test_load_configuration_inlines_include_relative_to_current_directory(): builtins = flexmock(sys.modules['builtins']) flexmock(module.os).should_receive('getcwd').and_return('/tmp') @@ -124,6 +132,37 @@ def test_load_configuration_raises_if_absolute_include_does_not_exist(): assert module.load_configuration('config.yaml') +def test_load_configuration_inlines_multiple_file_include_as_list(): + builtins = flexmock(sys.modules['builtins']) + flexmock(module.os).should_receive('getcwd').and_return('/tmp') + flexmock(module.os.path).should_receive('isabs').and_return(True) + flexmock(module.os.path).should_receive('exists').never() + include1_file = io.StringIO('value1') + include1_file.name = '/root/include1.yaml' + builtins.should_receive('open').with_args('/root/include1.yaml').and_return(include1_file) + include2_file = io.StringIO('value2') + include2_file.name = '/root/include2.yaml' + builtins.should_receive('open').with_args('/root/include2.yaml').and_return(include2_file) + config_file = io.StringIO('key: !include [/root/include1.yaml, /root/include2.yaml]') + config_file.name = 'config.yaml' + builtins.should_receive('open').with_args('config.yaml').and_return(config_file) + + assert module.load_configuration('config.yaml') == {'key': ['value2', 'value1']} + + +def test_load_configuration_include_with_unsupported_filename_type_raises(): + builtins = flexmock(sys.modules['builtins']) + flexmock(module.os).should_receive('getcwd').and_return('/tmp') + flexmock(module.os.path).should_receive('isabs').and_return(True) + flexmock(module.os.path).should_receive('exists').never() + config_file = io.StringIO('key: !include {path: /root/include.yaml}') + config_file.name = 'config.yaml' + builtins.should_receive('open').with_args('config.yaml').and_return(config_file) + + with pytest.raises(ValueError): + module.load_configuration('config.yaml') + + def test_load_configuration_merges_include(): builtins = flexmock(sys.modules['builtins']) flexmock(module.os).should_receive('getcwd').and_return('/tmp') @@ -149,6 +188,43 @@ def test_load_configuration_merges_include(): assert module.load_configuration('config.yaml') == {'foo': 'override', 'baz': 'quux'} +def test_load_configuration_merges_multiple_file_include(): + builtins = flexmock(sys.modules['builtins']) + flexmock(module.os).should_receive('getcwd').and_return('/tmp') + flexmock(module.os.path).should_receive('isabs').and_return(False) + flexmock(module.os.path).should_receive('exists').and_return(True) + include1_file = io.StringIO( + ''' + foo: bar + baz: quux + original: yes + ''' + ) + include1_file.name = 'include1.yaml' + builtins.should_receive('open').with_args('/tmp/include1.yaml').and_return(include1_file) + include2_file = io.StringIO( + ''' + baz: second + ''' + ) + include2_file.name = 'include2.yaml' + builtins.should_receive('open').with_args('/tmp/include2.yaml').and_return(include2_file) + config_file = io.StringIO( + ''' + foo: override + <<: !include [include1.yaml, include2.yaml] + ''' + ) + config_file.name = 'config.yaml' + builtins.should_receive('open').with_args('config.yaml').and_return(config_file) + + assert module.load_configuration('config.yaml') == { + 'foo': 'override', + 'baz': 'second', + 'original': 'yes', + } + + def test_load_configuration_with_retain_tag_merges_include_but_keeps_local_values(): builtins = flexmock(sys.modules['builtins']) flexmock(module.os).should_receive('getcwd').and_return('/tmp') diff --git a/tests/unit/config/test_normalize.py b/tests/unit/config/test_normalize.py index 633e0781..6a9d640b 100644 --- a/tests/unit/config/test_normalize.py +++ b/tests/unit/config/test_normalize.py @@ -111,6 +111,13 @@ def test_normalize_sections_with_different_umask_values_raises(): module.normalize_sections('test.yaml', config) +def test_normalize_sections_with_only_scalar_raises(): + config = 33 + + with pytest.raises(ValueError): + module.normalize_sections('test.yaml', config) + + @pytest.mark.parametrize( 'config,expected_config,produces_logs', (