git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
* [PATCH 00/25] fix buggy tests, modernize tests, fix broken &&-chains
@ 2018-07-02  0:23 Eric Sunshine
  2018-07-02  0:23 ` [PATCH 01/25] t: use test_might_fail() instead of manipulating exit code manually Eric Sunshine
                   ` (26 more replies)
  0 siblings, 27 replies; 52+ messages in thread
From: Eric Sunshine @ 2018-07-02  0:23 UTC (permalink / raw)
  To: git
  Cc: Elijah Newren, Johannes Sixt, Jonathan Nieder, Jonathan Tan,
	Stefan Beller, Junio C Hamano, Luke Diamand, Jeff King,
	Eric Sunshine

This series fixes several buggy tests which went undetected due to
broken &&-chains in subshells, modernizes some tests to take advantage
of test functions (test_might_fail(), test_write_lines(), etc.), and
fixes a lot of broken &&-chains in subshells. It applies atop
'master'. Happily, there are no broken &&-chains in subshells in any
in-flight topic.

It is split out from an earlier series[1] which additionally extended
--chain-lint to work within subshells. I decided to make this an
independent series because these (hopefully) non-controversial changes
all stand on their own merit, and because I don't want to flood the list
repeatedly with this lengthy series as I re-roll the "extend
--chain-lint to work in subshells" functionality from [1].

To ease review burden, I largely avoided noisy modernizations and
cleanups, and limited changes to merely adding "&&" even when some other
transformation would have made the fix nicer overall. (For example, I
resisted the urge to replace a series of 'echo' statements with a simple
here-doc.)

Changes since [1]:

* Found and fixed more &&-chain breakage, and converted a couple more
  'unset' instances (which were hidden behind a MINGW prerequisite) to
  sane_unset().

* Rewrote commit messages to sell changes on their own merit rather than
  leaning on --chain-lint as a crutch. (junio, jrnieder)

* Changed a modernization/cleanup to use "! non-git-command" rather than
  test_must_fail(), moving it to its own patch in the process. (j6t)

* Changed "printf '%s\n'" idiom to test_write_lines(). (junio)

* Incorporated Stefan's fix[2] for a broken 't/lib-submodule-update'
  test since my interpretation of the problem was incorrect.

* Converted a subshell 'echo' sequence to write_script().

* Dropped patches which existed primarily to pacify --chain-lint; they
  are no longer needed since I re-wrote the "linter" to detect &&-chain
  breakage itself (by pure textual inspection) rather than by
  incorporating subshell bodies into the main &&-chain:

  t0001: use "{...}" block around "||" expression rather than subshell
  t3303: use standard here-doc tag "EOF" to avoid fooling --chain-lint
  t9104: use "{...}" block around "||" expression rather than subshell
  t9401: drop unnecessary nested subshell

* Dropped a patch which pretty much duplicated Junio's 037714252f
  (tests: clean after SANITY tests, 2018-06-15), which graduated to
  'master':

  t7508: use test_when_finished() instead of managing exit code manually

An interdiff against [1] is below, although I stripped out all the noisy
"printf '%s\n'" to test_write_lines() differences, of which there were a
lot, since they drowned out the other more significant changes.

Thanks to Elijah, Hannes, Jonathan Nieder, Jonathan Tan, Junio, Luke,
Peff, and Stefan for comments on [1].

[1]: https://public-inbox.org/git/20180626073001.6555-1-sunshine@sunshineco.com/
[2]: https://public-inbox.org/git/20180627183057.254467-1-sbeller@google.com/

Eric Sunshine (25):
  t: use test_might_fail() instead of manipulating exit code manually
  t: use test_write_lines() instead of series of 'echo' commands
  t: use sane_unset() rather than 'unset' with broken &&-chain
  t: drop unnecessary terminating semicolon in subshell
  t/lib-submodule-update: fix "absorbing" test
  t5405: use test_must_fail() instead of checking exit code manually
  t5406: use write_script() instead of birthing shell script manually
  t5505: modernize and simplify hard-to-digest test
  t6036: fix broken "merge fails but has appropriate contents" tests
  t7201: drop pointless "exit 0" at end of subshell
  t7400: fix broken "submodule add/reconfigure --force" test
  t7810: use test_expect_code() instead of hand-rolled comparison
  t9001: fix broken "invoke hook" test
  t9814: simplify convoluted check that command correctly errors out
  t0000-t0999: fix broken &&-chains
  t1000-t1999: fix broken &&-chains
  t2000-t2999: fix broken &&-chains
  t3000-t3999: fix broken &&-chains
  t3030: fix broken &&-chains
  t4000-t4999: fix broken &&-chains
  t5000-t5999: fix broken &&-chains
  t6000-t6999: fix broken &&-chains
  t7000-t7999: fix broken &&-chains
  t9000-t9999: fix broken &&-chains
  t9119: fix broken &&-chains

 t/lib-submodule-update.sh                     |   5 +-
 t/t0000-basic.sh                              |   2 +-
 t/t0001-init.sh                               |   4 +-
 t/t0003-attributes.sh                         |  24 +-
 t/t0021-conversion.sh                         |   4 +-
 t/t0090-cache-tree.sh                         |   2 +-
 t/t1004-read-tree-m-u-wf.sh                   |   8 +-
 t/t1005-read-tree-reset.sh                    |  10 +-
 t/t1008-read-tree-overlay.sh                  |   2 +-
 t/t1020-subdirectory.sh                       |   2 +-
 t/t1050-large.sh                              |   6 +-
 t/t1300-config.sh                             |   2 +-
 t/t1411-reflog-show.sh                        |   6 +-
 t/t1507-rev-parse-upstream.sh                 |   6 +-
 t/t1512-rev-parse-disambiguation.sh           |   6 +-
 t/t1700-split-index.sh                        |   2 +-
 t/t2016-checkout-patch.sh                     |  24 +-
 t/t2103-update-index-ignore-missing.sh        |   2 +-
 t/t2202-add-addremove.sh                      |  14 +-
 t/t3000-ls-files-others.sh                    |   2 +-
 t/t3006-ls-files-long.sh                      |   2 +-
 t/t3008-ls-files-lazy-init-name-hash.sh       |   8 +-
 t/t3030-merge-recursive.sh                    | 340 +++++++++---------
 t/t3050-subprojects-fetch.sh                  |   8 +-
 t/t3102-ls-tree-wildcards.sh                  |   2 +-
 t/t3210-pack-refs.sh                          |   4 +-
 t/t3301-notes.sh                              |   8 +-
 t/t3400-rebase.sh                             |   8 +-
 t/t3402-rebase-merge.sh                       |   4 +-
 t/t3404-rebase-interactive.sh                 |   6 +-
 t/t3418-rebase-continue.sh                    |   4 +-
 t/t3700-add.sh                                |   8 +-
 t/t3701-add-interactive.sh                    |  16 +-
 t/t3904-stash-patch.sh                        |   8 +-
 t/t4001-diff-rename.sh                        |   2 +-
 t/t4010-diff-pathspec.sh                      |   4 +-
 t/t4012-diff-binary.sh                        |   6 +-
 t/t4024-diff-optimize-common.sh               |  16 +-
 t/t4025-hunk-header.sh                        |   8 +-
 t/t4041-diff-submodule-option.sh              |   4 +-
 t/t4060-diff-submodule-option-diff-format.sh  |   2 +-
 t/t4121-apply-diffs.sh                        |   2 +-
 t/t5300-pack-object.sh                        |   2 +-
 t/t5302-pack-index.sh                         |   2 +-
 t/t5400-send-pack.sh                          |   4 +-
 t/t5401-update-hooks.sh                       |   4 +-
 t/t5405-send-pack-rewind.sh                   |   3 +-
 t/t5406-remote-rejects.sh                     |   5 +-
 t/t5500-fetch-pack.sh                         |   2 +-
 t/t5505-remote.sh                             |  10 +-
 t/t5512-ls-remote.sh                          |   4 +-
 t/t5516-fetch-push.sh                         |  10 +-
 t/t5517-push-mirror.sh                        |  10 +-
 t/t5526-fetch-submodules.sh                   |   2 +-
 t/t5531-deep-submodule-push.sh                |   2 +-
 t/t5543-atomic-push.sh                        |   2 +-
 t/t5601-clone.sh                              |   2 +-
 t/t5605-clone-local.sh                        |   2 +-
 t/t5801-remote-helpers.sh                     |   2 +-
 t/t6010-merge-base.sh                         |   2 +-
 t/t6029-merge-subtree.sh                      |  16 +-
 t/t6036-recursive-corner-cases.sh             |  14 +-
 t/t6042-merge-rename-corner-cases.sh          |   8 +-
 t/t6043-merge-rename-directories.sh           |   2 +-
 t/t7001-mv.sh                                 |   2 +-
 t/t7105-reset-patch.sh                        |  12 +-
 t/t7201-co.sh                                 |  41 ++-
 t/t7301-clean-interactive.sh                  |  41 ++-
 t/t7400-submodule-basic.sh                    |  12 +-
 t/t7406-submodule-update.sh                   |   6 +-
 t/t7408-submodule-reference.sh                |   2 +-
 t/t7501-commit.sh                             |  56 +--
 t/t7506-status-submodule.sh                   |  10 +-
 t/t7610-mergetool.sh                          |   8 +-
 t/t7810-grep.sh                               |   7 +-
 t/t9001-send-email.sh                         |  10 +-
 t/t9100-git-svn-basic.sh                      |   2 +-
 t/t9101-git-svn-props.sh                      |   2 +-
 t/t9119-git-svn-info.sh                       | 120 +++----
 t/t9122-git-svn-author.sh                     |   6 +-
 t/t9129-git-svn-i18n-commitencoding.sh        |   2 +-
 t/t9130-git-svn-authors-file.sh               |   4 +-
 t/t9134-git-svn-ignore-paths.sh               |   6 +-
 t/t9137-git-svn-dcommit-clobber-series.sh     |   2 +-
 t/t9138-git-svn-authors-prog.sh               |   6 +-
 t/t9146-git-svn-empty-dirs.sh                 |  20 +-
 t/t9147-git-svn-include-paths.sh              |   6 +-
 t/t9152-svn-empty-dirs-after-gc.sh            |   2 +-
 t/t9164-git-svn-dcommit-concurrent.sh         |   2 +-
 ...65-git-svn-fetch-merge-branch-of-branch.sh |   2 +-
 t/t9200-git-cvsexportcommit.sh                |   6 +-
 t/t9302-fast-import-unpack-limit.sh           |   2 +-
 t/t9400-git-cvsserver-server.sh               |   8 +-
 t/t9600-cvsimport.sh                          |   2 +-
 t/t9806-git-p4-options.sh                     |   2 +-
 t/t9810-git-p4-rcs.sh                         |   2 +-
 t/t9811-git-p4-label-import.sh                |   2 +-
 t/t9814-git-p4-rename.sh                      |  18 +-
 t/t9815-git-p4-submit-fail.sh                 |   2 +-
 t/t9830-git-p4-symlink-dir.sh                 |   2 +-
 t/t9831-git-p4-triggers.sh                    |   2 +-
 t/t9902-completion.sh                         |   4 +-
 t/t9903-bash-prompt.sh                        |   2 +-
 103 files changed, 567 insertions(+), 589 deletions(-)

-- 
2.18.0.203.gfac676dfb9


Interdiff since [1]:

diff --git a/t/lib-submodule-update.sh b/t/lib-submodule-update.sh
index 8a2edee1cb..e90ec79087 100755
--- a/t/lib-submodule-update.sh
+++ b/t/lib-submodule-update.sh
@@ -959,7 +959,7 @@ test_submodule_forced_switch_recursing_with_args () {
 		)
 	'
 	# ... absorbing a .git directory.
