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

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:

quoted

Jeff King [off-list ref] writes:

quoted

[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.

quoted

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