From: Eric Sunshine <sunshine@sunshineco.com>
Date: Sun, 17 Apr 2016 06:36:24 GMT
Subject: Re: [PATCH v2 1/2] test-lib: add a function to compare an expection with stdout from a command
Message-ID: <CAPig+cTOa2yaMikOJHQXpSjY_EtyUXaqVz4KobQwO2xn=Q6h_w@mail.gmail.com>
In-Reply-To: <20160417035414.GA30002@sigill.intra.peff.net>

On Sat, Apr 16, 2016 at 11:54 PM, Jeff King <peff@peff.net> wrote:
> On Sat, Apr 16, 2016 at 11:07:02PM -0400, Eric Sunshine wrote:
>> > test_stdout accepts an expection and a command to execute.  It will execute
>> > the command and then compare the stdout from that command to an expectation.
>> > If the expectation is not met, a mock diff output is written to stderr.
>>
>> I wonder if this deserves more flexibility by accepting a comparison
>> operator, such as = and !=, similar to test_line_count()? Although, I
>> suppose such functionality could be added later if deemed useful.
>
> [...] Though I do actually find that:
>
>   test_stdout false git rev-parse --whatever
>
> isn't great, because there's no syntactic separator between the expected
> output and the actual command to run. So I dunno, maybe it would be
> better as:
>
>   test_stdout false = git rev-parse --whatever
>
> [...] We could also do:
>
>   test_stdout git rev-parse --whatever <<-\EOF
>   false
>   EOF
>
> which is more robust for multi-line output, but I think part of the
> point is to keep these as simple one-liners. You're not buying all that
> much over:
>
>   cat >expect <<-\EOF &&
>   false
>   EOF
>   git rev-parse --whatever >actual &&
>   test_cmp expect actual
>
> Though I do admit I've considered such a helper for some tests where
> that pattern is repeated ad nauseam.

Agreed. I wouldn't mind the version where test_stdout grabs "expected"
from <<EOF, but, as you say, it doesn't buy much over the manually
prepared test_cmp version.

I suppose that the one-liner form of test_stdout could have its uses,
however, it bothers me for a couple reasons: (1) it's not generally
useful like the version which grabs "expected" from <<EOF, (2) it
squats on a nice concise name which would better suit the <<EOF
version.

Anyhow, this may all be moot (for now) since I think this patch series
is going in the wrong direction entirely by abandoning the systematic
approach taken by the original t1500 code, as explained in my
review[1]. If modernization of t1500 retains a systematic approach,
then the repetitive code which prompted the suggestion of test_stdout
won't exist in the first place.

[1]: http://article.gmane.org/gmane.comp.version-control.git/291745