Re: [PATCH 4/4] t: branch: improve test rollback

From: Felipe Contreras <felipe.contreras@gmail.com>
Date: Sep 3, 2013, 23:03 UTC
Message-ID: <CAMP44s1Q6YjVnVz3OnT=cE+ozUddBxFzM1r_WT5QoR7sP6-9Sg@mail.gmail.com>
In-Reply-To: <xmqqtxi1o6kf.fsf@gitster.dls.corp.google.com>

On Tue, Sep 3, 2013 at 5:59 PM, Junio C Hamano <gitster@pobox.com> wrote:

Show 18 quoted lines

> Felipe Contreras <felipe.contreras@gmail.com> writes:
>
>> On Tue, Sep 3, 2013 at 2:32 PM, Junio C Hamano <gitster@pobox.com> wrote:
>>> Felipe Contreras <felipe.contreras@gmail.com> writes:
>>
>>>>  test_expect_success 'refuse --edit-description on unborn branch for now' '
>>>> +     test_when_finished "git checkout -" &&
>>>
>>> I am not sure if this is a good change.  Depending on the outcome of
>>> the "git checkout" in the test (it may succeed and set @{-1} to the
>>> branch we were on when we entered the test, or it may fail and leave
>>> @{-1} to the branch before we were on when we entered the test),
>>> this will take us to a different place, no?
>>
>> It is better than leaving Git in an unborn branch.
>
> True, but it falls short of "After every test the environment should
> be as close as to how it was before as possible.", doesn't it?

No it doesn't. Closer is closer.

> I think the fix is just to use the name of a concrete branch,
> instead of "-", which could be far quickly done than making excuses
> like that.

The fix is to write a test_checkout that would save the previous HEAD and restore it after the test, or something similar. But I'm not going to write that, and I bet nobody else is, so unless you want the tests to remain in the crappy state they are, "git checkout -" is the best sensible option.

-- 
Felipe Contreras