git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Fabian Stelzer <fs@gigacodes.de>
To: Eric Sunshine <sunshine@sunshineco.com>
Cc: git@vger.kernel.org, Jeff King <peff@peff.net>,
	Eric Sunshine <ericsunshine@gmail.com>
Subject: Re: [PATCH 17/19] t4000-t4999: detect and signal failure within loop
Date: Fri, 10 Dec 2021 10:53:03 +0100	[thread overview]
Message-ID: <20211210095303.xx3fxxtp5ww3s24x@fs> (raw)
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).

  reply	other threads:[~2021-12-10  9:53 UTC|newest]

Thread overview: 47+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-12-09  5:10 [PATCH 00/19] tests: fix broken &&-chains & abort loops on error Eric Sunshine
2021-12-09  5:10 ` [PATCH 01/19] t/lib-pager: use sane_unset() to avoid breaking &&-chain Eric Sunshine
2021-12-09  5:10 ` [PATCH 02/19] t1010: fix unnoticed failure on Windows Eric Sunshine
2021-12-09 16:27   ` Elijah Newren
2021-12-09 16:45     ` Eric Sunshine
2021-12-09  5:10 ` [PATCH 03/19] t1020: avoid aborting entire test script when one test fails Eric Sunshine
2021-12-09  5:11 ` [PATCH 04/19] t4202: clarify intent by creating expected content less cleverly Eric Sunshine
2021-12-10  9:09   ` Jeff King
2021-12-09  5:11 ` [PATCH 05/19] t5516: drop unnecessary subshell and command invocation Eric Sunshine
2021-12-10  9:10   ` Jeff King
2021-12-09  5:11 ` [PATCH 06/19] t6300: make `%(raw:size) --shell` test more robust Eric Sunshine
2021-12-10  9:14   ` Jeff King
2021-12-09  5:11 ` [PATCH 07/19] t9107: use shell parameter expansion to avoid breaking &&-chain Eric Sunshine
2021-12-09  5:11 ` [PATCH 08/19] tests: simplify construction of large blocks of text Eric Sunshine
2021-12-09  5:11 ` [PATCH 09/19] tests: use test_write_lines() to generate line-oriented output Eric Sunshine
2021-12-10  9:22   ` Jeff King
2021-12-11  6:59     ` Eric Sunshine
2021-12-09  5:11 ` [PATCH 10/19] tests: fix broken &&-chains in compound statements Eric Sunshine
2021-12-09  5:11 ` [PATCH 11/19] tests: fix broken &&-chains in `$(...)` command substitutions Eric Sunshine
2021-12-09 16:44   ` Elijah Newren
2021-12-09 16:53     ` Eric Sunshine
2021-12-09 16:57       ` Elijah Newren
2021-12-10  9:27       ` Jeff King
2021-12-09  5:11 ` [PATCH 12/19] tests: fix broken &&-chains in `{...}` groups Eric Sunshine
2021-12-10  9:29   ` Jeff King
2021-12-11  7:14     ` Eric Sunshine
2021-12-10  9:38   ` Fabian Stelzer
2021-12-11  7:32     ` Eric Sunshine
2021-12-09  5:11 ` [PATCH 13/19] tests: apply modern idiom for signaling test failure Eric Sunshine
2021-12-10  9:32   ` Jeff King
2021-12-11  7:47     ` Eric Sunshine
2021-12-09  5:11 ` [PATCH 14/19] tests: apply modern idiom for exiting loop upon failure Eric Sunshine
2021-12-10  9:36   ` Jeff King
2021-12-09  5:11 ` [PATCH 15/19] tests: simplify by dropping unnecessary `for` loops Eric Sunshine
2021-12-09 16:50   ` Elijah Newren
2021-12-09  5:11 ` [PATCH 16/19] t0000-t3999: detect and signal failure within loop Eric Sunshine
2021-12-09  5:11 ` [PATCH 17/19] t4000-t4999: " Eric Sunshine
2021-12-10  9:53   ` Fabian Stelzer [this message]
2021-12-11  8:06     ` Eric Sunshine
2021-12-09  5:11 ` [PATCH 18/19] t5000-t5999: " Eric Sunshine
2021-12-09  5:11 ` [PATCH 19/19] t6000-t9999: " Eric Sunshine
2021-12-09 17:02 ` [PATCH 00/19] tests: fix broken &&-chains & abort loops on error Elijah Newren
2021-12-09 19:17   ` Eric Sunshine
2021-12-10  9:38     ` Jeff King
2021-12-10  9:57       ` Fabian Stelzer
2021-12-11  8:16         ` Eric Sunshine
2021-12-11  9:58 ` [PATCH v1.1 2/19] t1010: fix unnoticed failure on Windows Eric Sunshine

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20211210095303.xx3fxxtp5ww3s24x@fs \
    --to=fs@gigacodes.de \
    --cc=ericsunshine@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=peff@peff.net \
    --cc=sunshine@sunshineco.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).