From: Jakub Narębski Date: Fri, 13 Jun 2014 18:52:47 GMT Subject: Re: [PATCH v5 3/4] commit test: test_set_editor in each test Message-ID: <539B487F.4030104@gmail.com> In-Reply-To: <20140613174745.GA88614@sirius.local> W dniu 2014-06-13 19:47, Caleb Thompson pisze: > On Fri, Jun 13, 2014 at 07:16:53PM +0200, Jakub Narębski wrote: >> W dniu 2014-06-13 18:36, Caleb Thompson pisze: >>> 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. I meant here GIT_SKIP_TESTS, but I see that test_set_editor was not in first test i.e. test_expect_success 'setup', so it wouldn't matter. Before and after both work correctly with GIT_SKIP_TESTS, before because of global setup. I'm sorry for the noise, then. -- Jakub Narębski