Re: [PATCH v3 4/6] add a test for semantic errors in config files

From: Matthieu Moy <matthieu.moy@grenoble-inp.fr>
Date: Jul 28, 2014, 11:38 UTC
Message-ID: <vpqr415r9vi.fsf@anie.imag.fr>
In-Reply-To: <53D63389.5090205@gmail.com>

Tanay Abhra <tanayabh@gmail.com> writes:

Show 21 quoted lines

> On 7/28/2014 4:52 PM, Matthieu Moy wrote:
>> Tanay Abhra <tanayabh@gmail.com> writes:
>> 
>>> +test_expect_success 'check line errors for malformed values' '
>>> +	mv .git/config .git/config.old &&
>>> +	test_when_finished "mv .git/config.old .git/config" &&
>>> +	cat >.git/config <<-\EOF &&
>>> +	[alias]
>>> +		br
>>> +	EOF
>>> +	test_expect_code 128 git br 2>result &&
>>> +	grep "fatal: bad config file line 2 in .git/config" result
>>> +'
>> 
>> This is PATCH 4, and it tests a bug fixed in PATCH 1. It would have
>> eased review to group both patches, either
>> 
>> PATCH 1: introduce test_expect_failure test to demonstrate the failure
>
> Didn't Junio comment that he wouldn't recommend inserting a test_expect_failure
> for new tests and then flipping them after in the series.

No, he even said it was good practice:

http://thread.gmane.org/gmane.comp.version-control.git/254101/focus=254104

his point was to avoid breaking something and repairing in another patch (which your initial series was doing because the test patch was coming between "rewrite git_config() to use the config-set API" and "add line number and file name info to `config_set`").

The situation is different when you have a pre-existing bug.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/