Re: [PATCHv2 4/7] t7008: demonstrate behavior of grep with textconv

From: Junio C Hamano <gitster@pobox.com>
Date: Apr 24, 2013, 17:29 UTC
Message-ID: <7vmwsnet0s.fsf@alter.siamese.dyndns.org>
In-Reply-To: <5177AF62.30104@drmicha.warpmail.net>

Michael J Gruber <git@drmicha.warpmail.net> writes:

Show 14 quoted lines

>>> +test_expect_failure 'grep does not honor textconv' '
>>> +	echo "a:binaryQfile" >expect &&
>>> +	git grep Qfile >actual &&
>> 
>> This should pass --textconv to "git grep".
>
> But "git grep" does not know that option yet, so the test would fail for
> the wrong reason.
>
> The point ist that I expect "git grep" to apply textconv filters by
> default, which it does not. (I know I might be the only one with this
> expectation.)
>
> Or do we want to document the absence of that option?

First, whether you write expect_failure or expec_success, please label the test to say what is expected to happen in the ideal world. The test in question says "grep does not honor textconv", but if you want it to honor textconv in the ideal world, it should be "grep honors textconv (when it should)".

Now, from the point of view of testing "git grep honors textconv" missing support at the command line parser level and a buggy implementation of the command line parser that accepts but does not trigger the feature are the same thing. The command would not honor textconv either way.

Marking the above as "failure" without explicitly asking for the feature with "--textconv" means we want it to use textconv by default, but that is *not* what the test title says is testing.

In your patch, what the body of the text is really expecting is "grep uses textconv by default". If that is what it tests, then passing --textconv from the command line as I suggested would be wrong, but I was going by the title of the patch.