From: Junio C Hamano <gitster@pobox.com>
Date: Wed, 23 Jul 2014 22:11:05 GMT
Subject: Re: [PATCH 3/7] add a test for semantic errors in config files
Message-ID: <xmqq38drd8ue.fsf@gitster.dls.corp.google.com>
In-Reply-To: <vpq38drvohr.fsf@anie.imag.fr>

Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:

> Tanay Abhra <tanayabh@gmail.com> 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 ;-)