Re: [PATCH 2/3] t5304: use helper to report failure of "test foo = bar"

From: Jeff King <peff@peff.net>
Date: Oct 13, 2014, 21:15 UTC
Message-ID: <20141013211524.GB15969@peff.net>
In-Reply-To: <20141013161022.GA20520@google.com>

On Mon, Oct 13, 2014 at 09:10:22AM -0700, Jonathan Nieder wrote:

Show 11 quoted lines

> Jeff King wrote:
> 
> > For small outputs, we sometimes use:
> >
> >   test "$(some_cmd)" = "something we expect"
> >
> > instead of a full test_cmp. The downside of this is that
> > when it fails, there is no output at all from the script.
> 
> There's another downside to that construct: it loses the exit
> status from some_cmd.

Yes, although I think in many cases it's not a big deal. For example, here we lose the exit code of count-objects, but it also is very unlikely to fail _and_ produce our expected output.

Show 9 quoted lines

> Alternatively, maybe there could be a helper in the same spirit as
> test_cmp_rev?
> 
> 	test_object_count () {
> 		git count-objects >output &&
> 		sed "s/ .*//" output >count &&
> 		printf "%s\n" "$1" >expect &&
> 		test_cmp expect count
> 	}

One of my goals was to provide a more generic helper so that we don't have to make little helpers like this for every command. So I'd much rather something like:

  test_output () {
	printf "%s\n" "$1" >expect &&
	shift &&
	"$@" >output &&
	test_cmp expect output
  }

The "\n" handling there feels a little hacky, but is probably OK in practice (the few commands that really do generate an output without a newline can use test_cmp manually). It should probably also "rm" the files on success to avoid polluting the working tree.

It also doesn't help cases that want to do "test $foo -lt 3" or other non-equality checks. But those are probably the minority.

I dunno. I am OK with what I posted, but if we feel strongly about testing the exit code, I can re-roll as test_output as above.

-Peff