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

From: Junio C Hamano <[email-masked]>
Date: Wed, 24 Apr 2013 10:29:55 -0700

Michael J Gruber <[email-masked]> writes:

>>> +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.