From 67ab2acb82a8f9abec7d94cdf46873ffaaf3bc2a Mon Sep 17 00:00:00 2001 From: Dan Helfman Date: Thu, 12 Sep 2019 16:37:43 -0700 Subject: [PATCH] Fix for hook erroring with exit code 1 not being interpreted as an error (#214). --- NEWS | 1 + borgmatic/execute.py | 7 ++++++- tests/integration/test_execute.py | 9 +++++++++ 3 files changed, 16 insertions(+), 1 deletion(-) diff --git a/NEWS b/NEWS index 8c4f814..aa9c3e3 100644 --- a/NEWS +++ b/NEWS @@ -3,6 +3,7 @@ * #209: Bypass Borg error about a moved repository via "relocated_repo_access_is_ok" option in borgmatic storage configuration section. * #213: Reorder arguments passed to Borg to fix duplicate directories when using Borg patterns. + * #214: Fix for hook erroring with exit code 1 not being interpreted as an error. 1.3.14 * #204: Do not treat Borg warnings (exit code 1) as failures. diff --git a/borgmatic/execute.py b/borgmatic/execute.py index efa0385..44dc16d 100644 --- a/borgmatic/execute.py +++ b/borgmatic/execute.py @@ -32,7 +32,12 @@ def execute_and_log_output(full_command, output_log_level, shell): logger.log(output_log_level, remaining_output) exit_code = process.poll() - if exit_code >= BORG_ERROR_EXIT_CODE: + + # If shell is True, assume we're running something other than Borg and should treat all non-zero + # exit codes as errors. + error = bool(exit_code != 0) if shell else bool(exit_code >= BORG_ERROR_EXIT_CODE) + + if error: # If an error occurs, include its output in the raised exception so that we don't # inadvertently hide error output. if len(last_lines) == ERROR_OUTPUT_MAX_LINE_COUNT: diff --git a/tests/integration/test_execute.py b/tests/integration/test_execute.py index c9bd9a4..fa7d4dd 100644 --- a/tests/integration/test_execute.py +++ b/tests/integration/test_execute.py @@ -32,6 +32,15 @@ def test_execute_and_log_output_includes_borg_error_output_in_exception(): assert error.value.output +def test_execute_and_log_output_with_shell_error_raises(): + flexmock(module.logger).should_receive('log') + + with pytest.raises(subprocess.CalledProcessError) as error: + module.execute_and_log_output(['false'], output_log_level=logging.INFO, shell=True) + + assert error.value.returncode == 1 + + def test_execute_and_log_output_truncates_long_borg_error_output(): flexmock(module).ERROR_OUTPUT_MAX_LINE_COUNT = 0 flexmock(module.logger).should_receive('log')