From: Junio C Hamano <gitster@pobox.com>
Date: Fri, 26 Jan 2018 18:39:05 GMT
Subject: Re: [PATCH 08/10] t: forbid piping into 'test_i18ngrep'
Message-ID: <xmqqshas79di.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <xmqq1sic8omp.fsf@gitster.mtv.corp.google.com>

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

> SZEDER Gábor <szeder.dev@gmail.com> writes:
>
>> See two of the previous patches for the only such cases we had in our
>> test suite.  However, reliably preventing this antipattern is arguably
>> more important than supporting these cases, which can be worked around
>> by only minor inconveniences.
>
> I am not sure if that inconveniences will be minor.  Is this too
> contrived an example, for example?
>
>   check () {
>         pattern=$1 file=$2 script=./runme
>
>         test_i18ngrep "$pattern" "$file" &&
>         write_script "$script" &&
>         test_expect_success "check $pattern" '
>                 "$script"
>         '
>   }
>
>   check foo file <<-EOF
>   ... test script comes here ...
>   EOF

Is there a case where test_i18ngrep (after your clean-ups in this
series up to 06/10) needs to read from more than one file?

I actually think that the kind of inconveniences we *can* work with,
without risking breakage to legitimate test, would be to allow and
require test_i18ngrep to name and read only from one file that
appears at the end of its command line.  IOW, instead of doing a
probing "read" that you cannot undo and break legitimate test, I
think it is OK to see if the last token names a file that is on the
filesystem, e.g.

	test_i18ngrep () {
		eval test -f \"\${$#}\" ||
		error "bug in the test sript: test_i18ngrep must" \
		      "name a file to read as the last token on the command line"
		...