From: Jeff King <peff@peff.net>
Date: Wed, 25 Mar 2015 02:47:10 GMT
Subject: Re: [PATCH 21/25] t9001: use test_when_finished
Message-ID: <20150325024710.GB15051@peff.net>
In-Reply-To: <20150325030022.Horde.yvJScxCdU4w4HDIV_xUwrw1@webmail.informatik.kit.edu>

On Wed, Mar 25, 2015 at 03:00:22AM +0100, SZEDER Gábor wrote:

> >Instead, they can all use test_when_finished, and we can
> >even make the code simpler by factoring out the shared
> >lines.
> 
> I think that saving the value of 'sendemail.confirm' is not necessary.
> 
> There are two blocks of confirmation tests, this patch concerns only tests
> of the second block.  The first block of confirmation tests is nearly at
> the beginning of the file in order to check the "no confirm" cases early.
> If any of those fails the remainig tests in the file are skipped because
> they might hang.  The last of those tests sets 'sendemail.confirm' to
> 'never' and leaves it so to avoid unintentional prompting in the remaining
> tests and then its value is not modified until that second block of
> confirm tests are reached.  This means that when those tests save the
> value of 'sendemail.confirm' they always save 'never'.  Then why save it,
> just use test_when_finished to restore it to 'never' and all is well.

Yeah, I suspected this while writing it the patch, but I preferred to
keep it more obvious that there would be no accidental regression, since
the series was already so long (and also because calling save_confirm is
not any worse than test_when_finished).

I don't mind a patch on top simplifying out save_confirm, if you're
confident that's what we're always saving.

-Peff