Get existing tests passing again (#961).

borgmatic/hooks/dispatch.py

@@ -42,7 +42,11 @@ def call_hook(function_name, config, hook_name, *args, **kwargs):
     module_name = hook_name.split('_databases')[0]
 
     # Probe for a data source or monitoring hook module corresponding to the hook name.
-    for parent_module in (borgmatic.hooks.credential, borgmatic.hooks.data_source, borgmatic.hooks.monitoring):
+    for parent_module in (
+        borgmatic.hooks.credential,
+        borgmatic.hooks.data_source,
+        borgmatic.hooks.monitoring,
+    ):
         if module_name not in get_submodule_names(parent_module):
             continue
 

tests/unit/borg/test_environment.py

@@ -3,13 +3,22 @@ from flexmock import flexmock
 from borgmatic.borg import environment as module
 
 
-def test_make_environment_with_passcommand_should_set_environment():
+def test_make_environment_with_passcommand_should_call_it_and_set_passphrase_file_descriptor_in_environment():
+    flexmock(module.borgmatic.hooks.dispatch).should_receive('call_hook').and_return('passphrase')
+    flexmock(module.os).should_receive('pipe').and_return((3, 4))
+    flexmock(module.os).should_receive('write')
+    flexmock(module.os).should_receive('close')
+    flexmock(module.os).should_receive('set_inheritable')
+
     environment = module.make_environment({'encryption_passcommand': 'command'})
 
-    assert environment.get('BORG_PASSCOMMAND') == 'command'
+    assert not environment.get('BORG_PASSCOMMAND')
+    assert environment.get('BORG_PASSPHRASE_FD') == '3'
 
 
 def test_make_environment_with_passphrase_should_set_environment():
+    flexmock(module.borgmatic.hooks.dispatch).should_receive('call_hook').and_return(None)
+    flexmock(module.os).should_receive('pipe').never()
     flexmock(module.os.environ).should_receive('get').and_return(None)
     environment = module.make_environment({'encryption_passphrase': 'pass'})
 
@@ -17,6 +26,8 @@ def test_make_environment_with_passphrase_should_set_environment():
 
 
 def test_make_environment_with_ssh_command_should_set_environment():
+    flexmock(module.borgmatic.hooks.dispatch).should_receive('call_hook').and_return(None)
+    flexmock(module.os).should_receive('pipe').never()
     flexmock(module.os.environ).should_receive('get').and_return(None)
     environment = module.make_environment({'ssh_command': 'ssh -C'})
 
@@ -24,6 +35,8 @@ def test_make_environment_with_ssh_command_should_set_environment():
 
 
 def test_make_environment_without_configuration_sets_certain_environment_variables():
+    flexmock(module.borgmatic.hooks.dispatch).should_receive('call_hook').and_return(None)
+    flexmock(module.os).should_receive('pipe').never()
     flexmock(module.os.environ).should_receive('get').and_return(None)
     environment = module.make_environment({})
 
@@ -36,6 +49,8 @@ def test_make_environment_without_configuration_sets_certain_environment_variabl
 
 
 def test_make_environment_without_configuration_does_not_set_certain_environment_variables_if_already_set():
+    flexmock(module.borgmatic.hooks.dispatch).should_receive('call_hook').and_return(None)
+    flexmock(module.os).should_receive('pipe').never()
     flexmock(module.os.environ).should_receive('get').with_args(
         'BORG_RELOCATED_REPO_ACCESS_IS_OK'
     ).and_return('yup')
@@ -48,6 +63,8 @@ def test_make_environment_without_configuration_does_not_set_certain_environment
 
 
 def test_make_environment_with_relocated_repo_access_true_should_set_environment_yes():
+    flexmock(module.borgmatic.hooks.dispatch).should_receive('call_hook').and_return(None)
+    flexmock(module.os).should_receive('pipe').never()
     flexmock(module.os.environ).should_receive('get').and_return(None)
     environment = module.make_environment({'relocated_repo_access_is_ok': True})
 
@@ -55,6 +72,8 @@ def test_make_environment_with_relocated_repo_access_true_should_set_environment
 
 
 def test_make_environment_with_relocated_repo_access_false_should_set_environment_no():
+    flexmock(module.borgmatic.hooks.dispatch).should_receive('call_hook').and_return(None)
+    flexmock(module.os).should_receive('pipe').never()
     flexmock(module.os.environ).should_receive('get').and_return(None)
     environment = module.make_environment({'relocated_repo_access_is_ok': False})
 
@@ -62,6 +81,8 @@ def test_make_environment_with_relocated_repo_access_false_should_set_environmen
 
 
 def test_make_environment_check_i_know_what_i_am_doing_true_should_set_environment_YES():
+    flexmock(module.borgmatic.hooks.dispatch).should_receive('call_hook').and_return(None)
+    flexmock(module.os).should_receive('pipe').never()
     flexmock(module.os.environ).should_receive('get').and_return(None)
     environment = module.make_environment({'check_i_know_what_i_am_doing': True})
 
@@ -69,6 +90,8 @@ def test_make_environment_check_i_know_what_i_am_doing_true_should_set_environme
 
 
 def test_make_environment_check_i_know_what_i_am_doing_false_should_set_environment_NO():
+    flexmock(module.borgmatic.hooks.dispatch).should_receive('call_hook').and_return(None)
+    flexmock(module.os).should_receive('pipe').never()
     flexmock(module.os.environ).should_receive('get').and_return(None)
     environment = module.make_environment({'check_i_know_what_i_am_doing': False})
 
@@ -76,6 +99,8 @@ def test_make_environment_check_i_know_what_i_am_doing_false_should_set_environm
 
 
 def test_make_environment_with_integer_variable_value():
+    flexmock(module.borgmatic.hooks.dispatch).should_receive('call_hook').and_return(None)
+    flexmock(module.os).should_receive('pipe').never()
     flexmock(module.os.environ).should_receive('get').and_return(None)
     environment = module.make_environment({'borg_files_cache_ttl': 40})
     assert environment.get('BORG_FILES_CACHE_TTL') == '40'

tests/unit/hooks/test_dispatch.py

@@ -17,6 +17,9 @@ def test_call_hook_invokes_module_function_with_arguments_and_returns_value():
     config = {'super_hook': flexmock(), 'other_hook': flexmock()}
     expected_return_value = flexmock()
     test_module = sys.modules[__name__]
+    flexmock(module).should_receive('get_submodule_names').with_args(
+        module.borgmatic.hooks.credential
+    ).and_return(['other_hook'])
     flexmock(module).should_receive('get_submodule_names').with_args(
         module.borgmatic.hooks.data_source
     ).and_return(['other_hook'])
@@ -39,6 +42,9 @@ def test_call_hook_probes_config_with_databases_suffix():
     config = {'super_hook_databases': flexmock(), 'other_hook': flexmock()}
     expected_return_value = flexmock()
     test_module = sys.modules[__name__]
+    flexmock(module).should_receive('get_submodule_names').with_args(
+        module.borgmatic.hooks.credential
+    ).and_return(['other_hook'])
     flexmock(module).should_receive('get_submodule_names').with_args(
         module.borgmatic.hooks.data_source
     ).and_return(['other_hook'])
@@ -61,6 +67,9 @@ def test_call_hook_strips_databases_suffix_from_hook_name():
     config = {'super_hook_databases': flexmock(), 'other_hook_databases': flexmock()}
     expected_return_value = flexmock()
     test_module = sys.modules[__name__]
+    flexmock(module).should_receive('get_submodule_names').with_args(
+        module.borgmatic.hooks.credential
+    ).and_return(['other_hook'])
     flexmock(module).should_receive('get_submodule_names').with_args(
         module.borgmatic.hooks.data_source
     ).and_return(['other_hook'])
@@ -83,6 +92,9 @@ def test_call_hook_without_hook_config_invokes_module_function_with_arguments_an
     config = {'other_hook': flexmock()}
     expected_return_value = flexmock()
     test_module = sys.modules[__name__]
+    flexmock(module).should_receive('get_submodule_names').with_args(
+        module.borgmatic.hooks.credential
+    ).and_return(['other_hook'])
     flexmock(module).should_receive('get_submodule_names').with_args(
         module.borgmatic.hooks.data_source
     ).and_return(['other_hook'])
@@ -104,6 +116,9 @@ def test_call_hook_without_hook_config_invokes_module_function_with_arguments_an
 def test_call_hook_without_corresponding_module_raises():
     config = {'super_hook': flexmock(), 'other_hook': flexmock()}
     test_module = sys.modules[__name__]
+    flexmock(module).should_receive('get_submodule_names').with_args(
+        module.borgmatic.hooks.credential
+    ).and_return(['other_hook'])
     flexmock(module).should_receive('get_submodule_names').with_args(
         module.borgmatic.hooks.data_source
     ).and_return(['other_hook'])
@@ -121,6 +136,9 @@ def test_call_hook_without_corresponding_module_raises():
 
 def test_call_hook_skips_non_hook_modules():
     config = {'not_a_hook': flexmock(), 'other_hook': flexmock()}
+    flexmock(module).should_receive('get_submodule_names').with_args(
+        module.borgmatic.hooks.credential
+    ).and_return(['other_hook'])
     flexmock(module).should_receive('get_submodule_names').with_args(
         module.borgmatic.hooks.data_source
     ).and_return(['other_hook'])

tests/unit/test_execute.py

@@ -185,6 +185,7 @@ def test_execute_command_calls_full_command():
         shell=False,
         env=None,
         cwd=None,
+        close_fds=True,
     ).and_return(flexmock(stdout=None)).once()
     flexmock(module.borgmatic.logger).should_receive('Log_prefix').and_return(flexmock())
     flexmock(module).should_receive('log_outputs')
