Quoting Jeff King <peff@xxxxxxxx>:
This test contains a lot of hand-rolled messages to show
when the test fails. We can omit most of these by using
"verbose" and "test_must_fail". A few of them are for
update-index, but we can assume it produces reasonable error
messages when it fails.
Signed-off-by: Jeff King <peff@xxxxxxxx>
---
t/t0020-crlf.sh | 144
+++++++++++---------------------------------------------
1 file changed, 28 insertions(+), 116 deletions(-)
diff --git a/t/t0020-crlf.sh b/t/t0020-crlf.sh
index d2e51a8..9fa26df 100755
--- a/t/t0020-crlf.sh
+++ b/t/t0020-crlf.sh
@@ -104,18 +104,12 @@ test_expect_success 'update with autocrlf=input' '
for f in one dir/two
do
append_cr <$f >tmp && mv -f tmp $f &&
- git update-index -- $f || {
- echo Oops
- false
- break
- }
+ git update-index -- $f ||
+ break
done &&
Ah, these tests are evil, I remember them from the time when I was
fiddling with Jonathan's patch. They can fail silently without
testing what they were supposed to test.
If something in the loop fails, the break will leave the loop but it
will do so with zero return value and, consequently, the test will
continue as if everything were OK.
And unless it was 'git update-index' that failed in a way that left a
borked index behind, the 'git diff-index --cached' below will not
error out or produce some output that would cause the test to fail.
i.e. I tried e.g.
append_cr <$f >tmp && mv -f tmp $f && false &&
in the loop and the test succeeded.
I think the best fix would be to unroll the loop: after this patch the
loop body consists of only two significant lines and we iterate
through the loop only twice, so the test would be even shorter.
differs=$(git diff-index --cached HEAD) &&
- test -z "$differs" || {
- echo Oops "$differs"
- false
- }
+ verbose test -z "$differs"
'
@@ -128,18 +122,12 @@ test_expect_success 'update with autocrlf=true' '
for f in one dir/two
do
append_cr <$f >tmp && mv -f tmp $f &&
- git update-index -- $f || {
- echo "Oops $f"
- false
- break
- }
+ git update-index -- $f ||
+ break
done &&
differs=$(git diff-index --cached HEAD) &&
- test -z "$differs" || {
- echo Oops "$differs"
- false
- }
+ verbose test -z "$differs"
'
@@ -152,19 +140,13 @@ test_expect_success 'checkout with autocrlf=true' '
for f in one dir/two
do
remove_cr <"$f" >tmp && mv -f tmp $f &&
- git update-index -- $f || {
- echo "Eh? $f"
- false
- break
- }
+ verbose git update-index -- $f ||
+ break
done &&
test "$one" = $(git hash-object --stdin <one) &&
test "$two" = $(git hash-object --stdin <dir/two) &&
differs=$(git diff-index --cached HEAD) &&
- test -z "$differs" || {
- echo Oops "$differs"
- false
- }
+ verbose test -z "$differs"
'
test_expect_success 'checkout with autocrlf=input' '
@@ -187,10 +169,7 @@ test_expect_success 'checkout with autocrlf=input' '
test "$one" = $(git hash-object --stdin <one) &&
test "$two" = $(git hash-object --stdin <dir/two) &&
differs=$(git diff-index --cached HEAD) &&
- test -z "$differs" || {
- echo Oops "$differs"
- false
- }
+ verbose test -z "$differs"
'
test_expect_success 'apply patch (autocrlf=input)' '
@@ -200,10 +179,7 @@ test_expect_success 'apply patch (autocrlf=input)' '
git read-tree --reset -u HEAD &&
git apply patch.file &&
- test "$patched" = "$(git hash-object --stdin <one)" || {
- echo "Eh? apply without index"
- false
- }
+ verbose test "$patched" = "$(git hash-object --stdin <one)"
'
test_expect_success 'apply patch --cached (autocrlf=input)' '
@@ -213,10 +189,7 @@ test_expect_success 'apply patch --cached
(autocrlf=input)' '
git read-tree --reset -u HEAD &&
git apply --cached patch.file &&
- test "$patched" = $(git rev-parse :one) || {
- echo "Eh? apply with --cached"
- false
- }
+ verbose test "$patched" = $(git rev-parse :one)
'
test_expect_success 'apply patch --index (autocrlf=input)' '
@@ -226,11 +199,8 @@ test_expect_success 'apply patch --index
(autocrlf=input)' '
git read-tree --reset -u HEAD &&
git apply --index patch.file &&
- test "$patched" = $(git rev-parse :one) &&
- test "$patched" = $(git hash-object --stdin <one) || {
- echo "Eh? apply with --index"
- false
- }
+ verbose test "$patched" = $(git rev-parse :one) &&
+ verbose test "$patched" = $(git hash-object --stdin <one)
'
test_expect_success 'apply patch (autocrlf=true)' '
@@ -240,10 +210,7 @@ test_expect_success 'apply patch (autocrlf=true)' '
git read-tree --reset -u HEAD &&
git apply patch.file &&
- test "$patched" = "$(remove_cr <one | git hash-object --stdin)" || {
- echo "Eh? apply without index"
- false
- }
+ verbose test "$patched" = "$(remove_cr <one | git hash-object --stdin)"
'
test_expect_success 'apply patch --cached (autocrlf=true)' '
@@ -253,10 +220,7 @@ test_expect_success 'apply patch --cached
(autocrlf=true)' '
git read-tree --reset -u HEAD &&
git apply --cached patch.file &&
- test "$patched" = $(git rev-parse :one) || {
- echo "Eh? apply without index"
- false
- }
+ verbose test "$patched" = $(git rev-parse :one)
'
test_expect_success 'apply patch --index (autocrlf=true)' '
@@ -266,11 +230,8 @@ test_expect_success 'apply patch --index
(autocrlf=true)' '
git read-tree --reset -u HEAD &&
git apply --index patch.file &&
- test "$patched" = $(git rev-parse :one) &&
- test "$patched" = "$(remove_cr <one | git hash-object --stdin)" || {
- echo "Eh? apply with --index"
- false
- }
+ verbose test "$patched" = $(git rev-parse :one) &&
+ verbose test "$patched" = "$(remove_cr <one | git hash-object --stdin)"
'
test_expect_success '.gitattributes says two is binary' '
@@ -326,21 +287,8 @@ test_expect_success '.gitattributes says two and
three are text' '
echo "t* crlf" >.gitattributes &&
git read-tree --reset -u HEAD &&
- if has_cr dir/two
- then
- : happy
- else
- echo "Huh?"
- false
- fi &&
-
- if has_cr three
- then
- : happy
- else
- echo "Huh?"
- false
- fi
+ verbose has_cr dir/two &&
+ verbose has_cr three
'
test_expect_success 'in-tree .gitattributes (1)' '
@@ -352,17 +300,8 @@ test_expect_success 'in-tree .gitattributes (1)' '
rm -rf tmp one dir .gitattributes patch.file three &&
git read-tree --reset -u HEAD &&
- if has_cr one
- then
- echo "Eh? one should not have CRLF"
- false
- else
- : happy
- fi &&
- has_cr three || {
- echo "Eh? three should still have CRLF"
- false
- }
+ test_must_fail has_cr one &&
+ verbose has_cr three
'
test_expect_success 'in-tree .gitattributes (2)' '
@@ -371,17 +310,8 @@ test_expect_success 'in-tree .gitattributes (2)' '
git read-tree --reset HEAD &&
git checkout-index -f -q -u -a &&
- if has_cr one
- then
- echo "Eh? one should not have CRLF"
- false
- else
- : happy
- fi &&
- has_cr three || {
- echo "Eh? three should still have CRLF"
- false
- }
+ test_must_fail has_cr one &&
+ verbose has_cr three
'
test_expect_success 'in-tree .gitattributes (3)' '
@@ -391,17 +321,8 @@ test_expect_success 'in-tree .gitattributes (3)' '
git checkout-index -u .gitattributes &&
git checkout-index -u one dir/two three &&
- if has_cr one
- then
- echo "Eh? one should not have CRLF"
- false
- else
- : happy
- fi &&
- has_cr three || {
- echo "Eh? three should still have CRLF"
- false
- }
+ test_must_fail has_cr one &&
+ verbose has_cr three
'
test_expect_success 'in-tree .gitattributes (4)' '
@@ -411,17 +332,8 @@ test_expect_success 'in-tree .gitattributes (4)' '
git checkout-index -u one dir/two three &&
git checkout-index -u .gitattributes &&
- if has_cr one
- then
- echo "Eh? one should not have CRLF"
- false
- else
- : happy
- fi &&
- has_cr three || {
- echo "Eh? three should still have CRLF"
- false
- }
+ test_must_fail has_cr one &&
+ verbose has_cr three
'
test_expect_success 'checkout with existing .gitattributes' '
--
2.3.3.520.g3cfbb5d
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html