Git Mailing List Archive on lore.kernel.org
 help / color / Atom feed
From: Jeff King <peff@peff.net>
To: "SZEDER Gábor" <szeder@ira.uka.de>
Cc: git@vger.kernel.org
Subject: [PATCH 5/8] t: fix some trivial cases of ignored exit codes in loops
Date: Wed, 25 Mar 2015 01:29:52 -0400
Message-ID: <20150325052952.GE31924@peff.net> (raw)
In-Reply-To: <20150325052456.GA19394@peff.net>

These are all cases where we do a setup step of the form:

  for i in $foo; do
	  set_up $i || break
  done &&
  more_setup

would not notice a failure in set_up (because break always
returns a 0 exit code). These are just setup steps that we
do not expect to fail, but it does not hurt to be defensive.

Most can be fixed by converting the "break" to a "return 1"
(since we eval our tests inside a function for just this
purpose). A few of the loops are inside subshells, so we can
use just "exit 1" to break out of the subshell. And a few
can actually be made shorter by just unrolling the loop.

Signed-off-by: Jeff King <peff@peff.net>
---
 t/t3010-ls-files-killed-modified.sh | 11 ++++-------
 t/t3031-merge-criscross.sh          |  2 +-
 t/t3202-show-branch-octopus.sh      |  2 +-
 t/t4024-diff-optimize-common.sh     |  2 +-
 t/t4046-diff-unmerged.sh            |  8 ++++----
 t/t4151-am-abort.sh                 |  2 +-
 t/t5505-remote.sh                   |  8 ++++----
 t/t5514-fetch-multiple.sh           |  4 ++--
 t/t6026-merge-attr.sh               |  6 +++---
 t/t6040-tracking-info.sh            |  7 +++----
 10 files changed, 24 insertions(+), 28 deletions(-)

diff --git a/t/t3010-ls-files-killed-modified.sh b/t/t3010-ls-files-killed-modified.sh
index 62fce10..580e158 100755
--- a/t/t3010-ls-files-killed-modified.sh
+++ b/t/t3010-ls-files-killed-modified.sh
@@ -55,13 +55,10 @@ test_expect_success 'git update-index --add to add various paths.' '
 	: >path9 &&
 	date >path10 &&
 	git update-index --add -- path0 path?/file? pathx/ju path7 path8 path9 path10 &&
-	for i in 1 2
-	do
-		git init submod$i &&
-		(
-			cd submod$i && git commit --allow-empty -m "empty $i"
-		) || break
-	done &&
+	git init submod1 &&
+	git -C submod1 commit --allow-empty -m "empty 1" &&
+	git init submod2 &&
+	git -C submod2 commit --allow-empty -m "empty 2" &&
 	git update-index --add submod[12] &&
 	(
 		cd submod1 &&
diff --git a/t/t3031-merge-criscross.sh b/t/t3031-merge-criscross.sh
index 7f41607..e59b0a3 100755
--- a/t/t3031-merge-criscross.sh
+++ b/t/t3031-merge-criscross.sh
@@ -32,7 +32,7 @@ test_expect_success 'setup repo with criss-cross history' '
 	do
 		echo $n > data/$n &&
 		n=$(($n+1)) ||
-		break
+		return 1
 	done &&
 
 	# check them in
diff --git a/t/t3202-show-branch-octopus.sh b/t/t3202-show-branch-octopus.sh
index 0a5d5e6..6adf478 100755
--- a/t/t3202-show-branch-octopus.sh
+++ b/t/t3202-show-branch-octopus.sh
@@ -19,7 +19,7 @@ test_expect_success 'setup' '
 		> file$i &&
 		git add file$i &&
 		test_tick &&
-		git commit -m branch$i || break
+		git commit -m branch$i || return 1
 	done
 
 '
diff --git a/t/t4024-diff-optimize-common.sh b/t/t4024-diff-optimize-common.sh
index c4d733f..7e76018 100755
--- a/t/t4024-diff-optimize-common.sh
+++ b/t/t4024-diff-optimize-common.sh
@@ -139,7 +139,7 @@ test_expect_success setup '
 		( printf C; zs $n ) >file-c$n &&
 		( echo D; zs $n ) >file-d$n &&
 
-		expect_pattern $n || break
+		expect_pattern $n || return 1
 
 	done >expect
 '
diff --git a/t/t4046-diff-unmerged.sh b/t/t4046-diff-unmerged.sh
index 25d50a6..d0f1447 100755
--- a/t/t4046-diff-unmerged.sh
+++ b/t/t4046-diff-unmerged.sh
@@ -8,7 +8,7 @@ test_expect_success setup '
 	do
 		blob=$(echo $i | git hash-object --stdin) &&
 		eval "blob$i=$blob" &&
-		eval "m$i=\"100644 \$blob$i $i\"" || break
+		eval "m$i=\"100644 \$blob$i $i\"" || return 1
 	done &&
 	paths= &&
 	for b in o x
@@ -24,9 +24,9 @@ test_expect_success setup '
 				case "$b" in x) echo "$m1$p" ;; esac &&
 				case "$o" in x) echo "$m2$p" ;; esac &&
 				case "$t" in x) echo "$m3$p" ;; esac ||
