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

From: Felipe Contreras <felipe.contreras@gmail.com>
Date: Sep 8, 2013, 03:02 UTC
Message-ID: <CAMP44s2PFVAKRUkEyN8BAH8C5UUMn21ou9hNkcOXaCsXscywHQ@mail.gmail.com>
In-Reply-To: <xmqq61ugo67v.fsf@gitster.dls.corp.google.com>

On Wed, Sep 4, 2013 at 12:19 PM, Junio C Hamano <gitster@pobox.com> wrote:

Show 25 quoted lines

> Felipe Contreras <felipe.contreras@gmail.com> writes:
>
>> On Tue, Sep 3, 2013 at 5:59 PM, Junio C Hamano <gitster@pobox.com> wrote:
>>> 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.
>
> Closer is not "as close as ... possible".

Yes it is, because there are no better alternatives within our ability, capacity, or realization.

http://www.merriam-webster.com/dictionary/possible

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

No, what if that test is skipped for some reason? Ideally each test case should do 'git checkout $branch_i_need', not rely on the previous tests to checkout the branch they need, but that requires too much work to fix.

'git checkout -' works perfectly fine, and it's the closest we have to my suggested test_checkout() which I think would be ideal, but I'm not going to work on.

-- 
Felipe Contreras