From 0cb6316b08463d9ec491e6db5ce53c3a65c048a8 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Tue, 21 Apr 2026 09:34:14 +0200 Subject: [PATCH 01/12] t: prepare `test_match_signal ()` calls for `set -e` We have a couple of calls to `test_match_signal ()` where we execute a Git command and expect it to die with a specific signal. These calls will essentially execute the process in a subshell via `foo; echo $?`, but as we expect `foo` to fail this will cause the overall subshell to fail once we `set -e`. Fix this issue by using `foo && echo 0 || echo $?` instead. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- t/t0005-signals.sh | 4 ++-- t/t3600-rm.sh | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/t/t0005-signals.sh b/t/t0005-signals.sh index afba0fc3fc..84319cf169 100755 --- a/t/t0005-signals.sh +++ b/t/t0005-signals.sh @@ -42,12 +42,12 @@ test_expect_success 'create blob' ' ' test_expect_success !MINGW 'a constipated git dies with SIGPIPE' ' - OUT=$( ((large_git; echo $? 1>&3) | :) 3>&1 ) && + OUT=$( ((large_git && echo 0 1>&3 || echo $? 1>&3) | :) 3>&1 ) && test_match_signal 13 "$OUT" ' test_expect_success !MINGW 'a constipated git dies with SIGPIPE even if parent ignores it' ' - OUT=$( ((trap "" PIPE && large_git; echo $? 1>&3) | :) 3>&1 ) && + OUT=$( ((trap "" PIPE && large_git && echo 0 1>&3 || echo $? 1>&3) | :) 3>&1 ) && test_match_signal 13 "$OUT" ' diff --git a/t/t3600-rm.sh b/t/t3600-rm.sh index 1f16e6b522..a371ea690e 100755 --- a/t/t3600-rm.sh +++ b/t/t3600-rm.sh @@ -260,7 +260,7 @@ test_expect_success 'choking "git rm" should not let it die with cruft (induce S test_expect_success !MINGW 'choking "git rm" should not let it die with cruft (induce and check SIGPIPE)' ' choke_git_rm_setup && - OUT=$( ((trap "" PIPE && git rm -n "some-file-*"; echo $? 1>&3) | :) 3>&1 ) && + OUT=$( ((trap "" PIPE && git rm -n "some-file-*" && echo 0 1>&3 || echo $? 1>&3) | :) 3>&1 ) && test_match_signal 13 "$OUT" && test_path_is_missing .git/index.lock ' From 990fd368ac7fcb56f69a27ba341cac10f04e285b Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Tue, 21 Apr 2026 09:34:15 +0200 Subject: [PATCH 02/12] t: prepare `test_must_fail ()` for `set -e` The helper function `test_must_fail ()` executes a specific Git command that may or may not fail in a specific way. This is done by executing the command in question and then comparing its exit code against a set of conditions. This works, but once we run our test suite with `set -e` we may bail out of `test_must_fail ()` early in case the command actually fails, even though we expect it to fail. Prepare for this change by handling the failed case with `||`. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- t/test-lib-functions.sh | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh index f3af10fb7e..5fd5494ef1 100644 --- a/t/test-lib-functions.sh +++ b/t/test-lib-functions.sh @@ -1195,8 +1195,9 @@ test_must_fail () { echo >&7 "test_must_fail: only 'git' is allowed: $*" return 1 fi - "$@" 2>&7 - exit_code=$? + + exit_code=0; "$@" 2>&7 || exit_code=$? + if test $exit_code -eq 0 && ! list_contains "$_test_ok" success then echo >&4 "test_must_fail: command succeeded: $*" From f43d01ab90be2711927c8d265b3fb21d2213d2ab Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Tue, 21 Apr 2026 09:34:16 +0200 Subject: [PATCH 03/12] t: prepare `stop_git_daemon ()` for `set -e` We have a couple of calls to `stop_git_daemon ()` outside of specific test cases that will kill a backgrounded git-daemon(1) process and expect the process with a specific error code. While these function calls do end up killing git-daemon(1), the error handling we have in those contexts is basically ineffective. So while we expect the process to exit with a specific error code, we will just continue with any error in case it doesn't. This will change once we enable `set -e` in a subsequent commit. There's two issues though that will make this _always_ fail: - Our call to `wait` is expected to fail, but because it's not part of a condition it will cause us to bail out immediately with `set -e`. - We try to kill git-daemon(1) a second time via the pidfile. We can generally expect that this is the same PID though as we had in the "GIT_DAEMON_PID" environment variable, and thus it's more likely than not that we have already killed it, and the call to kill will fail. Prepare for this change by handling the failure of `wait` with `||` and by silencing failures of the second call to `kill`. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- t/lib-git-daemon.sh | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/t/lib-git-daemon.sh b/t/lib-git-daemon.sh index e62569222b..d172aa51f0 100644 --- a/t/lib-git-daemon.sh +++ b/t/lib-git-daemon.sh @@ -85,14 +85,16 @@ stop_git_daemon() { # kill git-daemon child of git say >&3 "Stopping git daemon ..." + kill "$GIT_DAEMON_PID" - wait "$GIT_DAEMON_PID" >&3 2>&4 - ret=$? + ret=0; wait "$GIT_DAEMON_PID" >&3 2>&4 || ret=$? + if ! test_match_signal 15 $ret then error "git daemon exited with status: $ret" fi - kill "$(cat "$GIT_DAEMON_PIDFILE")" 2>/dev/null + + kill "$(cat "$GIT_DAEMON_PIDFILE")" 2>/dev/null || : GIT_DAEMON_PID= rm -f git_daemon_output "$GIT_DAEMON_PIDFILE" } From 9c64add20b09cc47c191c2bc7b2503b02d290385 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Tue, 21 Apr 2026 09:34:17 +0200 Subject: [PATCH 04/12] t: prepare `git config --unset` calls for `set -e` We have a couple of calls to `git config --unset` that ultimately end up as no-ops as the configuration variables aren't set (anymore) in the first place. These calls are mostly intended to recover unconditionally from tests that may have executed only partially, but they'll ultimately fail during a normal test run. This hasn't been a problem until now as we aren't running tests with `set -e`. This is about to change though, so let's silence the case where we cannot unset the config keys. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- t/t4032-diff-inter-hunk-context.sh | 2 +- t/t7508-status.sh | 4 ++-- t/t9138-git-svn-authors-prog.sh | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/t/t4032-diff-inter-hunk-context.sh b/t/t4032-diff-inter-hunk-context.sh index bada0cbd32..c98eb6abb2 100755 --- a/t/t4032-diff-inter-hunk-context.sh +++ b/t/t4032-diff-inter-hunk-context.sh @@ -17,7 +17,7 @@ f() { t() { use_config= - git config --unset diff.interHunkContext + git config --unset diff.interHunkContext || : case $# in 4) hunks=$4; cmd="diff -U$3";; diff --git a/t/t7508-status.sh b/t/t7508-status.sh index a5e21bf8bf..1167b835a4 100755 --- a/t/t7508-status.sh +++ b/t/t7508-status.sh @@ -773,8 +773,8 @@ test_expect_success TTY 'status --porcelain ignores color.status' ' ' # recover unconditionally from color tests -git config --unset color.status -git config --unset color.ui +git config --unset color.status || : +git config --unset color.ui || : test_expect_success 'status --porcelain respects -b' ' diff --git a/t/t9138-git-svn-authors-prog.sh b/t/t9138-git-svn-authors-prog.sh index 784ec7fc2d..5bb38cb23a 100755 --- a/t/t9138-git-svn-authors-prog.sh +++ b/t/t9138-git-svn-authors-prog.sh @@ -68,8 +68,8 @@ test_expect_success 'authors-file overrode authors-prog' ' ) ' -git --git-dir=x/.git config --unset svn.authorsfile -git --git-dir=x/.git config --unset svn.authorsprog +git --git-dir=x/.git config --unset svn.authorsfile || : +git --git-dir=x/.git config --unset svn.authorsprog || : test_expect_success 'authors-prog imported user without email' ' svn mkdir -m gg --username gg-hermit "$svnrepo"/gg && From 0c6600cdc751e8c018e6baddc0edce070c7c3f55 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Tue, 21 Apr 2026 09:34:18 +0200 Subject: [PATCH 05/12] t: prepare conditional test execution for `set -e` We have some test in our test suite where we use the pattern of `test ... && test_expect_succeess` to conditionally execute a test. The problem is that when we decide to not execute the test, we'll indeed skip the test, but the overall statement will also be unsuccessful. This will become a problem once we enable `set -e`. Prepare for this future by turning this into a proper conditional, which is also a bit easier to read overall. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- t/t4032-diff-inter-hunk-context.sh | 12 +++++++----- t/t7450-bad-git-dotfiles.sh | 24 +++++++++++++----------- 2 files changed, 20 insertions(+), 16 deletions(-) diff --git a/t/t4032-diff-inter-hunk-context.sh b/t/t4032-diff-inter-hunk-context.sh index c98eb6abb2..2d216fb70f 100755 --- a/t/t4032-diff-inter-hunk-context.sh +++ b/t/t4032-diff-inter-hunk-context.sh @@ -40,11 +40,13 @@ t() { test $(git $cmd $file | grep '^@@ ' | wc -l) = $hunks " - test -f $expected && - test_expect_success "$label: check output" " - git $cmd $file | grep -v '^index ' >actual && - test_cmp $expected actual - " + if test -f $expected + then + test_expect_success "$label: check output" " + git $cmd $file | grep -v '^index ' >actual && + test_cmp $expected actual + " + fi } cat <expected.f1.0.1 || exit 1 diff --git a/t/t7450-bad-git-dotfiles.sh b/t/t7450-bad-git-dotfiles.sh index f512eed278..8cc86522b2 100755 --- a/t/t7450-bad-git-dotfiles.sh +++ b/t/t7450-bad-git-dotfiles.sh @@ -220,17 +220,19 @@ check_dotx_symlink () { ) ' - test -n "$refuse_index" && - test_expect_success "refuse to load symlinked $name into index ($type)" ' - test_must_fail \ - git -C $dir \ - -c core.protectntfs \ - -c core.protecthfs \ - read-tree $tree 2>err && - grep "invalid path.*$name" err && - git -C $dir ls-files -s >out && - test_must_be_empty out - ' + if test -n "$refuse_index" + then + test_expect_success "refuse to load symlinked $name into index ($type)" ' + test_must_fail \ + git -C $dir \ + -c core.protectntfs \ + -c core.protecthfs \ + read-tree $tree 2>err && + grep "invalid path.*$name" err && + git -C $dir ls-files -s >out && + test_must_be_empty out + ' + fi } check_dotx_symlink gitmodules vanilla .gitmodules From 5f0d596fe4a7a4ea8752d3e5115190906c1bea4a Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Tue, 21 Apr 2026 09:34:19 +0200 Subject: [PATCH 06/12] t: prepare execution of potentially failing commands for `set -e` Several of our tests verify whether a certain binary can be executed, potentially skipping tests in case we cannot, for example because the binary doesn't exist. In those cases we often run the binary outside of any conditionally. This will start to fail once we enable `set -e`, as that will cause us to bail out the test immediately. Improve these tests by executing them inside of a conditional instead. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- t/lib-git-svn.sh | 7 +++---- t/lib-httpd.sh | 3 +-- t/t3901-i18n-patch.sh | 3 ++- t/t5000-tar-tree.sh | 4 ++-- t/t7422-submodule-output.sh | 2 +- t/t9200-git-cvsexportcommit.sh | 3 +-- t/t9400-git-cvsserver-server.sh | 5 +++-- t/t9401-git-cvsserver-crlf.sh | 4 ++-- t/t9402-git-cvsserver-refs.sh | 4 ++-- t/test-lib-functions.sh | 3 +-- t/test-lib.sh | 10 ++++++---- 11 files changed, 24 insertions(+), 24 deletions(-) diff --git a/t/lib-git-svn.sh b/t/lib-git-svn.sh index 2fde2353fd..52843f667d 100644 --- a/t/lib-git-svn.sh +++ b/t/lib-git-svn.sh @@ -15,8 +15,7 @@ GIT_SVN_DIR=$GIT_DIR/svn/refs/remotes/git-svn SVN_TREE=$GIT_SVN_DIR/svn-tree test_set_port SVNSERVE_PORT -svn >/dev/null 2>&1 -if test $? -ne 1 +if ! svn help >/dev/null 2>&1 then skip_all='skipping git svn tests, svn not found' test_done @@ -27,13 +26,13 @@ export svnrepo svnconf=$PWD/svnconf export svnconf +x=0 perl -w -e " use SVN::Core; use SVN::Repos; \$SVN::Core::VERSION gt '1.1.0' or exit(42); system(qw/svnadmin create --fs-type fsfs/, \$ENV{svnrepo}) == 0 or exit(41); -" >&3 2>&4 -x=$? +" >&3 2>&4 || x=$? if test $x -ne 0 then if test $x -eq 42; then diff --git a/t/lib-httpd.sh b/t/lib-httpd.sh index 4c76e813e3..fc646447d5 100644 --- a/t/lib-httpd.sh +++ b/t/lib-httpd.sh @@ -235,11 +235,10 @@ start_httpd() { test_atexit stop_httpd - "$LIB_HTTPD_PATH" -d "$HTTPD_ROOT_PATH" \ + if ! "$LIB_HTTPD_PATH" -d "$HTTPD_ROOT_PATH" \ -f "$TEST_PATH/apache.conf" $HTTPD_PARA \ -c "Listen 127.0.0.1:$LIB_HTTPD_PORT" -k start \ >&3 2>&4 - if test $? -ne 0 then cat "$HTTPD_ROOT_PATH"/error.log >&4 2>/dev/null test_skip_or_die GIT_TEST_HTTPD "web server setup failed" diff --git a/t/t3901-i18n-patch.sh b/t/t3901-i18n-patch.sh index f03601b49a..ef7d7e1edc 100755 --- a/t/t3901-i18n-patch.sh +++ b/t/t3901-i18n-patch.sh @@ -28,7 +28,8 @@ check_encoding () { 8859) grep "^encoding ISO8859-1" ;; *) - grep "^encoding ISO8859-1"; test "$?" != 0 ;; + ret=0; grep "^encoding ISO8859-1" || ret=$? + test "$ret" != 0 ;; esac || return 1 j=$i i=$(($i+1)) diff --git a/t/t5000-tar-tree.sh b/t/t5000-tar-tree.sh index 5465054f17..a8c28533dc 100755 --- a/t/t5000-tar-tree.sh +++ b/t/t5000-tar-tree.sh @@ -503,8 +503,8 @@ test_expect_success LONG_IS_64BIT 'set up repository with huge blob' ' # would generate the whole 64GB). test_expect_success LONG_IS_64BIT 'generate tar with huge size' ' { - git archive HEAD - echo $? >exit-code + { ret=0 && git archive HEAD || ret=$?; } && + echo "$ret" >exit-code } | test_copy_bytes 4096 >huge.tar && echo 141 >expect && test_cmp expect exit-code diff --git a/t/t7422-submodule-output.sh b/t/t7422-submodule-output.sh index aea1ddf117..852136fdfd 100755 --- a/t/t7422-submodule-output.sh +++ b/t/t7422-submodule-output.sh @@ -198,7 +198,7 @@ test_expect_success !MINGW 'git submodule status --recursive propagates SIGPIPE' ( cd repo && GIT_ALLOW_PROTOCOL=file git submodule add "$(pwd)"/../submodule && - { git submodule status --recursive 2>err; echo $?>status; } | + { { ret=0 && git submodule status --recursive 2>err || ret=$?; } && echo $ret >status; } | grep -q recursive-submodule-path-1 && test_must_be_empty err && test_match_signal 13 "$(cat status)" diff --git a/t/t9200-git-cvsexportcommit.sh b/t/t9200-git-cvsexportcommit.sh index 14cbe96527..581cf3d28f 100755 --- a/t/t9200-git-cvsexportcommit.sh +++ b/t/t9200-git-cvsexportcommit.sh @@ -11,8 +11,7 @@ if ! test_have_prereq PERL; then test_done fi -cvs >/dev/null 2>&1 -if test $? -ne 1 +if ! cvs version >/dev/null 2>&1 then skip_all='skipping git cvsexportcommit tests, cvs not found' test_done diff --git a/t/t9400-git-cvsserver-server.sh b/t/t9400-git-cvsserver-server.sh index e499c7f955..4b45398bab 100755 --- a/t/t9400-git-cvsserver-server.sh +++ b/t/t9400-git-cvsserver-server.sh @@ -17,12 +17,13 @@ if ! test_have_prereq PERL; then skip_all='skipping git cvsserver tests, perl not available' test_done fi -cvs >/dev/null 2>&1 -if test $? -ne 1 + +if ! cvs version >/dev/null 2>&1 then skip_all='skipping git-cvsserver tests, cvs not found' test_done fi + perl -e 'use DBI; use DBD::SQLite' >/dev/null 2>&1 || { skip_all='skipping git-cvsserver tests, Perl SQLite interface unavailable' test_done diff --git a/t/t9401-git-cvsserver-crlf.sh b/t/t9401-git-cvsserver-crlf.sh index a34805acdc..6b4cbb1651 100755 --- a/t/t9401-git-cvsserver-crlf.sh +++ b/t/t9401-git-cvsserver-crlf.sh @@ -60,12 +60,12 @@ check_status_options() { return $stat } -cvs >/dev/null 2>&1 -if test $? -ne 1 +if ! cvs version >/dev/null 2>&1 then skip_all='skipping git-cvsserver tests, cvs not found' test_done fi + if ! test_have_prereq PERL then skip_all='skipping git-cvsserver tests, perl not available' diff --git a/t/t9402-git-cvsserver-refs.sh b/t/t9402-git-cvsserver-refs.sh index 2ee41f9443..65f2ceedec 100755 --- a/t/t9402-git-cvsserver-refs.sh +++ b/t/t9402-git-cvsserver-refs.sh @@ -68,12 +68,12 @@ check_diff() { ######### -cvs >/dev/null 2>&1 -if test $? -ne 1 +if ! cvs version >/dev/null 2>&1 then skip_all='skipping git-cvsserver tests, cvs not found' test_done fi + if ! test_have_prereq PERL then skip_all='skipping git-cvsserver tests, perl not available' diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh index 5fd5494ef1..879ee1ee59 100644 --- a/t/test-lib-functions.sh +++ b/t/test-lib-functions.sh @@ -1248,8 +1248,7 @@ test_might_fail () { test_expect_code () { want_code=$1 shift - "$@" 2>&7 - exit_code=$? + exit_code=0; "$@" 2>&7 || exit_code=$? if test $exit_code = $want_code then return 0 diff --git a/t/test-lib.sh b/t/test-lib.sh index 70fd3e9baf..de7d9e7b92 100644 --- a/t/test-lib.sh +++ b/t/test-lib.sh @@ -143,8 +143,8 @@ export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME ################################################################ # It appears that people try to run tests without building... GIT_BINARY="${GIT_TEST_INSTALLED:-$GIT_BUILD_DIR}/git$X" -"$GIT_BINARY" >/dev/null -if test $? != 1 + +if ! "$GIT_BINARY" version >/dev/null then if test -n "$GIT_TEST_INSTALLED" then @@ -454,8 +454,10 @@ then # from any previous runs. >"$GIT_TEST_TEE_OUTPUT_FILE" - (GIT_TEST_TEE_STARTED=done ${TEST_SHELL_PATH} "$0" "$@" 2>&1; - echo $? >"$TEST_RESULTS_BASE.exit") | tee -a "$GIT_TEST_TEE_OUTPUT_FILE" + ( + ret=0 && GIT_TEST_TEE_STARTED=done ${TEST_SHELL_PATH} "$0" "$@" 2>&1 || ret=$? + echo "$ret" >"$TEST_RESULTS_BASE.exit" + ) | tee -a "$GIT_TEST_TEE_OUTPUT_FILE" test "$(cat "$TEST_RESULTS_BASE.exit")" = 0 exit fi From c1e29bcfa88fb8997379c3c7c888961b728e581d Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Tue, 21 Apr 2026 09:34:20 +0200 Subject: [PATCH 07/12] t: prepare `test_when_finished ()`/`test_atexit()` for `set -e` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both `test_when_finished ()` and `test_atexit ()` build up a chain of cleanup commands by prepending each new command to the existing cleanup string. To preserve the exit code of the test body across cleanup execution, we append the following logic: } && (exit "$eval_ret"); eval_ret=$?; ... The intent of this is to run the cleanup block and then unconditionally restore `eval_ret`. The original behaviour of this is is: +------------------+---------+------------------------------------+ |test body │ cleanup │ old behaviour │ +------------------+---------+------------------------------------+ │pass (eval_ret=0) | pass │ && taken -> (exit 0) -> eval_ret=0 | +------------------+---------+------------------------------------+ │pass (eval_ret=0) | fail │ && not taken -> eval_ret=$? | +------------------+---------+------------------------------------+ │fail (eval_ret=1) | pass │ && taken -> (exit 1) -> eval_ret=1 | +------------------+---------+------------------------------------+ │fail (eval_ret=1) | fail | && not taken -> eval_ret=$? | +------------------+---------+------------------------------------+ This logic will start to fail once we enable `set -e`. When `$eval_ret` is non-zero, the subshell we create will fail, and with `set -e` we'll thus bail out without evaluating the logic after the semicolon. Fix this issue by instead using `|| eval_ret=\$?; ...`. Besides being a bit simpler, it also retains the original behaviour: +------------------+---------+------------------------------------+ |test body │ cleanup │ old behaviour │ +------------------+---------+------------------------------------+ │pass (eval_ret=0) | pass │ || not taken -> eval_ret unchanged | +------------------+---------+------------------------------------+ │pass (eval_ret=0) | fail │ || taken -> eval_ret=$? | +------------------+---------+------------------------------------+ │fail (eval_ret=1) | pass │ || not taken -> eval_ret unchanged | +------------------+---------+------------------------------------+ │fail (eval_ret=1) | fail | || taken -> eval_ret=$? | +------------------+---------+------------------------------------+ Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- t/test-lib-functions.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh index 879ee1ee59..502bb0ddcb 100644 --- a/t/test-lib-functions.sh +++ b/t/test-lib-functions.sh @@ -1512,7 +1512,7 @@ test_when_finished () { test "${BASH_SUBSHELL-0}" = 0 || BUG "test_when_finished does nothing in a subshell" test_cleanup="{ $* - } && (exit \"\$eval_ret\"); eval_ret=\$?; $test_cleanup" + } || eval_ret=\$?; $test_cleanup" } # This function can be used to schedule some commands to be run @@ -1540,7 +1540,7 @@ test_atexit () { test "${BASH_SUBSHELL-0}" = 0 || BUG "test_atexit does nothing in a subshell" test_atexit_cleanup="{ $* - } && (exit \"\$eval_ret\"); eval_ret=\$?; $test_atexit_cleanup" + } || eval_ret=\$?; $test_atexit_cleanup" } # Deprecated wrapper for "git init", use "git init" directly instead From b94900a0cf20e213038102a4cbf072dec5b1bb18 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Tue, 21 Apr 2026 09:34:21 +0200 Subject: [PATCH 08/12] t0008: silence error in subshell when using `grep -v` In t0008 we use `grep -v` in a subshell, but expect that this command will sometimes not match anything. This would cause grep(1) to return an error code, but given that we don't run with `set -e` we swallow this error. We're about to enable `set -e`. Prepare for this by ignoring any errors. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- t/t0008-ignores.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/t/t0008-ignores.sh b/t/t0008-ignores.sh index e716b5cdfa..d77a179bdd 100755 --- a/t/t0008-ignores.sh +++ b/t/t0008-ignores.sh @@ -122,8 +122,8 @@ test_expect_success_multiple () { fi testname="$1" expect_all="$2" code="$3" - expect_verbose=$( echo "$expect_all" | grep -v '^:: ' ) - expect=$( echo "$expect_verbose" | sed -e 's/.* //' ) + expect_verbose=$(echo "$expect_all" | grep -v '^:: ' || :) + expect=$(echo "$expect_verbose" | sed -e 's/.* //') test_expect_success $prereq "$testname${no_index_opt:+ with $no_index_opt}" ' expect "$expect" && From 4f917bbf718047dc1b3c4346a9c47c84a52a810b Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Tue, 21 Apr 2026 09:34:22 +0200 Subject: [PATCH 09/12] t1301: don't fail in case setfacl(1) doesn't exist or fails In t1301 we're trying to remove any potentially-existing default ACLs that might exist on the transh directory by executing setfacl(1). According to 8ed0a740dd (t1301-shared-repo.sh: don't let a default ACL interfere with the test, 2008-10-16), this is done because we play around with permissions and umasks in this test suite. The setfacl(1) binary may not exist on some systems though, even though tests ultimately still pass. This doesn't matter currently, but will cause the test to fail once we start running with `set -e`. Silence such failures by ignoring failures here. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- t/t1301-shared-repo.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/t1301-shared-repo.sh b/t/t1301-shared-repo.sh index 630a47af21..0e0d07a1a1 100755 --- a/t/t1301-shared-repo.sh +++ b/t/t1301-shared-repo.sh @@ -12,7 +12,7 @@ TEST_CREATE_REPO_NO_TEMPLATE=1 . ./test-lib.sh # Remove a default ACL from the test dir if possible. -setfacl -k . 2>/dev/null +setfacl -k . 2>/dev/null || : # User must have read permissions to the repo -> failure on --shared=0400 test_expect_success 'shared = 0400 (faulty permission u-w)' ' From 090af9957c14915461f302f422d2c147b3e9175b Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Tue, 21 Apr 2026 09:34:23 +0200 Subject: [PATCH 10/12] t6002: fix use of `expr` with `set -e` In `test_bisection_diff ()` we use `expr` to perform some math. This command has some gotchas though in that it will only return success when the result is neither null nor zero. In some of our cases though it actually _is_ zero, and that will cause the expressions to fail once we enable `set -e`. Prepare for this change by instead using `$(( ))`, which doesn't have the same issue. While at it, modernize the function a tiny bit. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- t/t6002-rev-list-bisect.sh | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/t/t6002-rev-list-bisect.sh b/t/t6002-rev-list-bisect.sh index daa009c9a1..f2de40b5ed 100755 --- a/t/t6002-rev-list-bisect.sh +++ b/t/t6002-rev-list-bisect.sh @@ -27,13 +27,16 @@ test_bisection_diff() # Test if bisection size is close to half of list size within # tolerance. # - _bisect_err=$(expr $_list_size - $_bisection_size \* 2) - test "$_bisect_err" -lt 0 && _bisect_err=$(expr 0 - $_bisect_err) - _bisect_err=$(expr $_bisect_err / 2) ; # floor + _bisect_err=$(($_list_size - $_bisection_size * 2)) + if test "$_bisect_err" -lt 0 + then + _bisect_err=$((0 - $_bisect_err)) + fi + _bisect_err=$(($_bisect_err / 2)) ; # floor - test_expect_success \ - "bisection diff $_bisect_option $_head $* <= $_max_diff" \ - 'test $_bisect_err -le $_max_diff' + test_expect_success "bisection diff $_bisect_option $_head $* <= $_max_diff" ' + test $_bisect_err -le $_max_diff + ' } date >path0 From 1ecf6538263cf81c151f2e720cd73fde3d50a07e Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Tue, 21 Apr 2026 09:34:24 +0200 Subject: [PATCH 11/12] t9902: fix use of `read` with `set -e` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In t9902 we're using the `read` builtin to read some values into a variable. This is done by using `-d ""`, which cause us to read until the end of the heredoc. As the read is terminated by EOF, the command will end up returning a non-zero error code. This hasn't been an issue until now as we didn't run with `set -e`, but that'll change in a subsequent commit. Prepare for this change by not using read at all, as we can simply store the multi-line value directly. Suggested-by: SZEDER Gábor Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- t/t9902-completion.sh | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh index 2f9a597ec7..28f61f08fb 100755 --- a/t/t9902-completion.sh +++ b/t/t9902-completion.sh @@ -590,12 +590,10 @@ test_expect_success '__gitcomp - doesnt fail because of invalid variable name' ' __gitcomp "$invalid_variable_name" ' -read -r -d "" refs <<-\EOF -main +refs='main maint next -seen -EOF +seen' test_expect_success '__gitcomp_nl - trailing space' ' test_gitcomp_nl "m" "$refs" <<-EOF From ffe8005b9d8e0cf1b5d5d796ca4da76fbe219011 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Tue, 21 Apr 2026 09:34:25 +0200 Subject: [PATCH 12/12] t: detect errors outside of test cases We have recently merged a patch series that had a simple misspelling of `test_expect_success`. Instead of making our tests fail though, this typo went completely undetected and all of our tests passed, which is of course unfortunate. This is a more general issue with our test suite: all commands that run outside of a specific test case can fail, and if we don't explicitly check for such failure then this failure will be silently ignored. Improve the status quo by enabling the errexit option so that any such unchecked failures will cause us to abort immediately. Note that for now, we only enable this option for Bash 5 and newer. This is because other shells have wildly different behaviour, and older versions of Bash (especially on macOS) are buggy. The list of enabled shells may be extended going forward. Helped-by: Jeff King Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- ci/run-build-and-tests.sh | 6 ++++++ t/test-lib.sh | 25 +++++++++++++++++++++++++ 2 files changed, 31 insertions(+) diff --git a/ci/run-build-and-tests.sh b/ci/run-build-and-tests.sh index 28cfe730ee..de08a08d59 100755 --- a/ci/run-build-and-tests.sh +++ b/ci/run-build-and-tests.sh @@ -7,6 +7,12 @@ export TEST_CONTRIB_TOO=yes +case "$jobname" in +almalinux-*|debian-*|fedora-*|linux-*) + export GIT_TEST_USE_SET_E=yes + ;; +esac + case "$jobname" in fedora-breaking-changes-musl|linux-breaking-changes) export WITH_BREAKING_CHANGES=YesPlease diff --git a/t/test-lib.sh b/t/test-lib.sh index de7d9e7b92..cded7bd693 100644 --- a/t/test-lib.sh +++ b/t/test-lib.sh @@ -15,6 +15,31 @@ # You should have received a copy of the GNU General Public License # along with this program. If not, see https://www.gnu.org/licenses/ . +# Enable the use of errexit so that any unexpected failures will cause us to +# abort tests, even when outside of a specific test case. +# +# Note that we only enable this on Bash 5 and newer, or when explicitly +# requested by the user via `GIT_TEST_USE_SET_E=true`. This ib secause `set -e` +# has wildly different behaviour across shells. The list of default-enabled +# shells may be extended going forward. +if test -z "$GIT_TEST_USE_SET_E" && test "${BASH_VERSINFO:=0}" -ge 5 +then + GIT_TEST_USE_SET_E=true +fi + +# We cannot use `test-tool env-helper` here, as it's not yet available. +case "${GIT_TEST_USE_SET_E:-false}" in +1|on|true|yes) + set -e + ;; +0|off|false|no) + ;; +*) + echo "GIT_TEST_USE_SET_E requires a boolean" >&2 + exit 1 + ;; +esac + # Test the binaries we have just built. The tests are kept in # t/ subdirectory and are run in 'trash directory' subdirectory. if test -z "$TEST_DIRECTORY"