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

From: Jeff King <peff@peff.net>
Date: Sep 8, 2013, 04:56 UTC
Message-ID: <20130908045606.GF14019@sigill.intra.peff.net>
In-Reply-To: <CAMP44s2PFVAKRUkEyN8BAH8C5UUMn21ou9hNkcOXaCsXscywHQ@mail.gmail.com>

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.

-Peff