Re: [PATCH v2 2/6] test-reach: add run_three_modes method

From: Junio C Hamano <gitster@pobox.com>
Date: Sep 19, 2018, 19:31 UTC
Message-ID: <xmqqbm8t473q.fsf@gitster-ct.c.googlers.com>
In-Reply-To: <20180918180200.GD27036@localhost>

SZEDER Gábor <szeder.dev@gmail.com> writes:

Show 7 quoted lines

>> While inspecting this code, I realized that the final test for
>> 'commit_contains --tag' is silently dropping the '--tag' argument.
>> It should be quoted to include both.
>
> Nit: while quoting the function's arguments does fix the issue, it
> leaves the tests prone to the same issue in the future.  Wouldn't it
> be better to use $@ inside the function to refer to all its arguments?

IOW, do it more like this?

Show 5 quoted lines

>> -test_three_modes () {
>> +run_three_modes () {
>>  	test_when_finished rm -rf .git/objects/info/commit-graph &&
>> -	test-tool reach $1 <input >actual &&
>> +	$1 <input >actual &&

	"$@" <input >actual

i.e. treat each parameter as separate things without further getting split at $IFS and ...

>> +test_three_modes () {
>> +	run_three_modes "test-tool reach $1"

	run_three_modes test-tool reach "$1"

... make sure there three things are sent as separate, by quoting "$1" inside dq.

I think that makes sense.