Re: [PATCH v2 2/3] test-lib-functions: optionally keep HOME, TERM and SHELL in 'test_pause'

[Elijah Newren <newren@gmail.com>]

On Fri, Aug 27, 2021 at 5:47 PM Philippe Blain via GitGitGadget
<gitgitgadget@gmail.com> wrote:
>
> From: Philippe Blain <levraiphilippeblain@gmail.com>
>
> The 'test_pause' function, which is designed to help interactive
> debugging and exploration of tests, currently inherits the value of HOME
> and TERM set by 'test-lib.sh': HOME="$TRASH_DIRECTORY" and TERM=dumb. It
> also invokes the shell defined by TEST_SHELL_PATH, which defaults to
> /bin/sh (through SHELL_PATH).
>
> Changing the value of HOME means that any customization configured in a
> developers' shell startup files and any Git aliases defined in their
> global Git configuration file are not available in the shell invoked by
> 'test_pause'.
>
> Changing the value of TERM to 'dumb' means that colored output
> is disabled for all commands in that shell.
>
> Using /bin/sh as the shell invoked by 'test_pause' is not ideal since
> some platforms (i.e. Debian and derivatives) use Dash as /bin/sh, and
> this shell is usually compiled without readline support, which makes for
> a poor interactive command line experience.
>
> To make the interactive command line experience in the shell invoked by
> 'test_pause' more pleasant, save the values of HOME and TERM in
> USER_HOME and USER_TERM before changing them in test-lib.sh, and add
> options to 'test_pause' to optionally use these variables to invoke the
> shell. Also add an option to invoke SHELL instead of TEST_SHELL_PATH, so
> that developer's interactive shell is used.
>
> We use options instead of changing the behaviour unconditionally since
> these three variables can break test reproducibility. Moreover, using the
> original HOME means tests could overwrite files in a user's home
> directory. Be explicit about these caveats in the new 'Usage' section in
> test-lib-functions.sh.
>
> Signed-off-by: Philippe Blain <levraiphilippeblain@gmail.com>
> ---
>  t/test-lib-functions.sh | 37 ++++++++++++++++++++++++++++++++++++-
>  t/test-lib.sh           |  6 ++++--
>  2 files changed, 40 insertions(+), 3 deletions(-)
>
> diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
> index 1884177e293..1b775343adc 100644
> --- a/t/test-lib-functions.sh
> +++ b/t/test-lib-functions.sh
> @@ -137,9 +137,44 @@ test_tick () {
>  # Stop execution and start a shell. This is useful for debugging tests.
>  #
>  # Be sure to remove all invocations of this command before submitting.
> +#
> +# Usage: test_pause [options]
> +#   -t
> +#      Use your original TERM instead of test-lib.sh's "dumb".
> +#      This usually restores color output in the invoked shell.
> +#      WARNING: this can break test reproducibility.
> +#   -s
> +#      Invoke $SHELL instead of $TEST_SHELL_PATH
> +#      WARNING: this can break test reproducibility.
> +#   -h
> +#      Use your original HOME instead of test-lib.sh's "$TRASH_DIRECTORY".
> +#      This allows you to use your regular shell environment and Git aliases.
> +#      WARNING: this can break test reproducibility.
> +#      CAUTION: this can overwrite files in your HOME.

Do you want to add an option that implies the other three, so that
folks can do something like `test_pause -a` (or some other single
letter) rather than `test_pause -t -s -h`?

>
>  test_pause () {
> -       "$TEST_SHELL_PATH" <&6 >&5 2>&7
> +       PAUSE_TERM=$TERM &&
> +       PAUSE_SHELL=$TEST_SHELL_PATH &&
> +       PAUSE_HOME=$HOME &&
> +       while test $# != 0
> +       do
> +               case "$1" in
> +               -t)
> +                       PAUSE_TERM="$USER_TERM"
> +                       ;;
> +               -s)
> +                       PAUSE_SHELL="$SHELL"
> +                       ;;
> +               -h)
> +                       PAUSE_HOME="$USER_HOME"
> +                       ;;
> +               *)
> +                       break
> +                       ;;
> +               esac
> +               shift
> +       done &&
> +       TERM="$PAUSE_TERM" HOME="$PAUSE_HOME" "$PAUSE_SHELL" <&6 >&5 2>&7
>  }
>
>  # Wrap git with a debugger. Adding this to a command can make it easier
> diff --git a/t/test-lib.sh b/t/test-lib.sh
> index abcfbed6d61..132618991e2 100644
> --- a/t/test-lib.sh
> +++ b/t/test-lib.sh
> @@ -585,8 +585,9 @@ else
>         }
>  fi
>
> +USER_TERM="$TERM"
>  TERM=dumb
> -export TERM
> +export TERM USER_TERM
>
>  error () {
>         say_color error "error: $*"
> @@ -1380,9 +1381,10 @@ then
>  fi
>
>  # Last-minute variable setup
> +USER_HOME="$HOME"
>  HOME="$TRASH_DIRECTORY"
>  GNUPGHOME="$HOME/gnupg-home-not-used"
> -export HOME GNUPGHOME
> +export HOME GNUPGHOME USER_HOME
>
>  # Test repository
>  rm -fr "$TRASH_DIRECTORY" || {
> --
> gitgitgadget