From: Josh Steadmon <steadmon@google.com>
Date: Mon, 06 May 2024 19:58:35 GMT
Subject: Re: [PATCH v4 6/7] t/Makefile: run unit tests alongside shell tests
Message-ID: <z3k5phrp5cbxyorie3fhkvhewltz5z7lqumgraxiieur3gm5qr@ei753rpieyyf>
In-Reply-To: <20240503180239.GA3634479@coredump.intra.peff.net>

On 2024.05.03 14:02, Jeff King wrote:
> On Wed, Apr 24, 2024 at 02:25:44PM -0700, Junio C Hamano wrote:
> 
> > 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

Fixed in V6.