@@ -207,6 +208,7 @@ def test_execute_command_calls_full_command_with_output_file():
         shell=False,
         env=None,
         cwd=None,
+        close_fds=True,
     ).and_return(flexmock(stderr=None)).once()
     flexmock(module.borgmatic.logger).should_receive('Log_prefix').and_return(flexmock())
     flexmock(module).should_receive('log_outputs')
@@ -221,7 +223,14 @@ def test_execute_command_calls_full_command_without_capturing_output():
     flexmock(module).should_receive('log_command')
     flexmock(module.os, environ={'a': 'b'})
     flexmock(module.subprocess).should_receive('Popen').with_args(
-        full_command, stdin=None, stdout=None, stderr=None, shell=False, env=None, cwd=None
+        full_command,
+        stdin=None,
+        stdout=None,
+        stderr=None,
+        shell=False,
+        env=None,
+        cwd=None,
+        close_fds=True,
     ).and_return(flexmock(wait=lambda: 0)).once()
     flexmock(module).should_receive('interpret_exit_code').and_return(module.Exit_status.SUCCESS)
     flexmock(module.borgmatic.logger).should_receive('Log_prefix').and_return(flexmock())
@@ -245,6 +254,7 @@ def test_execute_command_calls_full_command_with_input_file():
         shell=False,
         env=None,
         cwd=None,
