Jeff King [off-list ref] 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.
If the above sane-unset is changed to EDITOR=: then I think that is
probably sensible.
On Fri, Jun 13, 2014 at 10:42:26AM -0700, Junio C Hamano wrote:
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.
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).
-Peff
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