Subject: Re: [PATCH 3/7] add a test for semantic errors in config files

From: Junio C Hamano <[email-masked]>
Date: Wed, 23 Jul 2014 15:11:05 -0700

Matthieu Moy <[email-masked]> writes:

> Tanay Abhra <[email-masked]> writes:
>
>> +test_expect_success 'check line errors for malformed values' '
>> +	cp .git/config .git/config.old &&

Should this be "mv" not "cp"?  You will be overwriting the file from
scratch in the later part of this test.

>> +	test_when_finished "mv .git/config.old .git/config" &&
>> +	echo "[alias]\n br" >.git/config &&

Is the use of \n portable?

> Another option is to mark the test as test_expect_failure when you
> introduce it, and change it to test_expect_success when you fix it
> (probably not applicable here, but it's a trick I find elegant).

Yes, I agree that it is a good practice to document an existing
breakage in an early patch #1, and then make a fix and flip
expect-failure to expect-success in the patch #2.

Breaking the code and documenting the breakage by expecting a
failure in one patch, and then later fixing the breakage and
flipping the expectation in another patch, is a bit less nice,
though ;-)