Re: [PATCH 3/7] add a test for semantic errors in config files
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Jul 23, 2014, 22:11 UTC
- 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 ;-)