Re: t4151 missing quotes

[Eric Sunshine <sunshine@sunshineco.com>]

On Mon, May 9, 2016 at 12:22 PM, Eric Sunshine <sunshine@sunshineco.com> wrote:
> On Mon, May 9, 2016 at 12:09 PM, Armin Kunaschik
> <megabreit@googlemail.com> wrote:
>> *** t4151-am-abort.sh   Mon May  9 17:51:44 2016
>> --- t4151-am-abort.sh.orig      Fri Apr 29 23:37:00 2016
>> !       test 3 -eq "$(git ls-files -u | wc -l)" &&
>> !       test 3 -eq $(git ls-files -u | wc -l) &&
>
> 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 '='.

This isn't quite accurate. Since the test is using '-eq' rather than
'=', the leading whitespace won't be a problem, but it can still be
problematic if you're somehow getting an empty result from the
pipeline:

    % test 3 -eq "$(echo)"
    -bash: test: : integer expression expected

> 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.