Re: [PATCH v9 2/2] test-config: add tests for the config_set API

From: Tanay Abhra <tanayabh@gmail.com>
Date: Jul 15, 2014, 17:07 UTC
Message-ID: <53C55FB7.7060405@gmail.com>
In-Reply-To: <xmqqk37ewr5r.fsf@gitster.dls.corp.google.com>

On 7/15/2014 9:27 PM, Junio C Hamano wrote:

Show 10 quoted lines

>> +test_expect_success 'setup default config' '
>> +	cat >.git/config <<\EOF
> 
> So the default .git/config that was prepared by "git init" is
> discarded and replaced with this?  Shouldn't it be
> 
> 	cat >>.git/config <<\EOF
> 
> instead?
>

Most of tests like t1300-repo-config.sh or t1303-wacky-config.sh clears the default config, will it be okay to clear it in this test series also? I need it for test_expect_success 'proper error on error in default config files' ' which requires me to compare the line no at which the error was found.

Show 14 quoted lines

>> +test_expect_success 'find multiple values' '
>> +	cat >expect <<-\EOF &&
>> +	sam
>> +	bat
>> +	hask
>> +	EOF
>> +	test-config get_value_multi "case.baz">actual &&
>> +	test_cmp expect actual
>> +'
> 
> Hmmm, wasn't the whole point of the helper to allow us to make
> things like the above into a one-liner, perhaps like this?
> 
>       check_config get_value_multi case.baz sam bat hask

Noted and corrected.

> I suspect the same applies to most if not all uses of test-config
> in the remainder of this patch.
>

I cant use it for configset_get_value_* as it may have variable number of files as arguments. :)

Thanks, Tanay Abhra.