Subject: Re: [PATCH 06/12] t: prepare execution of potentially failing commands for `set -e`

From: Junio C Hamano <[email-masked]>
Date: Mon, 13 Apr 2026 10:09:27 -0700

Patrick Steinhardt <[email-masked]> writes:

> 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.

OK.  "svn help" and "cvs version" do exit with status 0 and the
rewrites that use them make sense.  I wonder if we can do something
similar to "git" instead of relying on "git<RETURN>" to exit with 1,
perhaps ...

    $ git version >/dev/null; echo $?
    0

... by using "git version" in the test-lib.sh change?

Other than that looking very good.

>
> Signed-off-by: Patrick Steinhardt <[email-masked]>
> ---
>  t/lib-git-svn.sh                | 3 +--
>  t/lib-httpd.sh                  | 3 +--
>  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.sh                   | 4 ++--
>  7 files changed, 12 insertions(+), 14 deletions(-)
>
> diff --git a/t/lib-git-svn.sh b/t/lib-git-svn.sh
> index 2fde2353fd..07d86ea244 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
> 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/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.sh b/t/test-lib.sh
> index 70fd3e9baf..81380fe978 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