Re: [PATCH] t1002: stop using sum(1)

From: René Scharfe <l.s.r@web.de>
Date: Aug 15, 2017, 14:43 UTC
Message-ID: <5c9752cb-3c47-19f0-42b2-591bd29134f9@web.de>
In-Reply-To: <20170815004546.GA78174@aiede.mtv.corp.google.com>

Am 15.08.2017 um 02:46 schrieb Jonathan Nieder:

> Hi,
> 
> René Scharfe wrote:

>> We already compare changed files with their expected new contents using
>> diff(1), so we don't need to check with "test_must_fail test_cmp" if
>> they differ from their original state.  A later patch could convert the
>> direct diff(1) calls to test_cmp as well.

Let's call that paragraph "A".

> Nicely analyzed.  May we forge your sign-off?

Oops, yes, thanks for reminding me, handed it in late now.

Show 22 quoted lines

> 
> [...]
>> --- a/t/t1002-read-tree-m-u-2way.sh
>> +++ b/t/t1002-read-tree-m-u-2way.sh
> [...]
>> @@ -132,8 +138,8 @@ test_expect_success \
>>        git ls-files --stage >7.out &&
>>        test_cmp M.out 7.out &&
>>        check_cache_at frotz dirty &&
>> -     sum bozbar frotz nitfol >actual7.sum &&
>> -     if cmp M.sum actual7.sum; then false; else :; fi &&
>> +     test_cmp bozbar.M bozbar &&
>> +     test_cmp nitfol.M nitfol &&
> 
> This one is strange.  What is that '! cmp' trying to check for?
> Does the replacement capture the same thing?
> 
> E.g., does it need a '! test_cmp frotz.M frotz &&' line?
> 
> I haven't looked at the context closely --- another option could be a
> note in the commit message about how that '! cmp' line was not testing
> anything useful in the first place.

That's what paragraph A refers to. And as Johannes mentioned: We already check for equality in the lines following the context you cited (it's in my original email), so there is no need to check for inequality as well. That's true for all the cases you spotted.

René