Ramkumar Ramachandra wrote: > Use test_cmp in preference to repeatedly comparing command outputs by > hand. That could mean one of several things. It could mean: 1. Use test_cmp instead of open-coding it. 2. Use test_cmp instead of using our knowledge of the underlying filesystem to retrieve the files from the block device, instead of relying on the perfectly good operating system facilities that could take care of it for us 3. Use test_cmp instead of calling a human over to compare command outputs by eye, which idiomatically might be described as "by hand". What I mean is, I actually don't have much of a clue what you mean by "by hand". Usually it means "not automated sufficiently", but I think that is not the entire problem here (since test "$expect" = "$actual" looks no less automatic than printf '%s\n' "$expect" >expect && printf '%s\n' "$actual" >actual && test_cmp expect actual to me). > > Signed-off-by: Ramkumar Ramachandra <artagnon@xxxxxxxxx> [...] > --- a/t/t1006-cat-file.sh > +++ b/t/t1006-cat-file.sh > @@ -36,66 +36,41 @@ $content" [...] > - expect="$(maybe_remove_timestamp "$content" $no_ts)" > - actual="$(maybe_remove_timestamp "$(git cat-file $type $sha1)" $no_ts)" > - > - if test "z$expect" = "z$actual" > - then > - : happy > - else > - echo "Oops: expected $expect" > - echo "but got $actual" > - false > - fi > + maybe_remove_timestamp "$content" $no_ts >expect && > + maybe_remove_timestamp "$(git cat-file $type $sha1)" $no_ts >actual && > + test_cmp expect actual Most of the early part of patch proper looks sane from a quick glance. Wow, the whitespace is a little strange in the original. [...] > @@ -175,30 +153,41 @@ do > done > > test_expect_success "--batch-check for a non-existent named object" ' > - test "foobar42 missing > -foobar84 missing" = \ > - "$( ( echo foobar42; echo_without_newline foobar84; ) | git cat-file --batch-check)" > + cat >expect <<\-EOF && > +foobar42 missing > +foobar84 missing > +EOF > + $(echo foobar42; echo_without_newline foobar84) \ > + | git cat-file --batch-check >actual && > + test_cmp expect actual Style: the | character goes at the end of the first line (think of it as a way to save backslashes until they're needed). How could this $(...) command substitution possibly work? Later tests have the same problem, so I'm stopping here. Ciao, Jonathan -- 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