From fda01a2b02b4d8f1fc8f173bbe9536e482cd6ff6 Mon Sep 17 00:00:00 2001 From: Chemaclass Date: Sun, 3 May 2026 13:47:29 +0200 Subject: [PATCH] fix: bashdep::_vlog must return 0 when verbose=false The 'is_verbose && _log' chain short-circuits with rc=1 when verbose is off. _vlog was the last statement in download_url, so the entire function returned 1 on a successful download. install() then counted it as a failure, so install_from also returned non-zero on every real install when verbose=false (the default). Unit tests didn't catch this: no test asserted download_url returns 0 on a real successful download (they were either snapshot-based, mock the curl call, or only check filesystem state). Fix: - _vlog now uses an if-block and explicit 'return 0' at the end. - 7 new tests assert non-zero RC on every relevant path: download_url returns 0 on success (default + verbose-on + verbose-off forms), install returns 0, install_from returns 0, _vlog returns 0 in both modes. --- bashdep | 4 ++- tests/unit/bashdep_test.sh | 53 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 56 insertions(+), 1 deletion(-) diff --git a/bashdep b/bashdep index 81558d9..5066676 100755 --- a/bashdep +++ b/bashdep @@ -369,8 +369,10 @@ function bashdep::_log() { } # Internal: print only when verbose mode is on (and silent is off). +# Always returns 0 — "not verbose" is a normal state, not a failure. function bashdep::_vlog() { - bashdep::is_verbose && bashdep::_log "$@" + if bashdep::is_verbose; then bashdep::_log "$@"; fi + return 0 } # Internal: return 0 when an existing dependency can be reused. diff --git a/tests/unit/bashdep_test.sh b/tests/unit/bashdep_test.sh index 92294bb..4f07b82 100644 --- a/tests/unit/bashdep_test.sh +++ b/tests/unit/bashdep_test.sh @@ -572,6 +572,59 @@ function test_bashdep_download_url_curl_failure_returns_non_zero() { assert_general_error } +function test_bashdep_download_url_returns_zero_on_success() { + # shellcheck disable=SC2016 + mock curl 'touch "$4"' + bashdep::download_url "https://example.com/tool" "$TEST_DIR" >/dev/null + assert_successful_code "$?" +} + +function test_bashdep_download_url_returns_zero_when_verbose_off() { + # shellcheck disable=SC2016 + mock curl 'touch "$4"' + BASHDEP_VERBOSE=false + bashdep::download_url "https://example.com/tool" "$TEST_DIR" >/dev/null + assert_successful_code "$?" +} + +function test_bashdep_download_url_returns_zero_when_verbose_on() { + # shellcheck disable=SC2016 + mock curl 'touch "$4"' + BASHDEP_VERBOSE=true + bashdep::download_url "https://example.com/tool" "$TEST_DIR" >/dev/null + assert_successful_code "$?" +} + +function test_bashdep_install_returns_zero_after_real_download() { + # shellcheck disable=SC2016 + mock curl 'touch "$4"' + BASHDEP_DIR="$TEST_DIR" + bashdep::install "https://example.com/a" >/dev/null + assert_successful_code "$?" +} + +function test_bashdep_install_from_returns_zero_after_real_download() { + local file="$TEST_DIR/.bashdep" + printf 'https://example.com/a\n' > "$file" + # shellcheck disable=SC2016 + mock curl 'touch "$4"' + BASHDEP_DIR="$TEST_DIR" + bashdep::install_from "$file" >/dev/null + assert_successful_code "$?" +} + +function test_bashdep_vlog_returns_zero_when_verbose_off() { + BASHDEP_VERBOSE=false + bashdep::_vlog hi + assert_successful_code "$?" +} + +function test_bashdep_vlog_returns_zero_when_verbose_on() { + BASHDEP_VERBOSE=true + bashdep::_vlog hi >/dev/null + assert_successful_code "$?" +} + function test_bashdep_download_url_writes_lockfile_entry() { local url="https://example.com/tool" mock curl "true"