-	test_expect_failure "$command: replace submodule containing a .git directory with a directory must fail" '
+	test_expect_success "$command: replace submodule containing a .git directory with a directory must fail" '
 		prolog &&
 		reset_work_tree_to_interested add_sub1 &&
 		(
@@ -967,9 +967,8 @@ test_submodule_forced_switch_recursing_with_args () {
 			git branch -t replace_sub1_with_directory origin/replace_sub1_with_directory &&
 			replace_gitfile_with_git_dir sub1 &&
 			rm -rf .git/modules/sub1 &&
-			test_must_fail $command replace_sub1_with_directory &&
+			$command replace_sub1_with_directory &&
 			test_superproject_content origin/replace_sub1_with_directory &&
-			test_submodule_content sub1 origin/modify_sub1 &&
 			test_git_directory_exists sub1
 		)
 	'
diff --git a/t/t0001-init.sh b/t/t0001-init.sh
index 4c865051e7..ca85aae51e 100755
--- a/t/t0001-init.sh
+++ b/t/t0001-init.sh
@@ -408,7 +408,7 @@ is_hidden () {
 test_expect_success MINGW '.git hidden' '
 	rm -rf newdir &&
 	(
-		unset GIT_DIR GIT_WORK_TREE
+		sane_unset GIT_DIR GIT_WORK_TREE &&
 		mkdir newdir &&
 		cd newdir &&
 		git init &&
@@ -420,7 +420,7 @@ test_expect_success MINGW '.git hidden' '
 test_expect_success MINGW 'bare git dir not hidden' '
 	rm -rf newdir &&
 	(
-		unset GIT_DIR GIT_WORK_TREE GIT_CONFIG
+		sane_unset GIT_DIR GIT_WORK_TREE GIT_CONFIG &&
 		mkdir newdir &&
 		cd newdir &&
 		git --bare init
diff --git a/t/t1008-read-tree-overlay.sh b/t/t1008-read-tree-overlay.sh
index e74b185b6c..cf96016844 100755
--- a/t/t1008-read-tree-overlay.sh
+++ b/t/t1008-read-tree-overlay.sh
@@ -23,7 +23,7 @@ test_expect_success setup '
 
 test_expect_success 'multi-read' '
 	read_tree_must_succeed initial master side &&
-	(echo a && echo b/c) >expect &&
+	test_write_lines a b/c >expect &&
 	git ls-files >actual &&
 	test_cmp expect actual
 '
diff --git a/t/t3210-pack-refs.sh b/t/t3210-pack-refs.sh
index afa27ffe2d..724b4b9bc0 100755
--- a/t/t3210-pack-refs.sh
+++ b/t/t3210-pack-refs.sh
@@ -231,9 +231,9 @@ test_expect_success 'timeout if packed-refs.lock exists' '
 test_expect_success 'retry acquiring packed-refs.lock' '
 	LOCK=.git/packed-refs.lock &&
 	>"$LOCK" &&
-	test_when_finished "wait; rm -f $LOCK" &&
+	test_when_finished "wait && rm -f $LOCK" &&
 	{
-		( sleep 1 ; rm -f $LOCK ) &
+		( sleep 1 && rm -f $LOCK ) &
 	} &&
 	git -c core.packedrefstimeout=3000 pack-refs --all --prune
 '
diff --git a/t/t4024-diff-optimize-common.sh b/t/t4024-diff-optimize-common.sh
index 7e76018296..6b44ce1493 100755
--- a/t/t4024-diff-optimize-common.sh
+++ b/t/t4024-diff-optimize-common.sh
@@ -127,17 +127,17 @@ test_expect_success setup '
 
 	for n in $sample
 	do
-		( zs $n ; echo a ) >file-a$n &&
-		( echo b; zs $n; echo ) >file-b$n &&
-		( printf c; zs $n ) >file-c$n &&
-		( echo d; zs $n ) >file-d$n &&
+		( zs $n && echo a ) >file-a$n &&
+		( echo b && zs $n && echo ) >file-b$n &&
+		( printf c && zs $n ) >file-c$n &&
+		( echo d && zs $n ) >file-d$n &&
 
 		git add file-a$n file-b$n file-c$n file-d$n &&
 
-		( zs $n ; echo A ) >file-a$n &&
-		( echo B; zs $n; echo ) >file-b$n &&
-		( printf C; zs $n ) >file-c$n &&
-		( echo D; zs $n ) >file-d$n &&
+		( zs $n && echo A ) >file-a$n &&
+		( echo B && zs $n && echo ) >file-b$n &&
+		( printf C && zs $n ) >file-c$n &&
+		( echo D && zs $n ) >file-d$n &&
 
 		expect_pattern $n || return 1
 
diff --git a/t/t5406-remote-rejects.sh b/t/t5406-remote-rejects.sh
index 350d2e6ea5..ff06f99649 100755
--- a/t/t5406-remote-rejects.sh
+++ b/t/t5406-remote-rejects.sh
@@ -6,8 +6,9 @@ test_description='remote push rejects are reported by client'
 
 test_expect_success 'setup' '
 	mkdir .git/hooks &&
-	(echo "#!/bin/sh" && echo "exit 1") >.git/hooks/update &&
-	chmod +x .git/hooks/update &&
+	write_script .git/hooks/update <<-\EOF &&
+	exit 1
+	EOF
 	echo 1 >file &&
 	git add file &&
 	git commit -m 1 &&
diff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh
index b141145fc2..f604ef7a72 100755
--- a/t/t7406-submodule-update.sh
+++ b/t/t7406-submodule-update.sh
@@ -886,7 +886,7 @@ test_expect_success 'submodule update properly revives a moved submodule' '
 	 git commit -am "pre move" &&
 	 H2=$(git rev-parse --short HEAD) &&
 	 git status | sed "s/$H/XXX/" >expect &&
-	 H=$(cd submodule2; git rev-parse HEAD) &&
+	 H=$(cd submodule2 && git rev-parse HEAD) &&
 	 git rm --cached submodule2 &&
 	 rm -rf submodule2 &&
 	 mkdir -p "moved/sub module" &&
diff --git a/t/t9146-git-svn-empty-dirs.sh b/t/t9146-git-svn-empty-dirs.sh
index 954948812c..5f91c0d68b 100755
--- a/t/t9146-git-svn-empty-dirs.sh
+++ b/t/t9146-git-svn-empty-dirs.sh
@@ -21,7 +21,7 @@ test_expect_success 'empty directories exist' '
 		do
 			if ! test -d "$i"
 			then
-				echo >&2 "$i does not exist"
+				echo >&2 "$i does not exist" &&
 				exit 1
 			fi
 		done
@@ -38,7 +38,7 @@ test_expect_success 'option automkdirs set to false' '
 		do
 			if test -d "$i"
 			then
-				echo >&2 "$i exists"
+				echo >&2 "$i exists" &&
 				exit 1
 			fi
 		done
@@ -63,7 +63,7 @@ test_expect_success 'git svn mkdirs recreates empty directories' '
 		do
 			if ! test -d "$i"
 			then
-				echo >&2 "$i does not exist"
+				echo >&2 "$i does not exist" &&
 				exit 1
 			fi
 		done
@@ -148,7 +148,7 @@ test_expect_success 'git svn gc-ed files work' '
 			do
 				if ! test -d "$i"
 				then
-					echo >&2 "$i does not exist"
+					echo >&2 "$i does not exist" &&
 					exit 1
 				fi
 			done
diff --git a/t/t9152-svn-empty-dirs-after-gc.sh b/t/t9152-svn-empty-dirs-after-gc.sh
index 301e779709..89f285d082 100755
--- a/t/t9152-svn-empty-dirs-after-gc.sh
+++ b/t/t9152-svn-empty-dirs-after-gc.sh
@@ -30,7 +30,7 @@ test_expect_success 'git svn mkdirs recreates empty directories after git svn gc
 		do
 			if ! test -d "$i"
 			then
-				echo >&2 "$i does not exist"
+				echo >&2 "$i does not exist" &&
 				exit 1
 			fi
 		done
diff --git a/t/t9302-fast-import-unpack-limit.sh b/t/t9302-fast-import-unpack-limit.sh
index a04de14677..bb1c39cfcc 100755
--- a/t/t9302-fast-import-unpack-limit.sh
+++ b/t/t9302-fast-import-unpack-limit.sh
@@ -80,7 +80,7 @@ test_expect_success 'lookups after checkpoint works' '
 		do
 			if test $n -gt 30
 			then
-				echo >&2 "checkpoint did not update branch"
+				echo >&2 "checkpoint did not update branch" &&
 				exit 1
 			else
 				n=$(($n + 1))
diff --git a/t/t9400-git-cvsserver-server.sh b/t/t9400-git-cvsserver-server.sh
index 4207e9caa5..a5e5dca753 100755
--- a/t/t9400-git-cvsserver-server.sh
+++ b/t/t9400-git-cvsserver-server.sh
@@ -328,7 +328,7 @@ test_expect_success 'cvs update (subdirectories)' \
   '(for dir in A A/B A/B/C A/D E; do
       mkdir $dir &&
       echo "test file in $dir" >"$dir/file_in_$(echo $dir|sed -e "s#/# #g")"  &&
-      git add $dir;
+      git add $dir
    done) &&
    git commit -q -m "deep sub directory structure" &&
    git push gitcvs.git >/dev/null &&
diff --git a/t/t9814-git-p4-rename.sh b/t/t9814-git-p4-rename.sh
index 80aac5ab16..60baa06e27 100755
--- a/t/t9814-git-p4-rename.sh
+++ b/t/t9814-git-p4-rename.sh
@@ -9,11 +9,11 @@ test_expect_success 'start p4d' '
 '
 
 # We rely on this behavior to detect for p4 move availability.
-test_expect_success 'p4 help unknown returns 1' '
+test_expect_success '"p4 help unknown" errors out' '
 	(
 		cd "$cli" &&
 		p4 help client &&
-		test_must_fail p4 help nosuchcommand
+		! p4 help nosuchcommand
 	)
 '

^ permalink raw reply related	[flat|nested] 52+ messages in thread

* [PATCH 01/25] t: use test_might_fail() instead of manipulating exit code manually
  2018-07-02  0:23 [PATCH 00/25] fix buggy tests, modernize tests, fix broken &&-chains Eric Sunshine
@ 2018-07-02  0:23 ` Eric Sunshine
  2018-07-02 17:44   ` Stefan Beller
  2018-07-03 19:20   ` Junio C Hamano
  2018-07-02  0:23 ` [PATCH 02/25] t: use test_write_lines() instead of series of 'echo' commands Eric Sunshine
                   ` (25 subsequent siblings)
  26 siblings, 2 replies; 52+ messages in thread
From: Eric Sunshine @ 2018-07-02  0:23 UTC (permalink / raw)
  To: git
  Cc: Elijah Newren, Johannes Sixt, Jonathan Nieder, Jonathan Tan,
	Stefan Beller, Junio C Hamano, Luke Diamand, Jeff King,
	Eric Sunshine

These tests manually coerce the exit code of invoked commands to
"success" when they don't care if the command succeeds or fails since
failure of those commands should not cause the test to fail overall.
In doing so, they intentionally break the &&-chain. Modernize by
replacing manual exit code management with test_might_fail() and a
normal &&-chain.

Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
---
 t/t1507-rev-parse-upstream.sh | 6 +++---
 t/t1700-split-index.sh        | 2 +-
 t/t4012-diff-binary.sh        | 6 ++----
 t/t5400-send-pack.sh          | 4 ++--
 4 files changed, 8 insertions(+), 10 deletions(-)

diff --git a/t/t1507-rev-parse-upstream.sh b/t/t1507-rev-parse-upstream.sh
index 93c77eac45..349f6e10af 100755
--- a/t/t1507-rev-parse-upstream.sh
+++ b/t/t1507-rev-parse-upstream.sh
@@ -123,9 +123,9 @@ test_expect_success 'checkout -b new my-side@{u} forks from the same' '
 
 test_expect_success 'merge my-side@{u} records the correct name' '
 (
-	cd clone || exit
-	git checkout master || exit
-	git branch -D new ;# can fail but is ok
+	cd clone &&
+	git checkout master &&
+	test_might_fail git branch -D new &&
 	git branch -t new my-side@{u} &&
 	git merge -s ours new@{u} &&
 	git show -s --pretty=tformat:%s >actual &&
diff --git a/t/t1700-split-index.sh b/t/t1700-split-index.sh
index 1e81b33b2e..39133bcbc8 100755
--- a/t/t1700-split-index.sh
+++ b/t/t1700-split-index.sh
@@ -435,7 +435,7 @@ test_expect_success 'writing split index with null sha1 does not write cache tre
 	commit=$(git commit-tree $tree -p HEAD <msg) &&
 	git update-ref HEAD "$commit" &&
 	GIT_ALLOW_NULL_SHA1=1 git reset --hard &&
-	(test-tool dump-cache-tree >cache-tree.out || true) &&
+	test_might_fail test-tool dump-cache-tree >cache-tree.out &&
 	test_line_count = 0 cache-tree.out
 '
 
diff --git a/t/t4012-diff-binary.sh b/t/t4012-diff-binary.sh
index 0a8af76aab..6579c81216 100755
--- a/t/t4012-diff-binary.sh
+++ b/t/t4012-diff-binary.sh
@@ -102,10 +102,8 @@ test_expect_success 'apply binary patch' '
 
 test_expect_success 'diff --no-index with binary creation' '
 	echo Q | q_to_nul >binary &&
-	(: hide error code from diff, which just indicates differences
-	 git diff --binary --no-index /dev/null binary >current ||
-	 true
-	) &&
+	# hide error code from diff, which just indicates differences
+	test_might_fail git diff --binary --no-index /dev/null binary >current &&
 	rm binary &&
 	git apply --binary <current &&
 	echo Q >expected &&
diff --git a/t/t5400-send-pack.sh b/t/t5400-send-pack.sh
index 911eae1bf7..f1932ea431 100755
--- a/t/t5400-send-pack.sh
+++ b/t/t5400-send-pack.sh
@@ -86,7 +86,7 @@ test_expect_success 'push can be used to delete a ref' '
 test_expect_success 'refuse deleting push with denyDeletes' '
 	(
 	    cd victim &&
-	    ( git branch -D extra || : ) &&
+	    test_might_fail git branch -D extra &&
 	    git config receive.denyDeletes true &&
 	    git branch extra master
 	) &&
@@ -119,7 +119,7 @@ test_expect_success 'override denyDeletes with git -c receive-pack' '
 test_expect_success 'denyNonFastforwards trumps --force' '
 	(
 	    cd victim &&
-	    ( git branch -D extra || : ) &&
+	    test_might_fail git branch -D extra &&
 	    git config receive.denyNonFastforwards true
 	) &&
 	victim_orig=$(cd victim && git rev-parse --verify master) &&
-- 
2.18.0.203.gfac676dfb9


^ permalink raw reply related	[flat|nested] 52+ messages in thread

* [PATCH 02/25] t: use test_write_lines() instead of series of 'echo' commands
  2018-07-02  0:23 [PATCH 00/25] fix buggy tests, modernize tests, fix broken &&-chains Eric Sunshine
  2018-07-02  0:23 ` [PATCH 01/25] t: use test_might_fail() instead of manipulating exit code manually Eric Sunshine
@ 2018-07-02  0:23 ` Eric Sunshine
  2018-07-02 17:53   ` Stefan Beller
  2018-07-02  0:23 ` [PATCH 03/25] t: use sane_unset() rather than 'unset' with broken &&-chain Eric Sunshine
                   ` (24 subsequent siblings)
  26 siblings, 1 reply; 52+ messages in thread
From: Eric Sunshine @ 2018-07-02  0:23 UTC (permalink / raw)
  To: git
  Cc: Elijah Newren, Johannes Sixt, Jonathan Nieder, Jonathan Tan,
	Stefan Beller, Junio C Hamano, Luke Diamand, Jeff King,
	Eric Sunshine

These tests employ a noisy subshell (with missing &&-chain) to feed
input into Git commands or files:

    (echo a; echo b; echo c) | git some-command ...

Simplify by taking advantage of test_write_lines():

    test_write_lines a b c | git some-command ...

Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
---
 t/t0090-cache-tree.sh         |  2 +-
 t/t1008-read-tree-overlay.sh  |  2 +-
 t/t2016-checkout-patch.sh     | 24 ++++++++++----------
 t/t3404-rebase-interactive.sh |  6 ++---
 t/t3701-add-interactive.sh    | 16 +++++++-------
 t/t3904-stash-patch.sh        |  8 +++----
 t/t7105-reset-patch.sh        | 12 +++++-----
 t/t7301-clean-interactive.sh  | 41 +++++++++++++++++------------------
 t/t7501-commit.sh             |  4 ++--
 t/t7610-mergetool.sh          |  8 +++----
 10 files changed, 61 insertions(+), 62 deletions(-)

diff --git a/t/t0090-cache-tree.sh b/t/t0090-cache-tree.sh
index 0c61268fd2..28ea93f509 100755
--- a/t/t0090-cache-tree.sh
+++ b/t/t0090-cache-tree.sh
@@ -156,7 +156,7 @@ test_expect_success PERL 'commit --interactive gives cache-tree on partial commi
 		return 44;
 	}
 	EOT
-	(echo p; echo 1; echo; echo s; echo n; echo y; echo q) |
+	test_write_lines p 1 "" s n y q |
 	git commit --interactive -m foo &&
 	test_cache_tree
 '
diff --git a/t/t1008-read-tree-overlay.sh b/t/t1008-read-tree-overlay.sh
index 4c50ed955e..cf96016844 100755
--- a/t/t1008-read-tree-overlay.sh
+++ b/t/t1008-read-tree-overlay.sh
@@ -23,7 +23,7 @@ test_expect_success setup '
 
 test_expect_success 'multi-read' '
 	read_tree_must_succeed initial master side &&
-	(echo a; echo b/c) >expect &&
+	test_write_lines a b/c >expect &&
 	git ls-files >actual &&
 	test_cmp expect actual
 '
diff --git a/t/t2016-checkout-patch.sh b/t/t2016-checkout-patch.sh
index 9cd0ac4ba3..47aeb0b167 100755
--- a/t/t2016-checkout-patch.sh
+++ b/t/t2016-checkout-patch.sh
@@ -20,33 +20,33 @@ test_expect_success PERL 'setup' '
 
 test_expect_success PERL 'saying "n" does nothing' '
 	set_and_save_state dir/foo work head &&
-	(echo n; echo n) | git checkout -p &&
+	test_write_lines n n | git checkout -p &&
 	verify_saved_state bar &&
 	verify_saved_state dir/foo
 '
 
 test_expect_success PERL 'git checkout -p' '
-	(echo n; echo y) | git checkout -p &&
+	test_write_lines n y | git checkout -p &&
 	verify_saved_state bar &&
 	verify_state dir/foo head head
 '
 
 test_expect_success PERL 'git checkout -p with staged changes' '
 	set_state dir/foo work index &&
-	(echo n; echo y) | git checkout -p &&
+	test_write_lines n y | git checkout -p &&
 	verify_saved_state bar &&
 	verify_state dir/foo index index
 '
 
 test_expect_success PERL 'git checkout -p HEAD with NO staged changes: abort' '
 	set_and_save_state dir/foo work head &&
-	(echo n; echo y; echo n) | git checkout -p HEAD &&
+	test_write_lines n y n | git checkout -p HEAD &&
 	verify_saved_state bar &&
 	verify_saved_state dir/foo
 '
 
 test_expect_success PERL 'git checkout -p HEAD with NO staged changes: apply' '
-	(echo n; echo y; echo y) | git checkout -p HEAD &&
+	test_write_lines n y y | git checkout -p HEAD &&
 	verify_saved_state bar &&
 	verify_state dir/foo head head
 '
@@ -54,14 +54,14 @@ test_expect_success PERL 'git checkout -p HEAD with NO staged changes: apply' '
 test_expect_success PERL 'git checkout -p HEAD with change already staged' '
 	set_state dir/foo index index &&
 	# the third n is to get out in case it mistakenly does not apply
-	(echo n; echo y; echo n) | git checkout -p HEAD &&
+	test_write_lines n y n | git checkout -p HEAD &&
 	verify_saved_state bar &&
 	verify_state dir/foo head head
 '
 
 test_expect_success PERL 'git checkout -p HEAD^' '
 	# the third n is to get out in case it mistakenly does not apply
-	(echo n; echo y; echo n) | git checkout -p HEAD^ &&
+	test_write_lines n y n | git checkout -p HEAD^ &&
 	verify_saved_state bar &&
 	verify_state dir/foo parent parent
 '
@@ -69,7 +69,7 @@ test_expect_success PERL 'git checkout -p HEAD^' '
 test_expect_success PERL 'git checkout -p handles deletion' '
 	set_state dir/foo work index &&
 	rm dir/foo &&
-	(echo n; echo y) | git checkout -p &&
+	test_write_lines n y | git checkout -p &&
 	verify_saved_state bar &&
 	verify_state dir/foo index index
 '
@@ -81,21 +81,21 @@ test_expect_success PERL 'git checkout -p handles deletion' '
 
 test_expect_success PERL 'path limiting works: dir' '
 	set_state dir/foo work head &&
-	(echo y; echo n) | git checkout -p dir &&
+	test_write_lines y n | git checkout -p dir &&
 	verify_saved_state bar &&
 	verify_state dir/foo head head
 '
 
 test_expect_success PERL 'path limiting works: -- dir' '
 	set_state dir/foo work head &&
-	(echo y; echo n) | git checkout -p -- dir &&
+	test_write_lines y n | git checkout -p -- dir &&
 	verify_saved_state bar &&
 	verify_state dir/foo head head
 '
 
 test_expect_success PERL 'path limiting works: HEAD^ -- dir' '
 	# the third n is to get out in case it mistakenly does not apply
-	(echo y; echo n; echo n) | git checkout -p HEAD^ -- dir &&
+	test_write_lines y n n | git checkout -p HEAD^ -- dir &&
 	verify_saved_state bar &&
 	verify_state dir/foo parent parent
 '
@@ -103,7 +103,7 @@ test_expect_success PERL 'path limiting works: HEAD^ -- dir' '
 test_expect_success PERL 'path limiting works: foo inside dir' '
 	set_state dir/foo work head &&
 	# the third n is to get out in case it mistakenly does not apply
-	(echo y; echo n; echo n) | (cd dir && git checkout -p foo) &&
+	test_write_lines y n n | (cd dir && git checkout -p foo) &&
 	verify_saved_state bar &&
 	verify_state dir/foo head head
 '
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index 352a52e59d..85e99aac13 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -509,7 +509,7 @@ test_expect_success 'interrupted squash works as expected' '
 	one=$(git rev-parse HEAD~3) &&
 	set_fake_editor &&
 	test_must_fail env FAKE_LINES="1 squash 3 2" git rebase -i HEAD~3 &&
-	(echo one; echo two; echo four) > conflict &&
+	test_write_lines one two four > conflict &&
 	git add conflict &&
 	test_must_fail git rebase --continue &&
 	echo resolved > conflict &&
@@ -523,10 +523,10 @@ test_expect_success 'interrupted squash works as expected (case 2)' '
 	one=$(git rev-parse HEAD~3) &&
 	set_fake_editor &&
 	test_must_fail env FAKE_LINES="3 squash 1 2" git rebase -i HEAD~3 &&
-	(echo one; echo four) > conflict &&
+	test_write_lines one four > conflict &&
 	git add conflict &&
 	test_must_fail git rebase --continue &&
-	(echo one; echo two; echo four) > conflict &&
+	test_write_lines one two four > conflict &&
 	git add conflict &&
 	test_must_fail git rebase --continue &&
 	echo resolved > conflict &&
diff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh
index 3e9139dca8..609fbfdc31 100755
--- a/t/t3701-add-interactive.sh
+++ b/t/t3701-add-interactive.sh
@@ -46,13 +46,13 @@ test_expect_success 'setup expected' '
 '
 
 test_expect_success 'diff works (initial)' '
-	(echo d; echo 1) | git add -i >output &&
+	test_write_lines d 1 | git add -i >output &&
 	sed -ne "/new file/,/content/p" <output >diff &&
 	diff_cmp expected diff
 '
 test_expect_success 'revert works (initial)' '
 	git add file &&
-	(echo r; echo 1) | git add -i &&
+	test_write_lines r 1 | git add -i &&
 	git ls-files >output &&
 	! grep . output
 '
@@ -83,13 +83,13 @@ test_expect_success 'setup expected' '
 '
 
 test_expect_success 'diff works (commit)' '
-	(echo d; echo 1) | git add -i >output &&
+	test_write_lines d 1 | git add -i >output &&
 	sed -ne "/^index/,/content/p" <output >diff &&
 	diff_cmp expected diff
 '
 test_expect_success 'revert works (commit)' '
 	git add file &&
-	(echo r; echo 1) | git add -i &&
+	test_write_lines r 1 | git add -i &&
 	git add -i </dev/null >output &&
 	grep "unchanged *+3/-0 file" output
 '
@@ -102,7 +102,7 @@ test_expect_success 'setup expected' '
 
 test_expect_success 'dummy edit works' '
 	test_set_editor : &&
-	(echo e; echo a) | git add -p &&
+	test_write_lines e a | git add -p &&
 	git diff > diff &&
 	diff_cmp expected diff
 '
@@ -127,7 +127,7 @@ test_expect_success 'setup fake editor' '
 
 test_expect_success 'bad edit rejected' '
 	git reset &&
-	(echo e; echo n; echo d) | git add -p >output &&
+	test_write_lines e n d | git add -p >output &&
 	grep "hunk does not apply" output
 '
 
@@ -140,7 +140,7 @@ test_expect_success 'setup patch' '
 
 test_expect_success 'garbage edit rejected' '
 	git reset &&
-	(echo e; echo n; echo d) | git add -p >output &&
+	test_write_lines e n d | git add -p >output &&
 	grep "hunk does not apply" output
 '
 
@@ -170,7 +170,7 @@ test_expect_success 'setup expected' '
 '
 
 test_expect_success 'real edit works' '
-	(echo e; echo n; echo d) | git add -p &&
+	test_write_lines e n d | git add -p &&
 	git diff >output &&
 	diff_cmp expected output
 '
diff --git a/t/t3904-stash-patch.sh b/t/t3904-stash-patch.sh
index 83744f8c93..9546b6f8a4 100755
--- a/t/t3904-stash-patch.sh
+++ b/t/t3904-stash-patch.sh
@@ -29,14 +29,14 @@ test_expect_success 'setup' '
 test_expect_success 'saying "n" does nothing' '
 	set_state HEAD HEADfile_work HEADfile_index &&
 	set_state dir/foo work index &&
-	(echo n; echo n; echo n) | test_must_fail git stash save -p &&
+	test_write_lines n n n | test_must_fail git stash save -p &&
 	verify_state HEAD HEADfile_work HEADfile_index &&
 	verify_saved_state bar &&
 	verify_state dir/foo work index
 '
 
 test_expect_success 'git stash -p' '
-	(echo y; echo n; echo y) | git stash save -p &&
+	test_write_lines y n y | git stash save -p &&
 	verify_state HEAD committed HEADfile_index &&
 	verify_saved_state bar &&
 	verify_state dir/foo head index &&
@@ -51,7 +51,7 @@ test_expect_success 'git stash -p --no-keep-index' '
 	set_state HEAD HEADfile_work HEADfile_index &&
 	set_state bar bar_work bar_index &&
 	set_state dir/foo work index &&
-	(echo y; echo n; echo y) | git stash save -p --no-keep-index &&
+	test_write_lines y n y | git stash save -p --no-keep-index &&
 	verify_state HEAD committed committed &&
 	verify_state bar bar_work dummy &&
 	verify_state dir/foo head head &&
@@ -66,7 +66,7 @@ test_expect_success 'git stash --no-keep-index -p' '
 	set_state HEAD HEADfile_work HEADfile_index &&
 	set_state bar bar_work bar_index &&
 	set_state dir/foo work index &&
-	(echo y; echo n; echo y) | git stash save --no-keep-index -p &&
+	test_write_lines y n y | git stash save --no-keep-index -p &&
 	verify_state HEAD committed committed &&
 	verify_state dir/foo head head &&
 	verify_state bar bar_work dummy &&
diff --git a/t/t7105-reset-patch.sh b/t/t7105-reset-patch.sh
index 98b7d7b969..bd10a96727 100755
--- a/t/t7105-reset-patch.sh
+++ b/t/t7105-reset-patch.sh
@@ -19,20 +19,20 @@ test_expect_success PERL 'setup' '
 
 test_expect_success PERL 'saying "n" does nothing' '
 	set_and_save_state dir/foo work work &&
-	(echo n; echo n) | git reset -p &&
+	test_write_lines n n | git reset -p &&
 	verify_saved_state dir/foo &&
 	verify_saved_state bar
 '
 
 test_expect_success PERL 'git reset -p' '
-	(echo n; echo y) | git reset -p >output &&
+	test_write_lines n y | git reset -p >output &&
 	verify_state dir/foo work head &&
 	verify_saved_state bar &&
 	test_i18ngrep "Unstage" output
 '
 
 test_expect_success PERL 'git reset -p HEAD^' '
-	(echo n; echo y) | git reset -p HEAD^ >output &&
+	test_write_lines n y | git reset -p HEAD^ >output &&
 	verify_state dir/foo work parent &&
 	verify_saved_state bar &&
 	test_i18ngrep "Apply" output
@@ -45,20 +45,20 @@ test_expect_success PERL 'git reset -p HEAD^' '
 
 test_expect_success PERL 'git reset -p dir' '
 	set_state dir/foo work work &&
-	(echo y; echo n) | git reset -p dir &&
+	test_write_lines y n | git reset -p dir &&
 	verify_state dir/foo work head &&
 	verify_saved_state bar
 '
 
 test_expect_success PERL 'git reset -p -- foo (inside dir)' '
 	set_state dir/foo work work &&
-	(echo y; echo n) | (cd dir && git reset -p -- foo) &&
+	test_write_lines y n | (cd dir && git reset -p -- foo) &&
 	verify_state dir/foo work head &&
 	verify_saved_state bar
 '
 
 test_expect_success PERL 'git reset -p HEAD^ -- dir' '
-	(echo y; echo n) | git reset -p HEAD^ -- dir &&
+	test_write_lines y n | git reset -p HEAD^ -- dir &&
 	verify_state dir/foo work parent &&
 	verify_saved_state bar
 '
diff --git a/t/t7301-clean-interactive.sh b/t/t7301-clean-interactive.sh
index 1bf9789c8a..a07e8b86de 100755
--- a/t/t7301-clean-interactive.sh
+++ b/t/t7301-clean-interactive.sh
@@ -107,7 +107,7 @@ test_expect_success 'git clean -id (filter all)' '
 	mkdir -p build docs &&
 	touch a.out src/part3.c src/part3.h src/part4.c src/part4.h \
 	docs/manual.txt obj.o build/lib.so &&
-	(echo f; echo "*"; echo; echo c) | \
+	test_write_lines f "*" "" c |
 	git clean -id &&
 	test -f Makefile &&
 	test -f README &&
@@ -129,7 +129,7 @@ test_expect_success 'git clean -id (filter patterns)' '
 	mkdir -p build docs &&
 	touch a.out src/part3.c src/part3.h src/part4.c src/part4.h \
 	docs/manual.txt obj.o build/lib.so &&
-	(echo f; echo "part3.* *.out"; echo; echo c) | \
+	test_write_lines f "part3.* *.out" "" c |
 	git clean -id &&
 	test -f Makefile &&
 	test -f README &&
@@ -151,7 +151,7 @@ test_expect_success 'git clean -id (filter patterns 2)' '
 	mkdir -p build docs &&
 	touch a.out src/part3.c src/part3.h src/part4.c src/part4.h \
 	docs/manual.txt obj.o build/lib.so &&
-	(echo f; echo "* !*.out"; echo; echo c) | \
+	test_write_lines f "* !*.out" "" c |
 	git clean -id &&
 	test -f Makefile &&
 	test -f README &&
@@ -173,7 +173,7 @@ test_expect_success 'git clean -id (select - all)' '
 	mkdir -p build docs &&
 	touch a.out src/part3.c src/part3.h src/part4.c src/part4.h \
 	docs/manual.txt obj.o build/lib.so &&
-	(echo s; echo "*"; echo; echo c) | \
+	test_write_lines s "*" "" c |
 	git clean -id &&
 	test -f Makefile &&
 	test -f README &&
@@ -195,7 +195,7 @@ test_expect_success 'git clean -id (select - none)' '
 	mkdir -p build docs &&
 	touch a.out src/part3.c src/part3.h src/part4.c src/part4.h \
 	docs/manual.txt obj.o build/lib.so &&
-	(echo s; echo; echo c) | \
+	test_write_lines s "" c |
 	git clean -id &&
 	test -f Makefile &&
 	test -f README &&
@@ -217,7 +217,7 @@ test_expect_success 'git clean -id (select - number)' '
 	mkdir -p build docs &&
 	touch a.out src/part3.c src/part3.h src/part4.c src/part4.h \
 	docs/manual.txt obj.o build/lib.so &&
-	(echo s; echo 3; echo; echo c) | \
+	test_write_lines s 3 "" c |
 	git clean -id &&
 	test -f Makefile &&
 	test -f README &&
@@ -239,7 +239,7 @@ test_expect_success 'git clean -id (select - number 2)' '
 	mkdir -p build docs &&
 	touch a.out src/part3.c src/part3.h src/part4.c src/part4.h \
 	docs/manual.txt obj.o build/lib.so &&
-	(echo s; echo 2 3; echo 5; echo; echo c) | \
+	test_write_lines s "2 3" 5 "" c |
 	git clean -id &&
 	test -f Makefile &&
 	test -f README &&
@@ -261,7 +261,7 @@ test_expect_success 'git clean -id (select - number 3)' '
 	mkdir -p build docs &&
 	touch a.out src/part3.c src/part3.h src/part4.c src/part4.h \
 	docs/manual.txt obj.o build/lib.so &&
-	(echo s; echo 3,4 5; echo; echo c) | \
+	test_write_lines s "3,4 5" "" c |
 	git clean -id &&
 	test -f Makefile &&
 	test -f README &&
@@ -282,7 +282,7 @@ test_expect_success 'git clean -id (select - filenames)' '
 
 	mkdir -p build docs &&
 	touch a.out foo.txt bar.txt baz.txt &&
-	(echo s; echo a.out fo ba bar; echo; echo c) | \
+	test_write_lines s "a.out fo ba bar" "" c |
 	git clean -id &&
 	test -f Makefile &&
 	test ! -f a.out &&
@@ -298,7 +298,7 @@ test_expect_success 'git clean -id (select - range)' '
 	mkdir -p build docs &&
 	touch a.out src/part3.c src/part3.h src/part4.c src/part4.h \
 	docs/manual.txt obj.o build/lib.so &&
-	(echo s; echo 1,3-4; echo 2; echo; echo c) | \
+	test_write_lines s "1,3-4" 2 "" c |
 	git clean -id &&
 	test -f Makefile &&
 	test -f README &&
@@ -320,7 +320,7 @@ test_expect_success 'git clean -id (select - range 2)' '
 	mkdir -p build docs &&
 	touch a.out src/part3.c src/part3.h src/part4.c src/part4.h \
 	docs/manual.txt obj.o build/lib.so &&
-	(echo s; echo 4- 1; echo; echo c) | \
+	test_write_lines s "4- 1" "" c |
 	git clean -id &&
 	test -f Makefile &&
 	test -f README &&
@@ -342,7 +342,7 @@ test_expect_success 'git clean -id (inverse select)' '
 	mkdir -p build docs &&
 	touch a.out src/part3.c src/part3.h src/part4.c src/part4.h \
 	docs/manual.txt obj.o build/lib.so &&
-	(echo s; echo "*"; echo -5- 1 -2; echo; echo c) | \
+	test_write_lines s "*" "-5- 1 -2" "" c |
 	git clean -id &&
 	test -f Makefile &&
 	test -f README &&
@@ -364,7 +364,7 @@ test_expect_success 'git clean -id (ask)' '
 	mkdir -p build docs &&
 	touch a.out src/part3.c src/part3.h src/part4.c src/part4.h \
 	docs/manual.txt obj.o build/lib.so &&
-	(echo a; echo Y; echo y; echo no; echo yes; echo bad; echo) | \
+	test_write_lines a Y y no yes bad "" |
 	git clean -id &&
 	test -f Makefile &&
 	test -f README &&
@@ -386,7 +386,7 @@ test_expect_success 'git clean -id (ask - Ctrl+D)' '
 	mkdir -p build docs &&
 	touch a.out src/part3.c src/part3.h src/part4.c src/part4.h \
 	docs/manual.txt obj.o build/lib.so &&
-	(echo a; echo Y; echo no; echo yes; echo "\04") | \
+	test_write_lines a Y no yes "\04" |
 	git clean -id &&
 	test -f Makefile &&
 	test -f README &&
@@ -408,8 +408,8 @@ test_expect_success 'git clean -id with prefix and path (filter)' '
 	mkdir -p build docs &&
 	touch a.out src/part3.c src/part3.h src/part4.c src/part4.h \
 	docs/manual.txt obj.o build/lib.so &&
-	(cd build/ && \
-	 (echo f; echo "docs"; echo "*.h"; echo ; echo c) | \
+	(cd build/ &&
+	 test_write_lines f docs "*.h" "" c |
 	 git clean -id ..) &&
 	test -f Makefile &&
 	test -f README &&
@@ -431,9 +431,8 @@ test_expect_success 'git clean -id with prefix and path (select by name)' '
 	mkdir -p build docs &&
 	touch a.out src/part3.c src/part3.h src/part4.c src/part4.h \
 	docs/manual.txt obj.o build/lib.so &&
-	(cd build/ && \
-	 (echo s; echo "../docs/"; echo "../src/part3.c"; \
-	  echo "../src/part4.c";  echo; echo c) | \
+	(cd build/ &&
+	 test_write_lines s ../docs/ ../src/part3.c ../src/part4.c "" c |
 	 git clean -id ..) &&
 	test -f Makefile &&
 	test -f README &&
@@ -455,8 +454,8 @@ test_expect_success 'git clean -id with prefix and path (ask)' '
 	mkdir -p build docs &&
 	touch a.out src/part3.c src/part3.h src/part4.c src/part4.h \
 	docs/manual.txt obj.o build/lib.so &&
-	(cd build/ && \
-	 (echo a; echo Y; echo y; echo no; echo yes; echo bad; echo) | \
+	(cd build/ &&
+	 test_write_lines a Y y no yes bad "" |
 	 git clean -id ..) &&
 	test -f Makefile &&
 	test -f README &&
diff --git a/t/t7501-commit.sh b/t/t7501-commit.sh
index 9dbbd01fc0..282ff42331 100755
--- a/t/t7501-commit.sh
+++ b/t/t7501-commit.sh
@@ -47,7 +47,7 @@ test_expect_success 'paths and -a do not mix' '
 test_expect_success PERL 'can use paths with --interactive' '
 	echo bong-o-bong >file &&
 	# 2: update, 1:st path, that is all, 7: quit
-	( echo 2; echo 1; echo; echo 7 ) |
+	test_write_lines 2 1 "" 7 |
 	git commit -m foo --interactive file &&
 	git reset --hard HEAD^
 '
@@ -293,7 +293,7 @@ test_expect_success PERL 'interactive add' '
 test_expect_success PERL "commit --interactive doesn't change index if editor aborts" '
 	echo zoo >file &&
 	test_must_fail git diff --exit-code >diff1 &&
-	(echo u ; echo "*" ; echo q) |
+	test_write_lines u "*" q |
 	(
 		EDITOR=: &&
 		export EDITOR &&
diff --git a/t/t7610-mergetool.sh b/t/t7610-mergetool.sh
index 1a430b9c40..047156e9d5 100755
--- a/t/t7610-mergetool.sh
+++ b/t/t7610-mergetool.sh
@@ -57,18 +57,18 @@ test_expect_success 'setup' '
 
 	git checkout -b delete-base branch1 &&
 	mkdir -p a/a &&
-	(echo one; echo two; echo 3; echo 4) >a/a/file.txt &&
+	test_write_lines one two 3 4 >a/a/file.txt &&
 	git add a/a/file.txt &&
 	git commit -m"base file" &&
 	git checkout -b move-to-b delete-base &&
 	mkdir -p b/b &&
 	git mv a/a/file.txt b/b/file.txt &&
-	(echo one; echo two; echo 4) >b/b/file.txt &&
+	test_write_lines one two 4 >b/b/file.txt &&
 	git commit -a -m"move to b" &&
 	git checkout -b move-to-c delete-base &&
 	mkdir -p c/c &&
 	git mv a/a/file.txt c/c/file.txt &&
-	(echo one; echo two; echo 3) >c/c/file.txt &&
+	test_write_lines one two 3 >c/c/file.txt &&
 	git commit -a -m"move to c" &&
 
 	git checkout -b stash1 master &&
@@ -349,7 +349,7 @@ test_expect_success 'mergetool keeps tempfiles when aborting delete/delete' '
 	git checkout -b test$test_count move-to-c &&
 	test_config mergetool.keepTemporaries true &&
 	test_must_fail git merge move-to-b &&
-	! (echo a; echo n) | git mergetool a/a/file.txt &&
+	! test_write_lines a n | git mergetool a/a/file.txt &&
 	test -d a/a &&
 	cat >expect <<-\EOF &&
 	file_BASE_.txt
-- 
2.18.0.203.gfac676dfb9


^ permalink raw reply related	[flat|nested] 52+ messages in thread

* [PATCH 03/25] t: use sane_unset() rather than 'unset' with broken &&-chain
  2018-07-02  0:23 [PATCH 00/25] fix buggy tests, modernize tests, fix broken &&-chains Eric Sunshine
  2018-07-02  0:23 ` [PATCH 01/25] t: use test_might_fail() instead of manipulating exit code manually Eric Sunshine
  2018-07-02  0:23 ` [PATCH 02/25] t: use test_write_lines() instead of series of 'echo' commands Eric Sunshine
@ 2018-07-02  0:23 ` Eric Sunshine
  2018-07-02  0:23 ` [PATCH 04/25] t: drop unnecessary terminating semicolon in subshell Eric Sunshine
                   ` (23 subsequent siblings)
  26 siblings, 0 replies; 52+ messages in thread
From: Eric Sunshine @ 2018-07-02  0:23 UTC (permalink / raw)
  To: git
  Cc: Elijah Newren, Johannes Sixt, Jonathan Nieder, Jonathan Tan,
	Stefan Beller, Junio C Hamano, Luke Diamand, Jeff King,
	Eric Sunshine

These tests intentionally break the &&-chain after using 'unset' since
they don't know if 'unset' will succeed or fail and don't want a local
'unset' failure to fail the test overall. We can do better by using
sane_unset(), which can be linked into the &&-chain as usual.

Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
---
 t/t0001-init.sh   | 4 ++--
 t/t1300-config.sh | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/t/t0001-init.sh b/t/t0001-init.sh
index 4c865051e7..ca85aae51e 100755
--- a/t/t0001-init.sh
+++ b/t/t0001-init.sh
@@ -408,7 +408,7 @@ is_hidden () {
 test_expect_success MINGW '.git hidden' '
 	rm -rf newdir &&
 	(
-		unset GIT_DIR GIT_WORK_TREE
+		sane_unset GIT_DIR GIT_WORK_TREE &&
 		mkdir newdir &&
 		cd newdir &&
 		git init &&
@@ -420,7 +420,7 @@ test_expect_success MINGW '.git hidden' '
 test_expect_success MINGW 'bare git dir not hidden' '
 	rm -rf newdir &&
 	(
-		unset GIT_DIR GIT_WORK_TREE GIT_CONFIG
+		sane_unset GIT_DIR GIT_WORK_TREE GIT_CONFIG &&
 		mkdir newdir &&
 		cd newdir &&
 		git --bare init
diff --git a/t/t1300-config.sh b/t/t1300-config.sh
index 03c223708e..24706ba412 100755
--- a/t/t1300-config.sh
+++ b/t/t1300-config.sh
@@ -888,7 +888,7 @@ EOF
 
 test_expect_success !MINGW 'get --path copes with unset $HOME' '
 	(
-		unset HOME;
+		sane_unset HOME &&
 		test_must_fail git config --get --path path.home \
 			>result 2>msg &&
 		git config --get --path path.normal >>result &&
-- 
2.18.0.203.gfac676dfb9


^ permalink raw reply related	[flat|nested] 52+ messages in thread

* [PATCH 04/25] t: drop unnecessary terminating semicolon in subshell
  2018-07-02  0:23 [PATCH 00/25] fix buggy tests, modernize tests, fix broken &&-chains Eric Sunshine
                   ` (2 preceding siblings ...)
  2018-07-02  0:23 ` [PATCH 03/25] t: use sane_unset() rather than 'unset' with broken &&-chain Eric Sunshine
@ 2018-07-02  0:23 ` Eric Sunshine
  2018-07-03 19:23   ` Junio C Hamano
  2018-07-02  0:23 ` [PATCH 05/25] t/lib-submodule-update: fix "absorbing" test Eric Sunshine
                   ` (22 subsequent siblings)
  26 siblings, 1 reply; 52+ messages in thread
From: Eric Sunshine @ 2018-07-02  0:23 UTC (permalink / raw)
  To: git
  Cc: Elijah Newren, Johannes Sixt, Jonathan Nieder, Jonathan Tan,
	Stefan Beller, Junio C Hamano, Luke Diamand, Jeff King,
	Eric Sunshine

Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
---
 t/t3102-ls-tree-wildcards.sh    | 2 +-
 t/t4010-diff-pathspec.sh        | 4 ++--
 t/t9400-git-cvsserver-server.sh | 2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/t/t3102-ls-tree-wildcards.sh b/t/t3102-ls-tree-wildcards.sh
index e804377f1c..1e16c6b8ea 100755
--- a/t/t3102-ls-tree-wildcards.sh
+++ b/t/t3102-ls-tree-wildcards.sh
@@ -23,7 +23,7 @@ test_expect_success 'ls-tree outside prefix' '
 	cat >expect <<-EOF &&
 	100644 blob $EMPTY_BLOB	../a[a]/three
 	EOF
-	( cd aa && git ls-tree -r HEAD "../a[a]"; ) >actual &&
+	( cd aa && git ls-tree -r HEAD "../a[a]" ) >actual &&
 	test_cmp expect actual
 '
 
diff --git a/t/t4010-diff-pathspec.sh b/t/t4010-diff-pathspec.sh
index 35b35a81c8..b7f25071cf 100755
--- a/t/t4010-diff-pathspec.sh
+++ b/t/t4010-diff-pathspec.sh
@@ -111,10 +111,10 @@ test_expect_success 'diff-tree -r with wildcard' '
 test_expect_success 'setup submodules' '
 	test_tick &&
 	git init submod &&
-	( cd submod && test_commit first; ) &&
+	( cd submod && test_commit first ) &&
 	git add submod &&
 	git commit -m first &&
-	( cd submod && test_commit second; ) &&
+	( cd submod && test_commit second ) &&
 	git add submod &&
 	git commit -m second
 '
diff --git a/t/t9400-git-cvsserver-server.sh b/t/t9400-git-cvsserver-server.sh
index 06742748e9..6b09da79bf 100755
--- a/t/t9400-git-cvsserver-server.sh
+++ b/t/t9400-git-cvsserver-server.sh
@@ -328,7 +328,7 @@ test_expect_success 'cvs update (subdirectories)' \
   '(for dir in A A/B A/B/C A/D E; do
       mkdir $dir &&
       echo "test file in $dir" >"$dir/file_in_$(echo $dir|sed -e "s#/# #g")"  &&
-      git add $dir;
+      git add $dir
    done) &&
    git commit -q -m "deep sub directory structure" &&
    git push gitcvs.git >/dev/null &&
-- 
2.18.0.203.gfac676dfb9


^ permalink raw reply related	[flat|nested] 52+ messages in thread

* [PATCH 05/25] t/lib-submodule-update: fix "absorbing" test
  2018-07-02  0:23 [PATCH 00/25] fix buggy tests, modernize tests, fix broken &&-chains Eric Sunshine
                   ` (3 preceding siblings ...)
  2018-07-02  0:23 ` [PATCH 04/25] t: drop unnecessary terminating semicolon in subshell Eric Sunshine
@ 2018-07-02  0:23 ` Eric Sunshine
  2018-07-02  0:23 ` [PATCH 06/25] t5405: use test_must_fail() instead of checking exit code manually Eric Sunshine
                   ` (21 subsequent siblings)
  26 siblings, 0 replies; 52+ messages in thread
From: Eric Sunshine @ 2018-07-02  0:23 UTC (permalink / raw)
  To: git
  Cc: Elijah Newren, Johannes Sixt, Jonathan Nieder, Jonathan Tan,
	Stefan Beller, Junio C Hamano, Luke Diamand, Jeff King,
	Eric Sunshine

This test has been dysfunctional since it was added by 259f3ee296
(lib-submodule-update.sh: define tests for recursing into submodules,
2017-03-14), however, the problem went unnoticed due to a broken
&&-chain.

The test wants to verify that replacing a submodule containing a .git
directory will absorb the .git directory into the .git/modules/ of the
superproject, and then replace the working tree content appropriate to
the superproject. It is, therefore, incorrect to check if the
submodule content still exists since the submodule will have been
replaced by the content of the superproject.

Fix this by removing the submodule content check, which also happens
to be the line that broke the &&-chain.

While at it, fix broken &&-chains in a couple neighboring tests.

Helped-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
---
 t/lib-submodule-update.sh | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/t/lib-submodule-update.sh b/t/lib-submodule-update.sh
index 1f38a85371..e90ec79087 100755
--- a/t/lib-submodule-update.sh
+++ b/t/lib-submodule-update.sh
@@ -755,7 +755,7 @@ test_submodule_recursing_with_args_common() {
 			: >sub1/untrackedfile &&
 			test_must_fail $command replace_sub1_with_file &&
 			test_superproject_content origin/add_sub1 &&
-			test_submodule_content sub1 origin/add_sub1
+			test_submodule_content sub1 origin/add_sub1 &&
 			test -f sub1/untracked_file
 		)
 	'
@@ -842,7 +842,7 @@ test_submodule_switch_recursing_with_args () {
 			cd submodule_update &&
 			git branch -t add_sub1 origin/add_sub1 &&
 			: >sub1 &&
-			echo sub1 >.git/info/exclude
+			echo sub1 >.git/info/exclude &&
 			$command add_sub1 &&
 			test_superproject_content origin/add_sub1 &&
 			test_submodule_content sub1 origin/add_sub1
@@ -969,7 +969,6 @@ test_submodule_forced_switch_recursing_with_args () {
 			rm -rf .git/modules/sub1 &&
 			$command replace_sub1_with_directory &&
 			test_superproject_content origin/replace_sub1_with_directory &&
-			test_submodule_content sub1 origin/modify_sub1
 			test_git_directory_exists sub1
 		)
 	'
-- 
2.18.0.203.gfac676dfb9


^ permalink raw reply related	[flat|nested] 52+ messages in thread

* [PATCH 06/25] t5405: use test_must_fail() instead of checking exit code manually
  2018-07-02  0:23 [PATCH 00/25] fix buggy tests, modernize tests, fix broken &&-chains Eric Sunshine
                   ` (4 preceding siblings ...)
  2018-07-02  0:23 ` [PATCH 05/25] t/lib-submodule-update: fix "absorbing" test Eric Sunshine
@ 2018-07-02  0:23 ` Eric Sunshine
  2018-07-02  0:23 ` [PATCH 07/25] t5406: use write_script() instead of birthing shell script manually Eric Sunshine
                   ` (20 subsequent siblings)
  26 siblings, 0 replies; 52+ messages in thread
From: Eric Sunshine @ 2018-07-02  0:23 UTC (permalink / raw)
  To: git
  Cc: Elijah Newren, Johannes Sixt, Jonathan Nieder, Jonathan Tan,
	Stefan Beller, Junio C Hamano, Luke Diamand, Jeff King,
	Eric Sunshine

This test expects "git push" to fail, thus it manually inverts that
local expected failure into a successful exit code for the test overall.
In doing so, it intentionally breaks the &&-chain. Modernize by
replacing manual exit code management with test_must_fail() and a normal
&&-chain.

Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
---
 t/t5405-send-pack-rewind.sh | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/t/t5405-send-pack-rewind.sh b/t/t5405-send-pack-rewind.sh
index 4bda18a662..235fb7686a 100755
--- a/t/t5405-send-pack-rewind.sh
+++ b/t/t5405-send-pack-rewind.sh
@@ -25,8 +25,7 @@ test_expect_success 'non forced push should die not segfault' '
 
 	(
 		cd another &&
-		git push .. master:master
-		test $? = 1
+		test_must_fail git push .. master:master
 	)
 
 '
-- 
2.18.0.203.gfac676dfb9


^ permalink raw reply related	[flat|nested] 52+ messages in thread

* [PATCH 07/25] t5406: use write_script() instead of birthing shell script manually
  2018-07-02  0:23 [PATCH 00/25] fix buggy tests, modernize tests, fix broken &&-chains Eric Sunshine
                   ` (5 preceding siblings ...)
  2018-07-02  0:23 ` [PATCH 06/25] t5405: use test_must_fail() instead of checking exit code manually Eric Sunshine
@ 2018-07-02  0:23 ` Eric Sunshine
  2018-07-02  0:23 ` [PATCH 08/25] t5505: modernize and simplify hard-to-digest test Eric Sunshine
                   ` (19 subsequent siblings)
  26 siblings, 0 replies; 52+ messages in thread
From: Eric Sunshine @ 2018-07-02  0:23 UTC (permalink / raw)
  To: git
  Cc: Elijah Newren, Johannes Sixt, Jonathan Nieder, Jonathan Tan,
	Stefan Beller, Junio C Hamano, Luke Diamand, Jeff King,
	Eric Sunshine

Take advantage of write_script() to abstract-away details of shell
script creation, thus allowing the reader to focus on script content.
Readability benefits, particularly in this case, since the script body
was buried in a noisy one-liner subshell responsible for emitting
boilerplate and body.

Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
---
 t/t5406-remote-rejects.sh | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/t/t5406-remote-rejects.sh b/t/t5406-remote-rejects.sh
index 59e80a5ea2..ff06f99649 100755
--- a/t/t5406-remote-rejects.sh
+++ b/t/t5406-remote-rejects.sh
@@ -6,8 +6,9 @@ test_description='remote push rejects are reported by client'
 
 test_expect_success 'setup' '
 	mkdir .git/hooks &&
-	(echo "#!/bin/sh" ; echo "exit 1") >.git/hooks/update &&
-	chmod +x .git/hooks/update &&
+	write_script .git/hooks/update <<-\EOF &&
+	exit 1
+	EOF
 	echo 1 >file &&
 	git add file &&
 	git commit -m 1 &&
-- 
2.18.0.203.gfac676dfb9


^ permalink raw reply related	[flat|nested] 52+ messages in thread

* [PATCH 08/25] t5505: modernize and simplify hard-to-digest test
  2018-07-02  0:23 [PATCH 00/25] fix buggy tests, modernize tests, fix broken &&-chains Eric Sunshine
                   ` (6 preceding siblings ...)
  2018-07-02  0:23 ` [PATCH 07/25] t5406: use write_script() instead of birthing shell script manually Eric Sunshine
@ 2018-07-02  0:23 ` Eric Sunshine
  2018-07-02  0:23 ` [PATCH 09/25] t6036: fix broken "merge fails but has appropriate contents" tests Eric Sunshine
                   ` (18 subsequent siblings)
  26 siblings, 0 replies; 52+ messages in thread
From: Eric Sunshine @ 2018-07-02  0:23 UTC (permalink / raw)
  To: git
  Cc: Elijah Newren, Johannes Sixt, Jonathan Nieder, Jonathan Tan,
	Stefan Beller, Junio C Hamano, Luke Diamand, Jeff King,
	Eric Sunshine

This test uses a subshell within a subshell but is formatted in such a
way as to suggests that the inner subshell is a sibling rather than a
child, which makes it difficult to digest the test's structure and
intent.

Worse, the inner subshell performs cleanup of actions from earlier in
the test, however, a failure between the initial actions and the cleanup
will prevent the cleanup from taking place.

Fix these problems by modernizing and simplifying the test and by using
test_when_finished() for the cleanup action.

Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
---
 t/t5505-remote.sh | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/t/t5505-remote.sh b/t/t5505-remote.sh
index a6c0178f3a..3552b51b4c 100755
--- a/t/t5505-remote.sh
+++ b/t/t5505-remote.sh
@@ -348,17 +348,13 @@ URL: $(pwd)/one
 EOF
 
 test_expect_success 'prune --dry-run' '
-	(
-		cd one &&
-		git branch -m side2 side) &&
+	git -C one branch -m side2 side &&
+	test_when_finished "git -C one branch -m side side2" &&
 	(
 		cd test &&
 		git remote prune --dry-run origin >output &&
 		git rev-parse refs/remotes/origin/side2 &&
 		test_must_fail git rev-parse refs/remotes/origin/side &&
-	(
-		cd ../one &&
-		git branch -m side side2) &&
 		test_i18ncmp expect output
 	)
 '
-- 
2.18.0.203.gfac676dfb9


^ permalink raw reply related	[flat|nested] 52+ messages in thread

* [PATCH 09/25] t6036: fix broken "merge fails but has appropriate contents" tests
  2018-07-02  0:23 [PATCH 00/25] fix buggy tests, modernize tests, fix broken &&-chains Eric Sunshine
                   ` (7 preceding siblings ...)
  2018-07-02  0:23 ` [PATCH 08/25] t5505: modernize and simplify hard-to-digest test Eric Sunshine
@ 2018-07-02  0:23 ` Eric Sunshine
  2018-07-02  0:23 ` [PATCH 10/25] t7201: drop pointless "exit 0" at end of subshell Eric Sunshine
                   ` (17 subsequent siblings)
  26 siblings, 0 replies; 52+ messages in thread
From: Eric Sunshine @ 2018-07-02  0:23 UTC (permalink / raw)
  To: git
  Cc: Elijah Newren, Johannes Sixt, Jonathan Nieder, Jonathan Tan,
	Stefan Beller, Junio C Hamano, Luke Diamand, Jeff King,
	Eric Sunshine

These tests reference non-existent object "c" when they really mean to
be referencing "C", however, these errors went unnoticed due to a broken
&&-chain later in the tests. Fix these errors, as well as the broken
&&-chains behind which they hid.

Reviewed-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
---
 t/t6036-recursive-corner-cases.sh | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/t/t6036-recursive-corner-cases.sh b/t/t6036-recursive-corner-cases.sh
index b5621303d6..b32ff8e1db 100755
--- a/t/t6036-recursive-corner-cases.sh
+++ b/t/t6036-recursive-corner-cases.sh
@@ -506,10 +506,10 @@ test_expect_success 'merge of D & E2 fails but has appropriate contents' '
 		test_line_count = 2 out &&
 
 		git rev-parse >expect    \
-			B:a   E2:a/file  c:a/file   A:ignore-me &&
+			B:a   E2:a/file  C:a/file   A:ignore-me &&
 		git rev-parse   >actual   \
 			:2:a  :3:a/file  :1:a/file  :0:ignore-me &&
-		test_cmp expect actual
+		test_cmp expect actual &&
 
 		test_path_is_file a~HEAD
 	)
@@ -533,10 +533,10 @@ test_expect_success 'merge of E2 & D fails but has appropriate contents' '
 		test_line_count = 2 out &&
 
 		git rev-parse >expect    \
-			B:a   E2:a/file  c:a/file   A:ignore-me &&
+			B:a   E2:a/file  C:a/file   A:ignore-me &&
 		git rev-parse   >actual   \
 			:3:a  :2:a/file  :1:a/file  :0:ignore-me &&
-		test_cmp expect actual
+		test_cmp expect actual &&
 
 		test_path_is_file a~D^0
 	)
-- 
2.18.0.203.gfac676dfb9


^ permalink raw reply related	[flat|nested] 52+ messages in thread

* [PATCH 10/25] t7201: drop pointless "exit 0" at end of subshell
  2018-07-02  0:23 [PATCH 00/25] fix buggy tests, modernize tests, fix broken &&-chains Eric Sunshine
                   ` (8 preceding siblings ...)
  2018-07-02  0:23 ` [PATCH 09/25] t6036: fix broken "merge fails but has appropriate contents" tests Eric Sunshine
@ 2018-07-02  0:23 ` Eric Sunshine
  2018-07-02  0:23 ` [PATCH 11/25] t7400: fix broken "submodule add/reconfigure --force" test Eric Sunshine
                   ` (16 subsequent siblings)
  26 siblings, 0 replies; 52+ messages in thread
From: Eric Sunshine @ 2018-07-02  0:23 UTC (permalink / raw)
  To: git
  Cc: Elijah Newren, Johannes Sixt, Jonathan Nieder, Jonathan Tan,
	Stefan Beller, Junio C Hamano, Luke Diamand, Jeff King,
	Eric Sunshine

This test employs a for-loop inside a subshell and correctly aborts the
loop and fails the test overall (via "exit 1") if any iteration of the
for-loop fails. Otherwise, it exits the subshell with an explicit but
entirely unnecessary "exit 0", presumably to indicate that all
iterations of the loop succeeded. The &&-chain is broken between the
for-loop and the "exit 0". Rather than fixing the &&-chain, just drop
the pointless "exit 0".

Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
---
 t/t7201-co.sh | 1 -
 1 file changed, 1 deletion(-)

diff --git a/t/t7201-co.sh b/t/t7201-co.sh
index ab9da61da3..8d8a63a24b 100755
--- a/t/t7201-co.sh
+++ b/t/t7201-co.sh
@@ -673,7 +673,6 @@ test_expect_success 'custom merge driver with checkout -m' '
 		do
 			grep $t arm || exit 1
 		done
-		exit 0
 	) &&
 
 	mv arm expect &&
-- 
2.18.0.203.gfac676dfb9


^ permalink raw reply related	[flat|nested] 52+ messages in thread

* [PATCH 11/25] t7400: fix broken "submodule add/reconfigure --force" test
  2018-07-02  0:23 [PATCH 00/25] fix buggy tests, modernize tests, fix broken &&-chains Eric Sunshine
                   ` (9 preceding siblings ...)
  2018-07-02  0:23 ` [PATCH 10/25] t7201: drop pointless "exit 0" at end of subshell Eric Sunshine
@ 2018-07-02  0:23 ` Eric Sunshine
  2018-07-16 14:43   ` Johannes Schindelin
  2018-07-02  0:23 ` [PATCH 12/25] t7810: use test_expect_code() instead of hand-rolled comparison Eric Sunshine
                   ` (15 subsequent siblings)
  26 siblings, 1 reply; 52+ messages in thread
From: Eric Sunshine @ 2018-07-02  0:23 UTC (permalink / raw)
  To: git
  Cc: Elijah Newren, Johannes Sixt, Jonathan Nieder, Jonathan Tan,
	Stefan Beller, Junio C Hamano, Luke Diamand, Jeff King,
	Eric Sunshine

This test has been dysfunctional since it was added by 619acfc78c
(submodule add: extend force flag to add existing repos, 2016-10-06),
however, two problems early in the test went unnoticed due to a broken
&&-chain later in the test.

First, it tries configuring the submodule with repository "bogus-url",
however, "git submodule add" insists that the repository be either an
absolute URL or a relative pathname requiring prefix "./" or "../" (this
is true even with --force), but "bogus-url" does not meet those
criteria, thus the command fails.

Second, it then tries configuring a submodule with a path which is
.gitignore'd, which is disallowed. This restriction can be overridden
with --force, but the test neglects to use that option.

Fix both problems, as well as the broken &&-chain behind which they hid.

Reviewed-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
---
 t/t7400-submodule-basic.sh | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
index 812db137b8..401adaed32 100755
--- a/t/t7400-submodule-basic.sh
+++ b/t/t7400-submodule-basic.sh
@@ -171,12 +171,12 @@ test_expect_success 'submodule add to .gitignored path with --force' '
 test_expect_success 'submodule add to reconfigure existing submodule with --force' '
 	(
 		cd addtest-ignore &&
-		git submodule add --force bogus-url submod &&
-		git submodule add -b initial "$submodurl" submod-branch &&
-		test "bogus-url" = "$(git config -f .gitmodules submodule.submod.url)" &&
-		test "bogus-url" = "$(git config submodule.submod.url)" &&
+		git submodule add --force /bogus-url submod &&
+		git submodule add --force -b initial "$submodurl" submod-branch &&
+		test "/bogus-url" = "$(git config -f .gitmodules submodule.submod.url)" &&
+		test "/bogus-url" = "$(git config submodule.submod.url)" &&
 		# Restore the url
-		git submodule add --force "$submodurl" submod
+		git submodule add --force "$submodurl" submod &&
 		test "$submodurl" = "$(git config -f .gitmodules submodule.submod.url)" &&
 		test "$submodurl" = "$(git config submodule.submod.url)"
 	)
-- 
2.18.0.203.gfac676dfb9


^ permalink raw reply related	[flat|nested] 52+ messages in thread

* [PATCH 12/25] t7810: use test_expect_code() instead of hand-rolled comparison
  2018-07-02  0:23 [PATCH 00/25] fix buggy tests, modernize tests, fix broken &&-chains Eric Sunshine
                   ` (10 preceding siblings ...)
  2018-07-02  0:23 ` [PATCH 11/25] t7400: fix broken "submodule add/reconfigure --force" test Eric Sunshine
@ 2018-07-02  0:23 ` Eric Sunshine
  2018-07-02 18:05   ` Stefan Beller
  2018-07-02  0:23 ` [PATCH 13/25] t9001: fix broken "invoke hook" test Eric Sunshine
                   ` (14 subsequent siblings)
  26 siblings, 1 reply; 52+ messages in thread
From: Eric Sunshine @ 2018-07-02  0:23 UTC (permalink / raw)
  To: git
  Cc: Elijah Newren, Johannes Sixt, Jonathan Nieder, Jonathan Tan,
	Stefan Beller, Junio C Hamano, Luke Diamand, Jeff King,
	Eric Sunshine

This test manually checks the exit code of git-grep for a particular
value. In doing so, it intentionally breaks the &&-chain. Modernize the
test by taking advantage of test_expect_code() and a normal &&-chain.

Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
---
 t/t7810-grep.sh | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/t/t7810-grep.sh b/t/t7810-grep.sh
index 1797f632a3..fecee602c1 100755
--- a/t/t7810-grep.sh
+++ b/t/t7810-grep.sh
@@ -845,10 +845,9 @@ test_expect_success 'grep from a subdirectory to search wider area (1)' '
 test_expect_success 'grep from a subdirectory to search wider area (2)' '
 	mkdir -p s &&
 	(
-		cd s || exit 1
-		( git grep xxyyzz .. >out ; echo $? >status )
-		! test -s out &&
-		test 1 = $(cat status)
+		cd s &&
+		test_expect_code 1 git grep xxyyzz .. >out &&
+		! test -s out
 	)
 '
 
-- 
2.18.0.203.gfac676dfb9


^ permalink raw reply related	[flat|nested] 52+ messages in thread

* [PATCH 13/25] t9001: fix broken "invoke hook" test
  2018-07-02  0:23 [PATCH 00/25] fix buggy tests, modernize tests, fix broken &&-chains Eric Sunshine
                   ` (11 preceding siblings ...)
  2018-07-02  0:23 ` [PATCH 12/25] t7810: use test_expect_code() instead of hand-rolled comparison Eric Sunshine
@ 2018-07-02  0:23 ` Eric Sunshine
  2018-07-02  0:23 ` [PATCH 14/25] t9814: simplify convoluted check that command correctly errors out Eric Sunshine
                   ` (13 subsequent siblings)
  26 siblings, 0 replies; 52+ messages in thread
From: Eric Sunshine @ 2018-07-02  0:23 UTC (permalink / raw)
  To: git
  Cc: Elijah Newren, Johannes Sixt, Jonathan Nieder, Jonathan Tan,
	Stefan Beller, Junio C Hamano, Luke Diamand, Jeff King,
	Eric Sunshine

This test has been dysfunctional since it was added by 6489660b4b
(send-email: support validate hook, 2017-05-12), however, the problem
went unnoticed due to a broken &&-chain late in the test.

The test wants to verify that a non-zero exit code from the
'sendemail-validate' hook causes git-send-email to abort with a
particular error message. A command which is expected to fail should be
run with 'test_must_fail', however, the test neglects to do so.

Fix this problem, as well as the broken &&-chain behind which the
problem hid.

Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
---
 t/t9001-send-email.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index e80eacbb1b..776769fe0d 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -1966,11 +1966,11 @@ test_expect_success $PREREQ 'invoke hook' '
 
 		# Verify error message when a patch is rejected by the hook
 		sed -e "s/add master/x/" ../0001-add-master.patch >../another.patch &&
-		git send-email \
+		test_must_fail git send-email \
 			--from="Example <nobody@example.com>" \
 			--to=nobody@example.com \
 			--smtp-server="$(pwd)/../fake.sendmail" \
-			../another.patch 2>err
+			../another.patch 2>err &&
 		test_i18ngrep "rejected by sendemail-validate hook" err
 	)
 '
-- 
2.18.0.203.gfac676dfb9


^ permalink raw reply related	[flat|nested] 52+ messages in thread

* [PATCH 14/25] t9814: simplify convoluted check that command correctly errors out
  2018-07-02  0:23 [PATCH 00/25] fix buggy tests, modernize tests, fix broken &&-chains Eric Sunshine
                   ` (12 preceding siblings ...)
  2018-07-02  0:23 ` [PATCH 13/25] t9001: fix broken "invoke hook" test Eric Sunshine
@ 2018-07-02  0:23 ` Eric Sunshine
  2018-07-02  0:23 ` [PATCH 15/25] t0000-t0999: fix broken &&-chains Eric Sunshine
                   ` (12 subsequent siblings)
  26 siblings, 0 replies; 52+ messages in thread
From: Eric Sunshine @ 2018-07-02  0:23 UTC (permalink / raw)
  To: git
  Cc: Elijah Newren, Johannes Sixt, Jonathan Nieder, Jonathan Tan,
	Stefan Beller, Junio C Hamano, Luke Diamand, Jeff King,
	Eric Sunshine

This test uses a convoluted method to verify that "p4 help" errors
out when asked for help about an unknown command. In doing so, it
intentionally breaks the &&-chain. Simplify by employing the typical
"! command" idiom and a normal &&-chain instead.

Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
---
 t/t9814-git-p4-rename.sh | 18 +++---------------
 1 file changed, 3 insertions(+), 15 deletions(-)

diff --git a/t/t9814-git-p4-rename.sh b/t/t9814-git-p4-rename.sh
index e7e0268e98..60baa06e27 100755
--- a/t/t9814-git-p4-rename.sh
+++ b/t/t9814-git-p4-rename.sh
@@ -9,23 +9,11 @@ test_expect_success 'start p4d' '
 '
 
 # We rely on this behavior to detect for p4 move availability.
-test_expect_success 'p4 help unknown returns 1' '
+test_expect_success '"p4 help unknown" errors out' '
 	(
 		cd "$cli" &&
-		(
-			p4 help client >errs 2>&1
-			echo $? >retval
-		)
-		echo 0 >expected &&
-		test_cmp expected retval &&
-		rm retval &&
-		(
-			p4 help nosuchcommand >errs 2>&1
-			echo $? >retval
-		)
-		echo 1 >expected &&
-		test_cmp expected retval &&
-		rm retval
+		p4 help client &&
+		! p4 help nosuchcommand
 	)
 '
 
-- 
2.18.0.203.gfac676dfb9


^ permalink raw reply related	[flat|nested] 52+ messages in thread

* [PATCH 15/25] t0000-t0999: fix broken &&-chains
  2018-07-02  0:23 [PATCH 00/25] fix buggy tests, modernize tests, fix broken &&-chains Eric Sunshine
                   ` (13 preceding siblings ...)
  2018-07-02  0:23 ` [PATCH 14/25] t9814: simplify convoluted check that command correctly errors out Eric Sunshine
@ 2018-07-02  0:23 ` Eric Sunshine
  2018-07-02  0:23 ` [PATCH 16/25] t1000-t1999: " Eric Sunshine
                   ` (11 subsequent siblings)
  26 siblings, 0 replies; 52+ messages in thread
From: Eric Sunshine @ 2018-07-02  0:23 UTC (permalink / raw)
  To: git
  Cc: Elijah Newren, Johannes Sixt, Jonathan Nieder, Jonathan Tan,
	Stefan Beller, Junio C Hamano, Luke Diamand, Jeff King,
	Eric Sunshine

Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
---
 t/t0000-basic.sh      |  2 +-
 t/t0003-attributes.sh | 24 ++++++++++++------------
 t/t0021-conversion.sh |  4 ++--
 3 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/t/t0000-basic.sh b/t/t0000-basic.sh
index af61d083b4..34859fe4a5 100755
--- a/t/t0000-basic.sh
+++ b/t/t0000-basic.sh
@@ -1081,7 +1081,7 @@ test_expect_success 'very long name in the index handled sanely' '
 	(
 		git ls-files -s path4 |
 		sed -e "s/	.*/	/" |
-		tr -d "\012"
+		tr -d "\012" &&
 		echo "$a"
 	) | git update-index --index-info &&
 	len=$(git ls-files "a*" | wc -c) &&
diff --git a/t/t0003-attributes.sh b/t/t0003-attributes.sh
index f19ae4f8cc..5c37c2e1f8 100755
--- a/t/t0003-attributes.sh
+++ b/t/t0003-attributes.sh
@@ -34,15 +34,15 @@ test_expect_success 'open-quoted pathname' '
 test_expect_success 'setup' '
 	mkdir -p a/b/d a/c b &&
 	(
-		echo "[attr]notest !test"
-		echo "\" d \"	test=d"
-		echo " e	test=e"
-		echo " e\"	test=e"
-		echo "f	test=f"
-		echo "a/i test=a/i"
-		echo "onoff test -test"
-		echo "offon -test test"
-		echo "no notest"
+		echo "[attr]notest !test" &&
+		echo "\" d \"	test=d" &&
+		echo " e	test=e" &&
+		echo " e\"	test=e" &&
+		echo "f	test=f" &&
+		echo "a/i test=a/i" &&
+		echo "onoff test -test" &&
+		echo "offon -test test" &&
+		echo "no notest" &&
 		echo "A/e/F test=A/e/F"
 	) >.gitattributes &&
 	(
@@ -51,7 +51,7 @@ test_expect_success 'setup' '
 	) >a/.gitattributes &&
 	(
 		echo "h test=a/b/h" &&
-		echo "d/* test=a/b/d/*"
+		echo "d/* test=a/b/d/*" &&
 		echo "d/yes notest"
 	) >a/b/.gitattributes &&
 	(
@@ -287,7 +287,7 @@ test_expect_success 'bare repository: check that .gitattribute is ignored' '
 	(
 		cd bare.git &&
 		(
-			echo "f	test=f"
+			echo "f	test=f" &&
 			echo "a/i test=a/i"
 		) >.gitattributes &&
 		attr_check f unspecified &&
@@ -312,7 +312,7 @@ test_expect_success 'bare repository: test info/attributes' '
 	(
 		cd bare.git &&
 		(
-			echo "f	test=f"
+			echo "f	test=f" &&
 			echo "a/i test=a/i"
 		) >info/attributes &&
 		attr_check f f &&
diff --git a/t/t0021-conversion.sh b/t/t0021-conversion.sh
index 9479a4aaab..6a213608cc 100755
--- a/t/t0021-conversion.sh
+++ b/t/t0021-conversion.sh
@@ -785,7 +785,7 @@ test_expect_success PERL 'missing file in delayed checkout' '
 		cd repo &&
 		git init &&
 		echo "*.a filter=bug" >.gitattributes &&
-		cp "$TEST_ROOT/test.o" missing-delay.a
+		cp "$TEST_ROOT/test.o" missing-delay.a &&
 		git add . &&
 		git commit -m "test commit"
 	) &&
@@ -807,7 +807,7 @@ test_expect_success PERL 'invalid file in delayed checkout' '
 		git init &&
 		echo "*.a filter=bug" >.gitattributes &&
 		cp "$TEST_ROOT/test.o" invalid-delay.a &&
-		cp "$TEST_ROOT/test.o" unfiltered
+		cp "$TEST_ROOT/test.o" unfiltered &&
 		git add . &&
 		git commit -m "test commit"
 	) &&
-- 
2.18.0.203.gfac676dfb9


^ permalink raw reply related	[flat|nested] 52+ messages in thread

* [PATCH 16/25] t1000-t1999: fix broken &&-chains
  2018-07-02  0:23 [PATCH 00/25] fix buggy tests, modernize tests, fix broken &&-chains Eric Sunshine
                   ` (14 preceding siblings ...)
  2018-07-02  0:23 ` [PATCH 15/25] t0000-t0999: fix broken &&-chains Eric Sunshine
@ 2018-07-02  0:23 ` Eric Sunshine
  2018-07-02  0:23 ` [PATCH 17/25] t2000-t2999: " Eric Sunshine
                   ` (10 subsequent siblings)
  26 siblings, 0 replies; 52+ messages in thread
From: Eric Sunshine @ 2018-07-02  0:23 UTC (permalink / raw)
  To: git
  Cc: Elijah Newren, Johannes Sixt, Jonathan Nieder, Jonathan Tan,
	Stefan Beller, Junio C Hamano, Luke Diamand, Jeff King,
	Eric Sunshine

Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
---
 t/t1004-read-tree-m-u-wf.sh         |  8 ++++----
 t/t1005-read-tree-reset.sh          | 10 +++++-----
 t/t1020-subdirectory.sh             |  2 +-
 t/t1050-large.sh                    |  6 +++---
 t/t1411-reflog-show.sh              |  6 +++---
 t/t1512-rev-parse-disambiguation.sh |  6 +++---
 6 files changed, 19 insertions(+), 19 deletions(-)

diff --git a/t/t1004-read-tree-m-u-wf.sh b/t/t1004-read-tree-m-u-wf.sh
index 826cd32e23..c13578a635 100755
--- a/t/t1004-read-tree-m-u-wf.sh
+++ b/t/t1004-read-tree-m-u-wf.sh
@@ -210,10 +210,10 @@ test_expect_success 'D/F' '
 	read_tree_u_must_succeed -m -u branch-point side-b side-a &&
 	git ls-files -u >actual &&
 	(
-		a=$(git rev-parse branch-point:subdir/file2)
-		b=$(git rev-parse side-a:subdir/file2/another)
-		echo "100644 $a 1	subdir/file2"
-		echo "100644 $a 2	subdir/file2"
+		a=$(git rev-parse branch-point:subdir/file2) &&
+		b=$(git rev-parse side-a:subdir/file2/another) &&
+		echo "100644 $a 1	subdir/file2" &&
+		echo "100644 $a 2	subdir/file2" &&
 		echo "100644 $b 3	subdir/file2/another"
 	) >expect &&
 	test_cmp expect actual
diff --git a/t/t1005-read-tree-reset.sh b/t/t1005-read-tree-reset.sh
index 074568500a..83b09e1310 100755
--- a/t/t1005-read-tree-reset.sh
+++ b/t/t1005-read-tree-reset.sh
@@ -33,7 +33,7 @@ test_expect_success 'reset should remove remnants from a failed merge' '
 	git ls-files -s >expect &&
 	sha1=$(git rev-parse :new) &&
 	(
-		echo "100644 $sha1 1	old"
+		echo "100644 $sha1 1	old" &&
 		echo "100644 $sha1 3	old"
 	) | git update-index --index-info &&
 	>old &&
@@ -48,7 +48,7 @@ test_expect_success 'two-way reset should remove remnants too' '
 	git ls-files -s >expect &&
 	sha1=$(git rev-parse :new) &&
 	(
-		echo "100644 $sha1 1	old"
+		echo "100644 $sha1 1	old" &&
 		echo "100644 $sha1 3	old"
 	) | git update-index --index-info &&
 	>old &&
@@ -63,7 +63,7 @@ test_expect_success 'Porcelain reset should remove remnants too' '
 	git ls-files -s >expect &&
 	sha1=$(git rev-parse :new) &&
 	(
-		echo "100644 $sha1 1	old"
+		echo "100644 $sha1 1	old" &&
 		echo "100644 $sha1 3	old"
 	) | git update-index --index-info &&
 	>old &&
@@ -78,7 +78,7 @@ test_expect_success 'Porcelain checkout -f should remove remnants too' '
 	git ls-files -s >expect &&
 	sha1=$(git rev-parse :new) &&
 	(
-		echo "100644 $sha1 1	old"
+		echo "100644 $sha1 1	old" &&
 		echo "100644 $sha1 3	old"
 	) | git update-index --index-info &&
 	>old &&
@@ -93,7 +93,7 @@ test_expect_success 'Porcelain checkout -f HEAD should remove remnants too' '
 	git ls-files -s >expect &&
 	sha1=$(git rev-parse :new) &&
 	(
-		echo "100644 $sha1 1	old"
+		echo "100644 $sha1 1	old" &&
 		echo "100644 $sha1 3	old"
 	) | git update-index --index-info &&
 	>old &&
diff --git a/t/t1020-subdirectory.sh b/t/t1020-subdirectory.sh
index df3183ea1a..c2df75e495 100755
--- a/t/t1020-subdirectory.sh
+++ b/t/t1020-subdirectory.sh
@@ -148,7 +148,7 @@ test_expect_success 'GIT_PREFIX for built-ins' '
 	(
 		cd dir &&
 		echo "change" >two &&
-		GIT_EXTERNAL_DIFF=./diff git diff >../actual
+		GIT_EXTERNAL_DIFF=./diff git diff >../actual &&
 		git checkout -- two
 	) &&
 	test_cmp expect actual
diff --git a/t/t1050-large.sh b/t/t1050-large.sh
index f9eb143f43..1a9b21b293 100755
--- a/t/t1050-large.sh
+++ b/t/t1050-large.sh
@@ -108,7 +108,7 @@ test_expect_success 'packsize limit' '
 		test-tool genrandom "c" $(( 128 * 1024 )) >mid3 &&
 		git add mid1 mid2 mid3 &&
 
-		count=0
+		count=0 &&
 		for pi in .git/objects/pack/pack-*.idx
 		do
 			test -f "$pi" && count=$(( $count + 1 ))
@@ -116,8 +116,8 @@ test_expect_success 'packsize limit' '
 		test $count = 2 &&
 
 		(
-			git hash-object --stdin <mid1
-			git hash-object --stdin <mid2
+			git hash-object --stdin <mid1 &&
+			git hash-object --stdin <mid2 &&
 			git hash-object --stdin <mid3
 		) |
 		sort >expect &&
diff --git a/t/t1411-reflog-show.sh b/t/t1411-reflog-show.sh
index 596907758d..4d62ceef9c 100755
--- a/t/t1411-reflog-show.sh
+++ b/t/t1411-reflog-show.sh
@@ -159,9 +159,9 @@ test_expect_success 'git log -g -p shows diffs vs. parents' '
 	git log -1 -p HEAD^ >log.one &&
 	git log -1 -p HEAD >log.two &&
 	(
-		cat log.one; echo
-		cat log.two; echo
-		cat log.one; echo
+		cat log.one && echo &&
+		cat log.two && echo &&
+		cat log.one && echo &&
 		cat log.two
 	) >expect &&
 	test_cmp expect actual
diff --git a/t/t1512-rev-parse-disambiguation.sh b/t/t1512-rev-parse-disambiguation.sh
index 96fe3754c8..e4d5b56014 100755
--- a/t/t1512-rev-parse-disambiguation.sh
+++ b/t/t1512-rev-parse-disambiguation.sh
@@ -34,8 +34,8 @@ test_expect_success 'blob and tree' '
 		for i in 0 1 2 3 4 5 6 7 8 9
 		do
 			echo $i
-		done
-		echo
+		done &&
+		echo &&
 		echo b1rwzyc3
 	) >a0blgqsjc &&
 
@@ -222,7 +222,7 @@ test_expect_success 'more history' '
 
 	test_might_fail git rm -f a0blgqsjc &&
 	(
-		git cat-file blob $side:f5518nwu
+		git cat-file blob $side:f5518nwu &&
 		echo j3l0i9s6
 	) >ab2gs879 &&
 	git add ab2gs879 &&
-- 
2.18.0.203.gfac676dfb9


^ permalink raw reply related	[flat|nested] 52+ messages in thread

* [PATCH 17/25] t2000-t2999: fix broken &&-chains
  2018-07-02  0:23 [PATCH 00/25] fix buggy tests, modernize tests, fix broken &&-chains Eric Sunshine
                   ` (15 preceding siblings ...)
  2018-07-02  0:23 ` [PATCH 16/25] t1000-t1999: " Eric Sunshine
@ 2018-07-02  0:23 ` Eric Sunshine
  2018-07-02  0:23 ` [PATCH 18/25] t3000-t3999: " Eric Sunshine
                   ` (9 subsequent siblings)
  26 siblings, 0 replies; 52+ messages in thread
From: Eric Sunshine @ 2018-07-02  0:23 UTC (permalink / raw)
  To: git
  Cc: Elijah Newren, Johannes Sixt, Jonathan Nieder, Jonathan Tan,
	Stefan Beller, Junio C Hamano, Luke Diamand, Jeff King,
	Eric Sunshine

Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
---
 t/t2103-update-index-ignore-missing.sh |  2 +-
 t/t2202-add-addremove.sh               | 14 +++++++-------
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/t/t2103-update-index-ignore-missing.sh b/t/t2103-update-index-ignore-missing.sh
index 332694e7d3..0114f05228 100755
--- a/t/t2103-update-index-ignore-missing.sh
+++ b/t/t2103-update-index-ignore-missing.sh
@@ -32,7 +32,7 @@ test_expect_success basics '
 		test_create_repo xyzzy &&
 		cd xyzzy &&
 		>file &&
-		git add file
+		git add file &&
 		git commit -m "sub initial"
 	) &&
 	git add xyzzy &&
diff --git a/t/t2202-add-addremove.sh b/t/t2202-add-addremove.sh
index 6a5a3166b1..17744e8c57 100755
--- a/t/t2202-add-addremove.sh
+++ b/t/t2202-add-addremove.sh
@@ -6,12 +6,12 @@ test_description='git add --all'
 
 test_expect_success setup '
 	(
-		echo .gitignore
+		echo .gitignore &&
 		echo will-remove
 	) >expect &&
 	(
-		echo actual
-		echo expect
+		echo actual &&
+		echo expect &&
 		echo ignored
 	) >.gitignore &&
 	git --literal-pathspecs add --all &&
@@ -25,10 +25,10 @@ test_expect_success setup '
 
 test_expect_success 'git add --all' '
 	(
-		echo .gitignore
-		echo not-ignored
-		echo "M	.gitignore"
-		echo "A	not-ignored"
+		echo .gitignore &&
+		echo not-ignored &&
+		echo "M	.gitignore" &&
+		echo "A	not-ignored" &&
 		echo "D	will-remove"
 	) >expect &&
 	>ignored &&
-- 
2.18.0.203.gfac676dfb9


^ permalink raw reply related	[flat|nested] 52+ messages in thread

* [PATCH 18/25] t3000-t3999: fix broken &&-chains
  2018-07-02  0:23 [PATCH 00/25] fix buggy tests, modernize tests, fix broken &&-chains Eric Sunshine
                   ` (16 preceding siblings ...)
  2018-07-02  0:23 ` [PATCH 17/25] t2000-t2999: " Eric Sunshine
@ 2018-07-02  0:23 ` Eric Sunshine
  2018-07-02  0:23 ` [PATCH 19/25] t3030: " Eric Sunshine
                   ` (8 subsequent siblings)
  26 siblings, 0 replies; 52+ messages in thread
From: Eric Sunshine @ 2018-07-02  0:23 UTC (permalink / raw)
  To: git
  Cc: Elijah Newren, Johannes Sixt, Jonathan Nieder, Jonathan Tan,
	Stefan Beller, Junio C Hamano, Luke Diamand, Jeff King,
	Eric Sunshine

Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
---
 t/t3000-ls-files-others.sh              | 2 +-
 t/t3006-ls-files-long.sh                | 2 +-
 t/t3008-ls-files-lazy-init-name-hash.sh | 8 ++++----
 t/t3050-subprojects-fetch.sh            | 8 ++++----
 t/t3210-pack-refs.sh                    | 4 ++--
 t/t3301-notes.sh                        | 8 ++++----
 t/t3400-rebase.sh                       | 8 ++++----
 t/t3402-rebase-merge.sh                 | 4 ++--
 t/t3418-rebase-continue.sh              | 4 ++--
 t/t3700-add.sh                          | 8 ++++----
 10 files changed, 28 insertions(+), 28 deletions(-)

diff --git a/t/t3000-ls-files-others.sh b/t/t3000-ls-files-others.sh
index c525656b2c..afd4756134 100755
--- a/t/t3000-ls-files-others.sh
+++ b/t/t3000-ls-files-others.sh
@@ -84,7 +84,7 @@ test_expect_success SYMLINKS 'ls-files --others with symlinked submodule' '
 	) &&
 	(
 		cd super &&
-		"$SHELL_PATH" "$TEST_DIRECTORY/../contrib/workdir/git-new-workdir" ../sub sub
+		"$SHELL_PATH" "$TEST_DIRECTORY/../contrib/workdir/git-new-workdir" ../sub sub &&
 		git ls-files --others --exclude-standard >../actual
 	) &&
 	echo sub/ >expect &&
diff --git a/t/t3006-ls-files-long.sh b/t/t3006-ls-files-long.sh
index 202ad658b8..e109c3fbfb 100755
--- a/t/t3006-ls-files-long.sh
+++ b/t/t3006-ls-files-long.sh
@@ -29,7 +29,7 @@ test_expect_success 'overly-long path does not replace another by mistake' '
 	printf "$pat" "$blob_a" "$path_a" "$blob_z" "$path_z" |
 	git update-index --add --index-info &&
 	(
-		echo "$path_a"
+		echo "$path_a" &&
 		echo "$path_z"
 	) >expect &&
 	git ls-files >actual &&
diff --git a/t/t3008-ls-files-lazy-init-name-hash.sh b/t/t3008-ls-files-lazy-init-name-hash.sh
index 08af596ba6..64f047332b 100755
--- a/t/t3008-ls-files-lazy-init-name-hash.sh
+++ b/t/t3008-ls-files-lazy-init-name-hash.sh
@@ -14,10 +14,10 @@ LAZY_THREAD_COST=2000
 
 test_expect_success 'no buffer overflow in lazy_init_name_hash' '
 	(
-	    test_seq $LAZY_THREAD_COST | sed "s/^/a_/"
-	    echo b/b/b
-	    test_seq $LAZY_THREAD_COST | sed "s/^/c_/"
-	    test_seq 50 | sed "s/^/d_/" | tr "\n" "/"; echo d
+	    test_seq $LAZY_THREAD_COST | sed "s/^/a_/" &&
+	    echo b/b/b &&
+	    test_seq $LAZY_THREAD_COST | sed "s/^/c_/" &&
+	    test_seq 50 | sed "s/^/d_/" | tr "\n" "/" && echo d
 	) |
 	sed "s/^/100644 $EMPTY_BLOB	/" |
 	git update-index --index-info &&
diff --git a/t/t3050-subprojects-fetch.sh b/t/t3050-subprojects-fetch.sh
index 2f5f41a012..f1f09abdd9 100755
--- a/t/t3050-subprojects-fetch.sh
+++ b/t/t3050-subprojects-fetch.sh
@@ -21,10 +21,10 @@ test_expect_success setup '
 
 test_expect_success clone '
 	git clone "file://$(pwd)/.git" cloned &&
-	(git rev-parse HEAD; git ls-files -s) >expected &&
+	(git rev-parse HEAD && git ls-files -s) >expected &&
 	(
 		cd cloned &&
-		(git rev-parse HEAD; git ls-files -s) >../actual
+		(git rev-parse HEAD && git ls-files -s) >../actual
 	) &&
 	test_cmp expected actual
 '
@@ -40,11 +40,11 @@ test_expect_success advance '
 '
 
 test_expect_success fetch '
-	(git rev-parse HEAD; git ls-files -s) >expected &&
+	(git rev-parse HEAD && git ls-files -s) >expected &&
 	(
 		cd cloned &&
 		git pull &&
-		(git rev-parse HEAD; git ls-files -s) >../actual
+		(git rev-parse HEAD && git ls-files -s) >../actual
 	) &&
 	test_cmp expected actual
 '
diff --git a/t/t3210-pack-refs.sh b/t/t3210-pack-refs.sh
index afa27ffe2d..724b4b9bc0 100755
--- a/t/t3210-pack-refs.sh
+++ b/t/t3210-pack-refs.sh
@@ -231,9 +231,9 @@ test_expect_success 'timeout if packed-refs.lock exists' '
 test_expect_success 'retry acquiring packed-refs.lock' '
 	LOCK=.git/packed-refs.lock &&
 	>"$LOCK" &&
-	test_when_finished "wait; rm -f $LOCK" &&
+	test_when_finished "wait && rm -f $LOCK" &&
 	{
-		( sleep 1 ; rm -f $LOCK ) &
+		( sleep 1 && rm -f $LOCK ) &
 	} &&
 	git -c core.packedrefstimeout=3000 pack-refs --all --prune
 '
diff --git a/t/t3301-notes.sh b/t/t3301-notes.sh
index 2d200fdf36..ac62dc0e8f 100755
--- a/t/t3301-notes.sh
+++ b/t/t3301-notes.sh
@@ -914,7 +914,7 @@ test_expect_success 'git notes copy --stdin' '
 		${indent}
 		${indent}yet another note
 	EOF
-	(echo $(git rev-parse HEAD~3) $(git rev-parse HEAD^); \
+	(echo $(git rev-parse HEAD~3) $(git rev-parse HEAD^) &&
 	echo $(git rev-parse HEAD~2) $(git rev-parse HEAD)) |
 	git notes copy --stdin &&
 	git log -2 >actual &&
@@ -939,7 +939,7 @@ test_expect_success 'git notes copy --for-rewrite (unconfigured)' '
 	EOF
 	test_commit 14th &&
 	test_commit 15th &&
-	(echo $(git rev-parse HEAD~3) $(git rev-parse HEAD^); \
+	(echo $(git rev-parse HEAD~3) $(git rev-parse HEAD^) &&
 	echo $(git rev-parse HEAD~2) $(git rev-parse HEAD)) |
 	git notes copy --for-rewrite=foo &&
 	git log -2 >actual &&
@@ -972,7 +972,7 @@ test_expect_success 'git notes copy --for-rewrite (enabled)' '
 	EOF
 	test_config notes.rewriteMode overwrite &&
 	test_config notes.rewriteRef "refs/notes/*" &&
-	(echo $(git rev-parse HEAD~3) $(git rev-parse HEAD^); \
+	(echo $(git rev-parse HEAD~3) $(git rev-parse HEAD^) &&
 	echo $(git rev-parse HEAD~2) $(git rev-parse HEAD)) |
 	git notes copy --for-rewrite=foo &&
 	git log -2 >actual &&
@@ -1059,7 +1059,7 @@ test_expect_success 'git notes copy --for-rewrite (append two to one)' '
 	git notes add -f -m"append 2" HEAD^^ &&
 	test_config notes.rewriteMode concatenate &&
 	test_config notes.rewriteRef "refs/notes/*" &&
-	(echo $(git rev-parse HEAD^) $(git rev-parse HEAD);
+	(echo $(git rev-parse HEAD^) $(git rev-parse HEAD) &&
 	echo $(git rev-parse HEAD^^) $(git rev-parse HEAD)) |
 	git notes copy --for-rewrite=foo &&
 	git log -1 >actual &&
diff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh
index 72d9564747..3996ee0135 100755
--- a/t/t3400-rebase.sh
+++ b/t/t3400-rebase.sh
@@ -200,10 +200,10 @@ test_expect_success 'rebase -q is quiet' '
 
 test_expect_success 'Rebase a commit that sprinkles CRs in' '
 	(
-		echo "One"
-		echo "TwoQ"
-		echo "Three"
-		echo "FQur"
+		echo "One" &&
+		echo "TwoQ" &&
+		echo "Three" &&
+		echo "FQur" &&
 		echo "Five"
 	) | q_to_cr >CR &&
 	git add CR &&
diff --git a/t/t3402-rebase-merge.sh b/t/t3402-rebase-merge.sh
index 488945e007..a1ec501a87 100755
--- a/t/t3402-rebase-merge.sh
+++ b/t/t3402-rebase-merge.sh
@@ -25,7 +25,7 @@ test_expect_success setup '
 	git commit -a -m"master updates a bit more." &&
 
 	git checkout side &&
-	(echo "0 $T" ; cat original) >renamed &&
+	(echo "0 $T" && cat original) >renamed &&
 	git add renamed &&
 	git update-index --force-remove original &&
 	git commit -a -m"side renames and edits." &&
@@ -143,7 +143,7 @@ test_expect_success 'rebase -s funny -Xopt' '
 	git checkout -b test-funny master^ &&
 	test_commit funny &&
 	(
-		PATH=./test-bin:$PATH
+		PATH=./test-bin:$PATH &&
 		git rebase -s funny -Xopt master
 	) &&
 	test -f funny.was.run
diff --git a/t/t3418-rebase-continue.sh b/t/t3418-rebase-continue.sh
index 03bf1b8a3b..853e015839 100755
--- a/t/t3418-rebase-continue.sh
+++ b/t/t3418-rebase-continue.sh
@@ -60,7 +60,7 @@ test_expect_success 'rebase --continue remembers merge strategy and options' '
 	EOF
 	chmod +x test-bin/git-merge-funny &&
 	(
-		PATH=./test-bin:$PATH
+		PATH=./test-bin:$PATH &&
 		test_must_fail git rebase -s funny -Xopt master topic
 	) &&
 	test -f funny.was.run &&
@@ -68,7 +68,7 @@ test_expect_success 'rebase --continue remembers merge strategy and options' '
 	echo "Resolved" >F2 &&
 	git add F2 &&
 	(
-		PATH=./test-bin:$PATH
+		PATH=./test-bin:$PATH &&
 		git rebase --continue
 	) &&
 	test -f funny.was.run
diff --git a/t/t3700-add.sh b/t/t3700-add.sh
index 07af05d7ae..618750167a 100755
--- a/t/t3700-add.sh
+++ b/t/t3700-add.sh
@@ -156,9 +156,9 @@ test_expect_success 'git add with filemode=0, symlinks=0, and unmerged entries'
 test_expect_success 'git add with filemode=0, symlinks=0 prefers stage 2 over stage 1' '
 	git rm --cached -f file symlink &&
 	(
-		echo "100644 $(git hash-object -w stage1) 1	file"
-		echo "100755 $(git hash-object -w stage2) 2	file"
-		echo "100644 $(printf 1 | git hash-object -w -t blob --stdin) 1	symlink"
+		echo "100644 $(git hash-object -w stage1) 1	file" &&
+		echo "100755 $(git hash-object -w stage2) 2	file" &&
+		echo "100644 $(printf 1 | git hash-object -w -t blob --stdin) 1	symlink" &&
 		echo "120000 $(printf 2 | git hash-object -w -t blob --stdin) 2	symlink"
 	) | git update-index --index-info &&
 	git config core.filemode 0 &&
@@ -265,7 +265,7 @@ test_expect_success 'git add to resolve conflicts on otherwise ignored path' '
 	git reset --hard &&
 	H=$(git rev-parse :1/2/a) &&
 	(
-		echo "100644 $H 1	track-this"
+		echo "100644 $H 1	track-this" &&
 		echo "100644 $H 3	track-this"
 	) | git update-index --index-info &&
 	echo track-this >>.gitignore &&
-- 
2.18.0.203.gfac676dfb9


^ permalink raw reply related	[flat|nested] 52+ messages in thread

* [PATCH 19/25] t3030: fix broken &&-chains
  2018-07-02  0:23 [PATCH 00/25] fix buggy tests, modernize tests, fix broken &&-chains Eric Sunshine
                   ` (17 preceding siblings ...)
  2018-07-02  0:23 ` [PATCH 18/25] t3000-t3999: " Eric Sunshine
@ 2018-07-02  0:23 ` Eric Sunshine
  2018-07-02  0:24 ` [PATCH 20/25] t4000-t4999: " Eric Sunshine
                   ` (7 subsequent siblings)
  26 siblings, 0 replies; 52+ messages in thread
From: Eric Sunshine @ 2018-07-02  0:23 UTC (permalink / raw)
  To: git
  Cc: Elijah Newren, Johannes Sixt, Jonathan Nieder, Jonathan Tan,
	Stefan Beller, Junio C Hamano, Luke Diamand, Jeff King,
	Eric Sunshine

Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
---
 t/t3030-merge-recursive.sh | 340 ++++++++++++++++++-------------------
 1 file changed, 170 insertions(+), 170 deletions(-)

diff --git a/t/t3030-merge-recursive.sh b/t/t3030-merge-recursive.sh
index 3563e77b37..ff641b348a 100755
--- a/t/t3030-merge-recursive.sh
+++ b/t/t3030-merge-recursive.sh
@@ -36,15 +36,15 @@ test_expect_success 'setup 1' '
 	test_tick &&
 	git commit -m "master modifies a and d/e" &&
 	c1=$(git rev-parse --verify HEAD) &&
-	( git ls-tree -r HEAD ; git ls-files -s ) >actual &&
+	( git ls-tree -r HEAD && git ls-files -s ) >actual &&
 	(
-		echo "100644 blob $o1	a"
-		echo "100644 blob $o0	b"
-		echo "100644 blob $o0	c"
-		echo "100644 blob $o1	d/e"
-		echo "100644 $o1 0	a"
-		echo "100644 $o0 0	b"
-		echo "100644 $o0 0	c"
+		echo "100644 blob $o1	a" &&
+		echo "100644 blob $o0	b" &&
+		echo "100644 blob $o0	c" &&
+		echo "100644 blob $o1	d/e" &&
+		echo "100644 $o1 0	a" &&
+		echo "100644 $o0 0	b" &&
+		echo "100644 $o0 0	c" &&
 		echo "100644 $o1 0	d/e"
 	) >expected &&
 	test_cmp expected actual
@@ -54,15 +54,15 @@ test_expect_success 'setup 2' '
 
 	rm -rf [abcd] &&
 	git checkout side &&
-	( git ls-tree -r HEAD ; git ls-files -s ) >actual &&
+	( git ls-tree -r HEAD && git ls-files -s ) >actual &&
 	(
-		echo "100644 blob $o0	a"
-		echo "100644 blob $o0	b"
-		echo "100644 blob $o0	c"
-		echo "100644 blob $o0	d/e"
-		echo "100644 $o0 0	a"
-		echo "100644 $o0 0	b"
-		echo "100644 $o0 0	c"
+		echo "100644 blob $o0	a" &&
+		echo "100644 blob $o0	b" &&
+		echo "100644 blob $o0	c" &&
+		echo "100644 blob $o0	d/e" &&
+		echo "100644 $o0 0	a" &&
+		echo "100644 $o0 0	b" &&
+		echo "100644 $o0 0	c" &&
 		echo "100644 $o0 0	d/e"
 	) >expected &&
 	test_cmp expected actual &&
@@ -75,15 +75,15 @@ test_expect_success 'setup 2' '
 	test_tick &&
 	git commit -m "side modifies a" &&
 	c2=$(git rev-parse --verify HEAD) &&
-	( git ls-tree -r HEAD ; git ls-files -s ) >actual &&
+	( git ls-tree -r HEAD && git ls-files -s ) >actual &&
 	(
-		echo "100644 blob $o2	a"
-		echo "100644 blob $o0	b"
-		echo "100644 blob $o0	c"
-		echo "100644 blob $o0	d/e"
-		echo "100644 $o2 0	a"
-		echo "100644 $o0 0	b"
-		echo "100644 $o0 0	c"
+		echo "100644 blob $o2	a" &&
+		echo "100644 blob $o0	b" &&
+		echo "100644 blob $o0	c" &&
+		echo "100644 blob $o0	d/e" &&
+		echo "100644 $o2 0	a" &&
+		echo "100644 $o0 0	b" &&
+		echo "100644 $o0 0	c" &&
 		echo "100644 $o0 0	d/e"
 	) >expected &&
 	test_cmp expected actual
@@ -93,15 +93,15 @@ test_expect_success 'setup 3' '
 
 	rm -rf [abcd] &&
 	git checkout df-1 &&
-	( git ls-tree -r HEAD ; git ls-files -s ) >actual &&
+	( git ls-tree -r HEAD && git ls-files -s ) >actual &&
 	(
-		echo "100644 blob $o0	a"
-		echo "100644 blob $o0	b"
-		echo "100644 blob $o0	c"
-		echo "100644 blob $o0	d/e"
-		echo "100644 $o0 0	a"
-		echo "100644 $o0 0	b"
-		echo "100644 $o0 0	c"
+		echo "100644 blob $o0	a" &&
+		echo "100644 blob $o0	b" &&
+		echo "100644 blob $o0	c" &&
+		echo "100644 blob $o0	d/e" &&
+		echo "100644 $o0 0	a" &&
+		echo "100644 $o0 0	b" &&
+		echo "100644 $o0 0	c" &&
 		echo "100644 $o0 0	d/e"
 	) >expected &&
 	test_cmp expected actual &&
@@ -112,15 +112,15 @@ test_expect_success 'setup 3' '
 	test_tick &&
 	git commit -m "df-1 makes b/c" &&
 	c3=$(git rev-parse --verify HEAD) &&
-	( git ls-tree -r HEAD ; git ls-files -s ) >actual &&
+	( git ls-tree -r HEAD && git ls-files -s ) >actual &&
 	(
-		echo "100644 blob $o0	a"
-		echo "100644 blob $o3	b/c"
-		echo "100644 blob $o0	c"
-		echo "100644 blob $o0	d/e"
-		echo "100644 $o0 0	a"
-		echo "100644 $o3 0	b/c"
-		echo "100644 $o0 0	c"
+		echo "100644 blob $o0	a" &&
+		echo "100644 blob $o3	b/c" &&
+		echo "100644 blob $o0	c" &&
+		echo "100644 blob $o0	d/e" &&
+		echo "100644 $o0 0	a" &&
+		echo "100644 $o3 0	b/c" &&
+		echo "100644 $o0 0	c" &&
 		echo "100644 $o0 0	d/e"
 	) >expected &&
 	test_cmp expected actual
@@ -130,15 +130,15 @@ test_expect_success 'setup 4' '
 
 	rm -rf [abcd] &&
 	git checkout df-2 &&
-	( git ls-tree -r HEAD ; git ls-files -s ) >actual &&
+	( git ls-tree -r HEAD && git ls-files -s ) >actual &&
 	(
-		echo "100644 blob $o0	a"
-		echo "100644 blob $o0	b"
-		echo "100644 blob $o0	c"
-		echo "100644 blob $o0	d/e"
-		echo "100644 $o0 0	a"
-		echo "100644 $o0 0	b"
-		echo "100644 $o0 0	c"
+		echo "100644 blob $o0	a" &&
+		echo "100644 blob $o0	b" &&
+		echo "100644 blob $o0	c" &&
+		echo "100644 blob $o0	d/e" &&
+		echo "100644 $o0 0	a" &&
+		echo "100644 $o0 0	b" &&
+		echo "100644 $o0 0	c" &&
 		echo "100644 $o0 0	d/e"
 	) >expected &&
 	test_cmp expected actual &&
@@ -149,15 +149,15 @@ test_expect_success 'setup 4' '
 	test_tick &&
 	git commit -m "df-2 makes a/c" &&
 	c4=$(git rev-parse --verify HEAD) &&
-	( git ls-tree -r HEAD ; git ls-files -s ) >actual &&
+	( git ls-tree -r HEAD && git ls-files -s ) >actual &&
 	(
-		echo "100644 blob $o4	a/c"
-		echo "100644 blob $o0	b"
-		echo "100644 blob $o0	c"
-		echo "100644 blob $o0	d/e"
-		echo "100644 $o4 0	a/c"
-		echo "100644 $o0 0	b"
-		echo "100644 $o0 0	c"
+		echo "100644 blob $o4	a/c" &&
+		echo "100644 blob $o0	b" &&
+		echo "100644 blob $o0	c" &&
+		echo "100644 blob $o0	d/e" &&
+		echo "100644 $o4 0	a/c" &&
+		echo "100644 $o0 0	b" &&
+		echo "100644 $o0 0	c" &&
 		echo "100644 $o0 0	d/e"
 	) >expected &&
 	test_cmp expected actual
@@ -167,15 +167,15 @@ test_expect_success 'setup 5' '
 
 	rm -rf [abcd] &&
 	git checkout remove &&
-	( git ls-tree -r HEAD ; git ls-files -s ) >actual &&
+	( git ls-tree -r HEAD && git ls-files -s ) >actual &&
 	(
-		echo "100644 blob $o0	a"
-		echo "100644 blob $o0	b"
-		echo "100644 blob $o0	c"
-		echo "100644 blob $o0	d/e"
-		echo "100644 $o0 0	a"
-		echo "100644 $o0 0	b"
-		echo "100644 $o0 0	c"
+		echo "100644 blob $o0	a" &&
+		echo "100644 blob $o0	b" &&
+		echo "100644 blob $o0	c" &&
+		echo "100644 blob $o0	d/e" &&
+		echo "100644 $o0 0	a" &&
+		echo "100644 $o0 0	b" &&
+		echo "100644 $o0 0	c" &&
 		echo "100644 $o0 0	d/e"
 	) >expected &&
 	test_cmp expected actual &&
@@ -190,13 +190,13 @@ test_expect_success 'setup 5' '
 	test_tick &&
 	git commit -m "remove removes b and modifies a" &&
 	c5=$(git rev-parse --verify HEAD) &&
-	( git ls-tree -r HEAD ; git ls-files -s ) >actual &&
+	( git ls-tree -r HEAD && git ls-files -s ) >actual &&
 	(
-		echo "100644 blob $o5	a"
-		echo "100644 blob $o0	c"
-		echo "100644 blob $o0	d/e"
-		echo "100644 $o5 0	a"
-		echo "100644 $o0 0	c"
+		echo "100644 blob $o5	a" &&
+		echo "100644 blob $o0	c" &&
+		echo "100644 blob $o0	d/e" &&
+		echo "100644 $o5 0	a" &&
+		echo "100644 $o0 0	c" &&
 		echo "100644 $o0 0	d/e"
 	) >expected &&
 	test_cmp expected actual
@@ -207,15 +207,15 @@ test_expect_success 'setup 6' '
 
 	rm -rf [abcd] &&
 	git checkout df-3 &&
-	( git ls-tree -r HEAD ; git ls-files -s ) >actual &&
+	( git ls-tree -r HEAD && git ls-files -s ) >actual &&
 	(
-		echo "100644 blob $o0	a"
-		echo "100644 blob $o0	b"
-		echo "100644 blob $o0	c"
-		echo "100644 blob $o0	d/e"
-		echo "100644 $o0 0	a"
-		echo "100644 $o0 0	b"
-		echo "100644 $o0 0	c"
+		echo "100644 blob $o0	a" &&
+		echo "100644 blob $o0	b" &&
+		echo "100644 blob $o0	c" &&
+		echo "100644 blob $o0	d/e" &&
+		echo "100644 $o0 0	a" &&
+		echo "100644 $o0 0	b" &&
+		echo "100644 $o0 0	c" &&
 		echo "100644 $o0 0	d/e"
 	) >expected &&
 	test_cmp expected actual &&
@@ -226,15 +226,15 @@ test_expect_success 'setup 6' '
 	test_tick &&
 	git commit -m "df-3 makes d" &&
 	c6=$(git rev-parse --verify HEAD) &&
-	( git ls-tree -r HEAD ; git ls-files -s ) >actual &&
+	( git ls-tree -r HEAD && git ls-files -s ) >actual &&
 	(
-		echo "100644 blob $o0	a"
-		echo "100644 blob $o0	b"
-		echo "100644 blob $o0	c"
-		echo "100644 blob $o6	d"
-		echo "100644 $o0 0	a"
-		echo "100644 $o0 0	b"
-		echo "100644 $o0 0	c"
+		echo "100644 blob $o0	a" &&
+		echo "100644 blob $o0	b" &&
+		echo "100644 blob $o0	c" &&
+		echo "100644 blob $o6	d" &&
+		echo "100644 $o0 0	a" &&
+		echo "100644 $o0 0	b" &&
+		echo "100644 $o0 0	c" &&
 		echo "100644 $o6 0	d"
 	) >expected &&
 	test_cmp expected actual
@@ -286,11 +286,11 @@ test_expect_success 'merge-recursive result' '
 
 	git ls-files -s >actual &&
 	(
-		echo "100644 $o0 1	a"
-		echo "100644 $o2 2	a"
-		echo "100644 $o1 3	a"
-		echo "100644 $o0 0	b"
-		echo "100644 $o0 0	c"
+		echo "100644 $o0 1	a" &&
+		echo "100644 $o2 2	a" &&
+		echo "100644 $o1 3	a" &&
+		echo "100644 $o0 0	b" &&
+		echo "100644 $o0 0	c" &&
 		echo "100644 $o1 0	d/e"
 	) >expected &&
 	test_cmp expected actual
@@ -325,10 +325,10 @@ test_expect_success 'merge-recursive remove conflict' '
 
 	git ls-files -s >actual &&
 	(
-		echo "100644 $o0 1	a"
-		echo "100644 $o1 2	a"
-		echo "100644 $o5 3	a"
-		echo "100644 $o0 0	c"
+		echo "100644 $o0 1	a" &&
+		echo "100644 $o1 2	a" &&
+		echo "100644 $o5 3	a" &&
+		echo "100644 $o0 0	c" &&
 		echo "100644 $o1 0	d/e"
 	) >expected &&
 	test_cmp expected actual
@@ -347,9 +347,9 @@ test_expect_success 'merge-recursive result' '
 
 	git ls-files -s >actual &&
 	(
-		echo "100644 $o1 0	a"
-		echo "100644 $o3 0	b/c"
-		echo "100644 $o0 0	c"
+		echo "100644 $o1 0	a" &&
+		echo "100644 $o3 0	b/c" &&
+		echo "100644 $o0 0	c" &&
 		echo "100644 $o1 0	d/e"
 	) >expected &&
 	test_cmp expected actual
@@ -369,11 +369,11 @@ test_expect_success 'merge-recursive d/f conflict result' '
 
 	git ls-files -s >actual &&
 	(
-		echo "100644 $o0 1	a"
-		echo "100644 $o1 2	a"
-		echo "100644 $o4 0	a/c"
-		echo "100644 $o0 0	b"
-		echo "100644 $o0 0	c"
+		echo "100644 $o0 1	a" &&
+		echo "100644 $o1 2	a" &&
+		echo "100644 $o4 0	a/c" &&
+		echo "100644 $o0 0	b" &&
+		echo "100644 $o0 0	c" &&
 		echo "100644 $o1 0	d/e"
 	) >expected &&
 	test_cmp expected actual
@@ -393,11 +393,11 @@ test_expect_success 'merge-recursive d/f conflict result the other way' '
 
 	git ls-files -s >actual &&
 	(
-		echo "100644 $o0 1	a"
-		echo "100644 $o1 3	a"
-		echo "100644 $o4 0	a/c"
-		echo "100644 $o0 0	b"
-		echo "100644 $o0 0	c"
+		echo "100644 $o0 1	a" &&
+		echo "100644 $o1 3	a" &&
+		echo "100644 $o4 0	a/c" &&
+		echo "100644 $o0 0	b" &&
+		echo "100644 $o0 0	c" &&
 		echo "100644 $o1 0	d/e"
 	) >expected &&
 	test_cmp expected actual
@@ -417,11 +417,11 @@ test_expect_success 'merge-recursive d/f conflict result' '
 
 	git ls-files -s >actual &&
 	(
-		echo "100644 $o1 0	a"
-		echo "100644 $o0 0	b"
-		echo "100644 $o0 0	c"
-		echo "100644 $o6 3	d"
-		echo "100644 $o0 1	d/e"
+		echo "100644 $o1 0	a" &&
+		echo "100644 $o0 0	b" &&
+		echo "100644 $o0 0	c" &&
+		echo "100644 $o6 3	d" &&
+		echo "100644 $o0 1	d/e" &&
 		echo "100644 $o1 2	d/e"
 	) >expected &&
 	test_cmp expected actual
@@ -441,11 +441,11 @@ test_expect_success 'merge-recursive d/f conflict result' '
 
 	git ls-files -s >actual &&
 	(
-		echo "100644 $o1 0	a"
-		echo "100644 $o0 0	b"
-		echo "100644 $o0 0	c"
-		echo "100644 $o6 2	d"
-		echo "100644 $o0 1	d/e"
+		echo "100644 $o1 0	a" &&
+		echo "100644 $o0 0	b" &&
+		echo "100644 $o0 0	c" &&
+		echo "100644 $o6 2	d" &&
+		echo "100644 $o0 1	d/e" &&
 		echo "100644 $o1 3	d/e"
 	) >expected &&
 	test_cmp expected actual
@@ -465,13 +465,13 @@ test_expect_success 'reset and bind merge' '
 	git read-tree --prefix=M/ master &&
 	git ls-files -s >actual &&
 	(
-		echo "100644 $o1 0	M/a"
-		echo "100644 $o0 0	M/b"
-		echo "100644 $o0 0	M/c"
-		echo "100644 $o1 0	M/d/e"
-		echo "100644 $o1 0	a"
-		echo "100644 $o0 0	b"
-		echo "100644 $o0 0	c"
+		echo "100644 $o1 0	M/a" &&
+		echo "100644 $o0 0	M/b" &&
+		echo "100644 $o0 0	M/c" &&
+		echo "100644 $o1 0	M/d/e" &&
+		echo "100644 $o1 0	a" &&
+		echo "100644 $o0 0	b" &&
+		echo "100644 $o0 0	c" &&
 		echo "100644 $o1 0	d/e"
 	) >expected &&
 	test_cmp expected actual &&
@@ -479,17 +479,17 @@ test_expect_success 'reset and bind merge' '
 	git read-tree --prefix=a1/ master &&
 	git ls-files -s >actual &&
 	(
-		echo "100644 $o1 0	M/a"
-		echo "100644 $o0 0	M/b"
-		echo "100644 $o0 0	M/c"
-		echo "100644 $o1 0	M/d/e"
-		echo "100644 $o1 0	a"
-		echo "100644 $o1 0	a1/a"
-		echo "100644 $o0 0	a1/b"
-		echo "100644 $o0 0	a1/c"
-		echo "100644 $o1 0	a1/d/e"
-		echo "100644 $o0 0	b"
-		echo "100644 $o0 0	c"
+		echo "100644 $o1 0	M/a" &&
+		echo "100644 $o0 0	M/b" &&
+		echo "100644 $o0 0	M/c" &&
+		echo "100644 $o1 0	M/d/e" &&
+		echo "100644 $o1 0	a" &&
+		echo "100644 $o1 0	a1/a" &&
+		echo "100644 $o0 0	a1/b" &&
+		echo "100644 $o0 0	a1/c" &&
+		echo "100644 $o1 0	a1/d/e" &&
+		echo "100644 $o0 0	b" &&
+		echo "100644 $o0 0	c" &&
 		echo "100644 $o1 0	d/e"
 	) >expected &&
 	test_cmp expected actual &&
@@ -497,21 +497,21 @@ test_expect_success 'reset and bind merge' '
 	git read-tree --prefix=z/ master &&
 	git ls-files -s >actual &&
 	(
-		echo "100644 $o1 0	M/a"
-		echo "100644 $o0 0	M/b"
-		echo "100644 $o0 0	M/c"
-		echo "100644 $o1 0	M/d/e"
-		echo "100644 $o1 0	a"
-		echo "100644 $o1 0	a1/a"
-		echo "100644 $o0 0	a1/b"
-		echo "100644 $o0 0	a1/c"
-		echo "100644 $o1 0	a1/d/e"
-		echo "100644 $o0 0	b"
-		echo "100644 $o0 0	c"
-		echo "100644 $o1 0	d/e"
-		echo "100644 $o1 0	z/a"
-		echo "100644 $o0 0	z/b"
-		echo "100644 $o0 0	z/c"
+		echo "100644 $o1 0	M/a" &&
+		echo "100644 $o0 0	M/b" &&
+		echo "100644 $o0 0	M/c" &&
+		echo "100644 $o1 0	M/d/e" &&
+		echo "100644 $o1 0	a" &&
+		echo "100644 $o1 0	a1/a" &&
+		echo "100644 $o0 0	a1/b" &&
+		echo "100644 $o0 0	a1/c" &&
+		echo "100644 $o1 0	a1/d/e" &&
+		echo "100644 $o0 0	b" &&
+		echo "100644 $o0 0	c" &&
+		echo "100644 $o1 0	d/e" &&
+		echo "100644 $o1 0	z/a" &&
+		echo "100644 $o0 0	z/b" &&
+		echo "100644 $o0 0	z/c" &&
 		echo "100644 $o1 0	z/d/e"
 	) >expected &&
 	test_cmp expected actual
@@ -589,8 +589,8 @@ test_expect_success 'merge-recursive simple w/submodule result' '
 
 	git ls-files -s >actual &&
 	(
-		echo "100644 $o5 0	a"
-		echo "100644 $o0 0	c"
+		echo "100644 $o5 0	a" &&
+		echo "100644 $o0 0	c" &&
 		echo "160000 $c1 0	d"
 	) >expected &&
 	test_cmp expected actual
@@ -601,13 +601,13 @@ test_expect_success 'merge-recursive copy vs. rename' '
 	git merge rename &&
 	( git ls-tree -r HEAD && git ls-files -s ) >actual &&
 	(
-		echo "100644 blob $o0	b"
-		echo "100644 blob $o0	c"
-		echo "100644 blob $o0	d/e"
-		echo "100644 blob $o0	e"
-		echo "100644 $o0 0	b"
-		echo "100644 $o0 0	c"
-		echo "100644 $o0 0	d/e"
+		echo "100644 blob $o0	b" &&
+		echo "100644 blob $o0	c" &&
+		echo "100644 blob $o0	d/e" &&
+		echo "100644 blob $o0	e" &&
+		echo "100644 $o0 0	b" &&
+		echo "100644 $o0 0	c" &&
+		echo "100644 $o0 0	d/e" &&
 		echo "100644 $o0 0	e"
 	) >expected &&
 	test_cmp expected actual
@@ -617,17 +617,17 @@ test_expect_failure 'merge-recursive rename vs. rename/symlink' '
 
 	git checkout -f rename &&
 	git merge rename-ln &&
-	( git ls-tree -r HEAD ; git ls-files -s ) >actual &&
+	( git ls-tree -r HEAD && git ls-files -s ) >actual &&
 	(
-		echo "120000 blob $oln	a"
-		echo "100644 blob $o0	b"
-		echo "100644 blob $o0	c"
-		echo "100644 blob $o0	d/e"
-		echo "100644 blob $o0	e"
-		echo "120000 $oln 0	a"
-		echo "100644 $o0 0	b"
-		echo "100644 $o0 0	c"
-		echo "100644 $o0 0	d/e"
+		echo "120000 blob $oln	a" &&
+		echo "100644 blob $o0	b" &&
+		echo "100644 blob $o0	c" &&
+		echo "100644 blob $o0	d/e" &&
+		echo "100644 blob $o0	e" &&
+		echo "120000 $oln 0	a" &&
+		echo "100644 $o0 0	b" &&
+		echo "100644 $o0 0	c" &&
+		echo "100644 $o0 0	d/e" &&
 		echo "100644 $o0 0	e"
 	) >expected &&
 	test_cmp expected actual
-- 
2.18.0.203.gfac676dfb9


^ permalink raw reply related	[flat|nested] 52+ messages in thread

* [PATCH 20/25] t4000-t4999: fix broken &&-chains
  2018-07-02  0:23 [PATCH 00/25] fix buggy tests, modernize tests, fix broken &&-chains Eric Sunshine
                   ` (18 preceding siblings ...)
  2018-07-02  0:23 ` [PATCH 19/25] t3030: " Eric Sunshine
@ 2018-07-02  0:24 ` Eric Sunshine
  2018-07-02  0:24 ` [PATCH 21/25] t5000-t5999: " Eric Sunshine
                   ` (6 subsequent siblings)
  26 siblings, 0 replies; 52+ messages in thread
From: Eric Sunshine @ 2018-07-02  0:24 UTC (permalink / raw)
  To: git
  Cc: Elijah Newren, Johannes Sixt, Jonathan Nieder, Jonathan Tan,
	Stefan Beller, Junio C Hamano, Luke Diamand, Jeff King,
	Eric Sunshine

Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
---
 t/t4001-diff-rename.sh                       |  2 +-
 t/t4024-diff-optimize-common.sh              | 16 ++++++++--------
 t/t4025-hunk-header.sh                       |  8 ++++----
 t/t4041-diff-submodule-option.sh             |  4 ++--
 t/t4060-diff-submodule-option-diff-format.sh |  2 +-
 t/t4121-apply-diffs.sh                       |  2 +-
 6 files changed, 17 insertions(+), 17 deletions(-)

diff --git a/t/t4001-diff-rename.sh b/t/t4001-diff-rename.sh
index bf4030371a..c16486a9d4 100755
--- a/t/t4001-diff-rename.sh
+++ b/t/t4001-diff-rename.sh
@@ -180,7 +180,7 @@ test_expect_success 'setup for many rename source candidates' '
 	git add "path??" &&
 	test_tick &&
 	git commit -m "hundred" &&
-	(cat path1; echo new) >new-path &&
+	(cat path1 && echo new) >new-path &&
 	echo old >>path1 &&
 	git add new-path path1 &&
 	git diff -l 4 -C -C --cached --name-status >actual 2>actual.err &&
diff --git a/t/t4024-diff-optimize-common.sh b/t/t4024-diff-optimize-common.sh
index 7e76018296..6b44ce1493 100755
--- a/t/t4024-diff-optimize-common.sh
+++ b/t/t4024-diff-optimize-common.sh
@@ -127,17 +127,17 @@ test_expect_success setup '
 
 	for n in $sample
 	do
-		( zs $n ; echo a ) >file-a$n &&
-		( echo b; zs $n; echo ) >file-b$n &&
-		( printf c; zs $n ) >file-c$n &&
-		( echo d; zs $n ) >file-d$n &&
+		( zs $n && echo a ) >file-a$n &&
+		( echo b && zs $n && echo ) >file-b$n &&
+		( printf c && zs $n ) >file-c$n &&
+		( echo d && zs $n ) >file-d$n &&
 
 		git add file-a$n file-b$n file-c$n file-d$n &&
 
-		( zs $n ; echo A ) >file-a$n &&
-		( echo B; zs $n; echo ) >file-b$n &&
-		( printf C; zs $n ) >file-c$n &&
-		( echo D; zs $n ) >file-d$n &&
+		( zs $n && echo A ) >file-a$n &&
+		( echo B && zs $n && echo ) >file-b$n &&
+		( printf C && zs $n ) >file-c$n &&
+		( echo D && zs $n ) >file-d$n &&
 
 		expect_pattern $n || return 1
 
diff --git a/t/t4025-hunk-header.sh b/t/t4025-hunk-header.sh
index 7a3dbc1ea2..fa44e78869 100755
--- a/t/t4025-hunk-header.sh
+++ b/t/t4025-hunk-header.sh
@@ -12,12 +12,12 @@ NS="$N$N$N$N$N$N$N$N$N$N$N$N$N"
 test_expect_success setup '
 
 	(
-		echo "A $NS"
+		echo "A $NS" &&
 		for c in B C D E F G H I J K
 		do
 			echo "  $c"
-		done
-		echo "L  $NS"
+		done &&
+		echo "L  $NS" &&
 		for c in M N O P Q R S T U V
 		do
 			echo "  $c"
@@ -34,7 +34,7 @@ test_expect_success 'hunk header truncation with an overly long line' '
 
 	git diff | sed -n -e "s/^.*@@//p" >actual &&
 	(
-		echo " A $N$N$N$N$N$N$N$N$N2"
+		echo " A $N$N$N$N$N$N$N$N$N2" &&
 		echo " L  $N$N$N$N$N$N$N$N$N1"
 	) >expected &&
 	test_cmp actual expected
diff --git a/t/t4041-diff-submodule-option.sh b/t/t4041-diff-submodule-option.sh
index 058ee0829d..4e3499ef84 100755
--- a/t/t4041-diff-submodule-option.sh
+++ b/t/t4041-diff-submodule-option.sh
@@ -498,7 +498,7 @@ test_expect_success 'given commit --submodule=short' '
 test_expect_success 'setup .git file for sm2' '
 	(cd sm2 &&
 	 REAL="$(pwd)/../.real" &&
-	 mv .git "$REAL"
+	 mv .git "$REAL" &&
 	 echo "gitdir: $REAL" >.git)
 '
 
@@ -527,7 +527,7 @@ test_expect_success 'diff --submodule with objects referenced by alternates' '
 		git commit -m "sub a"
 	) &&
 	(cd sub_alt &&
-		sha1_before=$(git rev-parse --short HEAD)
+		sha1_before=$(git rev-parse --short HEAD) &&
 		echo b >b &&
 		git add b &&
 		git commit -m b &&
diff --git a/t/t4060-diff-submodule-option-diff-format.sh b/t/t4060-diff-submodule-option-diff-format.sh
index 4b168d0ed7..0eba4620f0 100755
--- a/t/t4060-diff-submodule-option-diff-format.sh
+++ b/t/t4060-diff-submodule-option-diff-format.sh
@@ -721,7 +721,7 @@ test_expect_success 'given commit' '
 test_expect_success 'setup .git file for sm2' '
 	(cd sm2 &&
 	 REAL="$(pwd)/../.real" &&
-	 mv .git "$REAL"
+	 mv .git "$REAL" &&
 	 echo "gitdir: $REAL" >.git)
 '
 
diff --git a/t/t4121-apply-diffs.sh b/t/t4121-apply-diffs.sh
index aff551a1d7..66368effd5 100755
--- a/t/t4121-apply-diffs.sh
+++ b/t/t4121-apply-diffs.sh
@@ -27,6 +27,6 @@ test_expect_success 'setup' \
 
 test_expect_success \
 	'check if contextually independent diffs for the same file apply' \
-	'( git diff test~2 test~1; git diff test~1 test~0 )| git apply'
+	'( git diff test~2 test~1 && git diff test~1 test~0 )| git apply'
 
 test_done
-- 
2.18.0.203.gfac676dfb9


^ permalink raw reply related	[flat|nested] 52+ messages in thread

* [PATCH 21/25] t5000-t5999: fix broken &&-chains
  2018-07-02  0:23 [PATCH 00/25] fix buggy tests, modernize tests, fix broken &&-chains Eric Sunshine
                   ` (19 preceding siblings ...)
  2018-07-02  0:24 ` [PATCH 20/25] t4000-t4999: " Eric Sunshine
@ 2018-07-02  0:24 ` Eric Sunshine
  2018-07-12 12:37   ` SZEDER Gábor
  2018-07-02  0:24 ` [PATCH 22/25] t6000-t6999: " Eric Sunshine
                   ` (5 subsequent siblings)
  26 siblings, 1 reply; 52+ messages in thread
From: Eric Sunshine @ 2018-07-02  0:24 UTC (permalink / raw)
  To: git
  Cc: Elijah Newren, Johannes Sixt, Jonathan Nieder, Jonathan Tan,
	Stefan Beller, Junio C Hamano, Luke Diamand, Jeff King,
	Eric Sunshine

Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
---
 t/t5300-pack-object.sh         |  2 +-
 t/t5302-pack-index.sh          |  2 +-
 t/t5401-update-hooks.sh        |  4 ++--
 t/t5500-fetch-pack.sh          |  2 +-
 t/t5505-remote.sh              |  2 +-
 t/t5512-ls-remote.sh           |  4 ++--
 t/t5516-fetch-push.sh          | 10 +++++-----
 t/t5517-push-mirror.sh         | 10 +++++-----
 t/t5526-fetch-submodules.sh    |  2 +-
 t/t5531-deep-submodule-push.sh |  2 +-
 t/t5543-atomic-push.sh         |  2 +-
 t/t5601-clone.sh               |  2 +-
 t/t5605-clone-local.sh         |  2 +-
 t/t5801-remote-helpers.sh      |  2 +-
 14 files changed, 24 insertions(+), 24 deletions(-)

diff --git a/t/t5300-pack-object.sh b/t/t5300-pack-object.sh
index 2336d09dcc..6c620cd540 100755
--- a/t/t5300-pack-object.sh
+++ b/t/t5300-pack-object.sh
@@ -191,7 +191,7 @@ test_expect_success 'survive missing objects/pack directory' '
 		mkdir missing-pack &&
 		cd missing-pack &&
 		git init &&
-		GOP=.git/objects/pack
+		GOP=.git/objects/pack &&
 		rm -fr $GOP &&
 		git index-pack --stdin --keep=test <../test-3-${packname_3}.pack &&
 		test -f $GOP/pack-${packname_3}.pack &&
diff --git a/t/t5302-pack-index.sh b/t/t5302-pack-index.sh
index bb9b8bb309..91d51b35f9 100755
--- a/t/t5302-pack-index.sh
+++ b/t/t5302-pack-index.sh
@@ -237,7 +237,7 @@ test_expect_success 'running index-pack in the object store' '
     rm -f .git/objects/pack/* &&
     cp test-1-${pack1}.pack .git/objects/pack/pack-${pack1}.pack &&
     (
-	cd .git/objects/pack
+	cd .git/objects/pack &&
 	git index-pack pack-${pack1}.pack
     ) &&
     test -f .git/objects/pack/pack-${pack1}.idx
diff --git a/t/t5401-update-hooks.sh b/t/t5401-update-hooks.sh
index 7f278d8ce9..b5f886a0e2 100755
--- a/t/t5401-update-hooks.sh
+++ b/t/t5401-update-hooks.sh
@@ -82,13 +82,13 @@ test_expect_success 'hooks ran' '
 '
 
 test_expect_success 'pre-receive hook input' '
-	(echo $commit0 $commit1 refs/heads/master;
+	(echo $commit0 $commit1 refs/heads/master &&
 	 echo $commit1 $commit0 refs/heads/tofail
 	) | test_cmp - victim.git/pre-receive.stdin
 '
 
 test_expect_success 'update hook arguments' '
-	(echo refs/heads/master $commit0 $commit1;
+	(echo refs/heads/master $commit0 $commit1 &&
 	 echo refs/heads/tofail $commit1 $commit0
 	) | test_cmp - victim.git/update.args
 '
diff --git a/t/t5500-fetch-pack.sh b/t/t5500-fetch-pack.sh
index ea6570e819..ebbbbfe054 100755
--- a/t/t5500-fetch-pack.sh
+++ b/t/t5500-fetch-pack.sh
@@ -259,7 +259,7 @@ test_expect_success 'clone shallow object count' '
 test_expect_success 'pull in shallow repo with missing merge base' '
 	(
 		cd shallow &&
-		git fetch --depth 4 .. A
+		git fetch --depth 4 .. A &&
 		test_must_fail git merge --allow-unrelated-histories FETCH_HEAD
 	)
 '
diff --git a/t/t5505-remote.sh b/t/t5505-remote.sh
index 3552b51b4c..11e14a1e0f 100755
--- a/t/t5505-remote.sh
+++ b/t/t5505-remote.sh
@@ -844,7 +844,7 @@ test_expect_success 'migrate a remote from named file in $GIT_DIR/branches (2)'
 		git remote rename origin origin &&
 		test_path_is_missing .git/branches/origin &&
 		test "$(git config remote.origin.url)" = "quux" &&
-		test "$(git config remote.origin.fetch)" = "refs/heads/foom:refs/heads/origin"
+		test "$(git config remote.origin.fetch)" = "refs/heads/foom:refs/heads/origin" &&
 		test "$(git config remote.origin.push)" = "HEAD:refs/heads/foom"
 	)
 '
diff --git a/t/t5512-ls-remote.sh b/t/t5512-ls-remote.sh
index 6a949484d0..ea020040e8 100755
--- a/t/t5512-ls-remote.sh
+++ b/t/t5512-ls-remote.sh
@@ -15,7 +15,7 @@ test_expect_success setup '
 	git tag mark1.10 &&
 	git show-ref --tags -d | sed -e "s/ /	/" >expected.tag &&
 	(
-		echo "$(git rev-parse HEAD)	HEAD"
+		echo "$(git rev-parse HEAD)	HEAD" &&
 		git show-ref -d	| sed -e "s/ /	/"
 	) >expected.all &&
 
@@ -105,7 +105,7 @@ test_expect_success 'use branch.<name>.remote if possible' '
 	git clone . other.git &&
 	(
 		cd other.git &&
-		echo "$(git rev-parse HEAD)	HEAD"
+		echo "$(git rev-parse HEAD)	HEAD" &&
 		git show-ref	| sed -e "s/ /	/"
 	) >exp &&
 
diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
index a5077d8b7c..bd8f23e430 100755
--- a/t/t5516-fetch-push.sh
+++ b/t/t5516-fetch-push.sh
@@ -923,7 +923,7 @@ test_expect_success 'push into aliased refs (consistent)' '
 	(
 		cd child1 &&
 		git branch foo &&
-		git symbolic-ref refs/heads/bar refs/heads/foo
+		git symbolic-ref refs/heads/bar refs/heads/foo &&
 		git config receive.denyCurrentBranch false
 	) &&
 	(
@@ -945,7 +945,7 @@ test_expect_success 'push into aliased refs (inconsistent)' '
 	(
 		cd child1 &&
 		git branch foo &&
-		git symbolic-ref refs/heads/bar refs/heads/foo
+		git symbolic-ref refs/heads/bar refs/heads/foo &&
 		git config receive.denyCurrentBranch false
 	) &&
 	(
@@ -1011,7 +1011,7 @@ test_expect_success 'push --porcelain rejected' '
 	mk_empty testrepo &&
 	git push testrepo refs/heads/master:refs/remotes/origin/master &&
 	(cd testrepo &&
-		git reset --hard origin/master^
+		git reset --hard origin/master^ &&
 		git config receive.denyCurrentBranch true) &&
 
 	echo >.git/foo  "To testrepo"  &&
@@ -1025,7 +1025,7 @@ test_expect_success 'push --porcelain --dry-run rejected' '
 	mk_empty testrepo &&
 	git push testrepo refs/heads/master:refs/remotes/origin/master &&
 	(cd testrepo &&
-		git reset --hard origin/master
+		git reset --hard origin/master &&
 		git config receive.denyCurrentBranch true) &&
 
 	echo >.git/foo  "To testrepo"  &&
@@ -1333,7 +1333,7 @@ test_expect_success 'push --follow-tag only pushes relevant tags' '
 		git commit --allow-empty -m "future commit" &&
 		git tag -m "future" future &&
 		git checkout master &&
-		git for-each-ref refs/heads/master refs/tags/tag >../expect
+		git for-each-ref refs/heads/master refs/tags/tag >../expect &&
 		git push --follow-tag ../dst master
 	) &&
 	(
diff --git a/t/t5517-push-mirror.sh b/t/t5517-push-mirror.sh
index 02f160aae0..c05a661400 100755
--- a/t/t5517-push-mirror.sh
+++ b/t/t5517-push-mirror.sh
@@ -71,7 +71,7 @@ test_expect_success 'push mirror force updates existing branches' '
 		git push --mirror up &&
 		echo two >foo && git add foo && git commit -m two &&
 		git push --mirror up &&
-		git reset --hard HEAD^
+		git reset --hard HEAD^ &&
 		git push --mirror up
 	) &&
 	master_master=$(cd master && git show-ref -s --verify refs/heads/master) &&
@@ -88,7 +88,7 @@ test_expect_success 'push mirror removes branches' '
 		echo one >foo && git add foo && git commit -m one &&
 		git branch remove master &&
 		git push --mirror up &&
-		git branch -D remove
+		git branch -D remove &&
 		git push --mirror up
 	) &&
 	(
@@ -170,7 +170,7 @@ test_expect_success 'push mirror force updates existing tags' '
 		echo two >foo && git add foo && git commit -m two &&
 		git tag -f tmaster master &&
 		git push --mirror up &&
-		git reset --hard HEAD^
+		git reset --hard HEAD^ &&
 		git tag -f tmaster master &&
 		git push --mirror up
 	) &&
@@ -188,7 +188,7 @@ test_expect_success 'push mirror removes tags' '
 		echo one >foo && git add foo && git commit -m one &&
 		git tag -f tremove master &&
 		git push --mirror up &&
-		git tag -d tremove
+		git tag -d tremove &&
 		git push --mirror up
 	) &&
 	(
@@ -235,7 +235,7 @@ test_expect_success 'remote.foo.mirror adds and removes branches' '
 		git branch keep master &&
 		git branch remove master &&
 		git push up &&
-		git branch -D remove
+		git branch -D remove &&
 		git push up
 	) &&
 	(
diff --git a/t/t5526-fetch-submodules.sh b/t/t5526-fetch-submodules.sh
index 359e03ff83..0f730d7781 100755
--- a/t/t5526-fetch-submodules.sh
+++ b/t/t5526-fetch-submodules.sh
@@ -379,7 +379,7 @@ test_expect_success "'--recurse-submodules=on-demand' recurses as deep as necess
 			git config -f .gitmodules submodule.subdir/deepsubmodule.fetchRecursive false
 		) &&
 		git fetch --recurse-submodules=on-demand >../actual.out 2>../actual.err &&
-		git config --unset fetch.recurseSubmodules
+		git config --unset fetch.recurseSubmodules &&
 		(
 			cd submodule &&
 			git config --unset -f .gitmodules submodule.subdir/deepsubmodule.fetchRecursive
diff --git a/t/t5531-deep-submodule-push.sh b/t/t5531-deep-submodule-push.sh
index 39cb2c1c34..e2c37fd978 100755
--- a/t/t5531-deep-submodule-push.sh
+++ b/t/t5531-deep-submodule-push.sh
@@ -354,7 +354,7 @@ test_expect_success 'push succeeds if submodule has no remote and is on the firs
 	git clone a a1 &&
 	(
 		cd a1 &&
-		git init b
+		git init b &&
 		(
 			cd b &&
 			>junk &&
diff --git a/t/t5543-atomic-push.sh b/t/t5543-atomic-push.sh
index 3480b33007..7079bcf9a0 100755
--- a/t/t5543-atomic-push.sh
+++ b/t/t5543-atomic-push.sh
@@ -178,7 +178,7 @@ test_expect_success 'atomic push obeys update hook preventing a branch to be pus
 test_expect_success 'atomic push is not advertised if configured' '
 	mk_repo_pair &&
 	(
-		cd upstream
+		cd upstream &&
 		git config receive.advertiseatomic 0
 	) &&
 	(
diff --git a/t/t5601-clone.sh b/t/t5601-clone.sh
index 0b62037744..ddaa96ac4f 100755
--- a/t/t5601-clone.sh
+++ b/t/t5601-clone.sh
@@ -618,7 +618,7 @@ hex2oct () {
 test_expect_success 'clone on case-insensitive fs' '
 	git init icasefs &&
 	(
-		cd icasefs
+		cd icasefs &&
 		o=$(git hash-object -w --stdin </dev/null | hex2oct) &&
 		t=$(printf "100644 X\0${o}100644 x\0${o}" |
 			git hash-object -w -t tree --stdin) &&
diff --git a/t/t5605-clone-local.sh b/t/t5605-clone-local.sh
index 3c087e907c..af23419ebf 100755
--- a/t/t5605-clone-local.sh
+++ b/t/t5605-clone-local.sh
@@ -94,7 +94,7 @@ test_expect_success 'clone empty repository' '
 	 git config receive.denyCurrentBranch warn) &&
 	git clone empty empty-clone &&
 	test_tick &&
-	(cd empty-clone
+	(cd empty-clone &&
 	 echo "content" >> foo &&
 	 git add foo &&
 	 git commit -m "Initial commit" &&
diff --git a/t/t5801-remote-helpers.sh b/t/t5801-remote-helpers.sh
index 362b1581e0..ee5757966f 100755
--- a/t/t5801-remote-helpers.sh
+++ b/t/t5801-remote-helpers.sh
@@ -96,7 +96,7 @@ test_expect_success 'push new branch with old:new refspec' '
 
 test_expect_success 'push new branch with HEAD:new refspec' '
 	(cd local &&
-	 git checkout new-name
+	 git checkout new-name &&
 	 git push origin HEAD:new-refspec-2
 	) &&
 	compare_refs local HEAD server refs/heads/new-refspec-2
-- 
2.18.0.203.gfac676dfb9


^ permalink raw reply related	[flat|nested] 52+ messages in thread

* [PATCH 22/25] t6000-t6999: fix broken &&-chains
  2018-07-02  0:23 [PATCH 00/25] fix buggy tests, modernize tests, fix broken &&-chains Eric Sunshine
                   ` (20 preceding siblings ...)
  2018-07-02  0:24 ` [PATCH 21/25] t5000-t5999: " Eric Sunshine
@ 2018-07-02  0:24 ` Eric Sunshine
  2018-07-02  0:24 ` [PATCH 23/25] t7000-t7999: " Eric Sunshine
                   ` (4 subsequent siblings)
  26 siblings, 0 replies; 52+ messages in thread
From: Eric Sunshine @ 2018-07-02  0:24 UTC (permalink / raw)
  To: git
  Cc: Elijah Newren, Johannes Sixt, Jonathan Nieder, Jonathan Tan,
	Stefan Beller, Junio C Hamano, Luke Diamand, Jeff King,
	Eric Sunshine

Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
---
 t/t6010-merge-base.sh                |  2 +-
 t/t6029-merge-subtree.sh             | 16 ++++++++--------
 t/t6036-recursive-corner-cases.sh    |  6 +++---
 t/t6042-merge-rename-corner-cases.sh |  8 ++++----
 t/t6043-merge-rename-directories.sh  |  2 +-
 5 files changed, 17 insertions(+), 17 deletions(-)

diff --git a/t/t6010-merge-base.sh b/t/t6010-merge-base.sh
index aa2d360ce3..44c726ea39 100755
--- a/t/t6010-merge-base.sh
+++ b/t/t6010-merge-base.sh
@@ -245,7 +245,7 @@ test_expect_success 'using reflog to find the fork point' '
 			git commit --allow-empty -m "Derived #$count" &&
 			git rev-parse HEAD >derived$count &&
 			git checkout -B base $E || exit 1
-		done
+		done &&
 
 		for count in 1 2 3
 		do
diff --git a/t/t6029-merge-subtree.sh b/t/t6029-merge-subtree.sh
index 3e692454a7..7d5bc78472 100755
--- a/t/t6029-merge-subtree.sh
+++ b/t/t6029-merge-subtree.sh
@@ -55,7 +55,7 @@ test_expect_success 'initial merge' '
 	git checkout -b work &&
 	git ls-files -s >actual &&
 	(
-		echo "100644 $o1 0	git-gui/git-gui.sh"
+		echo "100644 $o1 0	git-gui/git-gui.sh" &&
 		echo "100644 $o2 0	git.c"
 	) >expected &&
 	test_cmp expected actual
@@ -72,7 +72,7 @@ test_expect_success 'merge update' '
 	git pull -s subtree gui master2 &&
 	git ls-files -s >actual &&
 	(
-		echo "100644 $o3 0	git-gui/git-gui.sh"
+		echo "100644 $o3 0	git-gui/git-gui.sh" &&
 		echo "100644 $o2 0	git.c"
 	) >expected &&
 	test_cmp expected actual
@@ -88,8 +88,8 @@ test_expect_success 'initial ambiguous subtree' '
 	git checkout -b work2 &&
 	git ls-files -s >actual &&
 	(
-		echo "100644 $o1 0	git-gui/git-gui.sh"
-		echo "100644 $o1 0	git-gui2/git-gui.sh"
+		echo "100644 $o1 0	git-gui/git-gui.sh" &&
+		echo "100644 $o1 0	git-gui2/git-gui.sh" &&
 		echo "100644 $o2 0	git.c"
 	) >expected &&
 	test_cmp expected actual
@@ -101,8 +101,8 @@ test_expect_success 'merge using explicit' '
 	git pull -Xsubtree=git-gui gui master2 &&
 	git ls-files -s >actual &&
 	(
-		echo "100644 $o3 0	git-gui/git-gui.sh"
-		echo "100644 $o1 0	git-gui2/git-gui.sh"
+		echo "100644 $o3 0	git-gui/git-gui.sh" &&
+		echo "100644 $o1 0	git-gui2/git-gui.sh" &&
 		echo "100644 $o2 0	git.c"
 	) >expected &&
 	test_cmp expected actual
@@ -114,8 +114,8 @@ test_expect_success 'merge2 using explicit' '
 	git pull -Xsubtree=git-gui2 gui master2 &&
 	git ls-files -s >actual &&
 	(
-		echo "100644 $o1 0	git-gui/git-gui.sh"
-		echo "100644 $o3 0	git-gui2/git-gui.sh"
+		echo "100644 $o1 0	git-gui/git-gui.sh" &&
+		echo "100644 $o3 0	git-gui2/git-gui.sh" &&
 		echo "100644 $o2 0	git.c"
 	) >expected &&
 	test_cmp expected actual
diff --git a/t/t6036-recursive-corner-cases.sh b/t/t6036-recursive-corner-cases.sh
index b32ff8e1db..892cf08743 100755
--- a/t/t6036-recursive-corner-cases.sh
+++ b/t/t6036-recursive-corner-cases.sh
@@ -72,7 +72,7 @@ test_expect_success 'merge simple rename+criss-cross with no modifications' '
 		git rev-parse   >actual     \
 			:2:three   :3:three &&
 		git hash-object >>actual    \
-			three~HEAD three~R2^0
+			three~HEAD three~R2^0 &&
 		test_cmp expect actual
 	)
 '
@@ -148,7 +148,7 @@ test_expect_success 'merge criss-cross + rename merges with basic modification'
 		git rev-parse   >actual     \
 			:2:three   :3:three &&
 		git hash-object >>actual    \
-			three~HEAD three~R2^0
+			three~HEAD three~R2^0 &&
 		test_cmp expect actual
 	)
 '
@@ -228,7 +228,7 @@ test_expect_success 'git detects differently handled merges conflict' '
 			D:new_a  E:new_a &&
 		git rev-parse   >actual     \
 			:2:new_a :3:new_a &&
-		test_cmp expect actual
+		test_cmp expect actual &&
 
 		git cat-file -p B:new_a >ours &&
 		git cat-file -p C:new_a >theirs &&
diff --git a/t/t6042-merge-rename-corner-cases.sh b/t/t6042-merge-rename-corner-cases.sh
index 1cbd946fc2..661b633478 100755
--- a/t/t6042-merge-rename-corner-cases.sh
+++ b/t/t6042-merge-rename-corner-cases.sh
@@ -352,7 +352,7 @@ test_expect_success 'rename/directory conflict + content merge conflict' '
 			base:file   left-conflict:newfile  right:file &&
 		git rev-parse >actual                                 \
 			:1:newfile  :2:newfile             :3:newfile &&
-		test_cmp expect actual
+		test_cmp expect actual &&
 
 		test_path_is_file newfile/realfile &&
 		test_path_is_file newfile~HEAD
@@ -580,7 +580,7 @@ test_expect_failure 'detect conflict with rename/rename(1to2)/add-source merge'
 			C:a   A:a   B:b   C:C &&
 		git rev-parse >actual          \
 			:3:a  :1:a  :2:b  :3:c &&
-		test_cmp expect actual
+		test_cmp expect actual &&
 
 		test_path_is_file a &&
 		test_path_is_file b &&
@@ -680,13 +680,13 @@ test_expect_success 'rename/rename/add-dest merge still knows about conflicting
 			A:a   C:b   B:b   C:c   B:c &&
 		git rev-parse >actual                \
 			:1:a  :2:b  :3:b  :2:c  :3:c &&
-		test_cmp expect actual
+		test_cmp expect actual &&
 
 		git rev-parse >expect               \
 			C:c     B:c     C:b     B:b &&
 		git hash-object >actual                \
 			c~HEAD  c~B\^0  b~HEAD  b~B\^0 &&
-		test_cmp expect actual
+		test_cmp expect actual &&
 
 		test_path_is_missing b &&
 		test_path_is_missing c
diff --git a/t/t6043-merge-rename-directories.sh b/t/t6043-merge-rename-directories.sh
index 2e28f2908d..4a71f17edd 100755
--- a/t/t6043-merge-rename-directories.sh
+++ b/t/t6043-merge-rename-directories.sh
@@ -3583,7 +3583,7 @@ test_expect_success '11d-check: Avoid losing not-uptodate with rename + D/F conf
 		grep -q stuff z/c &&
 		test_seq 1 10 >expected &&
 		echo stuff >>expected &&
-		test_cmp expected z/c
+		test_cmp expected z/c &&
 
 		git ls-files -s >out &&
 		test_line_count = 4 out &&
-- 
2.18.0.203.gfac676dfb9


^ permalink raw reply related	[flat|nested] 52+ messages in thread

* [PATCH 23/25] t7000-t7999: fix broken &&-chains
  2018-07-02  0:23 [PATCH 00/25] fix buggy tests, modernize tests, fix broken &&-chains Eric Sunshine
                   ` (21 preceding siblings ...)
  2018-07-02  0:24 ` [PATCH 22/25] t6000-t6999: " Eric Sunshine
@ 2018-07-02  0:24 ` Eric Sunshine
  2018-07-02  0:24 ` [PATCH 24/25] t9000-t9999: " Eric Sunshine
                   ` (3 subsequent siblings)
  26 siblings, 0 replies; 52+ messages in thread
From: Eric Sunshine @ 2018-07-02  0:24 UTC (permalink / raw)
  To: git
  Cc: Elijah Newren, Johannes Sixt, Jonathan Nieder, Jonathan Tan,
	Stefan Beller, Junio C Hamano, Luke Diamand, Jeff King,
	Eric Sunshine

Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
---
 t/t7001-mv.sh                  |  2 +-
 t/t7201-co.sh                  | 40 +++++++++++++-------------
 t/t7400-submodule-basic.sh     |  2 +-
 t/t7406-submodule-update.sh    |  6 ++--
 t/t7408-submodule-reference.sh |  2 +-
 t/t7501-commit.sh              | 52 +++++++++++++++++-----------------
 t/t7506-status-submodule.sh    | 10 +++----
 7 files changed, 57 insertions(+), 57 deletions(-)

diff --git a/t/t7001-mv.sh b/t/t7001-mv.sh
index cc3fd2baf2..9e59e5a5dd 100755
--- a/t/t7001-mv.sh
+++ b/t/t7001-mv.sh
@@ -509,7 +509,7 @@ test_expect_success 'moving nested submodules' '
 		touch nested_level1 &&
 		git init &&
 		git add . &&
-		git commit -m "nested level 1"
+		git commit -m "nested level 1" &&
 		git submodule add ../sub_nested_nested &&
 		git commit -m "add nested level 2"
 	) &&
diff --git a/t/t7201-co.sh b/t/t7201-co.sh
index 8d8a63a24b..94cb039a03 100755
--- a/t/t7201-co.sh
+++ b/t/t7201-co.sh
@@ -528,10 +528,10 @@ test_expect_success 'checkout with --merge' '
 	cat sample >filf &&
 	git checkout -m -- fild file filf &&
 	(
-		echo "<<<<<<< ours"
-		echo ourside
-		echo "======="
-		echo theirside
+		echo "<<<<<<< ours" &&
+		echo ourside &&
+		echo "=======" &&
+		echo theirside &&
 		echo ">>>>>>> theirs"
 	) >merged &&
 	test_cmp expect fild &&
@@ -549,12 +549,12 @@ test_expect_success 'checkout with --merge, in diff3 -m style' '
 	cat sample >filf &&
 	git checkout -m -- fild file filf &&
 	(
-		echo "<<<<<<< ours"
-		echo ourside
-		echo "||||||| base"
-		echo original
-		echo "======="
-		echo theirside
+		echo "<<<<<<< ours" &&
+		echo ourside &&
+		echo "||||||| base" &&
+		echo original &&
+		echo "=======" &&
+		echo theirside &&
 		echo ">>>>>>> theirs"
 	) >merged &&
 	test_cmp expect fild &&
@@ -572,10 +572,10 @@ test_expect_success 'checkout --conflict=merge, overriding config' '
 	cat sample >filf &&
 	git checkout --conflict=merge -- fild file filf &&
 	(
-		echo "<<<<<<< ours"
-		echo ourside
-		echo "======="
-		echo theirside
+		echo "<<<<<<< ours" &&
+		echo ourside &&
+		echo "=======" &&
+		echo theirside &&
 		echo ">>>>>>> theirs"
 	) >merged &&
 	test_cmp expect fild &&
@@ -593,12 +593,12 @@ test_expect_success 'checkout --conflict=diff3' '
 	cat sample >filf &&
 	git checkout --conflict=diff3 -- fild file filf &&
 	(
-		echo "<<<<<<< ours"
-		echo ourside
-		echo "||||||| base"
-		echo original
-		echo "======="
-		echo theirside
+		echo "<<<<<<< ours" &&
+		echo ourside &&
+		echo "||||||| base" &&
+		echo original &&
+		echo "=======" &&
+		echo theirside &&
 		echo ">>>>>>> theirs"
 	) >merged &&
 	test_cmp expect fild &&
diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
index 401adaed32..76cf522a08 100755
--- a/t/t7400-submodule-basic.sh
+++ b/t/t7400-submodule-basic.sh
@@ -818,7 +818,7 @@ test_expect_success '../bar/a/b/c works with relative local path - ../foo/bar.gi
 		cp pristine-.git-config .git/config &&
 		cp pristine-.gitmodules .gitmodules &&
 		mkdir -p a/b/c &&
-		(cd a/b/c; git init) &&
+		(cd a/b/c && git init) &&
 		git config remote.origin.url ../foo/bar.git &&
 		git submodule add ../bar/a/b/c ./a/b/c &&
 		git submodule init &&
diff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh
index 9e0d31700e..f604ef7a72 100755
--- a/t/t7406-submodule-update.sh
+++ b/t/t7406-submodule-update.sh
@@ -865,9 +865,9 @@ test_expect_success 'submodule update places git-dir in superprojects git-dir re
 	 (cd submodule/subsubmodule &&
 	  git log > ../../expected
 	 ) &&
-	 (cd .git/modules/submodule/modules/subsubmodule
+	 (cd .git/modules/submodule/modules/subsubmodule &&
 	  git log > ../../../../../actual
-	 )
+	 ) &&
 	 test_cmp actual expected
 	)
 '
@@ -886,7 +886,7 @@ test_expect_success 'submodule update properly revives a moved submodule' '
 	 git commit -am "pre move" &&
 	 H2=$(git rev-parse --short HEAD) &&
 	 git status | sed "s/$H/XXX/" >expect &&
-	 H=$(cd submodule2; git rev-parse HEAD) &&
+	 H=$(cd submodule2 && git rev-parse HEAD) &&
 	 git rm --cached submodule2 &&
 	 rm -rf submodule2 &&
 	 mkdir -p "moved/sub module" &&
diff --git a/t/t7408-submodule-reference.sh b/t/t7408-submodule-reference.sh
index 08d9add05e..34ac28c056 100755
--- a/t/t7408-submodule-reference.sh
+++ b/t/t7408-submodule-reference.sh
@@ -148,7 +148,7 @@ test_expect_success 'preparing second superproject with a nested submodule plus
 		cd supersuper &&
 		echo "I am super super." >file &&
 		git add file &&
-		git commit -m B-super-super-initial
+		git commit -m B-super-super-initial &&
 		git submodule add "file://$base_dir/super" subwithsub &&
 		git commit -m B-super-super-added &&
 		git submodule update --init --recursive &&
diff --git a/t/t7501-commit.sh b/t/t7501-commit.sh
index 282ff42331..51646d8019 100755
--- a/t/t7501-commit.sh
+++ b/t/t7501-commit.sh
@@ -411,8 +411,8 @@ test_expect_success 'sign off (1)' '
 	git commit -s -m "thank you" &&
 	git cat-file commit HEAD | sed -e "1,/^\$/d" >actual &&
 	(
-		echo thank you
-		echo
+		echo thank you &&
+		echo &&
 		git var GIT_COMMITTER_IDENT |
 		sed -e "s/>.*/>/" -e "s/^/Signed-off-by: /"
 	) >expected &&
@@ -430,9 +430,9 @@ test_expect_success 'sign off (2)' '
 $existing" &&
 	git cat-file commit HEAD | sed -e "1,/^\$/d" >actual &&
 	(
-		echo thank you
-		echo
-		echo $existing
+		echo thank you &&
+		echo &&
+		echo $existing &&
 		git var GIT_COMMITTER_IDENT |
 		sed -e "s/>.*/>/" -e "s/^/Signed-off-by: /"
 	) >expected &&
@@ -450,9 +450,9 @@ test_expect_success 'signoff gap' '
 $alt" &&
 	git cat-file commit HEAD | sed -e "1,/^\$/d" > actual &&
 	(
-		echo welcome
-		echo
-		echo $alt
+		echo welcome &&
+		echo &&
+		echo $alt &&
 		git var GIT_COMMITTER_IDENT |
 		sed -e "s/>.*/>/" -e "s/^/Signed-off-by: /"
 	) >expected &&
@@ -470,11 +470,11 @@ We have now
 $alt" &&
 	git cat-file commit HEAD | sed -e "1,/^\$/d" > actual &&
 	(
-		echo welcome
-		echo
-		echo We have now
-		echo $alt
-		echo
+		echo welcome &&
+		echo &&
+		echo We have now &&
+		echo $alt &&
+		echo &&
 		git var GIT_COMMITTER_IDENT |
 		sed -e "s/>.*/>/" -e "s/^/Signed-off-by: /"
 	) >expected &&
@@ -491,11 +491,11 @@ non-trailer line
 Myfooter: x" &&
 	git cat-file commit HEAD | sed -e "1,/^\$/d" > actual &&
 	(
-		echo subject
-		echo
-		echo non-trailer line
-		echo Myfooter: x
-		echo
+		echo subject &&
+		echo &&
+		echo non-trailer line &&
+		echo Myfooter: x &&
+		echo &&
 		echo "Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>"
 	) >expected &&
 	test_cmp expected actual &&
@@ -508,10 +508,10 @@ non-trailer line
 Myfooter: x" &&
 	git cat-file commit HEAD | sed -e "1,/^\$/d" > actual &&
 	(
-		echo subject
-		echo
-		echo non-trailer line
-		echo Myfooter: x
+		echo subject &&
+		echo &&
+		echo non-trailer line &&
+		echo Myfooter: x &&
 		echo "Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>"
 	) >expected &&
 	test_cmp expected actual
@@ -524,10 +524,10 @@ test_expect_success 'multiple -m' '
 	git commit -m "one" -m "two" -m "three" &&
 	git cat-file commit HEAD | sed -e "1,/^\$/d" >actual &&
 	(
-		echo one
-		echo
-		echo two
-		echo
+		echo one &&
+		echo &&
+		echo two &&
+		echo &&
 		echo three
 	) >expected &&
 	test_cmp expected actual
diff --git a/t/t7506-status-submodule.sh b/t/t7506-status-submodule.sh
index b4b74dbe29..943708fb04 100755
--- a/t/t7506-status-submodule.sh
+++ b/t/t7506-status-submodule.sh
@@ -193,9 +193,9 @@ test_expect_success 'status with added and untracked file in modified submodule
 
 test_expect_success 'setup .git file for sub' '
 	(cd sub &&
-	 rm -f new-file
+	 rm -f new-file &&
 	 REAL="$(pwd)/../.real" &&
-	 mv .git "$REAL"
+	 mv .git "$REAL" &&
 	 echo "gitdir: $REAL" >.git) &&
 	 echo .real >>.gitignore &&
 	 git commit -m "added .real to .gitignore" .gitignore
@@ -209,12 +209,12 @@ test_expect_success 'status with added file in modified submodule with .git file
 
 test_expect_success 'status with a lot of untracked files in the submodule' '
 	(
-		cd sub
+		cd sub &&
 		i=0 &&
 		while test $i -lt 1024
 		do
-			>some-file-$i
-			i=$(( $i + 1 ))
+			>some-file-$i &&
+			i=$(( $i + 1 )) || exit 1
 		done
 	) &&
 	git status --porcelain sub 2>err.actual &&
-- 
2.18.0.203.gfac676dfb9


^ permalink raw reply related	[flat|nested] 52+ messages in thread

* [PATCH 24/25] t9000-t9999: fix broken &&-chains
  2018-07-02  0:23 [PATCH 00/25] fix buggy tests, modernize tests, fix broken &&-chains Eric Sunshine
                   ` (22 preceding siblings ...)
  2018-07-02  0:24 ` [PATCH 23/25] t7000-t7999: " Eric Sunshine
@ 2018-07-02  0:24 ` Eric Sunshine
  2018-07-02  0:24 ` [PATCH 25/25] t9119: " Eric Sunshine
                   ` (2 subsequent siblings)
  26 siblings, 0 replies; 52+ messages in thread
From: Eric Sunshine @ 2018-07-02  0:24 UTC (permalink / raw)
  To: git
  Cc: Elijah Newren, Johannes Sixt, Jonathan Nieder, Jonathan Tan,
	Stefan Beller, Junio C Hamano, Luke Diamand, Jeff King,
	Eric Sunshine

Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
---
 t/t9001-send-email.sh                         |  6 +++---
 t/t9100-git-svn-basic.sh                      |  2 +-
 t/t9101-git-svn-props.sh                      |  2 +-
 t/t9122-git-svn-author.sh                     |  6 +++---
 t/t9129-git-svn-i18n-commitencoding.sh        |  2 +-
 t/t9130-git-svn-authors-file.sh               |  4 ++--
 t/t9134-git-svn-ignore-paths.sh               |  6 +++---
 t/t9137-git-svn-dcommit-clobber-series.sh     |  2 +-
 t/t9138-git-svn-authors-prog.sh               |  6 +++---
 t/t9146-git-svn-empty-dirs.sh                 | 20 +++++++++----------
 t/t9147-git-svn-include-paths.sh              |  6 +++---
 t/t9152-svn-empty-dirs-after-gc.sh            |  2 +-
 t/t9164-git-svn-dcommit-concurrent.sh         |  2 +-
 ...65-git-svn-fetch-merge-branch-of-branch.sh |  2 +-
 t/t9200-git-cvsexportcommit.sh                |  6 +++---
 t/t9302-fast-import-unpack-limit.sh           |  2 +-
 t/t9400-git-cvsserver-server.sh               |  6 +++---
 t/t9600-cvsimport.sh                          |  2 +-
 t/t9806-git-p4-options.sh                     |  2 +-
 t/t9810-git-p4-rcs.sh                         |  2 +-
 t/t9811-git-p4-label-import.sh                |  2 +-
 t/t9815-git-p4-submit-fail.sh                 |  2 +-
 t/t9830-git-p4-symlink-dir.sh                 |  2 +-
 t/t9831-git-p4-triggers.sh                    |  2 +-
 t/t9902-completion.sh                         |  4 ++--
 t/t9903-bash-prompt.sh                        |  2 +-
 26 files changed, 51 insertions(+), 51 deletions(-)

diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index 776769fe0d..53314ff54e 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -330,7 +330,7 @@ test_expect_success $PREREQ 'Show all headers' '
 
 test_expect_success $PREREQ 'Prompting works' '
 	clean_fake_sendmail &&
-	(echo "to@example.com"
+	(echo "to@example.com" &&
 	 echo ""
 	) | GIT_SEND_EMAIL_NOTTY=1 git send-email \
 		--smtp-server="$(pwd)/fake.sendmail" \
@@ -470,8 +470,8 @@ test_expect_success $PREREQ 'Invalid In-Reply-To' '
 
 test_expect_success $PREREQ 'Valid In-Reply-To when prompting' '
 	clean_fake_sendmail &&
-	(echo "From Example <from@example.com>"
-	 echo "To Example <to@example.com>"
+	(echo "From Example <from@example.com>" &&
+	 echo "To Example <to@example.com>" &&
 	 echo ""
 	) | GIT_SEND_EMAIL_NOTTY=1 git send-email \
 		--smtp-server="$(pwd)/fake.sendmail" \
diff --git a/t/t9100-git-svn-basic.sh b/t/t9100-git-svn-basic.sh
index c937330a5f..9af6078844 100755
--- a/t/t9100-git-svn-basic.sh
+++ b/t/t9100-git-svn-basic.sh
@@ -31,7 +31,7 @@ test_expect_success \
 	(
 		cd import &&
 		echo foo >foo &&
-		ln -s foo foo.link
+		ln -s foo foo.link &&
 		mkdir -p dir/a/b/c/d/e &&
 		echo "deep dir" >dir/a/b/c/d/e/file &&
 		mkdir bar &&
diff --git a/t/t9101-git-svn-props.sh b/t/t9101-git-svn-props.sh
index 07bfb63777..8a5c8dc1aa 100755
--- a/t/t9101-git-svn-props.sh
+++ b/t/t9101-git-svn-props.sh
@@ -149,7 +149,7 @@ test_expect_success 'test show-ignore' "
 		svn_cmd up &&
 		svn_cmd propset -R svn:ignore '
 no-such-file*
-' .
+' . &&
 		svn_cmd commit -m 'propset svn:ignore'
 	) &&
 	git svn show-ignore > show-ignore.got &&
diff --git a/t/t9122-git-svn-author.sh b/t/t9122-git-svn-author.sh
index 30013b7bb9..9e8fe38e7e 100755
--- a/t/t9122-git-svn-author.sh
+++ b/t/t9122-git-svn-author.sh
@@ -7,8 +7,8 @@ test_expect_success 'setup svn repository' '
 	svn_cmd checkout "$svnrepo" work.svn &&
 	(
 		cd work.svn &&
-		echo >file
-		svn_cmd add file
+		echo >file &&
+		svn_cmd add file &&
 		svn_cmd commit -m "first commit" file
 	)
 '
@@ -17,7 +17,7 @@ test_expect_success 'interact with it via git svn' '
 	mkdir work.git &&
 	(
 		cd work.git &&
-		git svn init "$svnrepo"
+		git svn init "$svnrepo" &&
 		git svn fetch &&
 
 		echo modification >file &&
diff --git a/t/t9129-git-svn-i18n-commitencoding.sh b/t/t9129-git-svn-i18n-commitencoding.sh
index 8dbd6476fa..2c213ae654 100755
--- a/t/t9129-git-svn-i18n-commitencoding.sh
+++ b/t/t9129-git-svn-i18n-commitencoding.sh
@@ -51,7 +51,7 @@ do
 		git add F &&
 		git commit -a -F "$TEST_DIRECTORY"/t3900/$H.txt &&
 		E=$(git cat-file commit HEAD | sed -ne "s/^encoding //p") &&
-		test "z$E" = "z$H"
+		test "z$E" = "z$H" &&
 		compare_git_head_with "$TEST_DIRECTORY"/t3900/$H.txt
 	)
 	'
diff --git a/t/t9130-git-svn-authors-file.sh b/t/t9130-git-svn-authors-file.sh
index d8262854bb..cb764bcadc 100755
--- a/t/t9130-git-svn-authors-file.sh
+++ b/t/t9130-git-svn-authors-file.sh
@@ -25,7 +25,7 @@ test_expect_success 'start import with incomplete authors file' '
 
 test_expect_success 'imported 2 revisions successfully' '
 	(
-		cd x
+		cd x &&
 		git rev-list refs/remotes/git-svn >actual &&
 		test_line_count = 2 actual &&
 		git rev-list -1 --pretty=raw refs/remotes/git-svn >actual &&
@@ -42,7 +42,7 @@ EOF
 
 test_expect_success 'continues to import once authors have been added' '
 	(
-		cd x
+		cd x &&
 		git svn fetch --authors-file=../svn-authors &&
 		git rev-list refs/remotes/git-svn >actual &&
 		test_line_count = 4 actual &&
diff --git a/t/t9134-git-svn-ignore-paths.sh b/t/t9134-git-svn-ignore-paths.sh
index 09ff10cd9b..fff49c4100 100755
--- a/t/t9134-git-svn-ignore-paths.sh
+++ b/t/t9134-git-svn-ignore-paths.sh
@@ -82,7 +82,7 @@ test_expect_success 'update git svn-cloned repo (option ignore)' '
 test_expect_success 'SVN-side change inside of ignored www' '
 	(
 		cd s &&
-		echo zaq >> www/test_www.txt
+		echo zaq >> www/test_www.txt &&
 		svn_cmd commit -m "SVN-side change inside of www/test_www.txt" &&
 		svn_cmd up &&
 		svn_cmd log -v | fgrep "SVN-side change inside of www/test_www.txt"
@@ -114,8 +114,8 @@ test_expect_success 'update git svn-cloned repo (option ignore)' '
 test_expect_success 'SVN-side change in and out of ignored www' '
 	(
 		cd s &&
-		echo cvf >> www/test_www.txt
-		echo ygg >> qqq/test_qqq.txt
+		echo cvf >> www/test_www.txt &&
+		echo ygg >> qqq/test_qqq.txt &&
 		svn_cmd commit -m "SVN-side change in and out of ignored www" &&
 		svn_cmd up &&
 		svn_cmd log -v | fgrep "SVN-side change in and out of ignored www"
diff --git a/t/t9137-git-svn-dcommit-clobber-series.sh b/t/t9137-git-svn-dcommit-clobber-series.sh
index 5fa07a369f..067b15bad2 100755
--- a/t/t9137-git-svn-dcommit-clobber-series.sh
+++ b/t/t9137-git-svn-dcommit-clobber-series.sh
@@ -7,7 +7,7 @@ test_description='git svn dcommit clobber series'
 test_expect_success 'initialize repo' '
 	mkdir import &&
 	(cd import &&
-	awk "BEGIN { for (i = 1; i < 64; i++) { print i } }" > file
+	awk "BEGIN { for (i = 1; i < 64; i++) { print i } }" > file &&
 	svn_cmd import -m "initial" . "$svnrepo"
 	) &&
 	git svn init "$svnrepo" &&
diff --git a/t/t9138-git-svn-authors-prog.sh b/t/t9138-git-svn-authors-prog.sh
index 93ef44fae8..027b416720 100755
--- a/t/t9138-git-svn-authors-prog.sh
+++ b/t/t9138-git-svn-authors-prog.sh
@@ -38,7 +38,7 @@ test_expect_success 'import authors with prog and file' '
 
 test_expect_success 'imported 6 revisions successfully' '
 	(
-		cd x
+		cd x &&
 		git rev-list refs/remotes/git-svn >actual &&
 		test_line_count = 6 actual
 	)
@@ -46,7 +46,7 @@ test_expect_success 'imported 6 revisions successfully' '
 
 test_expect_success 'authors-prog ran correctly' '
 	(
-		cd x
+		cd x &&
 		git rev-list -1 --pretty=raw refs/remotes/git-svn~1 >actual &&
 		grep "^author ee-foo <ee-foo@example\.com> " actual &&
 		git rev-list -1 --pretty=raw refs/remotes/git-svn~2 >actual &&
@@ -62,7 +62,7 @@ test_expect_success 'authors-prog ran correctly' '
 
 test_expect_success 'authors-file overrode authors-prog' '
 	(
-		cd x
+		cd x &&
 		git rev-list -1 --pretty=raw refs/remotes/git-svn >actual &&
 		grep "^author FFFFFFF FFFFFFF <fFf@other\.example\.com> " actual
 	)
diff --git a/t/t9146-git-svn-empty-dirs.sh b/t/t9146-git-svn-empty-dirs.sh
index 6d3130e618..5f91c0d68b 100755
--- a/t/t9146-git-svn-empty-dirs.sh
+++ b/t/t9146-git-svn-empty-dirs.sh
@@ -21,7 +21,7 @@ test_expect_success 'empty directories exist' '
 		do
 			if ! test -d "$i"
 			then
-				echo >&2 "$i does not exist"
+				echo >&2 "$i does not exist" &&
 				exit 1
 			fi
 		done
@@ -38,7 +38,7 @@ test_expect_success 'option automkdirs set to false' '
 		do
 			if test -d "$i"
 			then
-				echo >&2 "$i exists"
+				echo >&2 "$i exists" &&
 				exit 1
 			fi
 		done
@@ -63,7 +63,7 @@ test_expect_success 'git svn mkdirs recreates empty directories' '
 		do
 			if ! test -d "$i"
 			then
-				echo >&2 "$i does not exist"
+				echo >&2 "$i does not exist" &&
 				exit 1
 			fi
 		done
@@ -79,21 +79,21 @@ test_expect_success 'git svn mkdirs -r works' '
 		do
 			if ! test -d "$i"
 			then
-				echo >&2 "$i does not exist"
+				echo >&2 "$i does not exist" &&
 				exit 1
 			fi
-		done
+		done &&
 
 		if test -d "! !"
 		then
-			echo >&2 "$i should not exist"
+			echo >&2 "$i should not exist" &&
 			exit 1
-		fi
+		fi &&
 
 		git svn mkdirs -r8 &&
 		if ! test -d "! !"
 		then
-			echo >&2 "$i not exist"
+			echo >&2 "$i not exist" &&
 			exit 1
 		fi
 	)
@@ -115,7 +115,7 @@ test_expect_success 'empty directories in trunk exist' '
 		do
 			if ! test -d "$i"
 			then
-				echo >&2 "$i does not exist"
+				echo >&2 "$i does not exist" &&
 				exit 1
 			fi
 		done
@@ -148,7 +148,7 @@ test_expect_success 'git svn gc-ed files work' '
 			do
 				if ! test -d "$i"
 				then
-					echo >&2 "$i does not exist"
+					echo >&2 "$i does not exist" &&
 					exit 1
 				fi
 			done
diff --git a/t/t9147-git-svn-include-paths.sh b/t/t9147-git-svn-include-paths.sh
index a90ff58629..d292bf9f55 100755
--- a/t/t9147-git-svn-include-paths.sh
+++ b/t/t9147-git-svn-include-paths.sh
@@ -84,7 +84,7 @@ test_expect_success 'update git svn-cloned repo (option include)' '
 test_expect_success 'SVN-side change inside of ignored www' '
 	(
 		cd s &&
-		echo zaq >> www/test_www.txt
+		echo zaq >> www/test_www.txt &&
 		svn_cmd commit -m "SVN-side change inside of www/test_www.txt" &&
 		svn_cmd up &&
 		svn_cmd log -v | fgrep "SVN-side change inside of www/test_www.txt"
@@ -116,8 +116,8 @@ test_expect_success 'update git svn-cloned repo (option include)' '
 test_expect_success 'SVN-side change in and out of included qqq' '
 	(
 		cd s &&
-		echo cvf >> www/test_www.txt
-		echo ygg >> qqq/test_qqq.txt
+		echo cvf >> www/test_www.txt &&
+		echo ygg >> qqq/test_qqq.txt &&
 		svn_cmd commit -m "SVN-side change in and out of ignored www" &&
 		svn_cmd up &&
 		svn_cmd log -v | fgrep "SVN-side change in and out of ignored www"
diff --git a/t/t9152-svn-empty-dirs-after-gc.sh b/t/t9152-svn-empty-dirs-after-gc.sh
index 301e779709..89f285d082 100755
--- a/t/t9152-svn-empty-dirs-after-gc.sh
+++ b/t/t9152-svn-empty-dirs-after-gc.sh
@@ -30,7 +30,7 @@ test_expect_success 'git svn mkdirs recreates empty directories after git svn gc
 		do
 			if ! test -d "$i"
 			then
-				echo >&2 "$i does not exist"
+				echo >&2 "$i does not exist" &&
 				exit 1
 			fi
 		done
diff --git a/t/t9164-git-svn-dcommit-concurrent.sh b/t/t9164-git-svn-dcommit-concurrent.sh
index d8464d4218..90346ff4e9 100755
--- a/t/t9164-git-svn-dcommit-concurrent.sh
+++ b/t/t9164-git-svn-dcommit-concurrent.sh
@@ -12,7 +12,7 @@ test_expect_success 'setup svn repository' '
 	svn_cmd checkout "$svnrepo" work.svn &&
 	(
 		cd work.svn &&
-		echo >file && echo > auto_updated_file
+		echo >file && echo > auto_updated_file &&
 		svn_cmd add file auto_updated_file &&
 		svn_cmd commit -m "initial commit"
 	) &&
diff --git a/t/t9165-git-svn-fetch-merge-branch-of-branch.sh b/t/t9165-git-svn-fetch-merge-branch-of-branch.sh
index fa3ef3b1f7..a4813c2b09 100755
--- a/t/t9165-git-svn-fetch-merge-branch-of-branch.sh
+++ b/t/t9165-git-svn-fetch-merge-branch-of-branch.sh
@@ -39,7 +39,7 @@ test_expect_success 'initialize source svn repo' '
 		svn_cmd commit -m trunk &&
 		svn_cmd switch "$svnrepo"/branches/branch2 &&
 		svn_cmd merge "$svnrepo"/trunk &&
-		svn_cmd commit -m "merge trunk"
+		svn_cmd commit -m "merge trunk" &&
 		svn_cmd switch "$svnrepo"/trunk &&
 		svn_cmd merge --reintegrate "$svnrepo"/branches/branch2 &&
 		svn_cmd commit -m "merge branch2"
diff --git a/t/t9200-git-cvsexportcommit.sh b/t/t9200-git-cvsexportcommit.sh
index 1319415ba8..cd61288aa1 100755
--- a/t/t9200-git-cvsexportcommit.sh
+++ b/t/t9200-git-cvsexportcommit.sh
@@ -187,7 +187,7 @@ test_expect_success \
       git commit -a -m "Update with spaces" &&
       id=$(git rev-list --max-count=1 HEAD) &&
       (cd "$CVSWORK" &&
-      git cvsexportcommit -c $id
+      git cvsexportcommit -c $id &&
       check_entries "G g" "with spaces.png/1.2/-kb|with spaces.txt/1.2/"
       )'
 
@@ -245,7 +245,7 @@ test_expect_success FILEMODE \
       git add G/off &&
       git commit -a -m "Execute test" &&
       (cd "$CVSWORK" &&
-      git cvsexportcommit -c HEAD
+      git cvsexportcommit -c HEAD &&
       test -x G/on &&
       ! test -x G/off
       )'
@@ -303,7 +303,7 @@ test_expect_success 're-commit a removed filename which remains in CVS attic' '
     git add attic_gremlin &&
     git commit -m "Added attic_gremlin" &&
 	git cvsexportcommit -w "$CVSWORK" -c HEAD &&
-    (cd "$CVSWORK"; cvs -Q update -d) &&
+    (cd "$CVSWORK" && cvs -Q update -d) &&
     test -f "$CVSWORK/attic_gremlin"
 '
 
diff --git a/t/t9302-fast-import-unpack-limit.sh b/t/t9302-fast-import-unpack-limit.sh
index a04de14677..bb1c39cfcc 100755
--- a/t/t9302-fast-import-unpack-limit.sh
+++ b/t/t9302-fast-import-unpack-limit.sh
@@ -80,7 +80,7 @@ test_expect_success 'lookups after checkpoint works' '
 		do
 			if test $n -gt 30
 			then
-				echo >&2 "checkpoint did not update branch"
+				echo >&2 "checkpoint did not update branch" &&
 				exit 1
 			else
 				n=$(($n + 1))
diff --git a/t/t9400-git-cvsserver-server.sh b/t/t9400-git-cvsserver-server.sh
index 6b09da79bf..a5e5dca753 100755
--- a/t/t9400-git-cvsserver-server.sh
+++ b/t/t9400-git-cvsserver-server.sh
@@ -371,7 +371,7 @@ test_expect_success 'cvs update (merge)' \
   'echo Line 0 >expected &&
    for i in 1 2 3 4 5 6 7
    do
-     echo Line $i >>merge
+     echo Line $i >>merge &&
      echo Line $i >>expected
    done &&
    echo Line 8 >>expected &&
@@ -382,7 +382,7 @@ test_expect_success 'cvs update (merge)' \
    GIT_CONFIG="$git_config" cvs -Q update &&
    test "$(echo $(grep merge CVS/Entries|cut -d/ -f2,3,5))" = "merge/1.1/" &&
    test_cmp merge ../merge &&
-   ( echo Line 0; cat merge ) >merge.tmp &&
+   ( echo Line 0 && cat merge ) >merge.tmp &&
    mv merge.tmp merge &&
    cd "$WORKDIR" &&
    echo Line 8 >>merge &&
@@ -410,7 +410,7 @@ do
 done
 
 test_expect_success 'cvs update (conflict merge)' \
-  '( echo LINE 0; cat merge ) >merge.tmp &&
+  '( echo LINE 0 && cat merge ) >merge.tmp &&
    mv merge.tmp merge &&
    git add merge &&
    git commit -q -m "Merge test (conflict)" &&
diff --git a/t/t9600-cvsimport.sh b/t/t9600-cvsimport.sh
index 804ce3850f..5dfee07d9a 100755
--- a/t/t9600-cvsimport.sh
+++ b/t/t9600-cvsimport.sh
@@ -135,7 +135,7 @@ test_expect_success PERL 'second update has correct .git/cvs-revisions' '
 
 	(cd module-git &&
 	 git log --format="o_fortuna 1.1 %H" -1 HEAD^^ &&
-	 git log --format="o_fortuna 1.2 %H" -1 HEAD^
+	 git log --format="o_fortuna 1.2 %H" -1 HEAD^ &&
 	 git log --format="tick 1.1 %H" -1 HEAD) > expected &&
 	test_cmp expected module-git/.git/cvs-revisions
 '
diff --git a/t/t9806-git-p4-options.sh b/t/t9806-git-p4-options.sh
index 1ab76c4246..3f5291b857 100755
--- a/t/t9806-git-p4-options.sh
+++ b/t/t9806-git-p4-options.sh
@@ -134,7 +134,7 @@ test_expect_success 'clone --changesfile' '
 	(
 		cd "$git" &&
 		git log --oneline p4/master >lines &&
-		test_line_count = 2 lines
+		test_line_count = 2 lines &&
 		test_path_is_file file1 &&
 		test_path_is_missing file2 &&
 		test_path_is_file file3
diff --git a/t/t9810-git-p4-rcs.sh b/t/t9810-git-p4-rcs.sh
index 8134ab439b..cc53debe19 100755
--- a/t/t9810-git-p4-rcs.sh
+++ b/t/t9810-git-p4-rcs.sh
@@ -161,7 +161,7 @@ test_expect_success 'cleanup after failure' '
 test_expect_success 'ktext expansion should not expand multi-line $File::' '
 	(
 		cd "$cli" &&
-		cat >lv.pm <<-\EOF
+		cat >lv.pm <<-\EOF &&
 		my $wanted = sub { my $f = $File::Find::name;
 				    if ( -f && $f =~ /foo/ ) {
 		EOF
diff --git a/t/t9811-git-p4-label-import.sh b/t/t9811-git-p4-label-import.sh
index decb66ba30..602b0a5d5c 100755
--- a/t/t9811-git-p4-label-import.sh
+++ b/t/t9811-git-p4-label-import.sh
@@ -133,7 +133,7 @@ test_expect_success 'export git tags to p4' '
 		p4 labels ... | grep LIGHTWEIGHT_TAG &&
 		p4 label -o GIT_TAG_1 | grep "tag created in git:xyzzy" &&
 		p4 sync ...@GIT_TAG_1 &&
-		! test -f main/f10
+		! test -f main/f10 &&
 		p4 sync ...@GIT_TAG_2 &&
 		test -f main/f10
 	)
diff --git a/t/t9815-git-p4-submit-fail.sh b/t/t9815-git-p4-submit-fail.sh
index 37b42d03a2..eaf03a6563 100755
--- a/t/t9815-git-p4-submit-fail.sh
+++ b/t/t9815-git-p4-submit-fail.sh
@@ -394,7 +394,7 @@ test_expect_success 'cleanup rename after submit cancel' '
 	(
 		cd "$cli" &&
 		test_path_is_missing text2 &&
-		p4 fstat -T action text2 2>&1 | grep "no such file"
+		p4 fstat -T action text2 2>&1 | grep "no such file" &&
 		test_path_is_file text &&
 		! p4 fstat -T action text
 	)
diff --git a/t/t9830-git-p4-symlink-dir.sh b/t/t9830-git-p4-symlink-dir.sh
index 3dc528bb1e..2ad1b0810d 100755
--- a/t/t9830-git-p4-symlink-dir.sh
+++ b/t/t9830-git-p4-symlink-dir.sh
@@ -30,7 +30,7 @@ test_expect_success 'symlinked directory' '
 	(
 		cd "$cli" &&
 		p4 sync &&
-		test -L some/sub/directory/subdir2
+		test -L some/sub/directory/subdir2 &&
 		test_path_is_file some/sub/directory/subdir2/file.t
 	)
 
diff --git a/t/t9831-git-p4-triggers.sh b/t/t9831-git-p4-triggers.sh
index bbcf14c664..be44c9751a 100755
--- a/t/t9831-git-p4-triggers.sh
+++ b/t/t9831-git-p4-triggers.sh
@@ -13,7 +13,7 @@ test_expect_success 'init depot' '
 		cd "$cli" &&
 		echo file1 >file1 &&
 		p4 add file1 &&
-		p4 submit -d "change 1"
+		p4 submit -d "change 1" &&
 		echo file2 >file2 &&
 		p4 add file2 &&
 		p4 submit -d "change 2"
diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index 3b3a7b66e4..5ff43b9cbb 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -1103,7 +1103,7 @@ test_expect_success '__git_complete_refs - remote' '
 	master-in-other Z
 	EOF
 	(
-		cur=
+		cur= &&
 		__git_complete_refs --remote=other &&
 		print_comp
 	) &&
@@ -1122,7 +1122,7 @@ test_expect_success '__git_complete_refs - track' '
 	master-in-other Z
 	EOF
 	(
-		cur=
+		cur= &&
 		__git_complete_refs --track &&
 		print_comp
 	) &&
diff --git a/t/t9903-bash-prompt.sh b/t/t9903-bash-prompt.sh
index c3b89ae783..04440685a6 100755
--- a/t/t9903-bash-prompt.sh
+++ b/t/t9903-bash-prompt.sh
@@ -529,7 +529,7 @@ test_expect_success 'prompt - bash color pc mode - branch name' '
 	printf "BEFORE: (${c_green}\${__git_ps1_branch_name}${c_clear}):AFTER\\nmaster" >expected &&
 	(
 		GIT_PS1_SHOWCOLORHINTS=y &&
-		__git_ps1 "BEFORE:" ":AFTER" >"$actual"
+		__git_ps1 "BEFORE:" ":AFTER" >"$actual" &&
 		printf "%s\\n%s" "$PS1" "${__git_ps1_branch_name}" >"$actual"
 	) &&
 	test_cmp expected "$actual"
-- 
2.18.0.203.gfac676dfb9


^ permalink raw reply related	[flat|nested] 52+ messages in thread

* [PATCH 25/25] t9119: fix broken &&-chains
  2018-07-02  0:23 [PATCH 00/25] fix buggy tests, modernize tests, fix broken &&-chains Eric Sunshine
                   ` (23 preceding siblings ...)
  2018-07-02  0:24 ` [PATCH 24/25] t9000-t9999: " Eric Sunshine
@ 2018-07-02  0:24 ` Eric Sunshine
  2018-07-02 18:27 ` [PATCH 00/25] fix buggy tests, modernize tests, " Stefan Beller
  2018-07-03 19:40 ` Junio C Hamano
  26 siblings, 0 replies; 52+ messages in thread
From: Eric Sunshine @ 2018-07-02  0:24 UTC (permalink / raw)
  To: git
  Cc: Elijah Newren, Johannes Sixt, Jonathan Nieder, Jonathan Tan,
	Stefan Beller, Junio C Hamano, Luke Diamand, Jeff King,
	Eric Sunshine

Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
---
 t/t9119-git-svn-info.sh | 120 ++++++++++++++++++++--------------------
 1 file changed, 60 insertions(+), 60 deletions(-)

diff --git a/t/t9119-git-svn-info.sh b/t/t9119-git-svn-info.sh
index 88241baee3..8201c3e808 100755
--- a/t/t9119-git-svn-info.sh
+++ b/t/t9119-git-svn-info.sh
@@ -22,8 +22,8 @@ esac
 # same value as "svn info" (i.e. the commit timestamp that touched the
 # path most recently); do not expect that field to match.
 test_cmp_info () {
-	sed -e '/^Text Last Updated:/d' "$1" >tmp.expect
-	sed -e '/^Text Last Updated:/d' "$2" >tmp.actual
+	sed -e '/^Text Last Updated:/d' "$1" >tmp.expect &&
+	sed -e '/^Text Last Updated:/d' "$2" >tmp.actual &&
 	test_cmp tmp.expect tmp.actual &&
 	rm -f tmp.expect tmp.actual
 }
@@ -59,24 +59,24 @@ test_expect_success 'setup repository and import' '
 	'
 
 test_expect_success 'info' "
-	(cd svnwc; svn info) > expected.info &&
-	(cd gitwc; git svn info) > actual.info &&
+	(cd svnwc && svn info) > expected.info &&
+	(cd gitwc && git svn info) > actual.info &&
 	test_cmp_info expected.info actual.info
 	"
 
 test_expect_success 'info --url' '
-	test "$(cd gitwc; git svn info --url)" = "$quoted_svnrepo"
+	test "$(cd gitwc && git svn info --url)" = "$quoted_svnrepo"
 	'
 
 test_expect_success 'info .' "
-	(cd svnwc; svn info .) > expected.info-dot &&
-	(cd gitwc; git svn info .) > actual.info-dot &&
+	(cd svnwc && svn info .) > expected.info-dot &&
+	(cd gitwc && git svn info .) > actual.info-dot &&
 	test_cmp_info expected.info-dot actual.info-dot
 	"
 
 test_expect_success 'info $(pwd)' '
-	(cd svnwc; svn info "$(pwd)") >expected.info-pwd &&
-	(cd gitwc; git svn info "$(pwd)") >actual.info-pwd &&
+	(cd svnwc && svn info "$(pwd)") >expected.info-pwd &&
+	(cd gitwc && git svn info "$(pwd)") >actual.info-pwd &&
 	grep -v ^Path: <expected.info-pwd >expected.info-np &&
 	grep -v ^Path: <actual.info-pwd >actual.info-np &&
 	test_cmp_info expected.info-np actual.info-np &&
@@ -85,8 +85,8 @@ test_expect_success 'info $(pwd)' '
 	'
 
 test_expect_success 'info $(pwd)/../___wc' '
-	(cd svnwc; svn info "$(pwd)/../svnwc") >expected.info-pwd &&
-	(cd gitwc; git svn info "$(pwd)/../gitwc") >actual.info-pwd &&
+	(cd svnwc && svn info "$(pwd)/../svnwc") >expected.info-pwd &&
+	(cd gitwc && git svn info "$(pwd)/../gitwc") >actual.info-pwd &&
 	grep -v ^Path: <expected.info-pwd >expected.info-np &&
 	grep -v ^Path: <actual.info-pwd >actual.info-np &&
 	test_cmp_info expected.info-np actual.info-np &&
@@ -95,8 +95,8 @@ test_expect_success 'info $(pwd)/../___wc' '
 	'
 
 test_expect_success 'info $(pwd)/../___wc//file' '
-	(cd svnwc; svn info "$(pwd)/../svnwc//file") >expected.info-pwd &&
-	(cd gitwc; git svn info "$(pwd)/../gitwc//file") >actual.info-pwd &&
+	(cd svnwc && svn info "$(pwd)/../svnwc//file") >expected.info-pwd &&
+	(cd gitwc && git svn info "$(pwd)/../gitwc//file") >actual.info-pwd &&
 	grep -v ^Path: <expected.info-pwd >expected.info-np &&
 	grep -v ^Path: <actual.info-pwd >actual.info-np &&
 	test_cmp_info expected.info-np actual.info-np &&
@@ -105,56 +105,56 @@ test_expect_success 'info $(pwd)/../___wc//file' '
 	'
 
 test_expect_success 'info --url .' '
-	test "$(cd gitwc; git svn info --url .)" = "$quoted_svnrepo"
+	test "$(cd gitwc && git svn info --url .)" = "$quoted_svnrepo"
 	'
 
 test_expect_success 'info file' "
-	(cd svnwc; svn info file) > expected.info-file &&
-	(cd gitwc; git svn info file) > actual.info-file &&
+	(cd svnwc && svn info file) > expected.info-file &&
+	(cd gitwc && git svn info file) > actual.info-file &&
 	test_cmp_info expected.info-file actual.info-file
 	"
 
 test_expect_success 'info --url file' '
-	test "$(cd gitwc; git svn info --url file)" = "$quoted_svnrepo/file"
+	test "$(cd gitwc && git svn info --url file)" = "$quoted_svnrepo/file"
 	'
 
 test_expect_success 'info directory' "
-	(cd svnwc; svn info directory) > expected.info-directory &&
-	(cd gitwc; git svn info directory) > actual.info-directory &&
+	(cd svnwc && svn info directory) > expected.info-directory &&
+	(cd gitwc && git svn info directory) > actual.info-directory &&
 	test_cmp_info expected.info-directory actual.info-directory
 	"
 
 test_expect_success 'info inside directory' "
-	(cd svnwc/directory; svn info) > expected.info-inside-directory &&
-	(cd gitwc/directory; git svn info) > actual.info-inside-directory &&
+	(cd svnwc/directory && svn info) > expected.info-inside-directory &&
+	(cd gitwc/directory && git svn info) > actual.info-inside-directory &&
 	test_cmp_info expected.info-inside-directory actual.info-inside-directory
 	"
 
 test_expect_success 'info --url directory' '
-	test "$(cd gitwc; git svn info --url directory)" = "$quoted_svnrepo/directory"
+	test "$(cd gitwc && git svn info --url directory)" = "$quoted_svnrepo/directory"
 	'
 
 test_expect_success 'info symlink-file' "
-	(cd svnwc; svn info symlink-file) > expected.info-symlink-file &&
-	(cd gitwc; git svn info symlink-file) > actual.info-symlink-file &&
+	(cd svnwc && svn info symlink-file) > expected.info-symlink-file &&
+	(cd gitwc && git svn info symlink-file) > actual.info-symlink-file &&
 	test_cmp_info expected.info-symlink-file actual.info-symlink-file
 	"
 
 test_expect_success 'info --url symlink-file' '
-	test "$(cd gitwc; git svn info --url symlink-file)" \
+	test "$(cd gitwc && git svn info --url symlink-file)" \
 	     = "$quoted_svnrepo/symlink-file"
 	'
 
 test_expect_success 'info symlink-directory' "
-	(cd svnwc; svn info symlink-directory) \
+	(cd svnwc && svn info symlink-directory) \
 		> expected.info-symlink-directory &&
-	(cd gitwc; git svn info symlink-directory) \
+	(cd gitwc && git svn info symlink-directory) \
 		> actual.info-symlink-directory &&
 	test_cmp_info expected.info-symlink-directory actual.info-symlink-directory
 	"
 
 test_expect_success 'info --url symlink-directory' '
-	test "$(cd gitwc; git svn info --url symlink-directory)" \
+	test "$(cd gitwc && git svn info --url symlink-directory)" \
 	     = "$quoted_svnrepo/symlink-directory"
 	'
 
@@ -169,13 +169,13 @@ test_expect_success 'info added-file' "
 		cd svnwc &&
 		svn_cmd add added-file > /dev/null
 	) &&
-	(cd svnwc; svn info added-file) > expected.info-added-file &&
-	(cd gitwc; git svn info added-file) > actual.info-added-file &&
+	(cd svnwc && svn info added-file) > expected.info-added-file &&
+	(cd gitwc && git svn info added-file) > actual.info-added-file &&
 	test_cmp_info expected.info-added-file actual.info-added-file
 	"
 
 test_expect_success 'info --url added-file' '
-	test "$(cd gitwc; git svn info --url added-file)" \
+	test "$(cd gitwc && git svn info --url added-file)" \
 	     = "$quoted_svnrepo/added-file"
 	'
 
@@ -190,15 +190,15 @@ test_expect_success 'info added-directory' "
 		cd gitwc &&
 		git add added-directory
 	) &&
-	(cd svnwc; svn info added-directory) \
+	(cd svnwc && svn info added-directory) \
 		> expected.info-added-directory &&
-	(cd gitwc; git svn info added-directory) \
+	(cd gitwc && git svn info added-directory) \
 		> actual.info-added-directory &&
 	test_cmp_info expected.info-added-directory actual.info-added-directory
 	"
 
 test_expect_success 'info --url added-directory' '
-	test "$(cd gitwc; git svn info --url added-directory)" \
+	test "$(cd gitwc && git svn info --url added-directory)" \
 	     = "$quoted_svnrepo/added-directory"
 	'
 
@@ -213,16 +213,16 @@ test_expect_success 'info added-symlink-file' "
 		ln -s added-file added-symlink-file &&
 		svn_cmd add added-symlink-file > /dev/null
 	) &&
-	(cd svnwc; svn info added-symlink-file) \
+	(cd svnwc && svn info added-symlink-file) \
 		> expected.info-added-symlink-file &&
-	(cd gitwc; git svn info added-symlink-file) \
+	(cd gitwc && git svn info added-symlink-file) \
 		> actual.info-added-symlink-file &&
 	test_cmp_info expected.info-added-symlink-file \
 		actual.info-added-symlink-file
 	"
 
 test_expect_success 'info --url added-symlink-file' '
-	test "$(cd gitwc; git svn info --url added-symlink-file)" \
+	test "$(cd gitwc && git svn info --url added-symlink-file)" \
 	     = "$quoted_svnrepo/added-symlink-file"
 	'
 
@@ -237,16 +237,16 @@ test_expect_success 'info added-symlink-directory' "
 		ln -s added-directory added-symlink-directory &&
 		svn_cmd add added-symlink-directory > /dev/null
 	) &&
-	(cd svnwc; svn info added-symlink-directory) \
+	(cd svnwc && svn info added-symlink-directory) \
 		> expected.info-added-symlink-directory &&
-	(cd gitwc; git svn info added-symlink-directory) \
+	(cd gitwc && git svn info added-symlink-directory) \
 		> actual.info-added-symlink-directory &&
 	test_cmp_info expected.info-added-symlink-directory \
 		actual.info-added-symlink-directory
 	"
 
 test_expect_success 'info --url added-symlink-directory' '
-	test "$(cd gitwc; git svn info --url added-symlink-directory)" \
+	test "$(cd gitwc && git svn info --url added-symlink-directory)" \
 	     = "$quoted_svnrepo/added-symlink-directory"
 	'
 
@@ -259,13 +259,13 @@ test_expect_success 'info deleted-file' "
 		cd svnwc &&
 		svn_cmd rm --force file > /dev/null
 	) &&
-	(cd svnwc; svn info file) >expected.info-deleted-file &&
-	(cd gitwc; git svn info file) >actual.info-deleted-file &&
+	(cd svnwc && svn info file) >expected.info-deleted-file &&
+	(cd gitwc && git svn info file) >actual.info-deleted-file &&
 	test_cmp_info expected.info-deleted-file actual.info-deleted-file
 	"
 
 test_expect_success 'info --url file (deleted)' '
-	test "$(cd gitwc; git svn info --url file)" \
+	test "$(cd gitwc && git svn info --url file)" \
 	     = "$quoted_svnrepo/file"
 	'
 
@@ -278,13 +278,13 @@ test_expect_success 'info deleted-directory' "
 		cd svnwc &&
 		svn_cmd rm --force directory > /dev/null
 	) &&
-	(cd svnwc; svn info directory) >expected.info-deleted-directory &&
-	(cd gitwc; git svn info directory) >actual.info-deleted-directory &&
+	(cd svnwc && svn info directory) >expected.info-deleted-directory &&
+	(cd gitwc && git svn info directory) >actual.info-deleted-directory &&
 	test_cmp_info expected.info-deleted-directory actual.info-deleted-directory
 	"
 
 test_expect_success 'info --url directory (deleted)' '
-	test "$(cd gitwc; git svn info --url directory)" \
+	test "$(cd gitwc && git svn info --url directory)" \
 	     = "$quoted_svnrepo/directory"
 	'
 
@@ -297,13 +297,13 @@ test_expect_success 'info deleted-symlink-file' "
 		cd svnwc &&
 		svn_cmd rm --force symlink-file > /dev/null
 	) &&
-	(cd svnwc; svn info symlink-file) >expected.info-deleted-symlink-file &&
-	(cd gitwc; git svn info symlink-file) >actual.info-deleted-symlink-file &&
+	(cd svnwc && svn info symlink-file) >expected.info-deleted-symlink-file &&
+	(cd gitwc && git svn info symlink-file) >actual.info-deleted-symlink-file &&
 	test_cmp_info expected.info-deleted-symlink-file actual.info-deleted-symlink-file
 	"
 
 test_expect_success 'info --url symlink-file (deleted)' '
-	test "$(cd gitwc; git svn info --url symlink-file)" \
+	test "$(cd gitwc && git svn info --url symlink-file)" \
 	     = "$quoted_svnrepo/symlink-file"
 	'
 
@@ -316,13 +316,13 @@ test_expect_success 'info deleted-symlink-directory' "
 		cd svnwc &&
 		svn_cmd rm --force symlink-directory > /dev/null
 	) &&
-	(cd svnwc; svn info symlink-directory) >expected.info-deleted-symlink-directory &&
-	(cd gitwc; git svn info symlink-directory) >actual.info-deleted-symlink-directory &&
+	(cd svnwc && svn info symlink-directory) >expected.info-deleted-symlink-directory &&
+	(cd gitwc && git svn info symlink-directory) >actual.info-deleted-symlink-directory &&
 	test_cmp_info expected.info-deleted-symlink-directory actual.info-deleted-symlink-directory
 	"
 
 test_expect_success 'info --url symlink-directory (deleted)' '
-	test "$(cd gitwc; git svn info --url symlink-directory)" \
+	test "$(cd gitwc && git svn info --url symlink-directory)" \
 	     = "$quoted_svnrepo/symlink-directory"
 	'
 
@@ -331,27 +331,27 @@ test_expect_success 'info --url symlink-directory (deleted)' '
 
 test_expect_success 'info unknown-file' "
 	echo two > gitwc/unknown-file &&
-	(cd gitwc; test_must_fail git svn info unknown-file) \
+	(cd gitwc && test_must_fail git svn info unknown-file) \
 		 2> actual.info-unknown-file &&
 	grep unknown-file actual.info-unknown-file
 	"
 
 test_expect_success 'info --url unknown-file' '
 	echo two > gitwc/unknown-file &&
-	(cd gitwc; test_must_fail git svn info --url unknown-file) \
+	(cd gitwc && test_must_fail git svn info --url unknown-file) \
 		 2> actual.info-url-unknown-file &&
 	grep unknown-file actual.info-url-unknown-file
 	'
 
 test_expect_success 'info unknown-directory' "
 	mkdir gitwc/unknown-directory svnwc/unknown-directory &&
-	(cd gitwc; test_must_fail git svn info unknown-directory) \
+	(cd gitwc && test_must_fail git svn info unknown-directory) \
 		 2> actual.info-unknown-directory &&
 	grep unknown-directory actual.info-unknown-directory
 	"
 
 test_expect_success 'info --url unknown-directory' '
-	(cd gitwc; test_must_fail git svn info --url unknown-directory) \
+	(cd gitwc && test_must_fail git svn info --url unknown-directory) \
 		 2> actual.info-url-unknown-directory &&
 	grep unknown-directory actual.info-url-unknown-directory
 	'
@@ -361,13 +361,13 @@ test_expect_success 'info unknown-symlink-file' "
 		cd gitwc &&
 		ln -s unknown-file unknown-symlink-file
 	) &&
-	(cd gitwc; test_must_fail git svn info unknown-symlink-file) \
+	(cd gitwc && test_must_fail git svn info unknown-symlink-file) \
 		 2> actual.info-unknown-symlink-file &&
 	grep unknown-symlink-file actual.info-unknown-symlink-file
 	"
 
 test_expect_success 'info --url unknown-symlink-file' '
-	(cd gitwc; test_must_fail git svn info --url unknown-symlink-file) \
+	(cd gitwc && test_must_fail git svn info --url unknown-symlink-file) \
 		 2> actual.info-url-unknown-symlink-file &&
 	grep unknown-symlink-file actual.info-url-unknown-symlink-file
 	'
@@ -377,13 +377,13 @@ test_expect_success 'info unknown-symlink-directory' "
 		cd gitwc &&
 		ln -s unknown-directory unknown-symlink-directory
 	) &&
-	(cd gitwc; test_must_fail git svn info unknown-symlink-directory) \
+	(cd gitwc && test_must_fail git svn info unknown-symlink-directory) \
 		 2> actual.info-unknown-symlink-directory &&
 	grep unknown-symlink-directory actual.info-unknown-symlink-directory
 	"
 
 test_expect_success 'info --url unknown-symlink-directory' '
-	(cd gitwc; test_must_fail git svn info --url unknown-symlink-directory) \
+	(cd gitwc && test_must_fail git svn info --url unknown-symlink-directory) \
 		 2> actual.info-url-unknown-symlink-directory &&
 	grep unknown-symlink-directory actual.info-url-unknown-symlink-directory
 	'
-- 
2.18.0.203.gfac676dfb9


^ permalink raw reply related	[flat|nested] 52+ messages in thread

* Re: [PATCH 01/25] t: use test_might_fail() instead of manipulating exit code manually
  2018-07-02  0:23 ` [PATCH 01/25] t: use test_might_fail() instead of manipulating exit code manually Eric Sunshine
@ 2018-07-02 17:44   ` Stefan Beller
  2018-07-02 20:58     ` Eric Sunshine
  2018-07-03 19:20   ` Junio C Hamano
  1 sibling, 1 reply; 52+ messages in thread
From: Stefan Beller @ 2018-07-02 17:44 UTC (permalink / raw)
  To: Eric Sunshine
  Cc: git, Elijah Newren, Johannes Sixt, Jonathan Nieder, Jonathan Tan,
	Junio C Hamano, Luke Diamand, Jeff King

> diff --git a/t/t4012-diff-binary.sh b/t/t4012-diff-binary.sh
> index 0a8af76aab..6579c81216 100755
> --- a/t/t4012-diff-binary.sh
> +++ b/t/t4012-diff-binary.sh
> @@ -102,10 +102,8 @@ test_expect_success 'apply binary patch' '
>
>  test_expect_success 'diff --no-index with binary creation' '
>         echo Q | q_to_nul >binary &&
> -       (: hide error code from diff, which just indicates differences
> -        git diff --binary --no-index /dev/null binary >current ||
> -        true
> -       ) &&
> +       # hide error code from diff, which just indicates differences
> +       test_might_fail git diff --binary --no-index /dev/null binary >current &&

I am not sure why we need to be non-deterministic here, i.e. I would rather
test for success or non-success error code and not just *any* error code.

This code was introduced in
71b989e7dd1 (fix bogus "diff --git" header from "diff --no-index", 2008-10-05)
whereas the test_must_fail was introduced in
74359821020 (tests: introduce test_must_fail, 2008-02-28).
However this code was authored without Junios involvement
(he was AFK 2008-09-23..2008-10-08), so
maybe test_must_fail was not so popular back then?

While I think this patch is a strict improvement for the test suite,
I do wonder if we can tighten the exit code check here (maybe in
a follow up series?).

Thanks,
Stefan

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH 02/25] t: use test_write_lines() instead of series of 'echo' commands
  2018-07-02  0:23 ` [PATCH 02/25] t: use test_write_lines() instead of series of 'echo' commands Eric Sunshine
@ 2018-07-02 17:53   ` Stefan Beller
  0 siblings, 0 replies; 52+ messages in thread
From: Stefan Beller @ 2018-07-02 17:53 UTC (permalink / raw)
  To: Eric Sunshine
  Cc: git, Elijah Newren, Johannes Sixt, Jonathan Nieder, Jonathan Tan,
	Junio C Hamano, Luke Diamand, Jeff King

On Sun, Jul 1, 2018 at 5:24 PM Eric Sunshine <sunshine@sunshineco.com> wrote:
>
> These tests employ a noisy subshell (with missing &&-chain) to feed
> input into Git commands or files:
>
>     (echo a; echo b; echo c) | git some-command ...
>
> Simplify by taking advantage of test_write_lines():
>
>     test_write_lines a b c | git some-command ...

...
and as piped commands only return the exit code of
the last command in the piping structure this is ok,
as all git commands are the last command here.

If any of the non-last commands are failing (which
are the test_write_lines after this patch), we would
only care if the git command fails any way, as there is
no benefit in testing the correctness of test_write_lines.

This is why we chose to go this route instead of
writing the output to a file and then taking the file
as input for the git command. Although this alternative
might be easier to debug, we choose to not use that
as it would complicate this patch even more and these
tests did not need debugging for a long time.

Makes sense.

Stefan

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH 12/25] t7810: use test_expect_code() instead of hand-rolled comparison
  2018-07-02  0:23 ` [PATCH 12/25] t7810: use test_expect_code() instead of hand-rolled comparison Eric Sunshine
@ 2018-07-02 18:05   ` Stefan Beller
  2018-07-02 21:12     ` Eric Sunshine
  0 siblings, 1 reply; 52+ messages in thread
From: Stefan Beller @ 2018-07-02 18:05 UTC (permalink / raw)
  To: Eric Sunshine
  Cc: git, Elijah Newren, Johannes Sixt, Jonathan Nieder, Jonathan Tan,
	Junio C Hamano, Luke Diamand, Jeff King

On Sun, Jul 1, 2018 at 5:25 PM Eric Sunshine <sunshine@sunshineco.com> wrote:
>
> This test manually checks the exit code of git-grep for a particular
> value. In doing so, it intentionally breaks the &&-chain. Modernize the
> test by taking advantage of test_expect_code() and a normal &&-chain.
>
> Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
> ---
>  t/t7810-grep.sh | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/t/t7810-grep.sh b/t/t7810-grep.sh
> index 1797f632a3..fecee602c1 100755
> --- a/t/t7810-grep.sh
> +++ b/t/t7810-grep.sh
> @@ -845,10 +845,9 @@ test_expect_success 'grep from a subdirectory to search wider area (1)' '
>  test_expect_success 'grep from a subdirectory to search wider area (2)' '
>         mkdir -p s &&
>         (
> -               cd s || exit 1
> -               ( git grep xxyyzz .. >out ; echo $? >status )
> -               ! test -s out &&
> -               test 1 = $(cat status)
> +               cd s &&
> +               test_expect_code 1 git grep xxyyzz .. >out &&
> +               ! test -s out
>         )

Further optimisation would be possible if I understand the code correctly:

    test_expect_code git -C s grep xxyyzz .. >../out
    test_must_be_empty out

(dropping the subshell entirely)


>  '
>
> --
> 2.18.0.203.gfac676dfb9
>

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH 00/25] fix buggy tests, modernize tests, fix broken &&-chains
  2018-07-02  0:23 [PATCH 00/25] fix buggy tests, modernize tests, fix broken &&-chains Eric Sunshine
                   ` (24 preceding siblings ...)
  2018-07-02  0:24 ` [PATCH 25/25] t9119: " Eric Sunshine
@ 2018-07-02 18:27 ` Stefan Beller
  2018-07-02 21:20   ` Eric Sunshine
  2018-07-03 19:40 ` Junio C Hamano
  26 siblings, 1 reply; 52+ messages in thread
From: Stefan Beller @ 2018-07-02 18:27 UTC (permalink / raw)
  To: Eric Sunshine
  Cc: git, Elijah Newren, Johannes Sixt, Jonathan Nieder, Jonathan Tan,
	Junio C Hamano, Luke Diamand, Jeff King

On Sun, Jul 1, 2018 at 5:24 PM Eric Sunshine <sunshine@sunshineco.com> wrote:
>
> This series fixes several buggy tests which went undetected due to
> broken &&-chains in subshells, modernizes some tests to take advantage
> of test functions (test_might_fail(), test_write_lines(), etc.), and
> fixes a lot of broken &&-chains in subshells. It applies atop
> 'master'. Happily, there are no broken &&-chains in subshells in any
> in-flight topic.
>
> It is split out from an earlier series[1] which additionally extended
> --chain-lint to work within subshells. I decided to make this an
> independent series because these (hopefully) non-controversial changes
> all stand on their own merit, and because I don't want to flood the list
> repeatedly with this lengthy series as I re-roll the "extend
> --chain-lint to work in subshells" functionality from [1].
>
> To ease review burden, I largely avoided noisy modernizations and
> cleanups, and limited changes to merely adding "&&" even when some other
> transformation would have made the fix nicer overall. (For example, I
> resisted the urge to replace a series of 'echo' statements with a simple
> here-doc.)
>
> Changes since [1]:
>
> * Found and fixed more &&-chain breakage, and converted a couple more
>   'unset' instances (which were hidden behind a MINGW prerequisite) to
>   sane_unset().
>
> * Rewrote commit messages to sell changes on their own merit rather than
>   leaning on --chain-lint as a crutch. (junio, jrnieder)
>
> * Changed a modernization/cleanup to use "! non-git-command" rather than
>   test_must_fail(), moving it to its own patch in the process. (j6t)
>
> * Changed "printf '%s\n'" idiom to test_write_lines(). (junio)
>
> * Incorporated Stefan's fix[2] for a broken 't/lib-submodule-update'
>   test since my interpretation of the problem was incorrect.
>
> * Converted a subshell 'echo' sequence to write_script().
>
> * Dropped patches which existed primarily to pacify --chain-lint; they
>   are no longer needed since I re-wrote the "linter" to detect &&-chain
>   breakage itself (by pure textual inspection) rather than by
>   incorporating subshell bodies into the main &&-chain:
>
>   t0001: use "{...}" block around "||" expression rather than subshell
>   t3303: use standard here-doc tag "EOF" to avoid fooling --chain-lint
>   t9104: use "{...}" block around "||" expression rather than subshell
>   t9401: drop unnecessary nested subshell
>
> * Dropped a patch which pretty much duplicated Junio's 037714252f
>   (tests: clean after SANITY tests, 2018-06-15), which graduated to
>   'master':
>
>   t7508: use test_when_finished() instead of managing exit code manually
>
> An interdiff against [1] is below, although I stripped out all the noisy
> "printf '%s\n'" to test_write_lines() differences, of which there were a
> lot, since they drowned out the other more significant changes.
>
> Thanks to Elijah, Hannes, Jonathan Nieder, Jonathan Tan, Junio, Luke,
> Peff, and Stefan for comments on [1].

Thanks for this series,
I think it is good to include it as is and build on top if needed. I had some
comments on the earlier part of the series, but that is really just the cherry
on top of the cake.

Thanks,
Stefan

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH 01/25] t: use test_might_fail() instead of manipulating exit code manually
  2018-07-02 17:44   ` Stefan Beller
@ 2018-07-02 20:58     ` Eric Sunshine
  0 siblings, 0 replies; 52+ messages in thread
From: Eric Sunshine @ 2018-07-02 20:58 UTC (permalink / raw)
  To: Stefan Beller
  Cc: Git List, Elijah Newren, Johannes Sixt, Jonathan Nieder,
	Jonathan Tan, Junio C Hamano, Luke Diamand, Jeff King

On Mon, Jul 2, 2018 at 1:44 PM Stefan Beller <sbeller@google.com> wrote:
> > diff --git a/t/t4012-diff-binary.sh b/t/t4012-diff-binary.sh
> >  test_expect_success 'diff --no-index with binary creation' '
> >         echo Q | q_to_nul >binary &&
> > -       (: hide error code from diff, which just indicates differences
> > -        git diff --binary --no-index /dev/null binary >current ||
> > -        true
> > -       ) &&
> > +       # hide error code from diff, which just indicates differences
> > +       test_might_fail git diff --binary --no-index /dev/null binary >current &&
>
> I am not sure why we need to be non-deterministic here, i.e. I would rather
> test for success or non-success error code and not just *any* error code.
>
> While I think this patch is a strict improvement for the test suite,
> I do wonder if we can tighten the exit code check here (maybe in
> a follow up series?).

I agree that such a change is out of the scope of this patch series.

But, do keep in mind that the exit code of git-diff doesn't indicate
only "success" or "error". Various exit codes represent different
conditions; for instance, 1 means differences were found, 0 means no
differences, and -1 means error. The outcome of this test is based
upon output generated by git-diff, so the test itself will fail
overall if git-diff doesn't produce expected results. Also, there is
an entire test script (t4017-diff-retval.sh) concerned with verifying
git-diff exit code, so I'm not sure we need to worry about it too much
here.

(I'm not arguing against a follow-up patch, though, just thinking
aloud about its potential value.)

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH 12/25] t7810: use test_expect_code() instead of hand-rolled comparison
  2018-07-02 18:05   ` Stefan Beller
@ 2018-07-02 21:12     ` Eric Sunshine
  0 siblings, 0 replies; 52+ messages in thread
From: Eric Sunshine @ 2018-07-02 21:12 UTC (permalink / raw)
  To: Stefan Beller
  Cc: Git List, Elijah Newren, Johannes Sixt, Jonathan Nieder,
	Jonathan Tan, Junio C Hamano, Luke Diamand, Jeff King

On Mon, Jul 2, 2018 at 2:14 PM Stefan Beller <sbeller@google.com> wrote:
> On Sun, Jul 1, 2018 at 5:25 PM Eric Sunshine <sunshine@sunshineco.com> wrote:
> >  test_expect_success 'grep from a subdirectory to search wider area (2)' '
> >         mkdir -p s &&
> >         (
> > -               cd s || exit 1
> > -               ( git grep xxyyzz .. >out ; echo $? >status )
> > -               ! test -s out &&
> > -               test 1 = $(cat status)
> > +               cd s &&
> > +               test_expect_code 1 git grep xxyyzz .. >out &&
> > +               ! test -s out
> >         )
>
> Further optimisation would be possible if I understand the code correctly:
>
>     test_expect_code git -C s grep xxyyzz .. >../out
>     test_must_be_empty out
>
> (dropping the subshell entirely)

I did consider dropping the subshell in favor of -C, however, decided
to keep it since the subshell has value by making it very explicit to
the reader that this git-grep command is running in a subdirectory
(which is what this test is all about). Contrast that with -C which is
easy to overlook and might not fully convince the reader that the
command's entire execution is within the subdirectory, whereas there
is no question that the entire execution of git-diff in "(cd s &&
git-diff ...)" occurs in the subdirectory.

I can make the test_must_be_empty() change if I re-roll.

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH 00/25] fix buggy tests, modernize tests, fix broken &&-chains
  2018-07-02 18:27 ` [PATCH 00/25] fix buggy tests, modernize tests, " Stefan Beller
@ 2018-07-02 21:20   ` Eric Sunshine
  0 siblings, 0 replies; 52+ messages in thread
From: Eric Sunshine @ 2018-07-02 21:20 UTC (permalink / raw)
  To: Stefan Beller
  Cc: Git List, Elijah Newren, Johannes Sixt, Jonathan Nieder,
	Jonathan Tan, Junio C Hamano, Luke Diamand, Jeff King

On Mon, Jul 2, 2018 at 2:27 PM Stefan Beller <sbeller@google.com> wrote:
> On Sun, Jul 1, 2018 at 5:24 PM Eric Sunshine <sunshine@sunshineco.com> wrote:
> > This series fixes several buggy tests which went undetected due to
> > broken &&-chains in subshells, modernizes some tests to take advantage
> > of test functions (test_might_fail(), test_write_lines(), etc.), and
> > fixes a lot of broken &&-chains in subshells. [...]
>
> I think it is good to include it as is and build on top if needed. I had some
> comments on the earlier part of the series, but that is really just the cherry
> on top of the cake.

Thanks for the review comments.

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH 01/25] t: use test_might_fail() instead of manipulating exit code manually
  2018-07-02  0:23 ` [PATCH 01/25] t: use test_might_fail() instead of manipulating exit code manually Eric Sunshine
  2018-07-02 17:44   ` Stefan Beller
@ 2018-07-03 19:20   ` Junio C Hamano
  1 sibling, 0 replies; 52+ messages in thread
From: Junio C Hamano @ 2018-07-03 19:20 UTC (permalink / raw)
  To: Eric Sunshine
  Cc: git, Elijah Newren, Johannes Sixt, Jonathan Nieder, Jonathan Tan,
	Stefan Beller, Luke Diamand, Jeff King

Eric Sunshine <sunshine@sunshineco.com> writes:

>  test_expect_success 'merge my-side@{u} records the correct name' '
>  (
> -	cd clone || exit
> -	git checkout master || exit
> -	git branch -D new ;# can fail but is ok
> +	cd clone &&
> +	git checkout master &&
> +	test_might_fail git branch -D new &&

I wonder why we had these "|| exit" in the first place, but these
are obviously good changes.

> diff --git a/t/t1700-split-index.sh b/t/t1700-split-index.sh
> index 1e81b33b2e..39133bcbc8 100755
> --- a/t/t1700-split-index.sh
> +++ b/t/t1700-split-index.sh
> @@ -435,7 +435,7 @@ test_expect_success 'writing split index with null sha1 does not write cache tre
>  	commit=$(git commit-tree $tree -p HEAD <msg) &&
>  	git update-ref HEAD "$commit" &&
>  	GIT_ALLOW_NULL_SHA1=1 git reset --hard &&
> -	(test-tool dump-cache-tree >cache-tree.out || true) &&
> +	test_might_fail test-tool dump-cache-tree >cache-tree.out &&
>  	test_line_count = 0 cache-tree.out
>  '

I wonder where the unstable expectation for the exit status comes
from.  4bddd983 ("split-index: don't write cache tree with null oid
entries", 2018-01-07) is not exactly illuminating, either X-<.

> diff --git a/t/t4012-diff-binary.sh b/t/t4012-diff-binary.sh
> index 0a8af76aab..6579c81216 100755
> --- a/t/t4012-diff-binary.sh
> +++ b/t/t4012-diff-binary.sh
> @@ -102,10 +102,8 @@ test_expect_success 'apply binary patch' '
>  
>  test_expect_success 'diff --no-index with binary creation' '
>  	echo Q | q_to_nul >binary &&
> -	(: hide error code from diff, which just indicates differences
> -	 git diff --binary --no-index /dev/null binary >current ||
> -	 true
> -	) &&
> +	# hide error code from diff, which just indicates differences
> +	test_might_fail git diff --binary --no-index /dev/null binary >current &&

As you said in downthread, we only care about the correct output in
the "current" file, which is verified by using it as input to the
"apply --binary", so might-fail is fine here.  But we probably want
to make sure "diff --binary --no-index" to *notice* that there is a
difference and report that with its exit status.  I wouldn't say so
if this was about testing "git apply", but the test title tells us
that it is about "diff --no-index", so...


^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH 04/25] t: drop unnecessary terminating semicolon in subshell
  2018-07-02  0:23 ` [PATCH 04/25] t: drop unnecessary terminating semicolon in subshell Eric Sunshine
@ 2018-07-03 19:23   ` Junio C Hamano
  0 siblings, 0 replies; 52+ messages in thread
From: Junio C Hamano @ 2018-07-03 19:23 UTC (permalink / raw)
  To: Eric Sunshine
  Cc: git, Elijah Newren, Johannes Sixt, Jonathan Nieder, Jonathan Tan,
	Stefan Beller, Luke Diamand, Jeff King

Eric Sunshine <sunshine@sunshineco.com> writes:

> Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
> ---
>  t/t3102-ls-tree-wildcards.sh    | 2 +-
>  t/t4010-diff-pathspec.sh        | 4 ++--
>  t/t9400-git-cvsserver-server.sh | 2 +-
>  3 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/t/t3102-ls-tree-wildcards.sh b/t/t3102-ls-tree-wildcards.sh
> index e804377f1c..1e16c6b8ea 100755
> --- a/t/t3102-ls-tree-wildcards.sh
> +++ b/t/t3102-ls-tree-wildcards.sh
> @@ -23,7 +23,7 @@ test_expect_success 'ls-tree outside prefix' '
>  	cat >expect <<-EOF &&
>  	100644 blob $EMPTY_BLOB	../a[a]/three
>  	EOF
> -	( cd aa && git ls-tree -r HEAD "../a[a]"; ) >actual &&
> +	( cd aa && git ls-tree -r HEAD "../a[a]" ) >actual &&
>  	test_cmp expect actual
>  '

Nice clean-up.  

I still find it irritating that ( ...; ) is merely unnecessary but
not incorrect, while lack of semicolon in { ...; } is a hard error
;-)

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH 00/25] fix buggy tests, modernize tests, fix broken &&-chains
  2018-07-02  0:23 [PATCH 00/25] fix buggy tests, modernize tests, fix broken &&-chains Eric Sunshine
                   ` (25 preceding siblings ...)
  2018-07-02 18:27 ` [PATCH 00/25] fix buggy tests, modernize tests, " Stefan Beller
@ 2018-07-03 19:40 ` Junio C Hamano
  26 siblings, 0 replies; 52+ messages in thread
From: Junio C Hamano @ 2018-07-03 19:40 UTC (permalink / raw)
  To: Eric Sunshine
  Cc: git, Elijah Newren, Johannes Sixt, Jonathan Nieder, Jonathan Tan,
	Stefan Beller, Luke Diamand, Jeff King

Eric Sunshine <sunshine@sunshineco.com> writes:

> It is split out from an earlier series[1] which additionally extended
> --chain-lint to work within subshells. I decided to make this an
> independent series because these (hopefully) non-controversial changes
> all stand on their own merit, and because I don't want to flood the list
> repeatedly with this lengthy series as I re-roll the "extend
> --chain-lint to work in subshells" functionality from [1].

Thanks for doing so; very much appreciated.

Not that I vehemently oppose to the --chain-lint change, but I do
like the fact that I do not even have to worry about it when queuing
these 25 patches.

Will queue.

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH 21/25] t5000-t5999: fix broken &&-chains
  2018-07-02  0:24 ` [PATCH 21/25] t5000-t5999: " Eric Sunshine
@ 2018-07-12 12:37   ` SZEDER Gábor
  2018-07-12 17:44     ` Eric Sunshine
  0 siblings, 1 reply; 52+ messages in thread
From: SZEDER Gábor @ 2018-07-12 12:37 UTC (permalink / raw)
  To: Eric Sunshine
  Cc: SZEDER Gábor, git, Elijah Newren, Johannes Sixt,
	Jonathan Nieder, Jonathan Tan, Stefan Beller, Junio C Hamano,
	Luke Diamand, Jeff King


> Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
> ---
>  t/t5300-pack-object.sh         |  2 +-
>  t/t5302-pack-index.sh          |  2 +-
>  t/t5401-update-hooks.sh        |  4 ++--
>  t/t5500-fetch-pack.sh          |  2 +-
>  t/t5505-remote.sh              |  2 +-
>  t/t5512-ls-remote.sh           |  4 ++--
>  t/t5516-fetch-push.sh          | 10 +++++-----
>  t/t5517-push-mirror.sh         | 10 +++++-----
>  t/t5526-fetch-submodules.sh    |  2 +-
>  t/t5531-deep-submodule-push.sh |  2 +-
>  t/t5543-atomic-push.sh         |  2 +-
>  t/t5601-clone.sh               |  2 +-
>  t/t5605-clone-local.sh         |  2 +-
>  t/t5801-remote-helpers.sh      |  2 +-
>  14 files changed, 24 insertions(+), 24 deletions(-)

The change below should be squashed into this patch to fix a
previously unnoticed broken &&-chain.  I think you missed it, because
this test script is rather expensive and you didn't run it with
GIT_TEST_CLONE_2GB=YesPlease.


diff --git a/t/t5608-clone-2gb.sh b/t/t5608-clone-2gb.sh
index df822d9a3e..2c6bc07344 100755
--- a/t/t5608-clone-2gb.sh
+++ b/t/t5608-clone-2gb.sh
@@ -23,7 +23,7 @@ test_expect_success CLONE_2GB 'setup' '
 		printf "blob\nmark :$i\ndata $blobsize\n" &&
 		#test-tool genrandom $i $blobsize &&
 		printf "%-${blobsize}s" $i &&
-		echo "M 100644 :$i $i" >> commit
+		echo "M 100644 :$i $i" >> commit &&
 		i=$(($i+1)) ||
 		echo $? > exit-status
 	 done &&


^ permalink raw reply related	[flat|nested] 52+ messages in thread

* Re: [PATCH 21/25] t5000-t5999: fix broken &&-chains
  2018-07-12 12:37   ` SZEDER Gábor
@ 2018-07-12 17:44     ` Eric Sunshine
  2018-07-12 18:19       ` Jeff King
  2018-07-12 18:31       ` Junio C Hamano
  0 siblings, 2 replies; 52+ messages in thread
From: Eric Sunshine @ 2018-07-12 17:44 UTC (permalink / raw)
  To: SZEDER Gábor
  Cc: Git List, Elijah Newren, Johannes Sixt, Jonathan Nieder,
	Jonathan Tan, Stefan Beller, Junio C Hamano, Luke Diamand,
	Jeff King

On Thu, Jul 12, 2018 at 8:37 AM SZEDER Gábor <szeder.dev@gmail.com> wrote:
> The change below should be squashed into this patch to fix a
> previously unnoticed broken &&-chain.  I think you missed it, because
> this test script is rather expensive and you didn't run it with
> GIT_TEST_CLONE_2GB=YesPlease.
>
> diff --git a/t/t5608-clone-2gb.sh b/t/t5608-clone-2gb.sh
> @@ -23,7 +23,7 @@ test_expect_success CLONE_2GB 'setup' '
> -               echo "M 100644 :$i $i" >> commit
> +               echo "M 100644 :$i $i" >> commit &&

Thanks for finding this. I tried to get as much coverage as possible
by installing packages I don't normally have installed (Apache, cvs,
cvsps, Subversion, Perforce, etc.) and even temporarily modified a
script or two to force it run when I simply couldn't meet some
prerequisite, thus reducing the "skipped" messages to a minimum, but I
wasn't even aware of this prerequisite since I never saw a "skipped"
message for it.

Looking at it more closely, I think the reason it didn't come to my
attention is that this script doesn't use the standard skip_all="..."
mechanism for skipping the tests but instead "rolls its own", and
apparently 'prove' simply swallowed (or was unable to produce) an
overall "skipped" message for this script.

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH 21/25] t5000-t5999: fix broken &&-chains
  2018-07-12 17:44     ` Eric Sunshine
@ 2018-07-12 18:19       ` Jeff King
  2018-07-12 18:31       ` Junio C Hamano
  1 sibling, 0 replies; 52+ messages in thread
From: Jeff King @ 2018-07-12 18:19 UTC (permalink / raw)
  To: Eric Sunshine
  Cc: SZEDER Gábor, Git List, Elijah Newren, Johannes Sixt,
	Jonathan Nieder, Jonathan Tan, Stefan Beller, Junio C Hamano,
	Luke Diamand

On Thu, Jul 12, 2018 at 01:44:49PM -0400, Eric Sunshine wrote:

> On Thu, Jul 12, 2018 at 8:37 AM SZEDER Gábor <szeder.dev@gmail.com> wrote:
> > The change below should be squashed into this patch to fix a
> > previously unnoticed broken &&-chain.  I think you missed it, because
> > this test script is rather expensive and you didn't run it with
> > GIT_TEST_CLONE_2GB=YesPlease.
> >
> > diff --git a/t/t5608-clone-2gb.sh b/t/t5608-clone-2gb.sh
> > @@ -23,7 +23,7 @@ test_expect_success CLONE_2GB 'setup' '
> > -               echo "M 100644 :$i $i" >> commit
> > +               echo "M 100644 :$i $i" >> commit &&
> 
> Thanks for finding this. I tried to get as much coverage as possible
> by installing packages I don't normally have installed (Apache, cvs,
> cvsps, Subversion, Perforce, etc.) and even temporarily modified a
> script or two to force it run when I simply couldn't meet some
> prerequisite, thus reducing the "skipped" messages to a minimum, but I
> wasn't even aware of this prerequisite since I never saw a "skipped"
> message for it.

I think in theory we should be able to lint _every_ test, even if we're
not running it. After all, the point is that a proper linting runs zero
commands.

That said, it may not be worth the implementation effort. The linting
happens at a pretty low-level, and we've already decided to skip tests
long before then (even for single prereqs, let alone skip_all cases
where we exit the script early).

> Looking at it more closely, I think the reason it didn't come to my
> attention is that this script doesn't use the standard skip_all="..."
> mechanism for skipping the tests but instead "rolls its own", and
> apparently 'prove' simply swallowed (or was unable to produce) an
> overall "skipped" message for this script.

Yeah, that is a bit funny. For a whole-test skip like this, I think
skip_all is easier to read, as it is immediately apparent to the reader
that nothing that _doesn't_ meet that prereq should be in the file. So
I'd be happy to see it switched (though it's not _that_ big a deal, so
leaving it is fine, too).

By the way, "prove --directives" can help with finding individual
skipped tests.

-Peff

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH 21/25] t5000-t5999: fix broken &&-chains
  2018-07-12 17:44     ` Eric Sunshine
  2018-07-12 18:19       ` Jeff King
@ 2018-07-12 18:31       ` Junio C Hamano
  2018-07-12 18:35         ` Junio C Hamano
  1 sibling, 1 reply; 52+ messages in thread
From: Junio C Hamano @ 2018-07-12 18:31 UTC (permalink / raw)
  To: Eric Sunshine
  Cc: SZEDER Gábor, Git List, Elijah Newren, Johannes Sixt,
	Jonathan Nieder, Jonathan Tan, Stefan Beller, Luke Diamand,
	Jeff King

Eric Sunshine <sunshine@sunshineco.com> writes:

> On Thu, Jul 12, 2018 at 8:37 AM SZEDER Gábor <szeder.dev@gmail.com> wrote:
>> The change below should be squashed into this patch to fix a
>> previously unnoticed broken &&-chain.  I think you missed it, because
>> this test script is rather expensive and you didn't run it with
>> GIT_TEST_CLONE_2GB=YesPlease.
>>
>> diff --git a/t/t5608-clone-2gb.sh b/t/t5608-clone-2gb.sh
>> @@ -23,7 +23,7 @@ test_expect_success CLONE_2GB 'setup' '
>> -               echo "M 100644 :$i $i" >> commit
>> +               echo "M 100644 :$i $i" >> commit &&
>
> Thanks for finding this. I tried to get as much coverage as possible
> by installing packages I don't normally have installed (Apache, cvs,
> cvsps, Subversion, Perforce, etc.) and even temporarily modified a
> script or two to force it run ...

Thanks, both.  I think the &&-chain fix series is already large and
also otherwise seem to be pretty solid, so let's not reroll but
queue this addition on top.

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH 21/25] t5000-t5999: fix broken &&-chains
  2018-07-12 18:31       ` Junio C Hamano
@ 2018-07-12 18:35         ` Junio C Hamano
  2018-07-12 18:46           ` Eric Sunshine
  2018-07-17  8:15           ` SZEDER Gábor
  0 siblings, 2 replies; 52+ messages in thread
From: Junio C Hamano @ 2018-07-12 18:35 UTC (permalink / raw)
  To: Eric Sunshine
  Cc: SZEDER Gábor, Git List, Elijah Newren, Johannes Sixt,
	Jonathan Nieder, Jonathan Tan, Stefan Beller, Luke Diamand,
	Jeff King

Junio C Hamano <gitster@pobox.com> writes:

> Eric Sunshine <sunshine@sunshineco.com> writes:
>
>> On Thu, Jul 12, 2018 at 8:37 AM SZEDER Gábor <szeder.dev@gmail.com> wrote:
>>> The change below should be squashed into this patch to fix a
>>> previously unnoticed broken &&-chain.  I think you missed it, because
>>> this test script is rather expensive and you didn't run it with
>>> GIT_TEST_CLONE_2GB=YesPlease.
>>>
>>> diff --git a/t/t5608-clone-2gb.sh b/t/t5608-clone-2gb.sh
>>> @@ -23,7 +23,7 @@ test_expect_success CLONE_2GB 'setup' '
>>> -               echo "M 100644 :$i $i" >> commit
>>> +               echo "M 100644 :$i $i" >> commit &&
>>
>> Thanks for finding this. I tried to get as much coverage as possible
>> by installing packages I don't normally have installed (Apache, cvs,
>> cvsps, Subversion, Perforce, etc.) and even temporarily modified a
>> script or two to force it run ...
>
> Thanks, both.  I think the &&-chain fix series is already large and
> also otherwise seem to be pretty solid, so let's not reroll but
> queue this addition on top.

Oops, sent before completing the message.

	For that to happen, we need a sign-off ;-)

I guess this one would have been caught with the "sed script on
subshell" linter that does not execute?

-- >8 --
Subject: t5608: fix broken &&-chain

This is inside a loop that is run inside a subshell, in a test that
is protected with CLONE_2GB prerequisite, one or more which is quite
likely reason why it wasn't caught durin the previous clean-up.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 t/t5608-clone-2gb.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/t/t5608-clone-2gb.sh b/t/t5608-clone-2gb.sh
index df822d9a3e..2c6bc07344 100755
--- a/t/t5608-clone-2gb.sh
+++ b/t/t5608-clone-2gb.sh
@@ -23,7 +23,7 @@ test_expect_success CLONE_2GB 'setup' '
 		printf "blob\nmark :$i\ndata $blobsize\n" &&
 		#test-tool genrandom $i $blobsize &&
 		printf "%-${blobsize}s" $i &&
-		echo "M 100644 :$i $i" >> commit
+		echo "M 100644 :$i $i" >> commit &&
 		i=$(($i+1)) ||
 		echo $? > exit-status
 	 done &&

^ permalink raw reply related	[flat|nested] 52+ messages in thread

* Re: [PATCH 21/25] t5000-t5999: fix broken &&-chains
  2018-07-12 18:35         ` Junio C Hamano
@ 2018-07-12 18:46           ` Eric Sunshine
  2018-07-12 18:50             ` Junio C Hamano
  2018-07-17  8:15           ` SZEDER Gábor
  1 sibling, 1 reply; 52+ messages in thread
From: Eric Sunshine @ 2018-07-12 18:46 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: SZEDER Gábor, Git List, Elijah Newren, Johannes Sixt,
	Jonathan Nieder, Jonathan Tan, Stefan Beller, Luke Diamand,
	Jeff King

On Thu, Jul 12, 2018 at 2:35 PM Junio C Hamano <gitster@pobox.com> wrote:
> Junio C Hamano <gitster@pobox.com> writes:
> Oops, sent before completing the message.
>
>         For that to happen, we need a sign-off ;-)
>
> I guess this one would have been caught with the "sed script on
> subshell" linter that does not execute?

Yes, this is correctly caught when the prerequisite is met.

> -- >8 --
> Subject: t5608: fix broken &&-chain
>
> This is inside a loop that is run inside a subshell, in a test that
> is protected with CLONE_2GB prerequisite, one or more which is quite
> likely reason why it wasn't caught durin the previous clean-up.

s/durin/during/

The exact reason is that the prerequisite was not met (indeed, I
wasn't even aware of that prerequisite), so the commit message can be
more direct:

    This was missed by the previous clean-ups due to an unmet
    CLONE_2GB prerequisite.

Thanks for saving a round-trip.

> Signed-off-by: Junio C Hamano <gitster@pobox.com>

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH 21/25] t5000-t5999: fix broken &&-chains
  2018-07-12 18:46           ` Eric Sunshine
@ 2018-07-12 18:50             ` Junio C Hamano
  2018-07-12 18:53               ` Eric Sunshine
  0 siblings, 1 reply; 52+ messages in thread
From: Junio C Hamano @ 2018-07-12 18:50 UTC (permalink / raw)
  To: Eric Sunshine
  Cc: SZEDER Gábor, Git List, Elijah Newren, Johannes Sixt,
	Jonathan Nieder, Jonathan Tan, Stefan Beller, Luke Diamand,
	Jeff King

Eric Sunshine <sunshine@sunshineco.com> writes:

> On Thu, Jul 12, 2018 at 2:35 PM Junio C Hamano <gitster@pobox.com> wrote:
>> Junio C Hamano <gitster@pobox.com> writes:
>> Oops, sent before completing the message.
>>
>>         For that to happen, we need a sign-off ;-)
>>
>> I guess this one would have been caught with the "sed script on
>> subshell" linter that does not execute?
>
> Yes, this is correctly caught when the prerequisite is met.
>
>> -- >8 --
>> Subject: t5608: fix broken &&-chain
>>
>> This is inside a loop that is run inside a subshell, in a test that
>> is protected with CLONE_2GB prerequisite, one or more which is quite
>> likely reason why it wasn't caught durin the previous clean-up.
>
> s/durin/during/
>
> The exact reason is that the prerequisite was not met (indeed, I
> wasn't even aware of that prerequisite), so the commit message can be
> more direct:
>
>     This was missed by the previous clean-ups due to an unmet
>     CLONE_2GB prerequisite.
>
> Thanks for saving a round-trip.
>
>> Signed-off-by: Junio C Hamano <gitster@pobox.com>

There still need a sign-off, so ... ;-)

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH 21/25] t5000-t5999: fix broken &&-chains
  2018-07-12 18:50             ` Junio C Hamano
@ 2018-07-12 18:53               ` Eric Sunshine
  0 siblings, 0 replies; 52+ messages in thread
From: Eric Sunshine @ 2018-07-12 18:53 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: SZEDER Gábor, Git List, Elijah Newren, Johannes Sixt,
	Jonathan Nieder, Jonathan Tan, Stefan Beller, Luke Diamand,
	Jeff King

On Thu, Jul 12, 2018 at 2:50 PM Junio C Hamano <gitster@pobox.com> wrote:
> Eric Sunshine <sunshine@sunshineco.com> writes:
> > The exact reason is that the prerequisite was not met (indeed, I
> > wasn't even aware of that prerequisite), so the commit message can be
> > more direct:
> >
> >     This was missed by the previous clean-ups due to an unmet
> >     CLONE_2GB prerequisite.
> >
> > Thanks for saving a round-trip.
> >
> >> Signed-off-by: Junio C Hamano <gitster@pobox.com>
>
> There still need a sign-off, so ... ;-)

For whose sign-off are you waiting, SZEDER's or mine? (I presume SZEDER's.)

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH 11/25] t7400: fix broken "submodule add/reconfigure --force" test
  2018-07-02  0:23 ` [PATCH 11/25] t7400: fix broken "submodule add/reconfigure --force" test Eric Sunshine
@ 2018-07-16 14:43   ` Johannes Schindelin
  2018-07-16 15:51     ` Johannes Schindelin
  0 siblings, 1 reply; 52+ messages in thread
From: Johannes Schindelin @ 2018-07-16 14:43 UTC (permalink / raw)
  To: Eric Sunshine
  Cc: git, Elijah Newren, Johannes Sixt, Jonathan Nieder, Jonathan Tan,
	Stefan Beller, Junio C Hamano, Luke Diamand, Jeff King

Hi Eric,

On Sun, 1 Jul 2018, Eric Sunshine wrote:

> This test has been dysfunctional since it was added by 619acfc78c
> (submodule add: extend force flag to add existing repos, 2016-10-06),
> however, two problems early in the test went unnoticed due to a broken
> &&-chain later in the test.
> 
> First, it tries configuring the submodule with repository "bogus-url",
> however, "git submodule add" insists that the repository be either an
> absolute URL or a relative pathname requiring prefix "./" or "../" (this
> is true even with --force), but "bogus-url" does not meet those
> criteria, thus the command fails.
> 
> Second, it then tries configuring a submodule with a path which is
> .gitignore'd, which is disallowed. This restriction can be overridden
> with --force, but the test neglects to use that option.
> 
> Fix both problems, as well as the broken &&-chain behind which they hid.
> 
> Reviewed-by: Stefan Beller <sbeller@google.com>
> Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
> ---
>  t/t7400-submodule-basic.sh | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
> index 812db137b8..401adaed32 100755
> --- a/t/t7400-submodule-basic.sh
> +++ b/t/t7400-submodule-basic.sh
> @@ -171,12 +171,12 @@ test_expect_success 'submodule add to .gitignored path with --force' '
>  test_expect_success 'submodule add to reconfigure existing submodule with --force' '
>  	(
>  		cd addtest-ignore &&
> -		git submodule add --force bogus-url submod &&
> -		git submodule add -b initial "$submodurl" submod-branch &&
> -		test "bogus-url" = "$(git config -f .gitmodules submodule.submod.url)" &&
> -		test "bogus-url" = "$(git config submodule.submod.url)" &&
> +		git submodule add --force /bogus-url submod &&
> +		git submodule add --force -b initial "$submodurl" submod-branch &&
> +		test "/bogus-url" = "$(git config -f .gitmodules submodule.submod.url)" &&
> +		test "/bogus-url" = "$(git config submodule.submod.url)" &&
>  		# Restore the url
> -		git submodule add --force "$submodurl" submod
> +		git submodule add --force "$submodurl" submod &&
>  		test "$submodurl" = "$(git config -f .gitmodules submodule.submod.url)" &&
>  		test "$submodurl" = "$(git config submodule.submod.url)"
>  	)

This breaks on Windows because of the absolute Unix-y path having to be
translated to a Windows path:

	https://git-for-windows.visualstudio.com/git/git%20Team/_build/results?buildId=12365&view=logs

(In this case, it is prefixed with `C:/git-sdk-64-ci` because that is the
pseudo root when running in a Git for Windows SDK that is installed into
that directory.)

I could imagine that using "$(pwd)/bogus-url" (which will generate a
Windows-y absolute path on Windows) would work, though.

Ciao,
Dscho

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH 11/25] t7400: fix broken "submodule add/reconfigure --force" test
  2018-07-16 14:43   ` Johannes Schindelin
@ 2018-07-16 15:51     ` Johannes Schindelin
  2018-07-16 18:50       ` Eric Sunshine
  0 siblings, 1 reply; 52+ messages in thread
From: Johannes Schindelin @ 2018-07-16 15:51 UTC (permalink / raw)
  To: Eric Sunshine
  Cc: git, Elijah Newren, Johannes Sixt, Jonathan Nieder, Jonathan Tan,
	Stefan Beller, Junio C Hamano, Luke Diamand, Jeff King

Hi Eric,

On Mon, 16 Jul 2018, Johannes Schindelin wrote:

> On Sun, 1 Jul 2018, Eric Sunshine wrote:
> 
> > This test has been dysfunctional since it was added by 619acfc78c
> > (submodule add: extend force flag to add existing repos, 2016-10-06),
> > however, two problems early in the test went unnoticed due to a broken
> > &&-chain later in the test.
> > 
> > First, it tries configuring the submodule with repository "bogus-url",
> > however, "git submodule add" insists that the repository be either an
> > absolute URL or a relative pathname requiring prefix "./" or "../" (this
> > is true even with --force), but "bogus-url" does not meet those
> > criteria, thus the command fails.
> > 
> > Second, it then tries configuring a submodule with a path which is
> > .gitignore'd, which is disallowed. This restriction can be overridden
> > with --force, but the test neglects to use that option.
> > 
> > Fix both problems, as well as the broken &&-chain behind which they hid.
> > 
> > Reviewed-by: Stefan Beller <sbeller@google.com>
> > Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
> > ---
> >  t/t7400-submodule-basic.sh | 10 +++++-----
> >  1 file changed, 5 insertions(+), 5 deletions(-)
> > 
> > diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
> > index 812db137b8..401adaed32 100755
> > --- a/t/t7400-submodule-basic.sh
> > +++ b/t/t7400-submodule-basic.sh
> > @@ -171,12 +171,12 @@ test_expect_success 'submodule add to .gitignored path with --force' '
> >  test_expect_success 'submodule add to reconfigure existing submodule with --force' '
> >  	(
> >  		cd addtest-ignore &&
> > -		git submodule add --force bogus-url submod &&
> > -		git submodule add -b initial "$submodurl" submod-branch &&
> > -		test "bogus-url" = "$(git config -f .gitmodules submodule.submod.url)" &&
> > -		test "bogus-url" = "$(git config submodule.submod.url)" &&
> > +		git submodule add --force /bogus-url submod &&
> > +		git submodule add --force -b initial "$submodurl" submod-branch &&
> > +		test "/bogus-url" = "$(git config -f .gitmodules submodule.submod.url)" &&
> > +		test "/bogus-url" = "$(git config submodule.submod.url)" &&
> >  		# Restore the url
> > -		git submodule add --force "$submodurl" submod
> > +		git submodule add --force "$submodurl" submod &&
> >  		test "$submodurl" = "$(git config -f .gitmodules submodule.submod.url)" &&
> >  		test "$submodurl" = "$(git config submodule.submod.url)"
> >  	)
> 
> This breaks on Windows because of the absolute Unix-y path having to be
> translated to a Windows path:
> 
> 	https://git-for-windows.visualstudio.com/git/git%20Team/_build/results?buildId=12365&view=logs
> 
> (In this case, it is prefixed with `C:/git-sdk-64-ci` because that is the
> pseudo root when running in a Git for Windows SDK that is installed into
> that directory.)
> 
> I could imagine that using "$(pwd)/bogus-url" (which will generate a
> Windows-y absolute path on Windows) would work, though.

Yes, this works indeed, see the patch below. Could you please squash it in
if you send another iteration of your patch series? Junio, could you
please add this as a SQUASH??? commit so that `pu` is fixed on Windows?
Thanks.

-- snipsnap --
diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
index 5ffc2726aad..2c2c97e1441 100755
--- a/t/t7400-submodule-basic.sh
+++ b/t/t7400-submodule-basic.sh
@@ -171,10 +171,11 @@ test_expect_success 'submodule add to .gitignored path with --force' '
 test_expect_success 'submodule add to reconfigure existing submodule with --force' '
 	(
 		cd addtest-ignore &&
-		git submodule add --force /bogus-url submod &&
+		bogus_url="$(pwd)/bogus-url" &&
+		git submodule add --force "$bogus_url" submod &&
 		git submodule add --force -b initial "$submodurl" submod-branch &&
-		test "/bogus-url" = "$(git config -f .gitmodules submodule.submod.url)" &&
-		test "/bogus-url" = "$(git config submodule.submod.url)" &&
+		test "$bogus_url" = "$(git config -f .gitmodules submodule.submod.url)" &&
+		test "$bogus_url" = "$(git config submodule.submod.url)" &&
 		# Restore the url
 		git submodule add --force "$submodurl" submod &&
 		test "$submodurl" = "$(git config -f .gitmodules submodule.submod.url)" &&

^ permalink raw reply related	[flat|nested] 52+ messages in thread

* Re: [PATCH 11/25] t7400: fix broken "submodule add/reconfigure --force" test
  2018-07-16 15:51     ` Johannes Schindelin
@ 2018-07-16 18:50       ` Eric Sunshine
  2018-07-16 21:37         ` Junio C Hamano
  0 siblings, 1 reply; 52+ messages in thread
From: Eric Sunshine @ 2018-07-16 18:50 UTC (permalink / raw)
  To: Johannes Schindelin
  Cc: Git List, Elijah Newren, Johannes Sixt, Jonathan Nieder,
	Jonathan Tan, Stefan Beller, Junio C Hamano, Luke Diamand,
	Jeff King

On Mon, Jul 16, 2018 at 11:51 AM Johannes Schindelin
<Johannes.Schindelin@gmx.de> wrote:
> On Mon, 16 Jul 2018, Johannes Schindelin wrote:
> > > -           git submodule add --force bogus-url submod &&
> > > +           git submodule add --force /bogus-url submod &&
> >
> > This breaks on Windows because of the absolute Unix-y path having to be
> > translated to a Windows path:
> >
> >       https://git-for-windows.visualstudio.com/git/git%20Team/_build/results?buildId=12365&view=logs
> >
> > I could imagine that using "$(pwd)/bogus-url" (which will generate a
> > Windows-y absolute path on Windows) would work, though.
>
> Yes, this works indeed, see the patch below. Could you please squash it in
> if you send another iteration of your patch series? Junio, could you
> please add this as a SQUASH??? commit so that `pu` is fixed on Windows?

Thanks for reporting and diagnosing. I wondered briefly if we could
get by with simply "./bogus-url" instead of having to use $(pwd),
however, "./bogus-url" gets normalized internally into an absolute
path, so $(pwd) is needed anyhow to make the test '"$bogus_url" =
"$(git config ...)"' check work.

So, this SQUASH looks like the correct way forward. Hopefully, Junio
can just squash it so I don't have to flood the list again with this
long series.

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH 11/25] t7400: fix broken "submodule add/reconfigure --force" test
  2018-07-16 18:50       ` Eric Sunshine
@ 2018-07-16 21:37         ` Junio C Hamano
  2018-07-16 23:05           ` Eric Sunshine
  2018-07-17 17:53           ` Junio C Hamano
  0 siblings, 2 replies; 52+ messages in thread
From: Junio C Hamano @ 2018-07-16 21:37 UTC (permalink / raw)
  To: Eric Sunshine
  Cc: Johannes Schindelin, Git List, Elijah Newren, Johannes Sixt,
	Jonathan Nieder, Jonathan Tan, Stefan Beller, Luke Diamand,
	Jeff King

Eric Sunshine <sunshine@sunshineco.com> writes:

> On Mon, Jul 16, 2018 at 11:51 AM Johannes Schindelin
> <Johannes.Schindelin@gmx.de> wrote:
>> On Mon, 16 Jul 2018, Johannes Schindelin wrote:
>> > > -           git submodule add --force bogus-url submod &&
>> > > +           git submodule add --force /bogus-url submod &&
>> >
>> > This breaks on Windows because of the absolute Unix-y path having to be
>> > translated to a Windows path:
>> >
>> >       https://git-for-windows.visualstudio.com/git/git%20Team/_build/results?buildId=12365&view=logs
>> >
>> > I could imagine that using "$(pwd)/bogus-url" (which will generate a
>> > Windows-y absolute path on Windows) would work, though.
>>
>> Yes, this works indeed, see the patch below. Could you please squash it in
>> if you send another iteration of your patch series? Junio, could you
>> please add this as a SQUASH??? commit so that `pu` is fixed on Windows?
>
> Thanks for reporting and diagnosing. I wondered briefly if we could
> get by with simply "./bogus-url" instead of having to use $(pwd),
> however, "./bogus-url" gets normalized internally into an absolute
> path, so $(pwd) is needed anyhow to make the test '"$bogus_url" =
> "$(git config ...)"' check work.
>
> So, this SQUASH looks like the correct way forward. Hopefully, Junio
> can just squash it so I don't have to flood the list again with this
> long series.

The topic already has another dependent topic so rerolling or
squashing would be a bit cumbersome.  I'll see what I could do but
it may not be until tomorrow's pushout.

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH 11/25] t7400: fix broken "submodule add/reconfigure --force" test
  2018-07-16 21:37         ` Junio C Hamano
@ 2018-07-16 23:05           ` Eric Sunshine
  2018-07-17 10:09             ` Johannes Schindelin
  2018-07-17 17:53           ` Junio C Hamano
  1 sibling, 1 reply; 52+ messages in thread
From: Eric Sunshine @ 2018-07-16 23:05 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Johannes Schindelin, Git List, Elijah Newren, Johannes Sixt,
	Jonathan Nieder, Jonathan Tan, Stefan Beller, Luke Diamand,
	Jeff King

On Mon, Jul 16, 2018 at 02:37:32PM -0700, Junio C Hamano wrote:
> Eric Sunshine <sunshine@sunshineco.com> writes:
> > On Mon, Jul 16, 2018 at 11:51 AM Johannes Schindelin
> > <Johannes.Schindelin@gmx.de> wrote:
> >> On Mon, 16 Jul 2018, Johannes Schindelin wrote:
> >> > > -           git submodule add --force bogus-url submod &&
> >> > > +           git submodule add --force /bogus-url submod &&
> >> > This breaks on Windows because of the absolute Unix-y path having to be
> >> > translated to a Windows path:
> >> > I could imagine that using "$(pwd)/bogus-url" (which will generate a
> >> > Windows-y absolute path on Windows) would work, though.
> >>
> >> Yes, this works indeed, see the patch below. Could you please squash it in
> >> if you send another iteration of your patch series? Junio, could you
> >> please add this as a SQUASH??? commit so that `pu` is fixed on Windows?
> >
> > So, this SQUASH looks like the correct way forward. Hopefully, Junio
> > can just squash it so I don't have to flood the list again with this
> > long series.
> 
> The topic already has another dependent topic so rerolling or
> squashing would be a bit cumbersome.  I'll see what I could do but
> it may not be until tomorrow's pushout.

No problem. Here's Dscho's fix in the form of a proper patch if
that makes it easier for you (it just needs his sign-off):

--- >8 ---
From: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Subject: [PATCH] t7400: make "submodule add/reconfigure --force" work on
 Windows

On Windows, the "Unixy" path /bogus-url gets translated automatically to
a Windows-style path (such as C:/git-sdk/bogus_url). This is normally
not problem, since it's still a valid and usable path in that form,
however, this test wants to do a comparison against the original path.
$(pwd) was invented exactly for this case, so use it to make the path
comparison work.

[es: commit message]

Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
---
 t/t7400-submodule-basic.sh | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
index 76cf522a08..bfb09dd566 100755
--- a/t/t7400-submodule-basic.sh
+++ b/t/t7400-submodule-basic.sh
@@ -171,10 +171,11 @@ test_expect_success 'submodule add to .gitignored path with --force' '
 test_expect_success 'submodule add to reconfigure existing submodule with --force' '
 	(
 		cd addtest-ignore &&
-		git submodule add --force /bogus-url submod &&
+		bogus_url="$(pwd)/bogus-url" &&
+		git submodule add --force "$bogus_url" submod &&
 		git submodule add --force -b initial "$submodurl" submod-branch &&
-		test "/bogus-url" = "$(git config -f .gitmodules submodule.submod.url)" &&
-		test "/bogus-url" = "$(git config submodule.submod.url)" &&
+		test "$bogus_url" = "$(git config -f .gitmodules submodule.submod.url)" &&
+		test "$bogus_url" = "$(git config submodule.submod.url)" &&
 		# Restore the url
 		git submodule add --force "$submodurl" submod &&
 		test "$submodurl" = "$(git config -f .gitmodules submodule.submod.url)" &&
-- 
2.18.0.233.g985f88cf7e

^ permalink raw reply related	[flat|nested] 52+ messages in thread

* Re: [PATCH 21/25] t5000-t5999: fix broken &&-chains
  2018-07-12 18:35         ` Junio C Hamano
  2018-07-12 18:46           ` Eric Sunshine
@ 2018-07-17  8:15           ` SZEDER Gábor
  1 sibling, 0 replies; 52+ messages in thread
From: SZEDER Gábor @ 2018-07-17  8:15 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Eric Sunshine, Git mailing list, Elijah Newren, Johannes Sixt,
	Jonathan Nieder, Jonathan Tan, Stefan Beller, Luke Diamand,
	Jeff King

On Thu, Jul 12, 2018 at 8:35 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> Junio C Hamano <gitster@pobox.com> writes:
>
> > Eric Sunshine <sunshine@sunshineco.com> writes:
> >
> >> On Thu, Jul 12, 2018 at 8:37 AM SZEDER Gábor <szeder.dev@gmail.com> wrote:
> >>> The change below should be squashed into this patch to fix a
> >>> previously unnoticed broken &&-chain.  I think you missed it, because
> >>> this test script is rather expensive and you didn't run it with
> >>> GIT_TEST_CLONE_2GB=YesPlease.
> >>>
> >>> diff --git a/t/t5608-clone-2gb.sh b/t/t5608-clone-2gb.sh
> >>> @@ -23,7 +23,7 @@ test_expect_success CLONE_2GB 'setup' '
> >>> -               echo "M 100644 :$i $i" >> commit
> >>> +               echo "M 100644 :$i $i" >> commit &&
> >>
> >> Thanks for finding this. I tried to get as much coverage as possible
> >> by installing packages I don't normally have installed (Apache, cvs,
> >> cvsps, Subversion, Perforce, etc.) and even temporarily modified a
> >> script or two to force it run ...
> >
> > Thanks, both.  I think the &&-chain fix series is already large and
> > also otherwise seem to be pretty solid, so let's not reroll but
> > queue this addition on top.
>
> Oops, sent before completing the message.
>
>         For that to happen, we need a sign-off ;-)
>
> I guess this one would have been caught with the "sed script on
> subshell" linter that does not execute?

It was in fact caught by Eric's linter (Travis CI runs these expensive
tests, too), so I think it would be more appropriate to give him the
credit.

Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>


> -- >8 --
> Subject: t5608: fix broken &&-chain
>
> This is inside a loop that is run inside a subshell, in a test that
> is protected with CLONE_2GB prerequisite, one or more which is quite
> likely reason why it wasn't caught durin the previous clean-up.
>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
>  t/t5608-clone-2gb.sh | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/t/t5608-clone-2gb.sh b/t/t5608-clone-2gb.sh
> index df822d9a3e..2c6bc07344 100755
> --- a/t/t5608-clone-2gb.sh
> +++ b/t/t5608-clone-2gb.sh
> @@ -23,7 +23,7 @@ test_expect_success CLONE_2GB 'setup' '
>                 printf "blob\nmark :$i\ndata $blobsize\n" &&
>                 #test-tool genrandom $i $blobsize &&
>                 printf "%-${blobsize}s" $i &&
> -               echo "M 100644 :$i $i" >> commit
> +               echo "M 100644 :$i $i" >> commit &&
>                 i=$(($i+1)) ||
>                 echo $? > exit-status
>          done &&

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH 11/25] t7400: fix broken "submodule add/reconfigure --force" test
  2018-07-16 23:05           ` Eric Sunshine
@ 2018-07-17 10:09             ` Johannes Schindelin
  0 siblings, 0 replies; 52+ messages in thread
From: Johannes Schindelin @ 2018-07-17 10:09 UTC (permalink / raw)
  To: Eric Sunshine
  Cc: Junio C Hamano, Git List, Elijah Newren, Johannes Sixt,
	Jonathan Nieder, Jonathan Tan, Stefan Beller, Luke Diamand,
	Jeff King

Hi Eric,

On Mon, 16 Jul 2018, Eric Sunshine wrote:

> On Mon, Jul 16, 2018 at 02:37:32PM -0700, Junio C Hamano wrote:
> > Eric Sunshine <sunshine@sunshineco.com> writes:
> > > On Mon, Jul 16, 2018 at 11:51 AM Johannes Schindelin
> > > <Johannes.Schindelin@gmx.de> wrote:
> > >> On Mon, 16 Jul 2018, Johannes Schindelin wrote:
> > >> > > -           git submodule add --force bogus-url submod &&
> > >> > > +           git submodule add --force /bogus-url submod &&
> > >> > This breaks on Windows because of the absolute Unix-y path having to be
> > >> > translated to a Windows path:
> > >> > I could imagine that using "$(pwd)/bogus-url" (which will generate a
> > >> > Windows-y absolute path on Windows) would work, though.
> > >>
> > >> Yes, this works indeed, see the patch below. Could you please squash it in
> > >> if you send another iteration of your patch series? Junio, could you
> > >> please add this as a SQUASH??? commit so that `pu` is fixed on Windows?
> > >
> > > So, this SQUASH looks like the correct way forward. Hopefully, Junio
> > > can just squash it so I don't have to flood the list again with this
> > > long series.
> > 
> > The topic already has another dependent topic so rerolling or
> > squashing would be a bit cumbersome.  I'll see what I could do but
> > it may not be until tomorrow's pushout.
> 
> No problem. Here's Dscho's fix in the form of a proper patch if
> that makes it easier for you (it just needs his sign-off):
> 
> --- >8 ---
> From: Johannes Schindelin <Johannes.Schindelin@gmx.de>
> Subject: [PATCH] t7400: make "submodule add/reconfigure --force" work on
>  Windows
> 
> On Windows, the "Unixy" path /bogus-url gets translated automatically to
> a Windows-style path (such as C:/git-sdk/bogus_url). This is normally
> not problem, since it's still a valid and usable path in that form,

s/not problem/not a problem/

> however, this test wants to do a comparison against the original path.
> $(pwd) was invented exactly for this case, so use it to make the path
> comparison work.
> 
> [es: commit message]
> 

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>

> Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
> ---
>  t/t7400-submodule-basic.sh | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
> index 76cf522a08..bfb09dd566 100755
> --- a/t/t7400-submodule-basic.sh
> +++ b/t/t7400-submodule-basic.sh
> @@ -171,10 +171,11 @@ test_expect_success 'submodule add to .gitignored path with --force' '
>  test_expect_success 'submodule add to reconfigure existing submodule with --force' '
>  	(
>  		cd addtest-ignore &&
> -		git submodule add --force /bogus-url submod &&
> +		bogus_url="$(pwd)/bogus-url" &&
> +		git submodule add --force "$bogus_url" submod &&
>  		git submodule add --force -b initial "$submodurl" submod-branch &&
> -		test "/bogus-url" = "$(git config -f .gitmodules submodule.submod.url)" &&
> -		test "/bogus-url" = "$(git config submodule.submod.url)" &&
> +		test "$bogus_url" = "$(git config -f .gitmodules submodule.submod.url)" &&
> +		test "$bogus_url" = "$(git config submodule.submod.url)" &&
>  		# Restore the url
>  		git submodule add --force "$submodurl" submod &&
>  		test "$submodurl" = "$(git config -f .gitmodules submodule.submod.url)" &&
> -- 
> 2.18.0.233.g985f88cf7e

Thanks!
Dscho

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH 11/25] t7400: fix broken "submodule add/reconfigure --force" test
  2018-07-16 21:37         ` Junio C Hamano
  2018-07-16 23:05           ` Eric Sunshine
@ 2018-07-17 17:53           ` Junio C Hamano
  1 sibling, 0 replies; 52+ messages in thread
From: Junio C Hamano @ 2018-07-17 17:53 UTC (permalink / raw)
  To: Eric Sunshine
  Cc: Johannes Schindelin, Git List, Elijah Newren, Johannes Sixt,
	Jonathan Nieder, Jonathan Tan, Stefan Beller, Luke Diamand,
	Jeff King

Junio C Hamano <gitster@pobox.com> writes:

>> So, this SQUASH looks like the correct way forward. Hopefully, Junio
>> can just squash it so I don't have to flood the list again with this
>> long series.
>
> The topic already has another dependent topic so rerolling or
> squashing would be a bit cumbersome.  I'll see what I could do but
> it may not be until tomorrow's pushout.

OK, there was only one topic that forked fro this one, so squash
with rebase are all in the 'pu'.

Thanks, all.

^ permalink raw reply	[flat|nested] 52+ messages in thread

end of thread, other threads:[~2018-07-17 17:53 UTC | newest]

Thread overview: 52+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-07-02  0:23 [PATCH 00/25] fix buggy tests, modernize tests, fix broken &&-chains Eric Sunshine
2018-07-02  0:23 ` [PATCH 01/25] t: use test_might_fail() instead of manipulating exit code manually Eric Sunshine
2018-07-02 17:44   ` Stefan Beller
2018-07-02 20:58     ` Eric Sunshine
2018-07-03 19:20   ` Junio C Hamano
2018-07-02  0:23 ` [PATCH 02/25] t: use test_write_lines() instead of series of 'echo' commands Eric Sunshine
2018-07-02 17:53   ` Stefan Beller
2018-07-02  0:23 ` [PATCH 03/25] t: use sane_unset() rather than 'unset' with broken &&-chain Eric Sunshine
2018-07-02  0:23 ` [PATCH 04/25] t: drop unnecessary terminating semicolon in subshell Eric Sunshine
2018-07-03 19:23   ` Junio C Hamano
2018-07-02  0:23 ` [PATCH 05/25] t/lib-submodule-update: fix "absorbing" test Eric Sunshine
2018-07-02  0:23 ` [PATCH 06/25] t5405: use test_must_fail() instead of checking exit code manually Eric Sunshine
2018-07-02  0:23 ` [PATCH 07/25] t5406: use write_script() instead of birthing shell script manually Eric Sunshine
2018-07-02  0:23 ` [PATCH 08/25] t5505: modernize and simplify hard-to-digest test Eric Sunshine
2018-07-02  0:23 ` [PATCH 09/25] t6036: fix broken "merge fails but has appropriate contents" tests Eric Sunshine
2018-07-02  0:23 ` [PATCH 10/25] t7201: drop pointless "exit 0" at end of subshell Eric Sunshine
2018-07-02  0:23 ` [PATCH 11/25] t7400: fix broken "submodule add/reconfigure --force" test Eric Sunshine
2018-07-16 14:43   ` Johannes Schindelin
2018-07-16 15:51     ` Johannes Schindelin
2018-07-16 18:50       ` Eric Sunshine
2018-07-16 21:37         ` Junio C Hamano
2018-07-16 23:05           ` Eric Sunshine
2018-07-17 10:09             ` Johannes Schindelin
2018-07-17 17:53           ` Junio C Hamano
2018-07-02  0:23 ` [PATCH 12/25] t7810: use test_expect_code() instead of hand-rolled comparison Eric Sunshine
2018-07-02 18:05   ` Stefan Beller
2018-07-02 21:12     ` Eric Sunshine
2018-07-02  0:23 ` [PATCH 13/25] t9001: fix broken "invoke hook" test Eric Sunshine
2018-07-02  0:23 ` [PATCH 14/25] t9814: simplify convoluted check that command correctly errors out Eric Sunshine
2018-07-02  0:23 ` [PATCH 15/25] t0000-t0999: fix broken &&-chains Eric Sunshine
2018-07-02  0:23 ` [PATCH 16/25] t1000-t1999: " Eric Sunshine
2018-07-02  0:23 ` [PATCH 17/25] t2000-t2999: " Eric Sunshine
2018-07-02  0:23 ` [PATCH 18/25] t3000-t3999: " Eric Sunshine
2018-07-02  0:23 ` [PATCH 19/25] t3030: " Eric Sunshine
2018-07-02  0:24 ` [PATCH 20/25] t4000-t4999: " Eric Sunshine
2018-07-02  0:24 ` [PATCH 21/25] t5000-t5999: " Eric Sunshine
2018-07-12 12:37   ` SZEDER Gábor
2018-07-12 17:44     ` Eric Sunshine
2018-07-12 18:19       ` Jeff King
2018-07-12 18:31       ` Junio C Hamano
2018-07-12 18:35         ` Junio C Hamano
2018-07-12 18:46           ` Eric Sunshine
2018-07-12 18:50             ` Junio C Hamano
2018-07-12 18:53               ` Eric Sunshine
2018-07-17  8:15           ` SZEDER Gábor
2018-07-02  0:24 ` [PATCH 22/25] t6000-t6999: " Eric Sunshine
2018-07-02  0:24 ` [PATCH 23/25] t7000-t7999: " Eric Sunshine
2018-07-02  0:24 ` [PATCH 24/25] t9000-t9999: " Eric Sunshine
2018-07-02  0:24 ` [PATCH 25/25] t9119: " Eric Sunshine
2018-07-02 18:27 ` [PATCH 00/25] fix buggy tests, modernize tests, " Stefan Beller
2018-07-02 21:20   ` Eric Sunshine
2018-07-03 19:40 ` Junio C Hamano

Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

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 read-only IMAP folder(s) and NNTP newsgroup(s).