All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Philippe Blain via GitGitGadget" <gitgitgadget@gmail.com>
To: git@vger.kernel.org
Cc: "SZEDER Gábor" <szeder.dev@gmail.com>,
	"Elijah Newren" <newren@gmail.com>,
	"Johannes Schindelin" <Johannes.Schindelin@gmx.de>,
	"Jens Lehmann" <Jens.Lehmann@web.de>,
	"Eric Sunshine" <sunshine@sunshineco.com>,
	"Taylor Blau" <me@ttaylorr.com>,
	"Carlo Arenas" <carenas@gmail.com>, "Jeff King" <peff@peff.net>,
	"Philippe Blain" <levraiphilippeblain@gmail.com>
Subject: [PATCH v2 0/3] test-lib-functions.sh: keep user's HOME, TERM and SHELL for 'test_pause' and 'debug'
Date: Sat, 28 Aug 2021 00:47:30 +0000	[thread overview]
Message-ID: <pull.1022.v2.git.1630111653.gitgitgadget@gmail.com> (raw)
In-Reply-To: <pull.1022.git.1629393395.gitgitgadget@gmail.com>

Changes since v1:

 * added 1/3 as a preliminary step to use TEST_SHELL_PATH in test_pause
   instead of SHELL_PATH, as suggested by Carlos
 * implemented the change in behaviour through optional flags in both
   test_pause and debug. This seemed to be the simplest way to keep the
   current behaviour but also provide a way to improve the UX.

v1: This series proposes two small quality-of-life improvements (in my
opinion) to the 'test_pause' and 'debug' test functions: using the original
values of HOME and TERM (before they are changed by the test framework) and
using SHELL instead of SHELL_PATH.

The later might be too big of a change, but I think it makes sense. We could
add a new GIT_TEST_* to conditionnaly change the behaviour, but I kept it
simple for v1.

Cheers, Philippe.

Philippe Blain (3):
  test-lib-functions: use 'TEST_SHELL_PATH' in 'test_pause'
  test-lib-functions: optionally keep HOME, TERM and SHELL in
    'test_pause'
  test-lib-functions: optionally keep HOME and TERM in 'debug'

 t/test-lib-functions.sh | 91 ++++++++++++++++++++++++++++++++++-------
 t/test-lib.sh           |  6 ++-
 2 files changed, 80 insertions(+), 17 deletions(-)


base-commit: 225bc32a989d7a22fa6addafd4ce7dcd04675dbf
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1022%2Fphil-blain%2Ftest-pause-and-debug-easier-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1022/phil-blain/test-pause-and-debug-easier-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/1022

