On Fri, Mar 03, 2017 at 03:04:02AM +0100, Johannes Schindelin wrote:
> The intention of that test case, however, was to verify that the
> setup_git_directory() function has not run, because it changes global
> state such as the current working directory.
>
> To keep that spirit, but fix the incorrect assumption, this patch
> replaces that test case by a new one that verifies that the pager is
> run in the subdirectory, i.e. that the current working directory has
> not been changed at the time the pager is configured and launched, even
> if the `rev-parse` command requires a .git/ directory and *will* change
> the working directory.
This is a great solution to the question I had in v1 ("how do we know
when setup_git_directory() is run?"). It not only checks the
internal-code case that we care about, but it also shows how real users
would get bit if we do the wrong thing.
> +test_expect_failure TTY 'core.pager in repo config works and retains cwd' '
> + sane_unset GIT_PAGER &&
> + test_config core.pager "cat >cwd-retained" &&
> + (
> + cd sub &&
> + rm -f cwd-retained &&
> + test_terminal git -p rev-parse HEAD &&
> + test -e cwd-retained
> + )
> +'
Does this actually need TTY and test_terminal? You replace the pager
with something that doesn't care about the terminal, and "-p" should
unconditionally turn it on.
(Also a minor nit: we usually prefer test_path_is_file to "test -e"
these days).
-Peff