Re: t4151 missing quotes

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, May 9, 2016 at 12:09 PM, Armin Kunaschik
<megabreit@xxxxxxxxxxxxxx> wrote:
> skipping through some failed tests I found more (smaller) problems
> inside the test... when test arguments are empty they need to be
> quoted (quite a lot test in this sentence).
>
> Error is like
> t4151-am-abort.sh[5]: test: argument expected
>
> My patch:
>
> *** t4151-am-abort.sh   Mon May  9 17:51:44 2016
> --- t4151-am-abort.sh.orig      Fri Apr 29 23:37:00 2016
> ***************
> *** 67,73 ****
>   test_expect_success 'am -3 --skip removes otherfile-4' '
>         git reset --hard initial &&
>         test_must_fail git am -3 0003-*.patch &&
> !       test 3 -eq "$(git ls-files -u | wc -l)" &&
>         test 4 = "$(cat otherfile-4)" &&
>         git am --skip &&
>         test_cmp_rev initial HEAD &&
> --- 67,73 ----
>   test_expect_success 'am -3 --skip removes otherfile-4' '
>         git reset --hard initial &&
>         test_must_fail git am -3 0003-*.patch &&
> !       test 3 -eq $(git ls-files -u | wc -l) &&
>         test 4 = "$(cat otherfile-4)" &&
>         git am --skip &&
>         test_cmp_rev initial HEAD &&
> ***************

Some comments:

Quoting the output of 'wc -l' will break the tests on Mac OS X and BSD
since the output contains leading whitespace which won't match the "3"
on the other side of the '='.

Your diff is backward, comparing 'current' against 'original', which
makes it difficult to read. Reviewers on this list expect to see
'original' compared against 'current'.

Use a unified format to make the diff easier to read; or just use
git-diff or git-format patch, which is even simpler.

It's not clear how the output of 'wc -l' could ever be the empty
string. Perhaps git-ls-files is dying and causing the pipe to abort
before 'wc -l' ever outputs anything? Without additional information
about the problem you're experiencing, it's difficult to judge if this
change is a good idea.
--
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



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]