Re: [PATCH v5 3/4] commit test: test_set_editor in each test

[Caleb Thompson <caleb@calebthompson.io>]

[-- Attachment #1: Type: text/plain, Size: 1253 bytes --]

On Fri, Jun 13, 2014 at 07:41:29PM -0400, Jeff King wrote:
> On Fri, Jun 13, 2014 at 10:42:26AM -0700, Junio C Hamano wrote:
>
> > Jeff King <peff@peff.net> writes:
> >
> > > [1] It might make sense for test_set_editor, when run from within a
> > >     test, to behave more like test_config, and do:
> > >
> > >       test_when_finished '
> > >         sane_unset FAKE_EDITOR &&
> > >         sane_unset EDITOR
> > >       '
> > >
> > >     I don't know if there would be fallouts with other test scripts,
> > >     though.
> >
> > The default environment for tests is to set EDITOR=: to avoid
> > accidentally triggering interactive cruft and interfering with
> > automated tests, I thought.
>
> Ah, yeah, that would make more sense.
>
> > If the above sane-unset is changed to EDITOR=: then I think that is
> > probably sensible.
>
> I think the trick is that other scripts may be relying on the global
> side-effect, and would need to be fixed up (and it is not always obvious
> which spots will need it; they might fail the tests, or they might start
> silently passing for the wrong reason).

For this reason, and that the scope of this change has already ballooned, I'd
rather not make this change in this patch if that's alright.

Caleb Thompson

[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]