From: Fabian Stelzer <fs@gigacodes.de>
Date: Fri, 10 Dec 2021 09:53:03 GMT
Subject: Re: [PATCH 17/19] t4000-t4999: detect and signal failure within loop
Message-ID: <20211210095303.xx3fxxtp5ww3s24x@fs>
In-Reply-To: <20211209051115.52629-18-sunshine@sunshineco.com>

On 09.12.2021 00:11, Eric Sunshine wrote:
>Failures within `for` and `while` loops can go unnoticed if not detected
>and signaled manually since the loop itself does not abort when a
>contained command fails, nor will a failure necessarily be detected when
>the loop finishes since the loop returns the exit code of the last
>command it ran on the final iteration, which may not be the command
>which failed. Therefore, detect and signal failures manually within
>loops using the idiom `|| return 1` (or `|| exit 1` within subshells).
>
>Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
>---
>diff --git a/t/t4015-diff-whitespace.sh b/t/t4015-diff-whitespace.sh
>index 2c13b62d3c..ca5adabe14 100755
>--- a/t/t4015-diff-whitespace.sh
>+++ b/t/t4015-diff-whitespace.sh
>@@ -843,7 +843,7 @@ test_expect_success 'whitespace changes with modification reported (diffstat)' '
>
> test_expect_success 'whitespace-only changes reported across renames (diffstat)' '
> 	git reset --hard &&
>-	for i in 1 2 3 4 5 6 7 8 9; do echo "$i$i$i$i$i$i"; done >x &&
>+	for i in 1 2 3 4 5 6 7 8 9; do echo "$i$i$i$i$i$i" || return 1; done >x &&
> 	git add x &&
> 	git commit -m "base" &&
> 	sed -e "5s/^/ /" x >z &&
>@@ -859,7 +859,7 @@ test_expect_success 'whitespace-only changes reported across renames (diffstat)'
>
> test_expect_success 'whitespace-only changes reported across renames' '
> 	git reset --hard HEAD~1 &&
>-	for i in 1 2 3 4 5 6 7 8 9; do echo "$i$i$i$i$i$i"; done >x &&
>+	for i in 1 2 3 4 5 6 7 8 9; do echo "$i$i$i$i$i$i" || return 1; done >x &&
> 	git add x &&
> 	hash_x=$(git hash-object x) &&
> 	before=$(git rev-parse --short "$hash_x") &&

I understand why the `|| return` in loops makes sense. But for these very 
simple ones just using `echo` I'll probably be confused if a linter starts 
to complain about them (probably depending on how specific the lint error 
will be).