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@xxxxxxxxxxxxxx/
[2]: https://public-inbox.org/git/20180627183057.254467-1-sbeller@xxxxxxxxxx/
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
)
'
