diff --git a/NEWS b/NEWS index 97499bde..c4f89344 100644 --- a/NEWS +++ b/NEWS @@ -3,6 +3,7 @@ handy for fetching borgmatic configuration from external scripts. See the documentation for more information: https://torsion.org/borgmatic/reference/command-line/actions/config-show/ + * #1228: Adjust the "spot" check so error output includes more information about what failed. * #1236: Fix the "spot" check to skip hard links, as Borg doesn't produces hashes for them. * #1269: Fix the ZFS hook to support datasets with a "canmount" property of "noauto". * #1270: Follow symlinks when backing up borgmatic configuration files to support the "bootstrap" diff --git a/borgmatic/actions/check.py b/borgmatic/actions/check.py index ce0d894f..8418c5d9 100644 --- a/borgmatic/actions/check.py +++ b/borgmatic/actions/check.py @@ -717,7 +717,7 @@ def spot_check( ) logger.debug(f'Paths in latest archive but not source paths: {truncated_archive_paths}') raise ValueError( - 'Spot check failed: There are no source paths to compare against the archive', + 'Spot check failed; there are no source paths to compare against the archive', ) # Calculate the percentage delta between the source paths count and the archive paths count, and @@ -731,19 +731,13 @@ def spot_check( width=MAX_SPOT_CHECK_PATHS_LENGTH, placeholder=' ...', ) - logger.debug( - f'Paths in source paths but not latest archive: {truncated_exclusive_source_paths}', - ) truncated_exclusive_archive_paths = textwrap.shorten( ', '.join(set(archive_paths) - rootless_source_paths) or 'none', width=MAX_SPOT_CHECK_PATHS_LENGTH, placeholder=' ...', ) - logger.debug( - f'Paths in latest archive but not source paths: {truncated_exclusive_archive_paths}', - ) raise ValueError( - f'Spot check failed: {count_delta_percentage:.2f}% file count delta between source paths and latest archive (tolerance is {spot_check_config["count_tolerance_percentage"]}%)', + f'Spot check failed\n{count_delta_percentage:.2f}% file count delta between source paths ({len(source_paths)} total) and latest archive ({len(archive_paths)} total); tolerance is {spot_check_config["count_tolerance_percentage"]}%\nOnly in source paths: {truncated_exclusive_source_paths}\nOnly in latest archive: {truncated_exclusive_archive_paths}', ) failing_paths = compare_spot_check_hashes( @@ -768,11 +762,8 @@ def spot_check( width=MAX_SPOT_CHECK_PATHS_LENGTH, placeholder=' ...', ) - logger.debug( - f'Source paths with data not matching the latest archive: {truncated_failing_paths}', - ) raise ValueError( - f'Spot check failed: {failing_percentage:.2f}% of source paths with data not matching the latest archive (tolerance is {data_tolerance_percentage}%)', + f'Spot check failed\n{failing_percentage:.2f}% of source paths ({len(failing_paths)} out of {len(source_paths)} checked) with data not matching the latest archive; tolerance is {data_tolerance_percentage}%\nSource paths with non-matching data: {truncated_failing_paths}', ) logger.info( diff --git a/borgmatic/config/generate.py b/borgmatic/config/generate.py index 4d3cda78..e22c452c 100644 --- a/borgmatic/config/generate.py +++ b/borgmatic/config/generate.py @@ -168,9 +168,11 @@ def render_configuration(config): rendered, # Dumping certain values (integers, for instance) causes ruamel.yaml to append an # end-of-document "..." marker. Strip it. - transform=lambda dumped: dumped[: -len(RUAMEL_YAML_END_OF_DOCUMENT_MARKER)] - if dumped.endswith(RUAMEL_YAML_END_OF_DOCUMENT_MARKER) - else dumped, + transform=lambda dumped: ( + dumped[: -len(RUAMEL_YAML_END_OF_DOCUMENT_MARKER)] + if dumped.endswith(RUAMEL_YAML_END_OF_DOCUMENT_MARKER) + else dumped + ), ) return rendered.getvalue() diff --git a/tests/unit/commands/test_arguments.py b/tests/unit/commands/test_arguments.py index 0fad32e7..0606a1d2 100644 --- a/tests/unit/commands/test_arguments.py +++ b/tests/unit/commands/test_arguments.py @@ -329,10 +329,12 @@ def test_parse_arguments_for_actions_consumes_action_arguments_after_action_name remaining = flexmock() flexmock(module).should_receive('get_subaction_parsers').and_return({}) flexmock(module).should_receive('parse_and_record_action_arguments').replace_with( - lambda unparsed, parsed, parser, action, canonical=None: parsed.update( - {action: action_namespace}, - ) - or remaining, + lambda unparsed, parsed, parser, action, canonical=None: ( + parsed.update( + {action: action_namespace}, + ) + or remaining + ), ) flexmock(module).should_receive('get_subactions_for_actions').and_return({}) action_parsers = {'action': flexmock(), 'other': flexmock()} @@ -355,10 +357,12 @@ def test_parse_arguments_for_actions_consumes_action_arguments_with_alias(): remaining = flexmock() flexmock(module).should_receive('get_subaction_parsers').and_return({}) flexmock(module).should_receive('parse_and_record_action_arguments').replace_with( - lambda unparsed, parsed, parser, action, canonical=None: parsed.update( - {canonical or action: action_namespace}, - ) - or remaining, + lambda unparsed, parsed, parser, action, canonical=None: ( + parsed.update( + {canonical or action: action_namespace}, + ) + or remaining + ), ) flexmock(module).should_receive('get_subactions_for_actions').and_return({}) action_parsers = { @@ -387,10 +391,12 @@ def test_parse_arguments_for_actions_consumes_multiple_action_arguments(): other_namespace = flexmock(bar=3) flexmock(module).should_receive('get_subaction_parsers').and_return({}) flexmock(module).should_receive('parse_and_record_action_arguments').replace_with( - lambda unparsed, parsed, parser, action, canonical=None: parsed.update( - {action: action_namespace if action == 'action' else other_namespace}, - ) - or (), + lambda unparsed, parsed, parser, action, canonical=None: ( + parsed.update( + {action: action_namespace if action == 'action' else other_namespace}, + ) + or () + ), ).and_return(('other', '--bar', '3')).and_return('action', '--foo', 'true') flexmock(module).should_receive('get_subactions_for_actions').and_return({}) action_parsers = { @@ -420,10 +426,12 @@ def test_parse_arguments_for_actions_respects_command_line_action_ordering(): action_namespace = flexmock(foo=True) flexmock(module).should_receive('get_subaction_parsers').and_return({}) flexmock(module).should_receive('parse_and_record_action_arguments').replace_with( - lambda unparsed, parsed, parser, action, canonical=None: parsed.update( - {action: other_namespace if action == 'other' else action_namespace}, - ) - or (), + lambda unparsed, parsed, parser, action, canonical=None: ( + parsed.update( + {action: other_namespace if action == 'other' else action_namespace}, + ) + or () + ), ).and_return(('action',)).and_return(('other', '--foo', 'true')) flexmock(module).should_receive('get_subactions_for_actions').and_return({}) action_parsers = { @@ -458,10 +466,12 @@ def test_parse_arguments_for_actions_applies_default_action_parsers(): flexmock(module).should_receive('get_subaction_parsers').and_return({}) flexmock(module).should_receive('parse_and_record_action_arguments').replace_with( - lambda unparsed, parsed, parser, action, canonical=None: parsed.update( - {action: namespaces.get(action)}, - ) - or (), + lambda unparsed, parsed, parser, action, canonical=None: ( + parsed.update( + {action: namespaces.get(action)}, + ) + or () + ), ).and_return(()) flexmock(module).should_receive('get_subactions_for_actions').and_return({}) action_parsers = { @@ -488,10 +498,12 @@ def test_parse_arguments_for_actions_consumes_global_arguments(): action_namespace = flexmock() flexmock(module).should_receive('get_subaction_parsers').and_return({}) flexmock(module).should_receive('parse_and_record_action_arguments').replace_with( - lambda unparsed, parsed, parser, action, canonical=None: parsed.update( - {action: action_namespace}, - ) - or ('--verbosity', 'lots'), + lambda unparsed, parsed, parser, action, canonical=None: ( + parsed.update( + {action: action_namespace}, + ) + or ('--verbosity', 'lots') + ), ) flexmock(module).should_receive('get_subactions_for_actions').and_return({}) action_parsers = { @@ -516,10 +528,12 @@ def test_parse_arguments_for_actions_passes_through_unknown_arguments_before_act action_namespace = flexmock() flexmock(module).should_receive('get_subaction_parsers').and_return({}) flexmock(module).should_receive('parse_and_record_action_arguments').replace_with( - lambda unparsed, parsed, parser, action, canonical=None: parsed.update( - {action: action_namespace}, - ) - or ('--wtf', 'yes'), + lambda unparsed, parsed, parser, action, canonical=None: ( + parsed.update( + {action: action_namespace}, + ) + or ('--wtf', 'yes') + ), ) flexmock(module).should_receive('get_subactions_for_actions').and_return({}) action_parsers = { @@ -544,10 +558,12 @@ def test_parse_arguments_for_actions_passes_through_unknown_arguments_after_acti action_namespace = flexmock() flexmock(module).should_receive('get_subaction_parsers').and_return({}) flexmock(module).should_receive('parse_and_record_action_arguments').replace_with( - lambda unparsed, parsed, parser, action, canonical=None: parsed.update( - {action: action_namespace}, - ) - or ('--wtf', 'yes'), + lambda unparsed, parsed, parser, action, canonical=None: ( + parsed.update( + {action: action_namespace}, + ) + or ('--wtf', 'yes') + ), ) flexmock(module).should_receive('get_subactions_for_actions').and_return({}) action_parsers = { @@ -572,10 +588,12 @@ def test_parse_arguments_for_actions_with_borg_action_skips_other_action_parsers action_namespace = flexmock(options=[]) flexmock(module).should_receive('get_subaction_parsers').and_return({}) flexmock(module).should_receive('parse_and_record_action_arguments').replace_with( - lambda unparsed, parsed, parser, action, canonical=None: parsed.update( - {action: action_namespace}, - ) - or (), + lambda unparsed, parsed, parser, action, canonical=None: ( + parsed.update( + {action: action_namespace}, + ) + or () + ), ).and_return(()) flexmock(module).should_receive('get_subactions_for_actions').and_return({}) action_parsers = {