Re: [PATCH 0/2] test-lib-functions.sh: keep user's HOME, TERM and SHELL for 'test_pause' and 'debug'

On Fri, Aug 20, 2021 at 8:12 AM Philippe Blain
[off-list ref] wrote:

Le 2021-08-19 à 16:11, Eric Sunshine a écrit :

quoted

Oh, indeed. I didn't mean to imply that HOME is the only problematic
one; they all are since, as you say, they can impact correctness and
reproducibility of the tests themselves. I called out HOME specially
because of the potential danger involved with pointing it at the
user's real home directory since it could very well lead to clobbering
of precious files and other settings belonging to the user.

I also did not think about the implications of changing $HOME that could lead to the
test framework overwriting stuff in my home. I checked the tests and there are only
a handful of them that seem to reference HOME, but still, for those tests it would be
undesirable to reset HOME.

It's not just tests which reference HOME explicitly which are
problematic. Git commands themselves access files and configuration
pointed at by HOME. Worse, Git commands invoked by tests can alter
configuration, so the potential for damage is wider than the few tests
which reference HOME explicitly.

In light of this I'm thinking of simply adding flags to 'test_pause' and 'debug' to signal
that one wants to use their original TERM, HOME and SHELL, with appropriate  caveats in
the description of the functions:

test_pause     # original behaviour
test_pause -t  # use USER_TERM
test_pause -s  # use SHELL instead of TEST_SHELL_PATH
test_pause -h  # use USER_HOME

A (very) stray thought I had: Rather than mucking with the environment
variables -- which can impact test reproducibility and correctness and
can potentially damage the user's precious files and configuration --
another possibility might be to detect which shell is being used
(whether it be bash, zsh, etc.), and then enable certain useful
options specific to the shell, such as tab-completion, colors, etc.
This approach doesn't give developers the customized shell experience
they're used to (it wouldn't have their aliases or configuration, for
instance), but it at least might make the test_pause() experience a
bit more pleasant.