+        close_fds=True,
     ).and_return(flexmock(stdout=None)).once()
     flexmock(module.borgmatic.logger).should_receive('Log_prefix').and_return(flexmock())
     flexmock(module).should_receive('log_outputs')
@@ -266,6 +276,7 @@ def test_execute_command_calls_full_command_with_shell():
         shell=True,
         env=None,
         cwd=None,
+        close_fds=True,
     ).and_return(flexmock(stdout=None)).once()
     flexmock(module.borgmatic.logger).should_receive('Log_prefix').and_return(flexmock())
     flexmock(module).should_receive('log_outputs')
@@ -287,6 +298,7 @@ def test_execute_command_calls_full_command_with_extra_environment():
         shell=False,
         env={'a': 'b', 'c': 'd'},
         cwd=None,
+        close_fds=True,
     ).and_return(flexmock(stdout=None)).once()
     flexmock(module.borgmatic.logger).should_receive('Log_prefix').and_return(flexmock())
     flexmock(module).should_receive('log_outputs')
@@ -308,6 +320,7 @@ def test_execute_command_calls_full_command_with_working_directory():
         shell=False,
         env=None,
         cwd='/working',
+        close_fds=True,
     ).and_return(flexmock(stdout=None)).once()
     flexmock(module.borgmatic.logger).should_receive('Log_prefix').and_return(flexmock())
     flexmock(module).should_receive('log_outputs')
