Re: [PATCH v2 5/6] t: convert grep assertions to test_grep

[Junio C Hamano <gitster@pobox.com>]

Junio C Hamano <gitster@pobox.com> writes:

> Junio C Hamano <gitster@pobox.com> writes:
>
>> SZEDER Gábor <szeder.dev@gmail.com> writes:
>>
>>> I think in this case checking the file3's contents is wrong, because
>>> at this point file3 should not exist in the first place.  I've sent a
>>> patch to fix this long ago, but apparently didn't manage to follow
>>> through back then.
>>>
>>>   https://lore.kernel.org/git/20211010172809.1472914-1-szeder.dev@gmail.com/
>>
>> Thanks.  I guess the test_grep can be extended to catch this case,
>> where 
>>
>>     test_grep ! -e pattern1 -e pattern2 file
>>
>> does not find any hits, but only because 'file' is missing, as an
>> error, ...
>
> Wait.  The necessary check is already there, isn't it?
>
>         test_grep () {
>                 eval "last_arg=\${$#}"
>
>                 test -f "$last_arg" ||
>                 BUG "test_grep requires a file to read as the last parameter"
>
> So why don't we see it every time we run that test that inspects
> file3's contents with Michael's series merged in?  Puzzled...

Ah, of course.  Michael sidesteps this mechanism by not using
"test_grep !", with

       ! grep dirty file3 && # lint-ok: file may not exist after --quit

and if we realize that "may not exist" is actually "never exists",
then your other patch from 5 years ago would become the most
sensible fix for this line.

It may not be a bad idea to go through "# lint-ok:" introduced by
Michael's series with finer toothed comb (there are only a handful
of them) and see if there are similar "look, the file we are
grepping in never exists with correctly running Git" gotchas.

Thanks.