Browse Source

Some additional test coverage (#635).

Dan Helfman 5 months ago
parent
commit
71f1819f05

+ 2 - 6
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.
     '''

+ 2 - 6
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.
     '''

+ 2 - 6
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'
 

+ 3 - 9
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.
     '''

+ 1 - 3
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.
     '''

+ 1 - 3
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.
     '''

+ 2 - 6
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.
     '''

+ 2 - 6
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.
     '''

+ 1 - 3
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.
     '''

+ 5 - 3
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

+ 1 - 3
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
 
 

+ 5 - 15
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():

+ 1 - 3
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
 

+ 61 - 0
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