From: Jakub Narębski <jnareb@gmail.com>
Date: Fri, 13 Jun 2014 17:16:53 GMT
Subject: Re: [PATCH v5 3/4] commit test: test_set_editor in each test
Message-ID: <539B3205.5010208@gmail.com>
In-Reply-To: <20140613163644.GB85151@sirius.local>

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?

-- 
Jakub Narębski