Re: [PATCH v4 6/7] t/Makefile: run unit tests alongside shell tests

From: Jeff King <peff@peff.net>
Date: May 3, 2024, 18:02 UTC
Message-ID: <20240503180239.GA3634479@coredump.intra.peff.net>
In-Reply-To: <xmqqo79yxxk7.fsf@gitster.g>

On Wed, Apr 24, 2024 at 02:25:44PM -0700, Junio C Hamano wrote:

Show 17 quoted lines

> Josh Steadmon <steadmon@google.com> writes:
> 
> > +case "$1" in
> > +*.sh)
> > +	if test -z "${TEST_SHELL_PATH+set}" ; then
> > +		echo "ERROR: TEST_SHELL_PATH is not set" >&2
> 
> Style.
> 
> As an empty string is not a reasonable value for this variable (and
> you do not quote ${TEST_SHELL_PATH} when you use it in "exec" below),
> 
> 	if test -z "${TEST_SHELL_PATH:+set}"
> 	then
> 		echo >&2 "ERROR: TEST_SHELL_PATH is not set or empty"
> 
> may be what we want here.

If we are using ":+" to handle the empty string, I think just:

  if test -z "$TEST_SHELL_PATH"

is sufficient, no?

(not that the other is incorrect, but whenever I see something like ":+set" I wonder if something more clever is going on, and of course I get nightmare flashbacks to looking at generated autoconf code).

-Peff