Re: [PATCH 21/25] t9001: use test_when_finished
- From
Jeff King <peff@peff.net>
- Date
- Mar 25, 2015, 02:47 UTC
- 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:
Show 16 quoted lines
> >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