From 71f1819f05ede67823cb3b60d20cf91e04798469 Mon Sep 17 00:00:00 2001 From: Dan Helfman Date: Mon, 27 Jan 2025 09:27:12 -0800 Subject: [PATCH] Some additional test coverage (#635). --- borgmatic/hooks/monitoring/cronhub.py | 8 +-- borgmatic/hooks/monitoring/cronitor.py | 8 +-- borgmatic/hooks/monitoring/healthchecks.py | 8 +-- borgmatic/hooks/monitoring/ntfy.py | 12 +--- borgmatic/hooks/monitoring/pagerduty.py | 4 +- borgmatic/hooks/monitoring/pushover.py | 4 +- borgmatic/hooks/monitoring/sentry.py | 8 +-- borgmatic/hooks/monitoring/uptime_kuma.py | 8 +-- borgmatic/hooks/monitoring/zabbix.py | 4 +- borgmatic/logger.py | 8 ++- tests/unit/config/test_paths.py | 4 +- .../unit/hooks/data_source/test_postgresql.py | 20 ++---- tests/unit/hooks/test_dispatch.py | 4 +- tests/unit/test_logger.py | 61 +++++++++++++++++++ 14 files changed, 89 insertions(+), 72 deletions(-) diff --git a/borgmatic/hooks/monitoring/cronhub.py b/borgmatic/hooks/monitoring/cronhub.py index dfc4892f..837d770b 100644 --- a/borgmatic/hooks/monitoring/cronhub.py +++ b/borgmatic/hooks/monitoring/cronhub.py @@ -28,9 +28,7 @@ def ping_monitor(hook_config, config, config_filename, state, monitoring_log_lev filename in any log entries. If this is a dry run, then don't actually ping anything. ''' if state not in MONITOR_STATE_TO_CRONHUB: - logger.debug( - f'Ignoring unsupported monitoring {state.name.lower()} in Cronhub hook' - ) + logger.debug(f'Ignoring unsupported monitoring {state.name.lower()} in Cronhub hook') return dry_run_label = ' (dry run; not actually pinging)' if dry_run else '' @@ -54,9 +52,7 @@ def ping_monitor(hook_config, config, config_filename, state, monitoring_log_lev logger.warning(f'Cronhub error: {error}') -def destroy_monitor( - ping_url_or_uuid, config, monitoring_log_level, dry_run -): # pragma: no cover +def destroy_monitor(ping_url_or_uuid, config, monitoring_log_level, dry_run): # pragma: no cover ''' No destruction is necessary for this monitor. ''' diff --git a/borgmatic/hooks/monitoring/cronitor.py b/borgmatic/hooks/monitoring/cronitor.py index 994dab31..e8169312 100644 --- a/borgmatic/hooks/monitoring/cronitor.py +++ b/borgmatic/hooks/monitoring/cronitor.py @@ -28,9 +28,7 @@ def ping_monitor(hook_config, config, config_filename, state, monitoring_log_lev filename in any log entries. If this is a dry run, then don't actually ping anything. ''' if state not in MONITOR_STATE_TO_CRONITOR: - logger.debug( - f'Ignoring unsupported monitoring {state.name.lower()} in Cronitor hook' - ) + logger.debug(f'Ignoring unsupported monitoring {state.name.lower()} in Cronitor hook') return dry_run_label = ' (dry run; not actually pinging)' if dry_run else '' @@ -49,9 +47,7 @@ def ping_monitor(hook_config, config, config_filename, state, monitoring_log_lev logger.warning(f'Cronitor error: {error}') -def destroy_monitor( - ping_url_or_uuid, config, monitoring_log_level, dry_run -): # pragma: no cover +def destroy_monitor(ping_url_or_uuid, config, monitoring_log_level, dry_run): # pragma: no cover ''' No destruction is necessary for this monitor. ''' diff --git a/borgmatic/hooks/monitoring/healthchecks.py b/borgmatic/hooks/monitoring/healthchecks.py index 1e145106..0b30d923 100644 --- a/borgmatic/hooks/monitoring/healthchecks.py +++ b/borgmatic/hooks/monitoring/healthchecks.py @@ -55,9 +55,7 @@ def ping_monitor(hook_config, config, config_filename, state, monitoring_log_lev dry_run_label = ' (dry run; not actually pinging)' if dry_run else '' if 'states' in hook_config and state.name.lower() not in hook_config['states']: - logger.info( - f'Skipping Healthchecks {state.name.lower()} ping due to configured states' - ) + logger.info(f'Skipping Healthchecks {state.name.lower()} ping due to configured states') return ping_url_is_uuid = re.search(r'\w{8}-\w{4}-\w{4}-\w{4}-\w{12}$', ping_url) @@ -68,9 +66,7 @@ def ping_monitor(hook_config, config, config_filename, state, monitoring_log_lev if hook_config.get('create_slug'): if ping_url_is_uuid: - logger.warning( - 'Healthchecks UUIDs do not support auto provisionning; ignoring' - ) + logger.warning('Healthchecks UUIDs do not support auto provisionning; ignoring') else: ping_url = f'{ping_url}?create=1' diff --git a/borgmatic/hooks/monitoring/ntfy.py b/borgmatic/hooks/monitoring/ntfy.py index 9fa28c80..f56a6047 100644 --- a/borgmatic/hooks/monitoring/ntfy.py +++ b/borgmatic/hooks/monitoring/ntfy.py @@ -62,13 +62,9 @@ def ping_monitor(hook_config, config, config_filename, state, monitoring_log_lev auth = requests.auth.HTTPBasicAuth(username, password) logger.info(f'Using basic auth with user {username} for ntfy') elif username is not None: - logger.warning( - 'Password missing for ntfy authentication, defaulting to no auth' - ) + logger.warning('Password missing for ntfy authentication, defaulting to no auth') elif password is not None: - logger.warning( - 'Username missing for ntfy authentication, defaulting to no auth' - ) + logger.warning('Username missing for ntfy authentication, defaulting to no auth') if not dry_run: logging.getLogger('urllib3').setLevel(logging.ERROR) @@ -80,9 +76,7 @@ def ping_monitor(hook_config, config, config_filename, state, monitoring_log_lev logger.warning(f'ntfy error: {error}') -def destroy_monitor( - ping_url_or_uuid, config, monitoring_log_level, dry_run -): # pragma: no cover +def destroy_monitor(ping_url_or_uuid, config, monitoring_log_level, dry_run): # pragma: no cover ''' No destruction is necessary for this monitor. ''' diff --git a/borgmatic/hooks/monitoring/pagerduty.py b/borgmatic/hooks/monitoring/pagerduty.py index 5e4aea4f..6afc27c2 100644 --- a/borgmatic/hooks/monitoring/pagerduty.py +++ b/borgmatic/hooks/monitoring/pagerduty.py @@ -72,9 +72,7 @@ def ping_monitor(hook_config, config, config_filename, state, monitoring_log_lev logger.warning(f'PagerDuty error: {error}') -def destroy_monitor( - ping_url_or_uuid, config, monitoring_log_level, dry_run -): # pragma: no cover +def destroy_monitor(ping_url_or_uuid, config, monitoring_log_level, dry_run): # pragma: no cover ''' No destruction is necessary for this monitor. ''' diff --git a/borgmatic/hooks/monitoring/pushover.py b/borgmatic/hooks/monitoring/pushover.py index 40f93d93..8211c32d 100644 --- a/borgmatic/hooks/monitoring/pushover.py +++ b/borgmatic/hooks/monitoring/pushover.py @@ -78,9 +78,7 @@ def ping_monitor(hook_config, config, config_filename, state, monitoring_log_lev logger.warning(f'Pushover error: {error}') -def destroy_monitor( - ping_url_or_uuid, config, monitoring_log_level, dry_run -): # pragma: no cover +def destroy_monitor(ping_url_or_uuid, config, monitoring_log_level, dry_run): # pragma: no cover ''' No destruction is necessary for this monitor. ''' diff --git a/borgmatic/hooks/monitoring/sentry.py b/borgmatic/hooks/monitoring/sentry.py index dbc8f014..857f85bc 100644 --- a/borgmatic/hooks/monitoring/sentry.py +++ b/borgmatic/hooks/monitoring/sentry.py @@ -38,9 +38,7 @@ def ping_monitor(hook_config, config, config_filename, state, monitoring_log_lev match = DATA_SOURCE_NAME_URL_PATTERN.match(data_source_name_url) if not match: - logger.warning( - f'Invalid Sentry data source name URL: {data_source_name_url}' - ) + logger.warning(f'Invalid Sentry data source name URL: {data_source_name_url}') return cron_url = f'{match.group("protocol")}://{match.group("hostname")}/api/{match.group("project_id")}/cron/{monitor_slug}/{match.group("username")}/' @@ -70,9 +68,7 @@ def ping_monitor(hook_config, config, config_filename, state, monitoring_log_lev logger.warning(f'Sentry error: {error}') -def destroy_monitor( - ping_url_or_uuid, config, monitoring_log_level, dry_run -): # pragma: no cover +def destroy_monitor(ping_url_or_uuid, config, monitoring_log_level, dry_run): # pragma: no cover ''' No destruction is necessary for this monitor. ''' diff --git a/borgmatic/hooks/monitoring/uptime_kuma.py b/borgmatic/hooks/monitoring/uptime_kuma.py index 9b06b750..015dde0b 100644 --- a/borgmatic/hooks/monitoring/uptime_kuma.py +++ b/borgmatic/hooks/monitoring/uptime_kuma.py @@ -28,9 +28,7 @@ def ping_monitor(hook_config, config, config_filename, state, monitoring_log_lev status = 'down' if state.name.lower() == 'fail' else 'up' push_url = hook_config.get('push_url', 'https://example.uptime.kuma/api/push/abcd1234') query = f'status={status}&msg={state.name.lower()}' - logger.info( - f'Pushing Uptime Kuma push_url {push_url}?{query} {dry_run_label}' - ) + logger.info(f'Pushing Uptime Kuma push_url {push_url}?{query} {dry_run_label}') logger.debug(f'Full Uptime Kuma state URL {push_url}?{query}') if dry_run: @@ -46,9 +44,7 @@ def ping_monitor(hook_config, config, config_filename, state, monitoring_log_lev logger.warning(f'Uptime Kuma error: {error}') -def destroy_monitor( - push_url_or_uuid, config, monitoring_log_level, dry_run -): # pragma: no cover +def destroy_monitor(push_url_or_uuid, config, monitoring_log_level, dry_run): # pragma: no cover ''' No destruction is necessary for this monitor. ''' diff --git a/borgmatic/hooks/monitoring/zabbix.py b/borgmatic/hooks/monitoring/zabbix.py index 17744a21..96b0a9da 100644 --- a/borgmatic/hooks/monitoring/zabbix.py +++ b/borgmatic/hooks/monitoring/zabbix.py @@ -126,9 +126,7 @@ def ping_monitor(hook_config, config, config_filename, state, monitoring_log_lev logger.warning(f'Zabbix error: {error}') -def destroy_monitor( - ping_url_or_uuid, config, monitoring_log_level, dry_run -): # pragma: no cover +def destroy_monitor(ping_url_or_uuid, config, monitoring_log_level, dry_run): # pragma: no cover ''' No destruction is necessary for this monitor. ''' diff --git a/borgmatic/logger.py b/borgmatic/logger.py index e6203fcd..4acaf50d 100644 --- a/borgmatic/logger.py +++ b/borgmatic/logger.py @@ -88,7 +88,7 @@ class Multi_stream_handler(logging.Handler): class Console_no_color_formatter(logging.Formatter): - def __init__(self, *args, **kwargs): + def __init__(self, *args, **kwargs): # pragma: no cover super(Console_no_color_formatter, self).__init__( '{prefix}{message}', style='{', defaults={'prefix': ''}, *args, **kwargs ) @@ -188,7 +188,8 @@ def add_custom_log_levels(): # pragma: no cover def get_log_prefix(): ''' - Return the current log prefix from the defaults for the formatter on the first logging handler. + Return the current log prefix from the defaults for the formatter on the first logging handler, + set by set_log_prefix(). Return None if no such prefix exists. ''' try: return next( @@ -219,13 +220,14 @@ class Log_prefix: Example use as a context manager: - + with borgmatic.logger.Log_prefix('myprefix'): do_something_that_logs() For the scope of that "with" statement, any logs created are prefixed with "myprefix: ". Afterwards, the prefix gets restored to whatever it was prior to the context manager. ''' + def __init__(self, prefix): ''' Given the desired log prefix, save it for use below. Set prefix to None to disable any diff --git a/tests/unit/config/test_paths.py b/tests/unit/config/test_paths.py index 12975b9d..65e3e477 100644 --- a/tests/unit/config/test_paths.py +++ b/tests/unit/config/test_paths.py @@ -84,9 +84,7 @@ def test_runtime_directory_with_relative_config_option_errors(): config = {'user_runtime_directory': 'run', 'borgmatic_source_directory': '/nope'} with pytest.raises(ValueError): - with module.Runtime_directory( - config - ) as borgmatic_runtime_directory: # noqa: F841 + with module.Runtime_directory(config) as borgmatic_runtime_directory: # noqa: F841 pass diff --git a/tests/unit/hooks/data_source/test_postgresql.py b/tests/unit/hooks/data_source/test_postgresql.py index 4188f96d..845859f7 100644 --- a/tests/unit/hooks/data_source/test_postgresql.py +++ b/tests/unit/hooks/data_source/test_postgresql.py @@ -61,25 +61,19 @@ def test_make_extra_environment_without_ssl_mode_does_not_set_ssl_mode(): def test_database_names_to_dump_passes_through_individual_database_name(): database = {'name': 'foo'} - assert module.database_names_to_dump(database, flexmock(), dry_run=False) == ( - 'foo', - ) + assert module.database_names_to_dump(database, flexmock(), dry_run=False) == ('foo',) def test_database_names_to_dump_passes_through_individual_database_name_with_format(): database = {'name': 'foo', 'format': 'custom'} - assert module.database_names_to_dump(database, flexmock(), dry_run=False) == ( - 'foo', - ) + assert module.database_names_to_dump(database, flexmock(), dry_run=False) == ('foo',) def test_database_names_to_dump_passes_through_all_without_format(): database = {'name': 'all'} - assert module.database_names_to_dump(database, flexmock(), dry_run=False) == ( - 'all', - ) + assert module.database_names_to_dump(database, flexmock(), dry_run=False) == ('all',) def test_database_names_to_dump_with_all_and_format_and_dry_run_bails(): @@ -166,9 +160,7 @@ def test_database_names_to_dump_with_all_and_format_excludes_particular_database 'foo,test,\ntemplate0,test,blah' ) - assert module.database_names_to_dump(database, flexmock(), dry_run=False) == ( - 'foo', - ) + assert module.database_names_to_dump(database, flexmock(), dry_run=False) == ('foo',) def test_database_names_to_dump_with_all_and_psql_command_uses_custom_command(): @@ -194,9 +186,7 @@ def test_database_names_to_dump_with_all_and_psql_command_uses_custom_command(): extra_environment=object, ).and_return('foo,text').once() - assert module.database_names_to_dump(database, flexmock(), dry_run=False) == ( - 'foo', - ) + assert module.database_names_to_dump(database, flexmock(), dry_run=False) == ('foo',) def test_use_streaming_true_for_any_non_directory_format_databases(): diff --git a/tests/unit/hooks/test_dispatch.py b/tests/unit/hooks/test_dispatch.py index d0eca83d..80f8c046 100644 --- a/tests/unit/hooks/test_dispatch.py +++ b/tests/unit/hooks/test_dispatch.py @@ -74,9 +74,7 @@ def test_call_hook_strips_databases_suffix_from_hook_name(): config['super_hook_databases'], config, 55, value=66 ).and_return(expected_return_value).once() - return_value = module.call_hook( - 'hook_function', config, 'super_hook_databases', 55, value=66 - ) + return_value = module.call_hook('hook_function', config, 'super_hook_databases', 55, value=66) assert return_value == expected_return_value diff --git a/tests/unit/test_logger.py b/tests/unit/test_logger.py index 97aeb63a..5a4d60c9 100644 --- a/tests/unit/test_logger.py +++ b/tests/unit/test_logger.py @@ -228,6 +228,67 @@ def test_add_logging_level_skips_global_setting_if_already_set(): module.add_logging_level('PLAID', 99) +def test_get_log_prefix_gets_prefix_from_first_handler(): + flexmock(module.logging).should_receive('getLogger').and_return( + flexmock( + handlers=[ + flexmock( + formatter=flexmock( + _style=flexmock(_defaults=flexmock(get=lambda name: 'myprefix: ')) + ) + ), + flexmock(), + ], + removeHandler=lambda handler: None, + ) + ) + + assert module.get_log_prefix() == 'myprefix' + + +def test_get_log_prefix_with_no_handlers_does_not_raise(): + flexmock(module.logging).should_receive('getLogger').and_return( + flexmock( + handlers=[], + removeHandler=lambda handler: None, + ) + ) + + assert module.get_log_prefix() == None + + +def test_get_log_prefix_with_no_formatters_does_not_raise(): + flexmock(module.logging).should_receive('getLogger').and_return( + flexmock( + handlers=[ + flexmock(), + flexmock(), + ], + removeHandler=lambda handler: None, + ) + ) + + assert module.get_log_prefix() == None + + +def test_get_log_prefix_with_no_prefix_does_not_raise(): + flexmock(module.logging).should_receive('getLogger').and_return( + flexmock( + handlers=[ + flexmock( + formatter=flexmock( + _style=flexmock(_defaults=flexmock(get=lambda name: None)) + ) + ), + flexmock(), + ], + removeHandler=lambda handler: None, + ) + ) + + assert module.get_log_prefix() == None + + def test_configure_logging_with_syslog_log_level_probes_for_log_socket_on_linux(): flexmock(module).should_receive('add_custom_log_levels') flexmock(module.logging).ANSWER = module.ANSWER