On Fri, Jun 13, 2014 at 07:16:53PM +0200, Jakub Narębski wrote: > W dniu 2014-06-13 18:36, Caleb Thompson pisze: > >On Fri, Jun 13, 2014 at 02:59:42AM -0400, Jeff King wrote: > > >>[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 > >> ' > > > >It might, but it's a little out of scope in addition to your concern > >about other test scripts. > > > >> > >> I don't know if there would be fallouts with other test scripts, > >> though. > > > >How is this for a reword of that commit description: > > > > t/t7507-commit-verbose.sh was using a global test_set_editor call to > > build its environment. The $EDITOR being used was not necessary for > > all tests, and was in fact circumvented using subshells in some > > cases. > > > > To improve robustness against global state changes and avoid the > > use of subshells to temporarily switch the editor, set the editor > > explicitly wherever it will be important. > > > > Specifically, in tests that need to check for the presence of a diff in the > > editor, make calls to set_test_editor to set $EDITOR to check-for-diff > > rather than relying on that editor being configured globally. This also > > helps readers grok the tests as the setup is closer to the verification. > > This also allows to run only specified subset of tests > with TEST_SKIP without requiring to remember which tests > are setup tests and have to be not skipped, isn't it? I don't see any references to TEST_SKIP in the code. Do you mean test_skip() from t/test_lib.sh? If so, it isn't clear to me what the use case would be for that, so I'd have to take your word. Caleb Thompson
Attachment:
pgpHcpUt4cKdt.pgp
Description: PGP signature