@@ -317,6 +330,28 @@ def test_execute_command_calls_full_command_with_working_directory():
     assert output is None
 
 
+def test_execute_command_with_BORG_PASSPHRASE_FD_leaves_file_descriptors_open():
+    full_command = ['foo', 'bar']
+    flexmock(module).should_receive('log_command')
+    flexmock(module.os, environ={'a': 'b'})
+    flexmock(module.subprocess).should_receive('Popen').with_args(
+        full_command,
+        stdin=None,
+        stdout=module.subprocess.PIPE,
+        stderr=module.subprocess.STDOUT,
+        shell=False,
+        env={'a': 'b', 'BORG_PASSPHRASE_FD': '4'},
+        cwd=None,
+        close_fds=False,
+    ).and_return(flexmock(stdout=None)).once()
+    flexmock(module.borgmatic.logger).should_receive('Log_prefix').and_return(flexmock())
+    flexmock(module).should_receive('log_outputs')
+
+    output = module.execute_command(full_command, extra_environment={'BORG_PASSPHRASE_FD': '4'})
+
+    assert output is None
+
+
 def test_execute_command_without_run_to_completion_returns_process():
     full_command = ['foo', 'bar']
     process = flexmock()
@@ -330,6 +365,7 @@ def test_execute_command_without_run_to_completion_returns_process():
         shell=False,
         env=None,
         cwd=None,
+        close_fds=True,
     ).and_return(process).once()
     flexmock(module.borgmatic.logger).should_receive('Log_prefix').and_return(flexmock())
     flexmock(module).should_receive('log_outputs')
@@ -343,7 +379,12 @@ def test_execute_command_and_capture_output_returns_stdout():
     flexmock(module).should_receive('log_command')
     flexmock(module.os, environ={'a': 'b'})
     flexmock(module.subprocess).should_receive('check_output').with_args(
-        full_command, stderr=None, shell=False, env=None, cwd=None
+        full_command,
+        stderr=None,
+        shell=False,
+        env=None,
+        cwd=None,
+        close_fds=True,
     ).and_return(flexmock(decode=lambda: expected_output)).once()
 
     output = module.execute_command_and_capture_output(full_command)
@@ -357,7 +398,12 @@ def test_execute_command_and_capture_output_with_capture_stderr_returns_stderr()
     flexmock(module).should_receive('log_command')
     flexmock(module.os, environ={'a': 'b'})
     flexmock(module.subprocess).should_receive('check_output').with_args(
-        full_command, stderr=module.subprocess.STDOUT, shell=False, env=None, cwd=None
+        full_command,
+        stderr=module.subprocess.STDOUT,
+        shell=False,
+        env=None,
+        cwd=None,
+        close_fds=True,
     ).and_return(flexmock(decode=lambda: expected_output)).once()
 
     output = module.execute_command_and_capture_output(full_command, capture_stderr=True)
