From: Jeff King <peff@peff.net>
Date: Fri, 31 Jul 2020 20:30:04 GMT
Subject: Re: [PATCH v1] t6300: fix issues related to %(contents:size)
Message-ID: <20200731203004.GA1440843@coredump.intra.peff.net>
In-Reply-To: <xmqqpn8b30zp.fsf@gitster.c.googlers.com>

On Fri, Jul 31, 2020 at 01:04:10PM -0700, Junio C Hamano wrote:

> > Definitely fixes the issue, though I wonder:
> >
> >> -		echo $expect >expected
> >> -		test_expect_${4:-sucess} $PREREQ "basic atom: $1 contents:size" '
> >> +		echo $expect >expect
> >> +		test_expect_${4:-success} $PREREQ "basic atom: $1 contents:size" '
> >>  			git for-each-ref --format="%(contents:size)" "$ref" >actual &&
> >>  			test_cmp expect actual
> >>  		'
> >
> > Should we instead switch the test_cmp to look at "expected" to be
> > consistent with the rest of the tests in this file?
> 
> If I recall correctly, "expect vs actual" were more common when I
> counted across all the tests last time.  Matching local convention
> is fine, though.

Yes, I agree that "expect" is where we should be heading overall. I
think matching local convention is best here to avoid introducing new
mistakes like this one, but I wouldn't be opposed to somebody switching
out s/expected/expect/ in the whole file.

-Peff