-				break
-			done || break
-		done || break
+				return 1
+			done
+		done
 	done >ls-files-s.expect &&
 	git update-index --index-info <ls-files-s.expect &&
 	git ls-files -s >ls-files-s.actual &&
diff --git a/t/t4151-am-abort.sh b/t/t4151-am-abort.sh
index 1176bcc..8d90634 100755
--- a/t/t4151-am-abort.sh
+++ b/t/t4151-am-abort.sh
@@ -20,7 +20,7 @@ test_expect_success setup '
 		echo $i >otherfile-$i &&
 		git add otherfile-$i &&
 		test_tick &&
-		git commit -a -m $i || break
+		git commit -a -m $i || return 1
 	done &&
 	git format-patch --no-numbered initial &&
 	git checkout -b side initial &&
diff --git a/t/t5505-remote.sh b/t/t5505-remote.sh
index 17c6330..7a8499c 100755
--- a/t/t5505-remote.sh
+++ b/t/t5505-remote.sh
@@ -579,7 +579,7 @@ test_expect_success 'update with arguments' '
 		cd one &&
 		for b in $(git branch -r)
 		do
-		git branch -r -d $b || break
+		git branch -r -d $b || exit 1
 		done &&
 		git remote add manduca ../mirror &&
 		git remote add megaloprepus ../mirror &&
@@ -622,7 +622,7 @@ test_expect_success 'update default' '
 		cd one &&
 		for b in $(git branch -r)
 		do
-		git branch -r -d $b || break
+		git branch -r -d $b || exit 1
 		done &&
 		git config remote.drosophila.skipDefaultUpdate true &&
 		git remote update default &&
@@ -642,7 +642,7 @@ test_expect_success 'update default (overridden, with funny whitespace)' '
 		cd one &&
 		for b in $(git branch -r)
 		do
-		git branch -r -d $b || break
+		git branch -r -d $b || exit 1
 		done &&
 		git config remotes.default "$(printf "\t drosophila  \n")" &&
 		git remote update default &&
@@ -656,7 +656,7 @@ test_expect_success 'update (with remotes.default defined)' '
 		cd one &&
 		for b in $(git branch -r)
 		do
-		git branch -r -d $b || break
+		git branch -r -d $b || exit 1
 		done &&
 		git config remotes.default "drosophila" &&
 		git remote update &&
