Re: [PATCH v5 3/4] commit test: test_set_editor in each test

[Caleb Thompson <caleb@calebthompson.io>]

[-- Attachment #1: Type: text/plain, Size: 1892 bytes --]

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 #2: Type: application/pgp-signature, Size: 473 bytes --]