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

[Felipe Contreras <felipe.contreras@gmail.com>]

On Sat, Sep 7, 2013 at 11:56 PM, Jeff King <peff@peff.net> wrote:
> On Sat, Sep 07, 2013 at 10:02:59PM -0500, Felipe Contreras wrote:
>
>> '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.
>
> I do not think it works perfectly fine if the checkout itself fails. For
> example, after your patch, one of the tests reads:
>
>   test_when_finished "git checkout -" &&
>   git checkout HEAD^{} &&
>   test_must_fail git branch --set-upstream-to master
>
> The cleanup is undoing what happened on the second line. So if we fail
> on the third line, it does what we want. If we fail on the second line,
> then what branch are switching to?
>
> I think you would at least want to reverse the order as:
>
>   git checkout HEAD^{} &&
>   test_when_finished "git checkout -" &&
>   test_must_fail git branch --set-upstream-to master
>
> so that you know you are actually going to the pre-test state.

Yeah, that would be better and easily achievable.

-- 
Felipe Contreras