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

From: Philippe Blain <levraiphilippeblain@gmail.com>
Date: Aug 20, 2021, 12:12 UTC
Message-ID: <53752b7d-c294-9507-a7f0-af2aee491251@gmail.com>
In-Reply-To: <CAPig+cRk-BB79+pO83A6Qov9W75_91gzaHY4Le1MYKDUWagugg@mail.gmail.com>

Hi everyone,

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

> On Thu, Aug 19, 2021 at 4:03 PM Elijah Newren <newren@gmail.com> wrote:
>> On Thu, Aug 19, 2021 at 11:10 AM Eric Sunshine <sunshine@sunshineco.com> wrote:

Show 19 quoted lines

>>> In such a
>>> scenario, HOME must be pointing at the test's home directory, not at
>>> my real home directory.
>>
>> I agree, but I worry that it's not just HOME.  I'd think the point of
>> test_pause is to let you interact with the repository state while
>> getting the same results that the test framework would, and I think
>> some tests could be affected by TERM and SHELL too (e.g. perhaps the
>> recent issues with COLUMNS).  Granted, I suspect far fewer tests would
>> be affected by those, but I'm not sure I like the idea of inability to
>> reproduce the same issues.
> 
> 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.
>

Thanks everyone for sharing their input and concerns. I understand that the behaviour change might not be wanted all the time, or by everyone.

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.

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

and combinations of these three.

For 'debug', Carlo's idea of just symlinking/copying gdbinit and/or llldbinit to the test HOME might be easier, and would cover the majority of developers, I think. As for TERM, we could do 'debug -t' as above, or use USER_TERM always...

I'll explore these ideas before sending v2.

Thanks,

Philippe.