@@ -372,7 +418,12 @@ def test_execute_command_and_capture_output_returns_output_when_process_error_is
     flexmock(module).should_receive('log_command')
     flexmock(module.os, environ={'a': 'b'})
     flexmock(module.subprocess).should_receive('check_output').with_args(
-        full_command, stderr=None, shell=False, env=None, cwd=None
+        full_command,
+        stderr=None,
+        shell=False,
+        env=None,
+        cwd=None,
+        close_fds=True,
     ).and_raise(subprocess.CalledProcessError(1, full_command, err_output)).once()
     flexmock(module).should_receive('interpret_exit_code').and_return(
         module.Exit_status.SUCCESS
@@ -389,7 +440,12 @@ def test_execute_command_and_capture_output_raises_when_command_errors():
     flexmock(module).should_receive('log_command')
     flexmock(module.os, environ={'a': 'b'})
     flexmock(module.subprocess).should_receive('check_output').with_args(
-        full_command, stderr=None, shell=False, env=None, cwd=None
+        full_command,
+        stderr=None,
+        shell=False,
+        env=None,
+        cwd=None,
+        close_fds=True,
     ).and_raise(subprocess.CalledProcessError(2, full_command, expected_output)).once()
     flexmock(module).should_receive('interpret_exit_code').and_return(
         module.Exit_status.ERROR
@@ -405,7 +461,12 @@ def test_execute_command_and_capture_output_returns_output_with_shell():
     flexmock(module).should_receive('log_command')
     flexmock(module.os, environ={'a': 'b'})
     flexmock(module.subprocess).should_receive('check_output').with_args(
-        'foo bar', stderr=None, shell=True, env=None, cwd=None
+        'foo bar',
+        stderr=None,
+        shell=True,
+        env=None,
+        cwd=None,
+        close_fds=True,
     ).and_return(flexmock(decode=lambda: expected_output)).once()
 
     output = module.execute_command_and_capture_output(full_command, shell=True)
@@ -424,6 +485,7 @@ def test_execute_command_and_capture_output_returns_output_with_extra_environmen
         shell=False,
         env={'a': 'b', 'c': 'd'},
         cwd=None,
+        close_fds=True,
     ).and_return(flexmock(decode=lambda: expected_output)).once()
 
     output = module.execute_command_and_capture_output(
@@ -439,7 +501,12 @@ def test_execute_command_and_capture_output_returns_output_with_working_director
     flexmock(module).should_receive('log_command')
     flexmock(module.os, environ={'a': 'b'})
     flexmock(module.subprocess).should_receive('check_output').with_args(
-        full_command, stderr=None, shell=False, env=None, cwd='/working'
+        full_command,
+        stderr=None,
+        shell=False,
+        env=None,
+        cwd='/working',
+        close_fds=True,
     ).and_return(flexmock(decode=lambda: expected_output)).once()
 
     output = module.execute_command_and_capture_output(
@@ -449,6 +516,29 @@ def test_execute_command_and_capture_output_returns_output_with_working_director
     assert output == expected_output
 
 
+def test_execute_command_and_capture_output_with_BORG_PASSPHRASE_FD_leaves_file_descriptors_open():
+    full_command = ['foo', 'bar']
+    expected_output = '[]'
+    flexmock(module).should_receive('log_command')
+    flexmock(module.os, environ={'a': 'b'})
+    flexmock(module.subprocess).should_receive('check_output').with_args(
+        full_command,
+        stderr=None,
+        shell=False,
+        env={'a': 'b', 'BORG_PASSPHRASE_FD': '4'},
+        cwd=None,
+        close_fds=False,
+    ).and_return(flexmock(decode=lambda: expected_output)).once()
+
+    output = module.execute_command_and_capture_output(
+        full_command,
+        shell=False,
+        extra_environment={'BORG_PASSPHRASE_FD': '4'},
+    )
+
+    assert output == expected_output
+
+
 def test_execute_command_with_processes_calls_full_command():
     full_command = ['foo', 'bar']
     processes = (flexmock(),)
@@ -462,6 +552,7 @@ def test_execute_command_with_processes_calls_full_command():
         shell=False,
         env=None,
         cwd=None,
+        close_fds=True,
     ).and_return(flexmock(stdout=None)).once()
     flexmock(module).should_receive('log_outputs')
 
@@ -484,6 +575,7 @@ def test_execute_command_with_processes_returns_output_with_output_log_level_non
         shell=False,
         env=None,
         cwd=None,
+        close_fds=True,
     ).and_return(process).once()
     flexmock(module).should_receive('log_outputs').and_return({process: 'out'})
 
@@ -506,6 +598,7 @@ def test_execute_command_with_processes_calls_full_command_with_output_file():
         shell=False,
         env=None,
         cwd=None,
+        close_fds=True,
     ).and_return(flexmock(stderr=None)).once()
     flexmock(module).should_receive('log_outputs')
 