diff --git a/t/t5514-fetch-multiple.sh b/t/t5514-fetch-multiple.sh
index 0f81409..4b4b667 100755
--- a/t/t5514-fetch-multiple.sh
+++ b/t/t5514-fetch-multiple.sh
@@ -120,7 +120,7 @@ test_expect_success 'git fetch --all (skipFetchAll)' '
 	(cd test4 &&
 	 for b in $(git branch -r)
 	 do
-		git branch -r -d $b || break
+		git branch -r -d $b || exit 1
 	 done &&
 	 git remote add three ../three &&
 	 git config remote.three.skipFetchAll true &&
@@ -144,7 +144,7 @@ test_expect_success 'git fetch --multiple (ignoring skipFetchAll)' '
 	(cd test4 &&
 	 for b in $(git branch -r)
 	 do
-		git branch -r -d $b || break
+		git branch -r -d $b || exit 1
 	 done &&
 	 git fetch --multiple one two three &&
 	 git branch -r > output &&
diff --git a/t/t6026-merge-attr.sh b/t/t6026-merge-attr.sh
index 5e43997..3c21938 100755
--- a/t/t6026-merge-attr.sh
+++ b/t/t6026-merge-attr.sh
@@ -11,7 +11,7 @@ test_expect_success setup '
 
 	for f in text binary union
 	do
-		echo Initial >$f && git add $f || break
+		echo Initial >$f && git add $f || return 1
 	done &&
 	test_tick &&
 	git commit -m Initial &&
@@ -19,7 +19,7 @@ test_expect_success setup '
 	git branch side &&
 	for f in text binary union
 	do
-		echo Master >>$f && git add $f || break
+		echo Master >>$f && git add $f || return 1
 	done &&
 	test_tick &&
 	git commit -m Master &&
@@ -27,7 +27,7 @@ test_expect_success setup '
 	git checkout side &&
 	for f in text binary union
 	do
-		echo Side >>$f && git add $f || break
+		echo Side >>$f && git add $f || return 1
 	done &&
 	test_tick &&
 	git commit -m Side &&
diff --git a/t/t6040-tracking-info.sh b/t/t6040-tracking-info.sh
index 7ac8fd0..3d5c238 100755
--- a/t/t6040-tracking-info.sh
+++ b/t/t6040-tracking-info.sh
@@ -12,10 +12,9 @@ advance () {
 }
 
 test_expect_success setup '
-	for i in a b c;
-	do
-		advance $i || break
-	done &&
+	advance a &&
+	advance b &&
+	advance c &&
 	git clone . test &&
 	(
 		cd test &&
-- 
2.3.4.635.gd6ffcfe

  parent reply index

Thread overview: 68+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-03-20 10:04 [PATCH 0/25] detecting &&-chain breakage Jeff King
2015-03-20 10:05 ` [PATCH 01/25] t/test-lib: introduce --chain-lint option Jeff King
2015-03-25  2:53   ` SZEDER Gábor
2015-03-25  3:05     ` Jeff King
2015-03-20 10:06 ` [PATCH 02/25] t: fix severe &&-chain breakage Jeff King
2015-03-20 10:06 ` [PATCH 03/25] t: fix moderate " Jeff King
2015-03-20 10:07 ` [PATCH 04/25] t: fix trivial " Jeff King
2015-03-20 10:07 ` [PATCH 05/25] t: assume test_cmp produces verbose output Jeff King
2015-03-20 10:09 ` [PATCH 06/25] t: use verbose instead of hand-rolled errors Jeff King
2015-03-20 10:09 ` [PATCH 07/25] t: use test_must_fail instead of hand-rolled blocks Jeff King
2015-03-20 10:10 ` [PATCH 08/25] t: fix &&-chaining issues around setup which might fail Jeff King
2015-03-20 10:11 ` [PATCH 09/25] t: use test_might_fail for diff and grep Jeff King
2015-03-20 10:11 ` [PATCH 10/25] t: use test_expect_code instead of hand-rolled comparison Jeff King
2015-03-20 10:12 ` [PATCH 11/25] t: wrap complicated expect_code users in a block Jeff King
2015-03-20 10:12 ` [PATCH 12/25] t: avoid using ":" for comments Jeff King
2015-03-20 10:12 ` [PATCH 13/25] t3600: fix &&-chain breakage for setup commands Jeff King
2015-03-20 10:12 ` [PATCH 14/25] t7201: fix &&-chain breakage Jeff King
2015-03-20 10:13 ` [PATCH 15/25] t9502: " Jeff King
2015-03-20 17:48   ` Johannes Sixt
2015-03-20 18:03     ` Jeff King
2015-03-20 10:13 ` [PATCH 16/25] t6030: use modern test_* helpers Jeff King
2015-03-20 10:13 ` [PATCH 17/25] t0020: " Jeff King
2015-03-25  0:23   ` SZEDER Gábor
2015-03-25  2:56     ` Jeff King
2015-03-25  5:24       ` [PATCH 0/8] more &&-chaining test fixups Jeff King
2015-03-25  5:25         ` [PATCH 1/8] perf-lib: fix ignored exit code inside loop Jeff King
2015-03-25  5:28         ` [PATCH 2/8] t0020: fix ignored exit code inside loops Jeff King
2015-03-25  5:28         ` [PATCH 3/8] t3305: fix ignored exit code inside loop Jeff King
2015-03-25  8:40           ` Johan Herland
2015-03-25  5:29         ` [PATCH 4/8] t7701: " Jeff King
2015-03-25  5:29         ` Jeff King [this message]
2015-03-25  5:30         ` [PATCH 6/8] t: simplify loop exit-code status variables Jeff King
2015-03-25 17:27           ` Junio C Hamano
2015-03-25 17:43             ` Jeff King
2015-03-25  5:31         ` [PATCH 7/8] t0020: use test_* helpers instead of hand-rolled messages Jeff King
2015-03-25  5:32         ` [PATCH 8/8] t9001: drop save_confirm helper Jeff King
2015-03-25 17:29         ` [PATCH 0/8] more &&-chaining test fixups Junio C Hamano
2015-03-20 10:13 ` [PATCH 18/25] t1301: use modern test_* helpers Jeff King
2015-03-24 23:51   ` SZEDER Gábor
2015-03-25  2:45     ` Jeff King
2015-03-20 10:13 ` [PATCH 19/25] t6034: " Jeff King
2015-03-24 23:43   ` SZEDER Gábor
2015-03-20 10:13 ` [PATCH 20/25] t4117: " Jeff King
2015-03-20 10:13 ` [PATCH 21/25] t9001: use test_when_finished Jeff King
2015-03-25  2:00   ` SZEDER Gábor
2015-03-25  2:47     ` Jeff King
2015-03-20 10:13 ` [PATCH 22/25] t0050: appease --chain-lint Jeff King
2015-03-20 10:13 ` [PATCH 23/25] t7004: fix embedded single-quotes Jeff King
2015-03-20 10:13 ` [PATCH 24/25] t0005: fix broken &&-chains Jeff King
2015-03-20 10:13 ` [PATCH 25/25] t4104: drop hand-rolled error reporting Jeff King
2015-03-20 10:23 ` [PATCH 0/25] detecting &&-chain breakage Jeff King
2015-03-20 14:28 ` Michael J Gruber
2015-03-20 14:32   ` [PATCH 26/27] t/*svn*: fix moderate " Michael J Gruber
2015-03-20 14:32     ` [PATCH 27/27] t9104: fix test for following larger parents Michael J Gruber
2015-03-20 18:04     ` [PATCH 26/27] t/*svn*: fix moderate &&-chain breakage Junio C Hamano
2015-03-20 19:35       ` Junio C Hamano
2015-03-20 20:02         ` Jeff King
2015-03-20 20:13           ` Jeff King
2015-03-23  9:36             ` Michael J Gruber
2015-03-20 17:57   ` [PATCH 0/25] detecting " Jeff King
2015-03-20 17:44 ` Junio C Hamano
2015-03-20 18:00   ` Junio C Hamano
2015-03-20 18:04     ` Jeff King
2015-03-20 18:33       ` Junio C Hamano
2015-03-20 23:18 ` Eric Sunshine
2015-03-21  8:19   ` Jeff King
2015-03-21 18:01     ` Junio C Hamano
2015-03-21 22:23       ` Jeff King

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=20150325052952.GE31924@peff.net \
    --to=peff@peff.net \
    --cc=git@vger.kernel.org \
    --cc=szeder@ira.uka.de \
    /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

Git Mailing List Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/git/0 git/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 git git/ https://lore.kernel.org/git \
		git@vger.kernel.org
	public-inbox-index git

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.git


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git