Re: mac test failure -- test 3301

From: Jeff King <peff@peff.net>
Date: Nov 11, 2014, 02:41 UTC
Message-ID: <20141111024128.GA21328@peff.net>
In-Reply-To: <CALKQrgexnrCy-P6yTZB+U0--5peaTys9KqB+KGKqbAuyLA=NqA@mail.gmail.com>

On Tue, Nov 11, 2014 at 02:40:19AM +0100, Johan Herland wrote:

Show 10 quoted lines

> > This and all other failures are due to the output of 'wc -l', which on
> > Mac is "{whitespace}1" rather than just "1" as it is on other
> > platforms. fbe4f748 added quotes around the $(... | wc -l) invocation
> > which caused the whitespace to be retained. A minimum fix would be to
> > drop the quotes surrounding $().
> 
> Ah, thanks!
> 
> I thought that quoting command output was a good idea in general. Am I
> wrong, or is this just one exception to an otherwise good guideline?

It usually is a good idea to prevent whitespace splitting by the shell (and especially with things that might unexpectedly be empty, in which case they end up as "no argument" and not an empty one). So yeah, this is an exception.

> Anyway, here is the minimal diff to remove quoting from the $(... | wc
> -l) commands (probably whitespace damaged by GMail - sorry). Junio:
> feel free to squash this in, or ask for a re-roll:

I think we have test_line_count exactly because of this unportability with wc output.

You'd want:

  git ls-tree refs/notes/commits >actual &&
  test_line_count = 1 actual

or similar.

By the way, the point of that "ls-tree" appears to be to check the number of total notes stored. Since notes are stored in a hashed structure, should it be "ls-tree -r"? Otherwise, we see only the leading directories; two notes whose sha1s start with the same two characters would be considered a single entry.

-Peff