Re: [PATCH 14/19] tests: apply modern idiom for exiting loop upon failure

From: Jeff King <peff@peff.net>
Date: Dec 10, 2021, 09:36 UTC
Message-ID: <YbMfhGCCO2nvTVo6@coredump.intra.peff.net>
In-Reply-To: <20211209051115.52629-15-sunshine@sunshineco.com>

On Thu, Dec 09, 2021 at 12:11:10AM -0500, Eric Sunshine wrote:

Show 24 quoted lines

> diff --git a/t/t1050-large.sh b/t/t1050-large.sh
> index 99ff2866b7..0e4267c723 100755
> --- a/t/t1050-large.sh
> +++ b/t/t1050-large.sh
> @@ -51,27 +51,21 @@ EOF
>  test_expect_success 'add a large file or two' '
>  	git add large1 huge large2 &&
>  	# make sure we got a single packfile and no loose objects
> -	bad= count=0 idx= &&
> +	count=0 idx= &&
>  	for p in .git/objects/pack/pack-*.pack
>  	do
>  		count=$(( $count + 1 )) &&
> -		if test_path_is_file "$p" &&
> -		   idx=${p%.pack}.idx && test_path_is_file "$idx"
> -		then
> -			continue
> -		fi
> -		bad=t
> +		test_path_is_file "$p" &&
> +		idx=${p%.pack}.idx &&
> +		test_path_is_file "$idx" || return 1
>  	done &&
> -	test -z "$bad" &&

Thanks goodness. I had to read the original loop several times to understand what it was trying to do. The post-image is much nicer.

Show 14 quoted lines

> diff --git a/t/t9400-git-cvsserver-server.sh b/t/t9400-git-cvsserver-server.sh
> index 17f988edd2..a6a73effde 100755
> --- a/t/t9400-git-cvsserver-server.sh
> +++ b/t/t9400-git-cvsserver-server.sh
> @@ -350,10 +350,9 @@ test_expect_success 'cvs update (subdirectories)' \
>  	test_cmp "$dir/$filename" "../$dir/$filename"; then
>          :
>        else
> -        echo >failure
> +        exit 1
>        fi
> -    done) &&
> -   test ! -f failure'
> +    done)'

These all look good. I had to blink a few times to see the subshell in this one (it's finished by the closing paren after "done", for other reviewers).

-Peff