@@ -520,7 +613,14 @@ def test_execute_command_with_processes_calls_full_command_without_capturing_out
     flexmock(module).should_receive('log_command')
     flexmock(module.os, environ={'a': 'b'})
     flexmock(module.subprocess).should_receive('Popen').with_args(
-        full_command, stdin=None, stdout=None, stderr=None, shell=False, env=None, cwd=None
+        full_command,
+        stdin=None,
+        stdout=None,
+        stderr=None,
+        shell=False,
+        env=None,
+        cwd=None,
+        close_fds=True,
     ).and_return(flexmock(wait=lambda: 0)).once()
     flexmock(module).should_receive('interpret_exit_code').and_return(module.Exit_status.SUCCESS)
     flexmock(module).should_receive('log_outputs')
@@ -546,6 +646,7 @@ def test_execute_command_with_processes_calls_full_command_with_input_file():
         shell=False,
         env=None,
         cwd=None,
+        close_fds=True,
     ).and_return(flexmock(stdout=None)).once()
     flexmock(module).should_receive('log_outputs')
 
@@ -567,6 +668,7 @@ def test_execute_command_with_processes_calls_full_command_with_shell():
         shell=True,
         env=None,
         cwd=None,
+        close_fds=True,
     ).and_return(flexmock(stdout=None)).once()
     flexmock(module).should_receive('log_outputs')
 
@@ -588,6 +690,7 @@ def test_execute_command_with_processes_calls_full_command_with_extra_environmen
         shell=False,
         env={'a': 'b', 'c': 'd'},
         cwd=None,
+        close_fds=True,
     ).and_return(flexmock(stdout=None)).once()
     flexmock(module).should_receive('log_outputs')
 
@@ -611,6 +714,7 @@ def test_execute_command_with_processes_calls_full_command_with_working_director
         shell=False,
         env=None,
         cwd='/working',
+        close_fds=True,
     ).and_return(flexmock(stdout=None)).once()
     flexmock(module).should_receive('log_outputs')
 
@@ -621,6 +725,32 @@ def test_execute_command_with_processes_calls_full_command_with_working_director
     assert output is None
 
 
+def test_execute_command_with_processes_with_BORG_PASSPHRASE_FD_leaves_file_descriptors_open():
+    full_command = ['foo', 'bar']
+    processes = (flexmock(),)
+    flexmock(module).should_receive('log_command')
+    flexmock(module.os, environ={'a': 'b'})
+    flexmock(module.subprocess).should_receive('Popen').with_args(
+        full_command,
+        stdin=None,
+        stdout=module.subprocess.PIPE,
+        stderr=module.subprocess.STDOUT,
+        shell=False,
+        env={'a': 'b', 'BORG_PASSPHRASE_FD': '4'},
+        cwd=None,
+        close_fds=False,
+    ).and_return(flexmock(stdout=None)).once()
+    flexmock(module).should_receive('log_outputs')
+
+    output = module.execute_command_with_processes(
+        full_command,
+        processes,
+        extra_environment={'BORG_PASSPHRASE_FD': '4'},
+    )
+
+    assert output is None
+
+
 def test_execute_command_with_processes_kills_processes_on_error():
     full_command = ['foo', 'bar']
     flexmock(module).should_receive('log_command')
@@ -637,6 +767,7 @@ def test_execute_command_with_processes_kills_processes_on_error():
         shell=False,
         env=None,
         cwd=None,
+        close_fds=True,
     ).and_raise(subprocess.CalledProcessError(1, full_command, 'error')).once()
     flexmock(module).should_receive('log_outputs').never()