| Message ID | 20211209051115.52629-13-sunshine@sunshineco.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 7abcbcb7ea4303adffd169d5367ce70904e79bf5 |
| Headers | show |
| Series | tests: fix broken &&-chains & abort loops on error | expand |
On Thu, Dec 09, 2021 at 12:11:08AM -0500, Eric Sunshine wrote: > The top-level &&-chain checker built into t/test-lib.sh causes tests to > magically exit with code 117 if the &&-chain is broken. However, it has > the shortcoming that the magic does not work within `{...}` groups, > `(...)` subshells, `$(...)` substitutions, or within bodies of compound > statements, such as `if`, `for`, `while`, `case`, etc. `chainlint.sed` > partly fills in the gap by catching broken &&-chains in `(...)` > subshells, but bugs can still lurk behind broken &&-chains in the other > cases. > > Fix broken &&-chains in `{...}` groups in order to reduce the number of > possible lurking bugs. Seems good. This is mostly stuff we don't expect to fail (mostly "echo"), so I doubt they're important on their own. But getting a clean state for the linter _is_ important. -Peff
On 09.12.2021 00:11, Eric Sunshine wrote: >The top-level &&-chain checker built into t/test-lib.sh causes tests to >magically exit with code 117 if the &&-chain is broken. However, it has >the shortcoming that the magic does not work within `{...}` groups, >`(...)` subshells, `$(...)` substitutions, or within bodies of compound >statements, such as `if`, `for`, `while`, `case`, etc. `chainlint.sed` >partly fills in the gap by catching broken &&-chains in `(...)` >subshells, but bugs can still lurk behind broken &&-chains in the other >cases. > >Fix broken &&-chains in `{...}` groups in order to reduce the number of >possible lurking bugs. > >Signed-off-by: Eric Sunshine <sunshine@sunshineco.com> >--- > >diff --git a/t/t0021-conversion.sh b/t/t0021-conversion.sh >index 1a1a69ad92..bb3de2701a 100755 >--- a/t/t0021-conversion.sh >+++ b/t/t0021-conversion.sh >@@ -76,13 +76,13 @@ test_expect_success setup ' > git config filter.rot13.clean ./rot13.sh && > > { >- echo "*.t filter=rot13" >+ echo "*.t filter=rot13" && > echo "*.i ident" > } >.gitattributes && > > { >- echo a b c d e f g h i j k l m >- echo n o p q r s t u v w x y z >+ echo a b c d e f g h i j k l m && >+ echo n o p q r s t u v w x y z && > echo '\''$Id$'\'' > } >test && > cat test >test.t && >@@ -159,7 +159,7 @@ test_expect_success expanded_in_repo ' > printf "\$Id: NoTerminatingSymbolAtEOF" > } >expected-output-crlf && > { >- echo "expanded-keywords ident" >+ echo "expanded-keywords ident" && > echo "expanded-keywords-crlf ident text eol=crlf" > } >>.gitattributes && > Wouldn't some of these be better off as heredocs as well? There are a couple more below. I personally don't much mind either way but since you changed quite a few in an earlier commit why not these? >diff --git a/t/t0069-oidtree.sh b/t/t0069-oidtree.sh >index 74cc59bf8a..889db50818 100755 >--- a/t/t0069-oidtree.sh >+++ b/t/t0069-oidtree.sh >@@ -28,7 +28,7 @@ test_expect_success 'oidtree insert and contains' ' > EOF > { > echoid insert 444 1 2 3 4 5 a b c d e && >- echoid contains 44 441 440 444 4440 4444 >+ echoid contains 44 441 440 444 4440 4444 && > echo clear > } | test-tool oidtree >actual && > test_cmp expect actual >@@ -37,11 +37,11 @@ test_expect_success 'oidtree insert and contains' ' > test_expect_success 'oidtree each' ' > echoid "" 123 321 321 >expect && > { >- echoid insert f 9 8 123 321 a b c d e >- echo each 12300 >- echo each 3211 >- echo each 3210 >- echo each 32100 >+ echoid insert f 9 8 123 321 a b c d e && >+ echo each 12300 && >+ echo each 3211 && >+ echo each 3210 && >+ echo each 32100 && > echo clear > } | test-tool oidtree >actual && > test_cmp expect actual >diff --git a/t/t1006-cat-file.sh b/t/t1006-cat-file.sh >index 67a3f64c2d..f6f00c7039 100755 >--- a/t/t1006-cat-file.sh >+++ b/t/t1006-cat-file.sh >@@ -283,7 +283,7 @@ test_expect_success "--batch-check with multiple sha1s gives correct format" ' > > test_expect_success 'setup blobs which are likely to delta' ' > test-tool genrandom foo 10240 >foo && >- { cat foo; echo plus; } >foo-plus && >+ { cat foo && echo plus; } >foo-plus && > git add foo foo-plus && > git commit -m foo && > cat >blobs <<-\EOF >diff --git a/t/t1300-config.sh b/t/t1300-config.sh >index 9571649c42..516dd8bfa8 100755 >--- a/t/t1300-config.sh >+++ b/t/t1300-config.sh >@@ -901,7 +901,7 @@ test_expect_success 'get --expiry-date' ' > EOF > : "work around heredoc parsing bug fixed in dash 0.5.7 (in ec2c84d)" && > { >- echo "$rel_out $(git config --expiry-date date.valid1)" >+ echo "$rel_out $(git config --expiry-date date.valid1)" && > git config --expiry-date date.valid2 && > git config --expiry-date date.valid3 && > git config --expiry-date date.valid4 && >diff --git a/t/t1403-show-ref.sh b/t/t1403-show-ref.sh >index 17d3cc1405..bbc01aae34 100755 >--- a/t/t1403-show-ref.sh >+++ b/t/t1403-show-ref.sh >@@ -78,7 +78,7 @@ test_expect_success 'show-ref --verify -q' ' > test_expect_success 'show-ref -d' ' > { > echo $(git rev-parse refs/tags/A) refs/tags/A && >- echo $(git rev-parse refs/tags/A^0) "refs/tags/A^{}" >+ echo $(git rev-parse refs/tags/A^0) "refs/tags/A^{}" && > echo $(git rev-parse refs/tags/C) refs/tags/C > } >expect && > git show-ref -d A C >actual && >@@ -148,7 +148,7 @@ test_expect_success 'show-ref --heads, --tags, --head, pattern' ' > > { > echo $(git rev-parse HEAD) HEAD && >- echo $(git rev-parse refs/heads/B) refs/heads/B >+ echo $(git rev-parse refs/heads/B) refs/heads/B && > echo $(git rev-parse refs/tags/B) refs/tags/B > } >expect && > git show-ref --head B >actual && >@@ -156,8 +156,8 @@ test_expect_success 'show-ref --heads, --tags, --head, pattern' ' > > { > echo $(git rev-parse HEAD) HEAD && >- echo $(git rev-parse refs/heads/B) refs/heads/B >- echo $(git rev-parse refs/tags/B) refs/tags/B >+ echo $(git rev-parse refs/heads/B) refs/heads/B && >+ echo $(git rev-parse refs/tags/B) refs/tags/B && > echo $(git rev-parse refs/tags/B^0) "refs/tags/B^{}" > } >expect && > git show-ref --head -d B >actual && >diff --git a/t/t2200-add-update.sh b/t/t2200-add-update.sh >index 67b9cc752f..d2ef0041f9 100755 >--- a/t/t2200-add-update.sh >+++ b/t/t2200-add-update.sh >@@ -153,10 +153,10 @@ test_expect_success 'add -u resolves unmerged paths' ' > echo "100644 $one 1 $path" && > echo "100644 $two 2 $path" && > echo "100644 $three 3 $path" >- done >- echo "100644 $one 1 path3" >- echo "100644 $one 1 path4" >- echo "100644 $one 3 path5" >+ done && >+ echo "100644 $one 1 path3" && >+ echo "100644 $one 1 path4" && >+ echo "100644 $one 3 path5" && > echo "100644 $one 3 path6" > } | > git update-index --index-info && >@@ -173,8 +173,8 @@ test_expect_success 'add -u resolves unmerged paths' ' > git add -u && > git ls-files -s path1 path2 path3 path4 path5 path6 >actual && > { >- echo "100644 $three 0 path1" >- echo "100644 $two 0 path3" >+ echo "100644 $three 0 path1" && >+ echo "100644 $two 0 path3" && > echo "100644 $two 0 path5" > } >expect && > test_cmp expect actual >diff --git a/t/t2201-add-update-typechange.sh b/t/t2201-add-update-typechange.sh >index a4eec0a346..687be974d4 100755 >--- a/t/t2201-add-update-typechange.sh >+++ b/t/t2201-add-update-typechange.sh >@@ -97,17 +97,17 @@ test_expect_success modify ' > " > } >expect && > { >- cat expect >- echo ":100644 160000 $_empty $ZERO_OID T yonk" >+ cat expect && >+ echo ":100644 160000 $_empty $ZERO_OID T yonk" && > echo ":100644 000000 $_empty $ZERO_OID D zifmia" > } >expect-files && > { >- cat expect >+ cat expect && > echo ":000000 160000 $ZERO_OID $ZERO_OID A yonk" > } >expect-index && > { >- echo "100644 $_empty 0 nitfol" >- echo "160000 $yomin 0 yomin" >+ echo "100644 $_empty 0 nitfol" && >+ echo "160000 $yomin 0 yomin" && > echo "160000 $yonk 0 yonk" > } >expect-final > ' >diff --git a/t/t4023-diff-rename-typechange.sh b/t/t4023-diff-rename-typechange.sh >index 47d6f35dcc..7cb9909293 100755 >--- a/t/t4023-diff-rename-typechange.sh >+++ b/t/t4023-diff-rename-typechange.sh >@@ -55,7 +55,7 @@ test_expect_success 'cross renames to be detected for regular files' ' > > git diff-tree five six -r --name-status -B -M | sort >actual && > { >- echo "R100 foo bar" >+ echo "R100 foo bar" && > echo "R100 bar foo" > } | sort >expect && > test_cmp expect actual >@@ -66,7 +66,7 @@ test_expect_success 'cross renames to be detected for typechange' ' > > git diff-tree one two -r --name-status -B -M | sort >actual && > { >- echo "R100 foo bar" >+ echo "R100 foo bar" && > echo "R100 bar foo" > } | sort >expect && > test_cmp expect actual >@@ -78,7 +78,7 @@ test_expect_success 'moves and renames' ' > git diff-tree three four -r --name-status -B -M | sort >actual && > { > # see -B -M (#6) in t4008 >- echo "C100 foo bar" >+ echo "C100 foo bar" && > echo "T100 foo" > } | sort >expect && > test_cmp expect actual >diff --git a/t/t4124-apply-ws-rule.sh b/t/t4124-apply-ws-rule.sh >index ebff6c6883..ec5c10d2a0 100755 >--- a/t/t4124-apply-ws-rule.sh >+++ b/t/t4124-apply-ws-rule.sh >@@ -233,7 +233,7 @@ test_expect_success 'blank at EOF with --whitespace=fix (1)' ' > test_write_lines a b c >one && > git add one && > test_write_lines a b c >expect && >- { cat expect; echo; } >one && >+ { cat expect && echo; } >one && > git diff -- one >patch && > > git checkout one && >diff --git a/t/t4150-am.sh b/t/t4150-am.sh >index 2aaaa0d7de..103cd39148 100755 >--- a/t/t4150-am.sh >+++ b/t/t4150-am.sh >@@ -116,7 +116,7 @@ test_expect_success setup ' > git format-patch --stdout first | sed -e "1d" > } | append_cr >patch1-crlf.eml && > { >- printf "%255s\\n" "" >+ printf "%255s\\n" "" && > echo "X-Fake-Field: Line One" && > echo "X-Fake-Field: Line Two" && > echo "X-Fake-Field: Line Three" && >diff --git a/t/t4212-log-corrupt.sh b/t/t4212-log-corrupt.sh >index 03b952c90d..0244888a5a 100755 >--- a/t/t4212-log-corrupt.sh >+++ b/t/t4212-log-corrupt.sh >@@ -20,10 +20,10 @@ test_expect_success 'fsck notices broken commit' ' > > test_expect_success 'git log with broken author email' ' > { >- echo commit $(cat broken_email.hash) >- echo "Author: A U Thor <author@example.com>" >- echo "Date: Thu Apr 7 15:13:13 2005 -0700" >- echo >+ echo commit $(cat broken_email.hash) && >+ echo "Author: A U Thor <author@example.com>" && >+ echo "Date: Thu Apr 7 15:13:13 2005 -0700" && >+ echo && > echo " foo" > } >expect.out && > >diff --git a/t/t5316-pack-delta-depth.sh b/t/t5316-pack-delta-depth.sh >index 759169d074..df524f7b6d 100755 >--- a/t/t5316-pack-delta-depth.sh >+++ b/t/t5316-pack-delta-depth.sh >@@ -57,8 +57,11 @@ test_expect_success 'create series of packs' ' > git commit -m $i && > cur=$(git rev-parse HEAD^{tree}) && > { >- test -n "$prev" && echo "-$prev" >- echo $cur >+ if test -n "$prev" >+ then >+ echo "-$prev" >+ fi && >+ echo $cur && > echo "$(git rev-parse :file) file" > } | git pack-objects --stdout >tmp && > git index-pack --stdin --fix-thin <tmp || return 1 >diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh >index 1892d6615a..01468ce6d8 100755 >--- a/t/t5510-fetch.sh >+++ b/t/t5510-fetch.sh >@@ -71,7 +71,7 @@ test_expect_success "fetch test for-merge" ' > main_in_two=$(cd ../two && git rev-parse main) && > one_in_two=$(cd ../two && git rev-parse one) && > { >- echo "$one_in_two " >+ echo "$one_in_two " && > echo "$main_in_two not-for-merge" > } >expected && > cut -f -2 .git/FETCH_HEAD >actual && >diff --git a/t/t5515-fetch-merge-logic.sh b/t/t5515-fetch-merge-logic.sh >index 9d440e2821..c69cfd5c64 100755 >--- a/t/t5515-fetch-merge-logic.sh >+++ b/t/t5515-fetch-merge-logic.sh >@@ -191,17 +191,17 @@ do > cp "$expect_r" expect_r && > convert_expected expect_r sed_script && > { >- echo "# $cmd" >- set x $cmd; shift >- git symbolic-ref HEAD refs/heads/$1 ; shift >- rm -f .git/FETCH_HEAD >+ echo "# $cmd" && >+ set x $cmd && shift && >+ git symbolic-ref HEAD refs/heads/$1 && shift && >+ rm -f .git/FETCH_HEAD && > git for-each-ref \ > refs/heads refs/remotes/rem refs/tags | > while read val type refname > do > git update-ref -d "$refname" "$val" >- done >- git fetch "$@" >/dev/null >+ done && >+ git fetch "$@" >/dev/null && > cat .git/FETCH_HEAD > } >"$actual_f" && > git show-ref >"$actual_r" && >diff --git a/t/t5562-http-backend-content-length.sh b/t/t5562-http-backend-content-length.sh >index 05a58069b0..b68ec22d3f 100755 >--- a/t/t5562-http-backend-content-length.sh >+++ b/t/t5562-http-backend-content-length.sh >@@ -63,7 +63,7 @@ test_expect_success 'setup' ' > hash_next=$(git commit-tree -p HEAD -m next HEAD^{tree}) && > { > printf "%s %s refs/heads/newbranch\\0report-status object-format=%s\\n" \ >- "$ZERO_OID" "$hash_next" "$(test_oid algo)" | packetize_raw >+ "$ZERO_OID" "$hash_next" "$(test_oid algo)" | packetize_raw && > printf 0000 && > echo "$hash_next" | git pack-objects --stdout > } >push_body && >diff --git a/t/t5570-git-daemon.sh b/t/t5570-git-daemon.sh >index b87ca06a58..1131503b76 100755 >--- a/t/t5570-git-daemon.sh >+++ b/t/t5570-git-daemon.sh >@@ -194,7 +194,7 @@ test_expect_success 'hostname cannot break out of directory' ' > > test_expect_success FAKENC 'hostname interpolation works after LF-stripping' ' > { >- printf "git-upload-pack /interp.git\n\0host=localhost" | packetize_raw >+ printf "git-upload-pack /interp.git\n\0host=localhost" | packetize_raw && > printf "0000" > } >input && > fake_nc "$GIT_DAEMON_HOST_PORT" <input >output && >diff --git a/t/t5571-pre-push-hook.sh b/t/t5571-pre-push-hook.sh >index b043a279f1..80e86d8284 100755 >--- a/t/t5571-pre-push-hook.sh >+++ b/t/t5571-pre-push-hook.sh >@@ -114,7 +114,7 @@ test_expect_success 'push to URL' ' > > test_expect_success 'set up many-ref tests' ' > { >- nr=1000 >+ nr=1000 && > while test $nr -lt 2000 > do > nr=$(( $nr + 1 )) && >diff --git a/t/t7513-interpret-trailers.sh b/t/t7513-interpret-trailers.sh >index 04885d0a5e..97f10905d2 100755 >--- a/t/t7513-interpret-trailers.sh >+++ b/t/t7513-interpret-trailers.sh >@@ -156,7 +156,7 @@ test_expect_success 'with config option on the command line' ' > Acked-by: Johan > Reviewed-by: Peff > EOF >- { echo; echo "Acked-by: Johan"; } | >+ { echo && echo "Acked-by: Johan"; } | > git -c "trailer.Acked-by.ifexists=addifdifferent" interpret-trailers \ > --trailer "Reviewed-by: Peff" --trailer "Acked-by: Johan" >actual && > test_cmp expected actual >diff --git a/t/t8002-blame.sh b/t/t8002-blame.sh >index 5bb302b1ba..ee4fdd8f18 100755 >--- a/t/t8002-blame.sh >+++ b/t/t8002-blame.sh >@@ -97,7 +97,7 @@ test_expect_success 'set up abbrev tests' ' > test_commit abbrev && > sha1=$(git rev-parse --verify HEAD) && > check_abbrev () { >- expect=$1; shift >+ expect=$1 && shift && > echo $sha1 | cut -c 1-$expect >expect && > git blame "$@" abbrev.t >actual && > perl -lne "/[0-9a-f]+/ and print \$&" <actual >actual.sha && >-- >2.34.1.307.g9b7440fafd >
On Fri, Dec 10, 2021 at 4:29 AM Jeff King <peff@peff.net> wrote: > On Thu, Dec 09, 2021 at 12:11:08AM -0500, Eric Sunshine wrote: > > Fix broken &&-chains in `{...}` groups in order to reduce the number of > > possible lurking bugs. > > Seems good. This is mostly stuff we don't expect to fail (mostly > "echo"), so I doubt they're important on their own. But getting a clean > state for the linter _is_ important. In this patch, I think the only &&-chain which really matters (i.e. could hide a genuine failure if broken) is t5515-fetch-merge-logic.sh. Aside from most of these being unlikely to fail (`echo`), most of the exit codes are being lost down pipes anyhow. As such, I had a hard time justifying this patch since it exists mostly to satisfy the linter which isn't smart enough to distinguish between the cases. What ultimately convinced me that this patch was worthwhile was (1) that there are legitimate cases, such as t5515-fetch-merge-logic.sh, where we really do want to be told about the broken &&-chain, and (2) that it's easier to have a single simple rule which we can point test authors at ("chain all your test commands with `&&`") rather than complex rules laying out cases when you do and don't need to maintain the &&-chain.
On Fri, Dec 10, 2021 at 4:38 AM Fabian Stelzer <fs@gigacodes.de> wrote: > On 09.12.2021 00:11, Eric Sunshine wrote: > >Fix broken &&-chains in `{...}` groups in order to reduce the number of > >possible lurking bugs. > > { > >- echo "*.t filter=rot13" > >+ echo "*.t filter=rot13" && > > echo "*.i ident" > > } >.gitattributes && > > { > >- echo "expanded-keywords ident" > >+ echo "expanded-keywords ident" && > > echo "expanded-keywords-crlf ident text eol=crlf" > > } >>.gitattributes && > > > > Wouldn't some of these be better off as heredocs as well? > There are a couple more below. I personally don't much mind either way but > since you changed quite a few in an earlier commit why not these? It's been months since I made these changes, but I think there were at least a couple reasons for not converting these to here-docs. First, in these cases, there were only one or two missing `&&` per block. Had I bulk converted them to here-docs, it would have made for a much more noisy patch, which would have taxed reviewers more, and reviewer-fatigue is a real concern when crafting a lengthy patch series like this one. In the "here-doc conversion" patch, on the other hand, many of those cases involved a significant number of missing `&&`; often every line was missing `&&`. So, the changes in that patch was going to be very noisy anyhow, whether I added missing `&&` or converted to here-docs. Second... > > { > > echoid insert 444 1 2 3 4 5 a b c d e && > >- echoid contains 44 441 440 444 4440 4444 > >+ echoid contains 44 441 440 444 4440 4444 && > > echo clear > > } | test-tool oidtree >actual && ... there are a number of cases like this which look like they could easily be converted to here-doc, but in fact `echoid` is a function call, so a here-doc wouldn't work. Also... > > { > > echo $(git rev-parse refs/tags/A) refs/tags/A && > >- echo $(git rev-parse refs/tags/A^0) "refs/tags/A^{}" > >+ echo $(git rev-parse refs/tags/A^0) "refs/tags/A^{}" && > > echo $(git rev-parse refs/tags/C) refs/tags/C > > } >expect && ... this sort of thing could certainly become a here-doc because $(...) will work in a here-doc, but when there is a preponderance of this sort of `{ echo && ... }` block in the test script, it would feel inconsistent to convert a few of them to here-docs.
diff --git a/t/t0021-conversion.sh b/t/t0021-conversion.sh index 1a1a69ad92..bb3de2701a 100755 --- a/t/t0021-conversion.sh +++ b/t/t0021-conversion.sh @@ -76,13 +76,13 @@ test_expect_success setup ' git config filter.rot13.clean ./rot13.sh && { - echo "*.t filter=rot13" + echo "*.t filter=rot13" && echo "*.i ident" } >.gitattributes && { - echo a b c d e f g h i j k l m - echo n o p q r s t u v w x y z + echo a b c d e f g h i j k l m && + echo n o p q r s t u v w x y z && echo '\''$Id$'\'' } >test && cat test >test.t && @@ -159,7 +159,7 @@ test_expect_success expanded_in_repo ' printf "\$Id: NoTerminatingSymbolAtEOF" } >expected-output-crlf && { - echo "expanded-keywords ident" + echo "expanded-keywords ident" && echo "expanded-keywords-crlf ident text eol=crlf" } >>.gitattributes && diff --git a/t/t0069-oidtree.sh b/t/t0069-oidtree.sh index 74cc59bf8a..889db50818 100755 --- a/t/t0069-oidtree.sh +++ b/t/t0069-oidtree.sh @@ -28,7 +28,7 @@ test_expect_success 'oidtree insert and contains' ' EOF { echoid insert 444 1 2 3 4 5 a b c d e && - echoid contains 44 441 440 444 4440 4444 + echoid contains 44 441 440 444 4440 4444 && echo clear } | test-tool oidtree >actual && test_cmp expect actual @@ -37,11 +37,11 @@ test_expect_success 'oidtree insert and contains' ' test_expect_success 'oidtree each' ' echoid "" 123 321 321 >expect && { - echoid insert f 9 8 123 321 a b c d e - echo each 12300 - echo each 3211 - echo each 3210 - echo each 32100 + echoid insert f 9 8 123 321 a b c d e && + echo each 12300 && + echo each 3211 && + echo each 3210 && + echo each 32100 && echo clear } | test-tool oidtree >actual && test_cmp expect actual diff --git a/t/t1006-cat-file.sh b/t/t1006-cat-file.sh index 67a3f64c2d..f6f00c7039 100755 --- a/t/t1006-cat-file.sh +++ b/t/t1006-cat-file.sh @@ -283,7 +283,7 @@ test_expect_success "--batch-check with multiple sha1s gives correct format" ' test_expect_success 'setup blobs which are likely to delta' ' test-tool genrandom foo 10240 >foo && - { cat foo; echo plus; } >foo-plus && + { cat foo && echo plus; } >foo-plus && git add foo foo-plus && git commit -m foo && cat >blobs <<-\EOF diff --git a/t/t1300-config.sh b/t/t1300-config.sh index 9571649c42..516dd8bfa8 100755 --- a/t/t1300-config.sh +++ b/t/t1300-config.sh @@ -901,7 +901,7 @@ test_expect_success 'get --expiry-date' ' EOF : "work around heredoc parsing bug fixed in dash 0.5.7 (in ec2c84d)" && { - echo "$rel_out $(git config --expiry-date date.valid1)" + echo "$rel_out $(git config --expiry-date date.valid1)" && git config --expiry-date date.valid2 && git config --expiry-date date.valid3 && git config --expiry-date date.valid4 && diff --git a/t/t1403-show-ref.sh b/t/t1403-show-ref.sh index 17d3cc1405..bbc01aae34 100755 --- a/t/t1403-show-ref.sh +++ b/t/t1403-show-ref.sh @@ -78,7 +78,7 @@ test_expect_success 'show-ref --verify -q' ' test_expect_success 'show-ref -d' ' { echo $(git rev-parse refs/tags/A) refs/tags/A && - echo $(git rev-parse refs/tags/A^0) "refs/tags/A^{}" + echo $(git rev-parse refs/tags/A^0) "refs/tags/A^{}" && echo $(git rev-parse refs/tags/C) refs/tags/C } >expect && git show-ref -d A C >actual && @@ -148,7 +148,7 @@ test_expect_success 'show-ref --heads, --tags, --head, pattern' ' { echo $(git rev-parse HEAD) HEAD && - echo $(git rev-parse refs/heads/B) refs/heads/B + echo $(git rev-parse refs/heads/B) refs/heads/B && echo $(git rev-parse refs/tags/B) refs/tags/B } >expect && git show-ref --head B >actual && @@ -156,8 +156,8 @@ test_expect_success 'show-ref --heads, --tags, --head, pattern' ' { echo $(git rev-parse HEAD) HEAD && - echo $(git rev-parse refs/heads/B) refs/heads/B - echo $(git rev-parse refs/tags/B) refs/tags/B + echo $(git rev-parse refs/heads/B) refs/heads/B && + echo $(git rev-parse refs/tags/B) refs/tags/B && echo $(git rev-parse refs/tags/B^0) "refs/tags/B^{}" } >expect && git show-ref --head -d B >actual && diff --git a/t/t2200-add-update.sh b/t/t2200-add-update.sh index 67b9cc752f..d2ef0041f9 100755 --- a/t/t2200-add-update.sh +++ b/t/t2200-add-update.sh @@ -153,10 +153,10 @@ test_expect_success 'add -u resolves unmerged paths' ' echo "100644 $one 1 $path" && echo "100644 $two 2 $path" && echo "100644 $three 3 $path" - done - echo "100644 $one 1 path3" - echo "100644 $one 1 path4" - echo "100644 $one 3 path5" + done && + echo "100644 $one 1 path3" && + echo "100644 $one 1 path4" && + echo "100644 $one 3 path5" && echo "100644 $one 3 path6" } | git update-index --index-info && @@ -173,8 +173,8 @@ test_expect_success 'add -u resolves unmerged paths' ' git add -u && git ls-files -s path1 path2 path3 path4 path5 path6 >actual && { - echo "100644 $three 0 path1" - echo "100644 $two 0 path3" + echo "100644 $three 0 path1" && + echo "100644 $two 0 path3" && echo "100644 $two 0 path5" } >expect && test_cmp expect actual diff --git a/t/t2201-add-update-typechange.sh b/t/t2201-add-update-typechange.sh index a4eec0a346..687be974d4 100755 --- a/t/t2201-add-update-typechange.sh +++ b/t/t2201-add-update-typechange.sh @@ -97,17 +97,17 @@ test_expect_success modify ' " } >expect && { - cat expect - echo ":100644 160000 $_empty $ZERO_OID T yonk" + cat expect && + echo ":100644 160000 $_empty $ZERO_OID T yonk" && echo ":100644 000000 $_empty $ZERO_OID D zifmia" } >expect-files && { - cat expect + cat expect && echo ":000000 160000 $ZERO_OID $ZERO_OID A yonk" } >expect-index && { - echo "100644 $_empty 0 nitfol" - echo "160000 $yomin 0 yomin" + echo "100644 $_empty 0 nitfol" && + echo "160000 $yomin 0 yomin" && echo "160000 $yonk 0 yonk" } >expect-final ' diff --git a/t/t4023-diff-rename-typechange.sh b/t/t4023-diff-rename-typechange.sh index 47d6f35dcc..7cb9909293 100755 --- a/t/t4023-diff-rename-typechange.sh +++ b/t/t4023-diff-rename-typechange.sh @@ -55,7 +55,7 @@ test_expect_success 'cross renames to be detected for regular files' ' git diff-tree five six -r --name-status -B -M | sort >actual && { - echo "R100 foo bar" + echo "R100 foo bar" && echo "R100 bar foo" } | sort >expect && test_cmp expect actual @@ -66,7 +66,7 @@ test_expect_success 'cross renames to be detected for typechange' ' git diff-tree one two -r --name-status -B -M | sort >actual && { - echo "R100 foo bar" + echo "R100 foo bar" && echo "R100 bar foo" } | sort >expect && test_cmp expect actual @@ -78,7 +78,7 @@ test_expect_success 'moves and renames' ' git diff-tree three four -r --name-status -B -M | sort >actual && { # see -B -M (#6) in t4008 - echo "C100 foo bar" + echo "C100 foo bar" && echo "T100 foo" } | sort >expect && test_cmp expect actual diff --git a/t/t4124-apply-ws-rule.sh b/t/t4124-apply-ws-rule.sh index ebff6c6883..ec5c10d2a0 100755 --- a/t/t4124-apply-ws-rule.sh +++ b/t/t4124-apply-ws-rule.sh @@ -233,7 +233,7 @@ test_expect_success 'blank at EOF with --whitespace=fix (1)' ' test_write_lines a b c >one && git add one && test_write_lines a b c >expect && - { cat expect; echo; } >one && + { cat expect && echo; } >one && git diff -- one >patch && git checkout one && diff --git a/t/t4150-am.sh b/t/t4150-am.sh index 2aaaa0d7de..103cd39148 100755 --- a/t/t4150-am.sh +++ b/t/t4150-am.sh @@ -116,7 +116,7 @@ test_expect_success setup ' git format-patch --stdout first | sed -e "1d" } | append_cr >patch1-crlf.eml && { - printf "%255s\\n" "" + printf "%255s\\n" "" && echo "X-Fake-Field: Line One" && echo "X-Fake-Field: Line Two" && echo "X-Fake-Field: Line Three" && diff --git a/t/t4212-log-corrupt.sh b/t/t4212-log-corrupt.sh index 03b952c90d..0244888a5a 100755 --- a/t/t4212-log-corrupt.sh +++ b/t/t4212-log-corrupt.sh @@ -20,10 +20,10 @@ test_expect_success 'fsck notices broken commit' ' test_expect_success 'git log with broken author email' ' { - echo commit $(cat broken_email.hash) - echo "Author: A U Thor <author@example.com>" - echo "Date: Thu Apr 7 15:13:13 2005 -0700" - echo + echo commit $(cat broken_email.hash) && + echo "Author: A U Thor <author@example.com>" && + echo "Date: Thu Apr 7 15:13:13 2005 -0700" && + echo && echo " foo" } >expect.out && diff --git a/t/t5316-pack-delta-depth.sh b/t/t5316-pack-delta-depth.sh index 759169d074..df524f7b6d 100755 --- a/t/t5316-pack-delta-depth.sh +++ b/t/t5316-pack-delta-depth.sh @@ -57,8 +57,11 @@ test_expect_success 'create series of packs' ' git commit -m $i && cur=$(git rev-parse HEAD^{tree}) && { - test -n "$prev" && echo "-$prev" - echo $cur + if test -n "$prev" + then + echo "-$prev" + fi && + echo $cur && echo "$(git rev-parse :file) file" } | git pack-objects --stdout >tmp && git index-pack --stdin --fix-thin <tmp || return 1 diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh index 1892d6615a..01468ce6d8 100755 --- a/t/t5510-fetch.sh +++ b/t/t5510-fetch.sh @@ -71,7 +71,7 @@ test_expect_success "fetch test for-merge" ' main_in_two=$(cd ../two && git rev-parse main) && one_in_two=$(cd ../two && git rev-parse one) && { - echo "$one_in_two " + echo "$one_in_two " && echo "$main_in_two not-for-merge" } >expected && cut -f -2 .git/FETCH_HEAD >actual && diff --git a/t/t5515-fetch-merge-logic.sh b/t/t5515-fetch-merge-logic.sh index 9d440e2821..c69cfd5c64 100755 --- a/t/t5515-fetch-merge-logic.sh +++ b/t/t5515-fetch-merge-logic.sh @@ -191,17 +191,17 @@ do cp "$expect_r" expect_r && convert_expected expect_r sed_script && { - echo "# $cmd" - set x $cmd; shift - git symbolic-ref HEAD refs/heads/$1 ; shift - rm -f .git/FETCH_HEAD + echo "# $cmd" && + set x $cmd && shift && + git symbolic-ref HEAD refs/heads/$1 && shift && + rm -f .git/FETCH_HEAD && git for-each-ref \ refs/heads refs/remotes/rem refs/tags | while read val type refname do git update-ref -d "$refname" "$val" - done - git fetch "$@" >/dev/null + done && + git fetch "$@" >/dev/null && cat .git/FETCH_HEAD } >"$actual_f" && git show-ref >"$actual_r" && diff --git a/t/t5562-http-backend-content-length.sh b/t/t5562-http-backend-content-length.sh index 05a58069b0..b68ec22d3f 100755 --- a/t/t5562-http-backend-content-length.sh +++ b/t/t5562-http-backend-content-length.sh @@ -63,7 +63,7 @@ test_expect_success 'setup' ' hash_next=$(git commit-tree -p HEAD -m next HEAD^{tree}) && { printf "%s %s refs/heads/newbranch\\0report-status object-format=%s\\n" \ - "$ZERO_OID" "$hash_next" "$(test_oid algo)" | packetize_raw + "$ZERO_OID" "$hash_next" "$(test_oid algo)" | packetize_raw && printf 0000 && echo "$hash_next" | git pack-objects --stdout } >push_body && diff --git a/t/t5570-git-daemon.sh b/t/t5570-git-daemon.sh index b87ca06a58..1131503b76 100755 --- a/t/t5570-git-daemon.sh +++ b/t/t5570-git-daemon.sh @@ -194,7 +194,7 @@ test_expect_success 'hostname cannot break out of directory' ' test_expect_success FAKENC 'hostname interpolation works after LF-stripping' ' { - printf "git-upload-pack /interp.git\n\0host=localhost" | packetize_raw + printf "git-upload-pack /interp.git\n\0host=localhost" | packetize_raw && printf "0000" } >input && fake_nc "$GIT_DAEMON_HOST_PORT" <input >output && diff --git a/t/t5571-pre-push-hook.sh b/t/t5571-pre-push-hook.sh index b043a279f1..80e86d8284 100755 --- a/t/t5571-pre-push-hook.sh +++ b/t/t5571-pre-push-hook.sh @@ -114,7 +114,7 @@ test_expect_success 'push to URL' ' test_expect_success 'set up many-ref tests' ' { - nr=1000 + nr=1000 && while test $nr -lt 2000 do nr=$(( $nr + 1 )) && diff --git a/t/t7513-interpret-trailers.sh b/t/t7513-interpret-trailers.sh index 04885d0a5e..97f10905d2 100755 --- a/t/t7513-interpret-trailers.sh +++ b/t/t7513-interpret-trailers.sh @@ -156,7 +156,7 @@ test_expect_success 'with config option on the command line' ' Acked-by: Johan Reviewed-by: Peff EOF - { echo; echo "Acked-by: Johan"; } | + { echo && echo "Acked-by: Johan"; } | git -c "trailer.Acked-by.ifexists=addifdifferent" interpret-trailers \ --trailer "Reviewed-by: Peff" --trailer "Acked-by: Johan" >actual && test_cmp expected actual diff --git a/t/t8002-blame.sh b/t/t8002-blame.sh index 5bb302b1ba..ee4fdd8f18 100755 --- a/t/t8002-blame.sh +++ b/t/t8002-blame.sh @@ -97,7 +97,7 @@ test_expect_success 'set up abbrev tests' ' test_commit abbrev && sha1=$(git rev-parse --verify HEAD) && check_abbrev () { - expect=$1; shift + expect=$1 && shift && echo $sha1 | cut -c 1-$expect >expect && git blame "$@" abbrev.t >actual && perl -lne "/[0-9a-f]+/ and print \$&" <actual >actual.sha &&
The top-level &&-chain checker built into t/test-lib.sh causes tests to magically exit with code 117 if the &&-chain is broken. However, it has the shortcoming that the magic does not work within `{...}` groups, `(...)` subshells, `$(...)` substitutions, or within bodies of compound statements, such as `if`, `for`, `while`, `case`, etc. `chainlint.sed` partly fills in the gap by catching broken &&-chains in `(...)` subshells, but bugs can still lurk behind broken &&-chains in the other cases. Fix broken &&-chains in `{...}` groups in order to reduce the number of possible lurking bugs. Signed-off-by: Eric Sunshine <sunshine@sunshineco.com> --- t/t0021-conversion.sh | 8 ++++---- t/t0069-oidtree.sh | 12 ++++++------ t/t1006-cat-file.sh | 2 +- t/t1300-config.sh | 2 +- t/t1403-show-ref.sh | 8 ++++---- t/t2200-add-update.sh | 12 ++++++------ t/t2201-add-update-typechange.sh | 10 +++++----- t/t4023-diff-rename-typechange.sh | 6 +++--- t/t4124-apply-ws-rule.sh | 2 +- t/t4150-am.sh | 2 +- t/t4212-log-corrupt.sh | 8 ++++---- t/t5316-pack-delta-depth.sh | 7 +++++-- t/t5510-fetch.sh | 2 +- t/t5515-fetch-merge-logic.sh | 12 ++++++------ t/t5562-http-backend-content-length.sh | 2 +- t/t5570-git-daemon.sh | 2 +- t/t5571-pre-push-hook.sh | 2 +- t/t7513-interpret-trailers.sh | 2 +- t/t8002-blame.sh | 2 +- 19 files changed, 53 insertions(+), 50 deletions(-)