Re: [PATCH v5 3/4] commit test: test_set_editor in each test
- From
Jakub Narębski <jnareb@gmail.com>
- Date
- Jun 13, 2014, 17:16 UTC
- 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:
Show 30 quoted lines
>> [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