All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Fabian Stelzer <fs@gigacodes.de>
Cc: git@vger.kernel.org, "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>,
	"Adam Dinwoodie" <adam@dinwoodie.org>,
	"Jeff King" <peff@peff.net>
Subject: Re: [PATCH v4 3/3] test-lib: make BAIL_OUT() work in tests and prereq
Date: Wed, 01 Dec 2021 15:13:35 -0800	[thread overview]
Message-ID: <xmqqv907yl40.fsf@gitster.g> (raw)
In-Reply-To: <20211201085315.576865-4-fs@gigacodes.de> (Fabian Stelzer's message of "Wed, 1 Dec 2021 09:53:15 +0100")

Fabian Stelzer <fs@gigacodes.de> writes:

> BAIL_OUT() is meant to abort the whole test run and print a message with
> a standard prefix that can be parsed to stdout. Since for every test the
> normal fd`s are redirected in test_eval_ this output would not be seen
> when used within the context of a test or prereq like we do in
> test_have_prereq(). To make this function work in these contexts we move
> the setup of the fd aliases a few lines up before the first use of
> BAIL_OUT() and then have this function always print to the alias.
>
> Signed-off-by: Fabian Stelzer <fs@gigacodes.de>
> ---
>  t/test-lib.sh | 14 ++++++++++----
>  1 file changed, 10 insertions(+), 4 deletions(-)
>
> diff --git a/t/test-lib.sh b/t/test-lib.sh
> index 9090ce1225..14a7aeae0f 100644
> --- a/t/test-lib.sh
> +++ b/t/test-lib.sh
> @@ -589,6 +589,15 @@ USER_TERM="$TERM"
>  TERM=dumb
>  export TERM USER_TERM
>  
> +# What is written by tests to stdout and stderr is sent so different places

"sent so" -> "sent to".  I'll tweak locally, so no need to resend.

> +# depending on the test mode (e.g. /dev/null in non-verbose mode, piped to tee
> +# with --tee option, etc.). We save the original stdin to FD #6 and stdout and
> +# stderr to #5 and #7, so that the test framework can use them (e.g. for
> +# printing errors within the test framework) independently of the test mode.
> +exec 5>&1
> +exec 6<&0
> +exec 7>&2
> +
>  _error_exit () {
>  	finalize_junit_xml
>  	GIT_EXIT_OK=t
> @@ -612,7 +621,7 @@ BAIL_OUT () {
>  	local bail_out="Bail out! "
>  	local message="$1"
>  
> -	say_color error $bail_out "$message"
> +	say_color error $bail_out "$message" >&5

This is merely a style thing, but as commands get longer, it becomes
easier to spot redirection if it is written immediately after the
verb, i.e.

	say_color >&5 error $bail_out "$message"

>  	_error_exit
>  }
>  
> @@ -637,9 +646,6 @@ then
>  	exit 0
>  fi
>  
> -exec 5>&1
> -exec 6<&0
> -exec 7>&2
>  if test "$verbose_log" = "t"
>  then
>  	exec 3>>"$GIT_TEST_TEE_OUTPUT_FILE" 4>&3

Looks good.  Thanks.

  reply	other threads:[~2021-12-01 23:13 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-11-17  9:04 [PATCH v2 0/2] test-lib: improve missing prereq handling Fabian Stelzer
2021-11-17  9:04 ` [PATCH v2 1/2] test-lib: show missing prereq summary Fabian Stelzer
2021-11-17  9:04 ` [PATCH v2 2/2] test-lib: introduce required prereq for test runs Fabian Stelzer
2021-11-18 23:42   ` Junio C Hamano
2021-11-19  9:07     ` Fabian Stelzer
2021-11-19 11:13   ` Ævar Arnfjörð Bjarmason
2021-11-19 13:48     ` Fabian Stelzer
2021-11-19 14:09     ` Fabian Stelzer
2021-11-19 14:26       ` Ævar Arnfjörð Bjarmason
2021-11-19 15:40         ` Fabian Stelzer
2021-11-19 16:37           ` Ævar Arnfjörð Bjarmason
2021-11-20 15:03   ` [PATCH v3 0/3] test-lib: improve missing prereq handling Fabian Stelzer
2021-11-20 15:03     ` [PATCH v3 1/3] test-lib: show missing prereq summary Fabian Stelzer
2021-11-20 15:04     ` [PATCH v3 2/3] test-lib: introduce required prereq for test runs Fabian Stelzer
2021-11-20 15:04     ` [PATCH v3 3/3] test-lib: make BAIL_OUT() work in tests and prereq Fabian Stelzer
2021-11-22 11:52       ` Ævar Arnfjörð Bjarmason
2021-11-22 17:05         ` Junio C Hamano
2021-11-26  9:55           ` Fabian Stelzer
2021-11-26 21:02             ` Junio C Hamano
2021-11-27 12:47               ` Fabian Stelzer
2021-11-28 23:38                 ` Junio C Hamano
2021-11-30 14:38                   ` Fabian Stelzer
2021-11-30 14:59                     ` Ævar Arnfjörð Bjarmason
2021-12-01  8:53     ` [PATCH v4 0/3] test-lib: improve missing prereq handling Fabian Stelzer
2021-12-01  8:53       ` [PATCH v4 1/3] test-lib: show missing prereq summary Fabian Stelzer
2021-12-01  8:53       ` [PATCH v4 2/3] test-lib: introduce required prereq for test runs Fabian Stelzer
2021-12-01  8:53       ` [PATCH v4 3/3] test-lib: make BAIL_OUT() work in tests and prereq Fabian Stelzer
2021-12-01 23:13         ` Junio C Hamano [this message]
2021-12-01 21:05       ` [PATCH v4 0/3] test-lib: improve missing prereq handling Adam Dinwoodie

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=xmqqv907yl40.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=adam@dinwoodie.org \
    --cc=avarab@gmail.com \
    --cc=fs@gigacodes.de \
    --cc=git@vger.kernel.org \
    --cc=peff@peff.net \
    /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.