Paul Tan <pyokagan@xxxxxxxxx> writes: > Many tests in t5520 used the following to test the contents of files: > > test `cat file` = expected > > or > > test $(cat file) = expected > > These 2 forms, however, will be affected by field splitting and, > depending on the value of $IFS, may be split into multiple arguments, > making the test fail in mysterious ways. > > Replace the above 2 forms with: > > test "$(cat file)" = expected > > as quoting the command substitution will prevent field splitting. > > Signed-off-by: Paul Tan <pyokagan@xxxxxxxxx> > --- > * Removed use of "verbose" > > * The use of "wc -l" is not quoted, as the output of "wc -l" on a Mac contains > leading whitespace. See [1]. > > [1] http://thread.gmane.org/gmane.comp.version-control.git/268950/focus=269052 > > t/t5520-pull.sh | 70 ++++++++++++++++++++++++++++----------------------------- > 1 file changed, 35 insertions(+), 35 deletions(-) Overall the series was a pleasant read. Some common patterns (my mind is still continuing the verbose_test topic) I noticed were (1) comparing output from two rev-parse output for object names. We have test_cmp_rev we can use for better readability (but its implementaiton may want to be updated not to contaminate the working tree unnecessarily). e.g test "$(git rev-parse HEAD^)" = "$(git rev-parse copy)" => test_cmp_rev HEAD^ copy (2) checking contents of a file, either from a working tree, from the index or from a rev, with a fixed short string. test_contents () { case "$1" in *:*) git cat-file blob "$1" ;; *) cat "$1" ;; esac >actual && printf "%s" "$2" >expect && if ! cmp -s expect actual then echo "Unexpected contents in '$1'" test_cmp expect actual else : good ; fi } or something. I think it is a good idea *not* to address these patterns within this series, and have people come back to them _after_ the series settles, to reduce code churn and unnecessary conflicts. Thanks. -- 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