Range-diff vs v1:

 -:  ----------- > 1:  2f566f330e0 test-lib-functions: use 'TEST_SHELL_PATH' in 'test_pause'
 1:  bf916ad98cc ! 2:  00211457ece test-lib-functions: use user's SHELL, HOME and TERM for 'test_pause'
     @@ Metadata
      Author: Philippe Blain <levraiphilippeblain@gmail.com>
      
       ## Commit message ##
     -    test-lib-functions: use user's SHELL, HOME and TERM for 'test_pause'
     +    test-lib-functions: optionally keep HOME, TERM and SHELL in 'test_pause'
      
          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 SHELL_PATH, which defaults to /bin/sh.
     +    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
     @@ Commit message
      
          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 use
     -    these variables to invoke the shell in 'test_pause'. Also, invoke SHELL
     -    instead of SHELL_PATH, so that developer's interactive shell is used.
     +    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 ##
      @@ t/test-lib-functions.sh: 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.
       
       test_pause () {
     --	"$SHELL_PATH" <&6 >&5 2>&7
     -+	TERM="$USER_TERM" HOME="$USER_HOME" "$SHELL" <&6 >&5 2>&7
     +-	"$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
 2:  d51d0db6e25 < -:  ----------- test-lib-functions: use user's TERM and HOME for 'debug'
 -:  ----------- > 3:  1fac9baec1d test-lib-functions: optionally keep HOME and TERM in 'debug'

-- 
gitgitgadget

  parent reply	other threads:[~2021-08-28  0:47 UTC|newest]

Thread overview: 39+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-19 17:16 [PATCH 0/2] test-lib-functions.sh: keep user's HOME, TERM and SHELL for 'test_pause' and 'debug' Philippe Blain via GitGitGadget
2021-08-19 17:16 ` [PATCH 1/2] test-lib-functions: use user's SHELL, HOME and TERM for 'test_pause' Philippe Blain via GitGitGadget
2021-08-20  3:08   ` Carlo Arenas
2021-08-20 12:14     ` Philippe Blain
2021-08-19 17:16 ` [PATCH 2/2] test-lib-functions: use user's TERM and HOME for 'debug' Philippe Blain via GitGitGadget
2021-08-19 19:24   ` Taylor Blau
2021-08-20  3:18     ` Carlo Arenas
2021-08-19 18:10 ` [PATCH 0/2] test-lib-functions.sh: keep user's HOME, TERM and SHELL for 'test_pause' and 'debug' Eric Sunshine
2021-08-19 19:57   ` Junio C Hamano
2021-08-19 20:14     ` Eric Sunshine
2021-08-19 20:03   ` Elijah Newren
2021-08-19 20:11     ` Eric Sunshine
2021-08-20 12:12       ` Philippe Blain
2021-08-20 15:50         ` Eric Sunshine
2021-08-20 18:23         ` Jeff King
2021-08-28  0:47 ` Philippe Blain via GitGitGadget [this message]
2021-08-28  0:47   ` [PATCH v2 1/3] test-lib-functions: use 'TEST_SHELL_PATH' in 'test_pause' Philippe Blain via GitGitGadget
2021-08-28  0:47   ` [PATCH v2 2/3] test-lib-functions: optionally keep HOME, TERM and SHELL " Philippe Blain via GitGitGadget
2021-08-28  7:27     ` Elijah Newren
2021-08-28 14:50       ` Philippe Blain
2021-08-28  0:47   ` [PATCH v2 3/3] test-lib-functions: optionally keep HOME and TERM in 'debug' Philippe Blain via GitGitGadget
2021-09-01 13:31   ` [PATCH v3 0/3] test-lib-functions.sh: keep user's HOME, TERM and SHELL for 'test_pause' and 'debug' Philippe Blain via GitGitGadget
2021-09-01 13:31     ` [PATCH v3 1/3] test-lib-functions: use 'TEST_SHELL_PATH' in 'test_pause' Philippe Blain via GitGitGadget
2021-09-01 20:04       ` Junio C Hamano
2021-09-01 13:31     ` [PATCH v3 2/3] test-lib-functions: optionally keep HOME, TERM and SHELL " Philippe Blain via GitGitGadget
2021-09-01 20:26       ` Junio C Hamano
2021-09-01 21:52         ` Elijah Newren
2021-09-01 23:09           ` Junio C Hamano
2021-09-02 13:10             ` Philippe Blain
2021-09-01 13:31     ` [PATCH v3 3/3] test-lib-functions: optionally keep HOME and TERM in 'debug' Philippe Blain via GitGitGadget
2021-09-06  4:20     ` [PATCH v4 0/3] test-lib-functions.sh: keep user's HOME, TERM and SHELL for 'test_pause' and 'debug' Philippe Blain via GitGitGadget
2021-09-06  4:20       ` [PATCH v4 1/3] test-lib-functions: use 'TEST_SHELL_PATH' in 'test_pause' Philippe Blain via GitGitGadget
2021-09-06  4:20       ` [PATCH v4 2/3] test-lib-functions: optionally keep HOME, TERM and SHELL " Philippe Blain via GitGitGadget
2021-09-06  4:20       ` [PATCH v4 3/3] test-lib-functions: keep user's debugger config files and TERM in 'debug' Philippe Blain via GitGitGadget
2021-09-06  4:38       ` [PATCH v5 0/3] test-lib-functions.sh: keep user's HOME, TERM and SHELL for 'test_pause' and 'debug' Philippe Blain via GitGitGadget
2021-09-06  4:38         ` [PATCH v5 1/3] test-lib-functions: use 'TEST_SHELL_PATH' in 'test_pause' Philippe Blain via GitGitGadget
2021-09-06  4:38         ` [PATCH v5 2/3] test-lib-functions: optionally keep HOME, TERM and SHELL " Philippe Blain via GitGitGadget
2021-09-06  4:39         ` [PATCH v5 3/3] test-lib-functions: keep user's debugger config files and TERM in 'debug' Philippe Blain via GitGitGadget
2021-09-07  6:24         ` [PATCH v5 0/3] test-lib-functions.sh: keep user's HOME, TERM and SHELL for 'test_pause' and 'debug' Elijah Newren

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=pull.1022.v2.git.1630111653.gitgitgadget@gmail.com \
    --to=gitgitgadget@gmail.com \
    --cc=Jens.Lehmann@web.de \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=carenas@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=levraiphilippeblain@gmail.com \
    --cc=me@ttaylorr.com \
    --cc=newren@gmail.com \
    --cc=peff@peff.net \
    --cc=sunshine@sunshineco.com \
    --cc=szeder.dev@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.